From 2f9819f2471cdb50abb9ca2854f1bc7330ca5b9b Mon Sep 17 00:00:00 2001 From: Matt Hughes Date: Thu, 7 Feb 2019 13:54:52 -0500 Subject: [PATCH] Improve XSS lint for underscore templates See also https://edx.readthedocs.io/projects/edx-developer-guide/en/latest/conventions/preventing_xss.html#javascript-edx-namespace The failure totals in test_main appear to've been wrong previously; I'm just updating them to run clean via `pytest scripts/xsslint/tests` --- scripts/xsslint/tests/test_linters.py | 1 + scripts/xsslint/tests/test_main.py | 6 +++--- scripts/xsslint/xsslint/linters.py | 3 +++ 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/scripts/xsslint/tests/test_linters.py b/scripts/xsslint/tests/test_linters.py index bc7f560fbb..069b45656b 100644 --- a/scripts/xsslint/tests/test_linters.py +++ b/scripts/xsslint/tests/test_linters.py @@ -190,6 +190,7 @@ class TestUnderscoreTemplateLinter(TestLinter): self.assertEqual(results.violations[1].is_disabled, False) @data( + {'template': '<%= edx.HtmlUtils.ensureHtml(message) %>'}, {'template': '<%= HtmlUtils.ensureHtml(message) %>'}, {'template': '<%= _.escape(message) %>'}, ) diff --git a/scripts/xsslint/tests/test_main.py b/scripts/xsslint/tests/test_main.py index e8e272131b..c0cef69bfe 100644 --- a/scripts/xsslint/tests/test_main.py +++ b/scripts/xsslint/tests/test_main.py @@ -93,7 +93,7 @@ class TestXSSLinter(TestCase): # Assert no rule totals. self.assertIsNone(re.search(r'{}:\s*{} violations'.format(self.ruleset.python_parse_error.rule_id, 0), output)) # Assert final total - self.assertIsNotNone(re.search(r'{} violations total'.format(7), output)) + self.assertIsNotNone(re.search(r'{} violations total'.format(5), output)) def test_lint_with_verbose(self): """ @@ -125,7 +125,7 @@ class TestXSSLinter(TestCase): # Assert no rule totals. self.assertIsNone(re.search(r'{}:\s*{} violations'.format(self.ruleset.python_parse_error.rule_id, 0), output)) # Assert final total - self.assertIsNotNone(re.search(r'{} violations total'.format(7), output)) + self.assertIsNotNone(re.search(r'{} violations total'.format(5), output)) def test_lint_with_rule_totals(self): """ @@ -150,7 +150,7 @@ class TestXSSLinter(TestCase): # Assert totals output. self.assertIsNotNone(re.search(r'{}:\s*{} violations'.format(self.ruleset.python_parse_error.rule_id, 0), output)) self.assertIsNotNone(re.search(r'{}:\s*{} violations'.format(self.ruleset.python_wrap_html.rule_id, 1), output)) - self.assertIsNotNone(re.search(r'{} violations total'.format(7), output)) + self.assertIsNotNone(re.search(r'{} violations total'.format(5), output)) def test_lint_with_list_files(self): """ diff --git a/scripts/xsslint/xsslint/linters.py b/scripts/xsslint/xsslint/linters.py index 953cc5d8b2..840b2fce98 100644 --- a/scripts/xsslint/xsslint/linters.py +++ b/scripts/xsslint/xsslint/linters.py @@ -268,6 +268,7 @@ class UnderscoreTemplateLinter(BaseLinter): Safe examples:: + <%= edx.HtmlUtils.ensureHtml(message) %> <%= HtmlUtils.ensureHtml(message) %> <%= _.escape(message) %> @@ -278,6 +279,8 @@ class UnderscoreTemplateLinter(BaseLinter): True if the Expression has been safely escaped, and False otherwise. """ + if expression.expression_inner.startswith('edx.HtmlUtils.'): + return True if expression.expression_inner.startswith('HtmlUtils.'): return True if expression.expression_inner.startswith('_.escape('):