diff --git a/common/lib/xmodule/xmodule/js/src/capa/schematic.js b/common/lib/xmodule/xmodule/js/src/capa/schematic.js
index 63edb278d87381868666d5039c875a9e047dadd2..92c274c7bce219d99ef648092fe5bc0efc5048c5 100644
--- a/common/lib/xmodule/xmodule/js/src/capa/schematic.js
+++ b/common/lib/xmodule/xmodule/js/src/capa/schematic.js
@@ -2810,7 +2810,7 @@ schematic = (function() {
// for each requested freq, interpolate response value
for (var k = 1; k < flist.length; k++) {
var f = flist[k];
- var v = interpolate(f,x_values,values); //xss-lint: disable=javascript-interpolate
+ var v = interpolate(f,x_values,values);
// convert to dB
fvlist.push([f,v == undefined ? 'undefined' : 20.0 * Math.log(v)/Math.LN10]);
}
@@ -2932,7 +2932,7 @@ schematic = (function() {
// for each requested time, interpolate waveform value
for (var k = 1; k < tlist.length; k++) {
var t = tlist[k];
- var v = interpolate(t,times,values); // xss-lint: disable=javascript-interpolate
+ var v = interpolate(t,times,values);
tvlist.push([t,v == undefined ? 'undefined' : v]);
}
// save results as list of [t,value] pairs
@@ -2978,7 +2978,7 @@ schematic = (function() {
// t is the time at which we want a value
// times is a list of timepoints from the simulation
- function interpolate(t,times,values) { // xss-lint: disable=javascript-interpolate
+ function interpolate(t,times,values) {
if (values == undefined) return undefined;
for (var i = 0; i < times.length; i++)
@@ -5219,7 +5219,7 @@ schematic = (function() {
}
Wire.prototype = new Component();
Wire.prototype.constructor = Wire;
-
+
Wire.prototype.toString = function() {
return edx.StringUtils.interpolate(
'<Wire ({x},{y}) ({x_plus_dx},{y_plus_dy})>',
@@ -5348,7 +5348,7 @@ schematic = (function() {
y: this.y
});
}
-
+
Ground.prototype.draw = function(c) {
Component.prototype.draw.call(this,c); // give superclass a shot
this.draw_line(c,0,0,0,8);
diff --git a/common/lib/xmodule/xmodule/js/src/sequence/display.js b/common/lib/xmodule/xmodule/js/src/sequence/display.js
index 06f519bd3508ce5ee689876439e13f106417ccea..1a283e55b8450545f8b4ed2b831b0b0bd3baabf8 100644
--- a/common/lib/xmodule/xmodule/js/src/sequence/display.js
+++ b/common/lib/xmodule/xmodule/js/src/sequence/display.js
@@ -329,7 +329,6 @@
this.render(newPosition);
} else {
alertTemplate = gettext('Sequence error! Cannot navigate to %(tab_name)s in the current SequenceModule. Please contact the course staff.'); // eslint-disable-line max-len
- // xss-lint: disable=javascript-interpolate
alertText = interpolate(alertTemplate, {
tab_name: newPosition
}, true);
diff --git a/common/lib/xmodule/xmodule/js/src/video/06_video_progress_slider.js b/common/lib/xmodule/xmodule/js/src/video/06_video_progress_slider.js
index df7ac15e0829460c035b459808ac04aeb27303ed..3179e8b1534ceab555c4f7d5fd796e32560fb679 100644
--- a/common/lib/xmodule/xmodule/js/src/video/06_video_progress_slider.js
+++ b/common/lib/xmodule/xmodule/js/src/video/06_video_progress_slider.js
@@ -329,7 +329,6 @@ function() {
msg = ngettext('%(value)s second', '%(value)s seconds', value);
break;
}
- // xss-lint: disable=javascript-interpolate
return interpolate(msg, {value: value}, true);
};
diff --git a/common/static/js/capa/spec/formula_equation_preview_spec.js b/common/static/js/capa/spec/formula_equation_preview_spec.js
index ad0a903e0352b1b7be818f2e41739151e19eca6c..17c147a8d0cf7af68c029bb9010099b3163ce237 100644
--- a/common/static/js/capa/spec/formula_equation_preview_spec.js
+++ b/common/static/js/capa/spec/formula_equation_preview_spec.js
@@ -1,3 +1,32 @@
+describe('escapeSelector', function() {
+ 'use strict';
+ var escapeSelector = window.escapeSelector;
+
+ it('correctly escapes css', function() {
+ // tests borrowed from
+ // https://github.com/jquery/jquery/blob/3edfa1bcdc50bca41ac58b2642b12f3feee03a3b/test/unit/selector.js#L2030
+ expect(escapeSelector('-')).toEqual('\\-');
+ expect(escapeSelector('-a')).toEqual('-a');
+ expect(escapeSelector('--')).toEqual('--');
+ expect(escapeSelector('--a')).toEqual('--a');
+ expect(escapeSelector('\uFFFD')).toEqual('\uFFFD');
+ expect(escapeSelector('\uFFFDb')).toEqual('\uFFFDb');
+ expect(escapeSelector('a\uFFFDb')).toEqual('a\uFFFDb');
+ expect(escapeSelector('1a')).toEqual('\\31 a');
+ expect(escapeSelector('a\0b')).toEqual('a\uFFFDb');
+ expect(escapeSelector('a3b')).toEqual('a3b');
+ expect(escapeSelector('-4a')).toEqual('-\\34 a');
+ expect(escapeSelector('\x01\x02\x1E\x1F')).toEqual('\\1 \\2 \\1e \\1f ');
+
+ // This is the important one; xblocks and course ids often contain invalid characters, so if these aren't
+ // escaped when embedding/searching xblock IDs using css selectors, bad things happen.
+ expect(escapeSelector('course-v1:edX+DemoX+Demo_Course')).toEqual('course-v1\\:edX\\+DemoX\\+Demo_Course');
+ expect(escapeSelector('block-v1:edX+DemoX+Demo_Course+type@sequential+block')).toEqual(
+ 'block-v1\\:edX\\+DemoX\\+Demo_Course\\+type\\@sequential\\+block'
+ );
+ });
+});
+
describe('Formula Equation Preview', function() {
'use strict';
var formulaEquationPreview = window.formulaEquationPreview;
diff --git a/common/static/js/capa/src/formula_equation_preview.js b/common/static/js/capa/src/formula_equation_preview.js
index 646ba0809c021da12426bc9339b329640b185746..8456171972620a9bb186d0acf7d2ef335b31420f 100644
--- a/common/static/js/capa/src/formula_equation_preview.js
+++ b/common/static/js/capa/src/formula_equation_preview.js
@@ -1,3 +1,38 @@
+function escapeSelector(id) {
+ 'use strict';
+ // Wrapper around window.CSS.escape that uses a fallback method if CSS.escape is not available. This is designed to
+ // serialize a string to be used as a valid css selector. See
+ // https://drafts.csswg.org/cssom/#the-css.escape()-method For example, this can be used with xblock and course ids,
+ // which often contain invalid characters that must be escaped to function properly in css selectors.
+ // TODO: if this escaping is also required elsewhere, it may be useful to add a global CSS.escape polyfill and
+ // use that directly.
+
+ // CSS string/identifier serialization https://drafts.csswg.org/cssom/#common-serializing-idioms
+ // This code borrowed from https://api.jquery.com/jQuery.escapeSelector/ (source:
+ // https://github.com/jquery/jquery/blob/3edfa1bc/src/selector/escapeSelector.js). When we upgrade to jQuery 3.0, we
+ // can use $.escapeSelector() instead of this shim escapeSelector function.
+ var rcssescape = /([\0-\x1f\x7f]|^-?\d)|^-$|[^\x80-\uFFFF\w-]/g; // eslint-disable-line no-control-regex
+ function fcssescape(ch, asCodePoint) {
+ if (asCodePoint) {
+ // U+0000 NULL becomes U+FFFD REPLACEMENT CHARACTER
+ if (ch === '\0') {
+ return '\uFFFD';
+ }
+ // Control characters and (dependent upon position) numbers get escaped as code points
+ return ch.slice(0, -1) + '\\' + ch.charCodeAt(ch.length - 1).toString(16) + ' ';
+ }
+ // Other potentially-special ASCII characters get backslash-escaped
+ return '\\' + ch;
+ }
+
+ if (window.CSS && window.CSS.escape) {
+ return window.CSS.escape(id);
+ } else {
+ // ensure string and then run the replacements
+ return (id + '').replace(rcssescape, fcssescape);
+ }
+}
+
var formulaEquationPreview = {
minDelay: 300, // Minimum time between requests sent out.
errorDelay: 1500 // Wait time before showing error (prevent frustration).
@@ -13,7 +48,7 @@ formulaEquationPreview.enable = function() {
function setupInput() {
var $this = $(this); // cache the jQuery object
- var $preview = $('#' + this.id + '_preview');
+ var $preview = $('#' + escapeSelector(this.id) + '_preview');
var inputData = {
// These are the mutable values
diff --git a/lms/djangoapps/teams/static/teams/js/views/edit_team_members.js b/lms/djangoapps/teams/static/teams/js/views/edit_team_members.js
index 425d82e4773ca6adcc2c261443c2b3c9af992b3d..16aadbde49e4a80396899e0fccb0a7dc38fc58f2 100644
--- a/lms/djangoapps/teams/static/teams/js/views/edit_team_members.js
+++ b/lms/djangoapps/teams/static/teams/js/views/edit_team_members.js
@@ -52,7 +52,7 @@
_.each(this.model.get('membership'), function(membership) {
// eslint-disable-next-line no-undef
- dateJoined = interpolate( // xss-lint: disable=javascript-interpolate
+ dateJoined = interpolate(
/* Translators: 'date' is a placeholder for a fuzzy,
* relative timestamp (see: https://github.com/rmm5t/jquery-timeago)
*/
@@ -62,7 +62,7 @@
);
// eslint-disable-next-line no-undef
- lastActivity = interpolate( // xss-lint: disable=javascript-interpolate
+ lastActivity = interpolate(
/* Translators: 'date' is a placeholder for a fuzzy,
* relative timestamp (see: https://github.com/rmm5t/jquery-timeago)
*/
diff --git a/openedx/core/djangoapps/xblock/runtime/shims.py b/openedx/core/djangoapps/xblock/runtime/shims.py
index c9c172b4d9289c6ac56380cc11290e7333651aea..edf1d56951fd40750e6167c9a7a5e03b4e4f88ae 100644
--- a/openedx/core/djangoapps/xblock/runtime/shims.py
+++ b/openedx/core/djangoapps/xblock/runtime/shims.py
@@ -11,6 +11,7 @@ from django.core.cache import cache
from django.template import TemplateDoesNotExist
from django.utils.functional import cached_property
from fs.memoryfs import MemoryFS
+from openedx.core.djangoapps.xblock.apps import get_xblock_app_config
import six
from edxmako.shortcuts import render_to_string
@@ -270,7 +271,12 @@ class RuntimeShim(object):
Seems only to be used by capa. Remove this if capa can be refactored.
"""
# TODO: Refactor capa to access this directly, don't bother the runtime. Then remove it from here.
- return settings.STATIC_URL
+ static_url = settings.STATIC_URL
+ if static_url.startswith('/') and not static_url.startswith('//'):
+ # This is not a full URL - should start with https:// to support loading assets from an iframe sandbox
+ site_root_url = get_xblock_app_config().get_site_root_url()
+ static_url = site_root_url + static_url
+ return static_url
@cached_property
def user_is_staff(self):
diff --git a/scripts/xsslint/tests/test_linters.py b/scripts/xsslint/tests/test_linters.py
index ce18bfea13cb44b2c03dbeef91244207253e9a86..e3556fa80010579312d1f3bb1e912d460ec81773 100644
--- a/scripts/xsslint/tests/test_linters.py
+++ b/scripts/xsslint/tests/test_linters.py
@@ -403,27 +403,19 @@ class TestJavaScriptLinter(TestLinter):
linter.check_javascript_file_is_safe(data['template'], results)
self._validate_data_rules(data, results)
- @data(
- {'template': 'StringUtils.interpolate()', 'rule': None},
- {'template': 'HtmlUtils.interpolateHtml()', 'rule': None},
- {'template': 'interpolate(anything)', 'rule': JAVASCRIPT_LINTER_RULESET.javascript_interpolate},
- )
- def test_javascript_interpolate(self, data):
- """
- Test check_javascript_file_is_safe with interpolate()
- """
- linter = _build_javascript_linter()
- results = FileResults('')
-
- linter.check_javascript_file_is_safe(data['template'], results)
-
- self._validate_data_rules(data, results)
-
@data(
{'template': '_.escape(message)', 'rule': None},
- {'template': 'anything.escape(message)', 'rule': JAVASCRIPT_LINTER_RULESET.javascript_escape},
+ {'template': 'anything.escape(message)', 'rule': None},
+ {'template': 'anythingescape(message)', 'rule': None},
+ {'template': '$escape(message)', 'rule': None},
+ {'template': '_escape(message)', 'rule': None},
+ {'template': 'escape(message)', 'rule': JAVASCRIPT_LINTER_RULESET.javascript_escape},
+ {'template': '(escape(message))', 'rule': JAVASCRIPT_LINTER_RULESET.javascript_escape},
+ {'template': ' escape(message))', 'rule': JAVASCRIPT_LINTER_RULESET.javascript_escape},
+ {'template': 'window.escape(message)', 'rule': JAVASCRIPT_LINTER_RULESET.javascript_escape},
+ {'template': '(window.escape(message)', 'rule': JAVASCRIPT_LINTER_RULESET.javascript_escape},
)
- def test_javascript_interpolate(self, data):
+ def test_javascript_escape(self, data):
"""
Test check_javascript_file_is_safe with interpolate()
"""
diff --git a/scripts/xsslint/xsslint/linters.py b/scripts/xsslint/xsslint/linters.py
index d345483f0d0fa5dd913db589495fc778eee06855..50ef4b0ba468f6178bc0d4d58b619d66fd9352f9 100644
--- a/scripts/xsslint/xsslint/linters.py
+++ b/scripts/xsslint/xsslint/linters.py
@@ -329,7 +329,6 @@ class JavaScriptLinter(BaseLinter):
javascript_jquery_html='javascript-jquery-html',
javascript_concat_html='javascript-concat-html',
javascript_escape='javascript-escape',
- javascript_interpolate='javascript-interpolate',
)
def __init__(self, underscore_linter, javascript_skip_dirs=None):
@@ -401,7 +400,6 @@ class JavaScriptLinter(BaseLinter):
file_contents, "html", self.ruleset.javascript_jquery_html, no_caller_check,
self._is_jquery_html_argument_safe, results
)
- self._check_javascript_interpolate(file_contents, results)
self._check_javascript_escape(file_contents, results)
self._check_concat_with_html(file_contents, self.ruleset.javascript_concat_html, results)
self.underscore_linter.check_underscore_file_is_safe(file_contents, results)
@@ -435,37 +433,18 @@ class JavaScriptLinter(BaseLinter):
expression = Expression(start_index)
return expression
- def _check_javascript_interpolate(self, file_contents, results):
- """
- Checks that interpolate() calls are safe.
-
- Only use of StringUtils.interpolate() or HtmlUtils.interpolateText()
- are safe.
-
- Arguments:
- file_contents: The contents of the JavaScript file.
- results: A file results objects to which violations will be added.
-
- """
- # Ignores calls starting with "StringUtils.", because those are safe
- regex = re.compile(r"(?<!StringUtils).interpolate\(")
- for function_match in regex.finditer(file_contents):
- expression = self._get_expression_for_function(file_contents, function_match)
- results.violations.append(ExpressionRuleViolation(self.ruleset.javascript_interpolate, expression))
-
def _check_javascript_escape(self, file_contents, results):
"""
- Checks that only necessary escape() are used.
-
- Allows for _.escape(), although this shouldn't be the recommendation.
+ Checks that escape() is not used. escape() is not recommended.
+ ref. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/escape
Arguments:
file_contents: The contents of the JavaScript file.
results: A file results objects to which violations will be added.
"""
- # Ignores calls starting with "_.", because those are safe
- regex = regex = re.compile(r"(?<!_).escape\(")
+ # Regex to match uses of escape() or window.escape().
+ regex = re.compile(r"(?:^|(?<=window\.)|(?<![\w.$]))escape\(")
for function_match in regex.finditer(file_contents):
expression = self._get_expression_for_function(file_contents, function_match)
results.violations.append(ExpressionRuleViolation(self.ruleset.javascript_escape, expression))
diff --git a/scripts/xsslint_thresholds.json b/scripts/xsslint_thresholds.json
index f1e89e8adb116ebc4af9048950416e30be9670c9..bd51cf7943a3f9202284019d551836cb8f3f9295 100644
--- a/scripts/xsslint_thresholds.json
+++ b/scripts/xsslint_thresholds.json
@@ -2,7 +2,6 @@
"rules": {
"javascript-concat-html": 100,
"javascript-escape": 5,
- "javascript-interpolate": 9,
"javascript-jquery-append": 50,
"javascript-jquery-html": 112,
"javascript-jquery-insert-into-target": 18,