From edc98939b559b1ebed2d810dcc9afbc5d45c723d Mon Sep 17 00:00:00 2001 From: Anthony Mangano Date: Thu, 22 Mar 2018 12:07:12 -0400 Subject: [PATCH] Define XSS-linter rules on Linter classes. --- scripts/xsslint/tests/test_linters.py | 223 ++++++++++++++------------ scripts/xsslint/tests/test_main.py | 83 +++++----- scripts/xsslint/xsslint/linters.py | 105 ++++++++---- scripts/xsslint/xsslint/main.py | 21 ++- scripts/xsslint/xsslint/reporting.py | 8 +- scripts/xsslint/xsslint/rules.py | 57 +++---- scripts/xsslint/xsslint/visitors.py | 24 ++- 7 files changed, 292 insertions(+), 229 deletions(-) diff --git a/scripts/xsslint/tests/test_linters.py b/scripts/xsslint/tests/test_linters.py index bbe6937bab..bc7f560fbb 100644 --- a/scripts/xsslint/tests/test_linters.py +++ b/scripts/xsslint/tests/test_linters.py @@ -11,7 +11,6 @@ from ddt import data, ddt from xsslint.linters import JavaScriptLinter, MakoTemplateLinter, PythonLinter, UnderscoreTemplateLinter from xsslint.reporting import FileResults -from xsslint.rules import Rules from xsslint.utils import ParseString @@ -64,6 +63,8 @@ class TestUnderscoreTemplateLinter(TestLinter): Test UnderscoreTemplateLinter """ + ruleset = UnderscoreTemplateLinter.ruleset + def test_check_underscore_file_is_safe(self): """ Test check_underscore_file_is_safe with safe template @@ -101,8 +102,8 @@ class TestUnderscoreTemplateLinter(TestLinter): linter.check_underscore_file_is_safe(template, results) self.assertEqual(len(results.violations), 2) - self.assertEqual(results.violations[0].rule, Rules.underscore_not_escaped) - self.assertEqual(results.violations[1].rule, Rules.underscore_not_escaped) + self.assertEqual(results.violations[0].rule, self.ruleset.underscore_not_escaped) + self.assertEqual(results.violations[1].rule, self.ruleset.underscore_not_escaped) @data( { @@ -210,6 +211,9 @@ class TestJavaScriptLinter(TestLinter): """ Test JavaScriptLinter """ + + ruleset = JavaScriptLinter.ruleset + UnderscoreTemplateLinter.ruleset + @data( {'template': 'var m = "Plain text " + message + "plain text"', 'rule': None}, {'template': 'var m = "檌檒濦 " + message + "plain text"', 'rule': None}, @@ -219,14 +223,14 @@ class TestJavaScriptLinter(TestLinter): """ value: gettext("Copy Email To Editor"), id: 'copy_email_' + email_id))"""), 'rule': None }, - {'template': 'var m = "

" + message + "

"', 'rule': Rules.javascript_concat_html}, + {'template': 'var m = "

" + message + "

"', 'rule': ruleset.javascript_concat_html}, { 'template': r'var m = "

\"escaped quote\"" + message + "\"escaped quote\"

"', - 'rule': Rules.javascript_concat_html + 'rule': ruleset.javascript_concat_html }, {'template': ' // var m = "

" + commentedOutMessage + "

"', 'rule': None}, - {'template': 'var m = "

" + message + "

"', 'rule': Rules.javascript_concat_html}, - {'template': 'var m = "

" + message + " broken string', 'rule': Rules.javascript_concat_html}, + {'template': 'var m = "

" + message + "

"', 'rule': ruleset.javascript_concat_html}, + {'template': 'var m = "

" + message + " broken string', 'rule': ruleset.javascript_concat_html}, ) def test_concat_with_html(self, data): """ @@ -247,16 +251,16 @@ class TestJavaScriptLinter(TestLinter): # plain text is ok because any & will be escaped, and it stops false # negatives on some other objects with an append() method {'template': 'test.append("plain text")', 'rule': None}, - {'template': 'test.append("

")', 'rule': Rules.javascript_jquery_append}, + {'template': 'test.append("
")', 'rule': ruleset.javascript_jquery_append}, {'template': 'graph.svg.append("g")', 'rule': None}, {'template': 'test.append( $( "
" ) )', 'rule': None}, {'template': 'test.append($("
"))', 'rule': None}, {'template': 'test.append($("
"))', 'rule': None}, {'template': 'test.append(HtmlUtils.ensureHtml(htmlSnippet).toString())', 'rule': None}, {'template': 'HtmlUtils.append($el, someHtml)', 'rule': None}, - {'template': 'test.append("fail on concat" + test.render().el)', 'rule': Rules.javascript_jquery_append}, - {'template': 'test.append("fail on concat" + testEl)', 'rule': Rules.javascript_jquery_append}, - {'template': 'test.append(message)', 'rule': Rules.javascript_jquery_append}, + {'template': 'test.append("fail on concat" + test.render().el)', 'rule': ruleset.javascript_jquery_append}, + {'template': 'test.append("fail on concat" + testEl)', 'rule': ruleset.javascript_jquery_append}, + {'template': 'test.append(message)', 'rule': ruleset.javascript_jquery_append}, ) def test_jquery_append(self, data): """ @@ -281,10 +285,10 @@ class TestJavaScriptLinter(TestLinter): {'template': 'test.prepend($("
"))', 'rule': None}, {'template': 'test.prepend(HtmlUtils.ensureHtml(htmlSnippet).toString())', 'rule': None}, {'template': 'HtmlUtils.prepend($el, someHtml)', 'rule': None}, - {'template': 'test.prepend("broken string)', 'rule': Rules.javascript_jquery_prepend}, - {'template': 'test.prepend("fail on concat" + test.render().el)', 'rule': Rules.javascript_jquery_prepend}, - {'template': 'test.prepend("fail on concat" + testEl)', 'rule': Rules.javascript_jquery_prepend}, - {'template': 'test.prepend(message)', 'rule': Rules.javascript_jquery_prepend}, + {'template': 'test.prepend("broken string)', 'rule': ruleset.javascript_jquery_prepend}, + {'template': 'test.prepend("fail on concat" + test.render().el)', 'rule': ruleset.javascript_jquery_prepend}, + {'template': 'test.prepend("fail on concat" + testEl)', 'rule': ruleset.javascript_jquery_prepend}, + {'template': 'test.prepend(message)', 'rule': ruleset.javascript_jquery_prepend}, ) def test_jquery_prepend(self, data): """ @@ -307,14 +311,14 @@ class TestJavaScriptLinter(TestLinter): {'template': 'test.replaceAll(HtmlUtils.ensureHtml(htmlSnippet).toString())', 'rule': None}, {'template': 'test.replaceWith(HtmlUtils.ensureHtml(htmlSnippet).toString())', 'rule': None}, {'template': 'test.replaceWith(edx.HtmlUtils.HTML(htmlString).toString())', 'rule': None}, - {'template': 'test.unwrap(anything)', 'rule': Rules.javascript_jquery_insertion}, - {'template': 'test.wrap(anything)', 'rule': Rules.javascript_jquery_insertion}, - {'template': 'test.wrapAll(anything)', 'rule': Rules.javascript_jquery_insertion}, - {'template': 'test.wrapInner(anything)', 'rule': Rules.javascript_jquery_insertion}, - {'template': 'test.after(anything)', 'rule': Rules.javascript_jquery_insertion}, - {'template': 'test.before(anything)', 'rule': Rules.javascript_jquery_insertion}, - {'template': 'test.replaceAll(anything)', 'rule': Rules.javascript_jquery_insertion}, - {'template': 'test.replaceWith(anything)', 'rule': Rules.javascript_jquery_insertion}, + {'template': 'test.unwrap(anything)', 'rule': ruleset.javascript_jquery_insertion}, + {'template': 'test.wrap(anything)', 'rule': ruleset.javascript_jquery_insertion}, + {'template': 'test.wrapAll(anything)', 'rule': ruleset.javascript_jquery_insertion}, + {'template': 'test.wrapInner(anything)', 'rule': ruleset.javascript_jquery_insertion}, + {'template': 'test.after(anything)', 'rule': ruleset.javascript_jquery_insertion}, + {'template': 'test.before(anything)', 'rule': ruleset.javascript_jquery_insertion}, + {'template': 'test.replaceAll(anything)', 'rule': ruleset.javascript_jquery_insertion}, + {'template': 'test.replaceWith(anything)', 'rule': ruleset.javascript_jquery_insertion}, ) def test_jquery_insertion(self, data): """ @@ -341,11 +345,11 @@ class TestJavaScriptLinter(TestLinter): {'template': 'testEl.prependTo(target);', 'rule': None}, {'template': 'testEl.insertAfter(target);', 'rule': None}, {'template': 'testEl.insertBefore(target);', 'rule': None}, - {'template': 'anycall().appendTo(target)', 'rule': Rules.javascript_jquery_insert_into_target}, - {'template': 'anything.appendTo(target)', 'rule': Rules.javascript_jquery_insert_into_target}, - {'template': 'anything.prependTo(target)', 'rule': Rules.javascript_jquery_insert_into_target}, - {'template': 'anything.insertAfter(target)', 'rule': Rules.javascript_jquery_insert_into_target}, - {'template': 'anything.insertBefore(target)', 'rule': Rules.javascript_jquery_insert_into_target}, + {'template': 'anycall().appendTo(target)', 'rule': ruleset.javascript_jquery_insert_into_target}, + {'template': 'anything.appendTo(target)', 'rule': ruleset.javascript_jquery_insert_into_target}, + {'template': 'anything.prependTo(target)', 'rule': ruleset.javascript_jquery_insert_into_target}, + {'template': 'anything.insertAfter(target)', 'rule': ruleset.javascript_jquery_insert_into_target}, + {'template': 'anything.insertBefore(target)', 'rule': ruleset.javascript_jquery_insert_into_target}, ) def test_jquery_insert_to_target(self, data): """ @@ -368,10 +372,10 @@ class TestJavaScriptLinter(TestLinter): {'template': 'test.html("")', 'rule': None}, {'template': 'test.html(HtmlUtils.ensureHtml(htmlSnippet).toString())', 'rule': None}, {'template': 'HtmlUtils.setHtml($el, someHtml)', 'rule': None}, - {'template': 'test.html("any string")', 'rule': Rules.javascript_jquery_html}, - {'template': 'test.html("broken string)', 'rule': Rules.javascript_jquery_html}, - {'template': 'test.html("檌檒濦")', 'rule': Rules.javascript_jquery_html}, - {'template': 'test.html(anything)', 'rule': Rules.javascript_jquery_html}, + {'template': 'test.html("any string")', 'rule': ruleset.javascript_jquery_html}, + {'template': 'test.html("broken string)', 'rule': ruleset.javascript_jquery_html}, + {'template': 'test.html("檌檒濦")', 'rule': ruleset.javascript_jquery_html}, + {'template': 'test.html(anything)', 'rule': ruleset.javascript_jquery_html}, ) def test_jquery_html(self, data): """ @@ -386,7 +390,7 @@ class TestJavaScriptLinter(TestLinter): @data( {'template': 'StringUtils.interpolate()', 'rule': None}, {'template': 'HtmlUtils.interpolateHtml()', 'rule': None}, - {'template': 'interpolate(anything)', 'rule': Rules.javascript_interpolate}, + {'template': 'interpolate(anything)', 'rule': ruleset.javascript_interpolate}, ) def test_javascript_interpolate(self, data): """ @@ -401,7 +405,7 @@ class TestJavaScriptLinter(TestLinter): @data( {'template': '_.escape(message)', 'rule': None}, - {'template': 'anything.escape(message)', 'rule': Rules.javascript_escape}, + {'template': 'anything.escape(message)', 'rule': ruleset.javascript_escape}, ) def test_javascript_interpolate(self, data): """ @@ -420,13 +424,16 @@ class TestPythonLinter(TestLinter): """ Test PythonLinter """ + + ruleset = PythonLinter.ruleset + @data( {'template': 'm = "Plain text " + message + "plain text"', 'rule': None}, {'template': 'm = "檌檒濦 " + message + "plain text"', 'rule': None}, {'template': ' # m = "

" + commentedOutMessage + "

"', 'rule': None}, - {'template': 'm = "

" + message + "

"', 'rule': [Rules.python_concat_html, Rules.python_concat_html]}, - {'template': 'm = "

" + message + "

"', 'rule': [Rules.python_concat_html, Rules.python_concat_html]}, - {'template': 'm = "

" + message + " broken string', 'rule': Rules.python_parse_error}, + {'template': 'm = "

" + message + "

"', 'rule': [ruleset.python_concat_html, ruleset.python_concat_html]}, + {'template': 'm = "

" + message + "

"', 'rule': [ruleset.python_concat_html, ruleset.python_concat_html]}, + {'template': 'm = "

" + message + " broken string', 'rule': ruleset.python_parse_error}, ) def test_concat_with_html(self, data): """ @@ -456,7 +463,7 @@ class TestPythonLinter(TestLinter): linter.check_python_file_is_safe(python_file, results) self.assertEqual(len(results.violations), 1) - self.assertEqual(results.violations[0].rule, Rules.python_deprecated_display_name) + self.assertEqual(results.violations[0].rule, self.ruleset.python_deprecated_display_name) def test_check_custom_escaping(self): """ @@ -472,7 +479,7 @@ class TestPythonLinter(TestLinter): linter.check_python_file_is_safe(python_file, results) self.assertEqual(len(results.violations), 1) - self.assertEqual(results.violations[0].rule, Rules.python_custom_escape) + self.assertEqual(results.violations[0].rule, self.ruleset.python_custom_escape) @data( { @@ -493,7 +500,7 @@ class TestPythonLinter(TestLinter): span_end=HTML(""), ) """), - 'rule': Rules.python_requires_html_or_text + 'rule': ruleset.python_requires_html_or_text }, { 'python': @@ -504,7 +511,7 @@ class TestPythonLinter(TestLinter): span_end=HTML(""), ) """), - 'rule': Rules.python_requires_html_or_text + 'rule': ruleset.python_requires_html_or_text }, { 'python': @@ -514,7 +521,7 @@ class TestPythonLinter(TestLinter): link_end=HTML(""), )) """), - 'rule': [Rules.python_close_before_format, Rules.python_requires_html_or_text] + 'rule': [ruleset.python_close_before_format, ruleset.python_requires_html_or_text] }, { 'python': @@ -524,7 +531,7 @@ class TestPythonLinter(TestLinter): link_end=HTML(""), ) """), - 'rule': Rules.python_close_before_format + 'rule': ruleset.python_close_before_format }, { 'python': @@ -536,10 +543,10 @@ class TestPythonLinter(TestLinter): """), 'rule': [ - Rules.python_close_before_format, - Rules.python_requires_html_or_text, - Rules.python_close_before_format, - Rules.python_requires_html_or_text + ruleset.python_close_before_format, + ruleset.python_requires_html_or_text, + ruleset.python_close_before_format, + ruleset.python_requires_html_or_text ] }, { @@ -550,7 +557,7 @@ class TestPythonLinter(TestLinter): span_end="", ) """), - 'rule': [Rules.python_wrap_html, Rules.python_wrap_html] + 'rule': [ruleset.python_wrap_html, ruleset.python_wrap_html] }, { 'python': @@ -581,11 +588,11 @@ class TestPythonLinter(TestLinter): }, { 'python': r"""msg = '\d+)?$'.format(test)""", @@ -593,7 +600,7 @@ class TestPythonLinter(TestLinter): }, { 'python': r"""msg = ''", - 'rule': Rules.python_parse_error + 'rule': ruleset.python_parse_error }, ) def test_check_python_with_text_and_html(self, data): @@ -651,11 +658,11 @@ class TestPythonLinter(TestLinter): results.violations.sort(key=lambda violation: violation.sort_key()) self.assertEqual(len(results.violations), 5) - self.assertEqual(results.violations[0].rule, Rules.python_wrap_html) - self.assertEqual(results.violations[1].rule, Rules.python_requires_html_or_text) - self.assertEqual(results.violations[2].rule, Rules.python_close_before_format) - self.assertEqual(results.violations[3].rule, Rules.python_wrap_html) - self.assertEqual(results.violations[4].rule, Rules.python_interpolate_html) + self.assertEqual(results.violations[0].rule, self.ruleset.python_wrap_html) + self.assertEqual(results.violations[1].rule, self.ruleset.python_requires_html_or_text) + self.assertEqual(results.violations[2].rule, self.ruleset.python_close_before_format) + self.assertEqual(results.violations[3].rule, self.ruleset.python_wrap_html) + self.assertEqual(results.violations[4].rule, self.ruleset.python_interpolate_html) @data( { @@ -667,7 +674,7 @@ class TestPythonLinter(TestLinter):

''').format(response=response_text) """, - 'rule': Rules.python_wrap_html, + 'rule': ruleset.python_wrap_html, 'start_line': 2, }, { @@ -683,7 +690,7 @@ class TestPythonLinter(TestLinter):
''').format(response=response_text) """, - 'rule': Rules.python_wrap_html, + 'rule': ruleset.python_wrap_html, 'start_line': 6, }, { @@ -695,7 +702,7 @@ class TestPythonLinter(TestLinter): ''' response_str = '''

{response}

'''.format(response=response_text) """, - 'rule': Rules.python_wrap_html, + 'rule': ruleset.python_wrap_html, 'start_line': 6, }, { @@ -712,7 +719,7 @@ class TestPythonLinter(TestLinter):
''').format(response=response_text) """, - 'rule': Rules.python_wrap_html, + 'rule': ruleset.python_wrap_html, 'start_line': 6, }, ) @@ -738,6 +745,10 @@ class TestMakoTemplateLinter(TestLinter): Test MakoTemplateLinter """ + ruleset = ( + MakoTemplateLinter.ruleset + PythonLinter.ruleset + JavaScriptLinter.ruleset + UnderscoreTemplateLinter.ruleset + ) + @data( {'directory': 'lms/templates', 'expected': True}, {'directory': 'lms/templates/support', 'expected': True}, @@ -767,13 +778,13 @@ class TestMakoTemplateLinter(TestLinter): }, { 'template': '\n ## <%page expression_filter="h"/>', - 'rule': Rules.mako_missing_default + 'rule': ruleset.mako_missing_default }, { 'template': '\n <%page expression_filter="h" /> ' '\n <%page args="section_data"/>', - 'rule': Rules.mako_multiple_page_tags + 'rule': ruleset.mako_multiple_page_tags }, { 'template': @@ -783,16 +794,16 @@ class TestMakoTemplateLinter(TestLinter): }, { 'template': '\n <%page args="section_data" /> ', - 'rule': Rules.mako_missing_default + 'rule': ruleset.mako_missing_default }, { 'template': '\n <%page args="section_data"/> ', - 'rule': Rules.mako_missing_default + 'rule': ruleset.mako_missing_default }, { 'template': '\n', - 'rule': Rules.mako_missing_default + 'rule': ruleset.mako_missing_default }, ) def test_check_page_default(self, data): @@ -811,12 +822,12 @@ class TestMakoTemplateLinter(TestLinter): @data( {'expression': '${x}', 'rule': None}, - {'expression': '${{unbalanced}', 'rule': Rules.mako_unparseable_expression}, - {'expression': '${x | n}', 'rule': Rules.mako_invalid_html_filter}, + {'expression': '${{unbalanced}', 'rule': ruleset.mako_unparseable_expression}, + {'expression': '${x | n}', 'rule': ruleset.mako_invalid_html_filter}, {'expression': '${x | n, decode.utf8}', 'rule': None}, - {'expression': '${x | h}', 'rule': Rules.mako_unwanted_html_filter}, + {'expression': '${x | h}', 'rule': ruleset.mako_unwanted_html_filter}, {'expression': ' ## ${commented_out | h}', 'rule': None}, - {'expression': '${x | n, dump_js_escaped_json}', 'rule': Rules.mako_invalid_html_filter}, + {'expression': '${x | n, dump_js_escaped_json}', 'rule': ruleset.mako_invalid_html_filter}, ) def test_check_mako_expressions_in_html(self, data): """ @@ -850,7 +861,7 @@ class TestMakoTemplateLinter(TestLinter): linter._check_mako_file_is_safe(mako_template, results) self.assertEqual(len(results.violations), 1) - self.assertEqual(results.violations[0].rule, Rules.python_deprecated_display_name) + self.assertEqual(results.violations[0].rule, self.ruleset.python_deprecated_display_name) @data( { @@ -867,7 +878,7 @@ class TestMakoTemplateLinter(TestLinter): text = _("Introductions, prerequisites, FAQs that are used on %s (formatted in HTML)") % a_link %> """), - 'rule': [Rules.python_wrap_html, Rules.python_concat_html, Rules.python_wrap_html] + 'rule': [ruleset.python_wrap_html, ruleset.python_concat_html, ruleset.python_wrap_html] }, { 'expression': @@ -877,7 +888,7 @@ class TestMakoTemplateLinter(TestLinter): span_end=HTML(""), )} """), - 'rule': Rules.python_requires_html_or_text + 'rule': ruleset.python_requires_html_or_text }, { 'expression': @@ -898,7 +909,7 @@ class TestMakoTemplateLinter(TestLinter): span_end=HTML(""), )} """), - 'rule': Rules.python_requires_html_or_text + 'rule': ruleset.python_requires_html_or_text }, { 'expression': @@ -908,7 +919,7 @@ class TestMakoTemplateLinter(TestLinter): link_end=HTML(""), ))} """), - 'rule': [Rules.python_close_before_format, Rules.python_requires_html_or_text] + 'rule': [ruleset.python_close_before_format, ruleset.python_requires_html_or_text] }, { 'expression': @@ -918,7 +929,7 @@ class TestMakoTemplateLinter(TestLinter): link_end=HTML(""), )} """), - 'rule': Rules.python_close_before_format + 'rule': ruleset.python_close_before_format }, { 'expression': @@ -928,7 +939,7 @@ class TestMakoTemplateLinter(TestLinter): span_end="", )} """), - 'rule': [Rules.python_wrap_html, Rules.python_wrap_html] + 'rule': [ruleset.python_wrap_html, ruleset.python_wrap_html] }, { 'expression': @@ -950,11 +961,11 @@ class TestMakoTemplateLinter(TestLinter): }, { 'expression': "${''}", - 'rule': Rules.python_wrap_html + 'rule': ruleset.python_wrap_html }, { 'expression': "${'Embedded HTML '}", - 'rule': Rules.python_wrap_html + 'rule': ruleset.python_wrap_html }, { 'expression': "${ HTML('') }", @@ -966,19 +977,19 @@ class TestMakoTemplateLinter(TestLinter): }, { 'expression': "${ '' + 'some other text' }", - 'rule': [Rules.python_concat_html, Rules.python_wrap_html] + 'rule': [ruleset.python_concat_html, ruleset.python_wrap_html] }, { 'expression': "${ HTML('missing closing parentheses.' }", - 'rule': Rules.python_parse_error + 'rule': ruleset.python_parse_error }, { 'expression': "${'Rock & Roll'}", - 'rule': Rules.mako_html_entities + 'rule': ruleset.mako_html_entities }, { 'expression': "${'Rock & Roll'}", - 'rule': Rules.mako_html_entities + 'rule': ruleset.mako_html_entities }, ) def test_check_mako_with_text_and_html(self, data): @@ -1010,7 +1021,7 @@ class TestMakoTemplateLinter(TestLinter): linter._check_mako_file_is_safe(mako_template, results) self.assertEqual(len(results.violations), 1) - self.assertEqual(results.violations[0].rule, Rules.mako_missing_default) + self.assertEqual(results.violations[0].rule, self.ruleset.mako_missing_default) def test_check_mako_expression_default_disabled(self): """ @@ -1099,13 +1110,13 @@ class TestMakoTemplateLinter(TestLinter): linter._check_mako_file_is_safe(mako_template, results) self.assertEqual(len(results.violations), 1) - self.assertEqual(results.violations[0].rule, Rules.mako_missing_default) + self.assertEqual(results.violations[0].rule, self.ruleset.mako_missing_default) @data( - {'expression': '${x}', 'rule': Rules.mako_invalid_js_filter}, - {'expression': '${{unbalanced}', 'rule': Rules.mako_unparseable_expression}, - {'expression': '${x | n}', 'rule': Rules.mako_invalid_js_filter}, - {'expression': '${x | h}', 'rule': Rules.mako_invalid_js_filter}, + {'expression': '${x}', 'rule': ruleset.mako_invalid_js_filter}, + {'expression': '${{unbalanced}', 'rule': ruleset.mako_unparseable_expression}, + {'expression': '${x | n}', 'rule': ruleset.mako_invalid_js_filter}, + {'expression': '${x | h}', 'rule': ruleset.mako_invalid_js_filter}, {'expression': '${x | n, dump_js_escaped_json}', 'rule': None}, {'expression': '${x | n, decode.utf8}', 'rule': None}, ) @@ -1132,7 +1143,7 @@ class TestMakoTemplateLinter(TestLinter): self._validate_data_rules(data, results) @data( - {'expression': '${x}', 'rule': Rules.mako_invalid_js_filter}, + {'expression': '${x}', 'rule': ruleset.mako_invalid_js_filter}, {'expression': '"${x | n, js_escaped_string}"', 'rule': None}, ) def test_check_mako_expressions_in_require_module(self, data): @@ -1158,7 +1169,7 @@ class TestMakoTemplateLinter(TestLinter): self._validate_data_rules(data, results) @data( - {'expression': '${x}', 'rule': Rules.mako_invalid_js_filter}, + {'expression': '${x}', 'rule': ruleset.mako_invalid_js_filter}, {'expression': '"${x | n, js_escaped_string}"', 'rule': None}, ) def test_check_mako_expressions_in_require_js(self, data): @@ -1190,8 +1201,8 @@ class TestMakoTemplateLinter(TestLinter): {'media_type': 'application/javascript', 'rule': None}, {'media_type': 'text/x-mathjax-config', 'rule': None}, {'media_type': 'json/xblock-args', 'rule': None}, - {'media_type': 'text/template', 'rule': Rules.mako_invalid_html_filter}, - {'media_type': 'unknown/type', 'rule': Rules.mako_unknown_context}, + {'media_type': 'text/template', 'rule': ruleset.mako_invalid_html_filter}, + {'media_type': 'unknown/type', 'rule': ruleset.mako_unknown_context}, ) def test_check_mako_expressions_in_script_type(self, data): """ @@ -1242,13 +1253,13 @@ class TestMakoTemplateLinter(TestLinter): linter._check_mako_file_is_safe(mako_template, results) self.assertEqual(len(results.violations), 7) - self.assertEqual(results.violations[0].rule, Rules.mako_unwanted_html_filter) - self.assertEqual(results.violations[1].rule, Rules.mako_invalid_js_filter) - self.assertEqual(results.violations[2].rule, Rules.mako_unwanted_html_filter) - self.assertEqual(results.violations[3].rule, Rules.mako_invalid_js_filter) - self.assertEqual(results.violations[4].rule, Rules.mako_unwanted_html_filter) - self.assertEqual(results.violations[5].rule, Rules.mako_invalid_js_filter) - self.assertEqual(results.violations[6].rule, Rules.mako_unwanted_html_filter) + self.assertEqual(results.violations[0].rule, self.ruleset.mako_unwanted_html_filter) + self.assertEqual(results.violations[1].rule, self.ruleset.mako_invalid_js_filter) + self.assertEqual(results.violations[2].rule, self.ruleset.mako_unwanted_html_filter) + self.assertEqual(results.violations[3].rule, self.ruleset.mako_invalid_js_filter) + self.assertEqual(results.violations[4].rule, self.ruleset.mako_unwanted_html_filter) + self.assertEqual(results.violations[5].rule, self.ruleset.mako_invalid_js_filter) + self.assertEqual(results.violations[6].rule, self.ruleset.mako_unwanted_html_filter) def test_check_mako_expressions_javascript_strings(self): """ @@ -1282,9 +1293,9 @@ class TestMakoTemplateLinter(TestLinter): linter._check_mako_file_is_safe(mako_template, results) self.assertEqual(len(results.violations), 3) - self.assertEqual(results.violations[0].rule, Rules.mako_js_missing_quotes) - self.assertEqual(results.violations[1].rule, Rules.mako_js_html_string) - self.assertEqual(results.violations[2].rule, Rules.mako_js_html_string) + self.assertEqual(results.violations[0].rule, self.ruleset.mako_js_missing_quotes) + self.assertEqual(results.violations[1].rule, self.ruleset.mako_js_html_string) + self.assertEqual(results.violations[2].rule, self.ruleset.mako_js_html_string) def test_check_javascript_in_mako_javascript_context(self): """ @@ -1304,7 +1315,7 @@ class TestMakoTemplateLinter(TestLinter): linter._check_mako_file_is_safe(mako_template, results) self.assertEqual(len(results.violations), 1) - self.assertEqual(results.violations[0].rule, Rules.javascript_concat_html) + self.assertEqual(results.violations[0].rule, self.ruleset.javascript_concat_html) self.assertEqual(results.violations[0].start_line, 4) @data( @@ -1331,7 +1342,7 @@ class TestMakoTemplateLinter(TestLinter): linter._check_mako_file_is_safe(data['template'], results) self.assertEqual(len(results.violations), 2) - self.assertEqual(results.violations[0].rule, Rules.mako_missing_default) + self.assertEqual(results.violations[0].rule, self.ruleset.mako_missing_default) violation = results.violations[1] lines = list(data['template'].splitlines()) diff --git a/scripts/xsslint/tests/test_main.py b/scripts/xsslint/tests/test_main.py index 2d7b735562..d0a6d0ab35 100644 --- a/scripts/xsslint/tests/test_main.py +++ b/scripts/xsslint/tests/test_main.py @@ -10,9 +10,8 @@ from unittest import TestCase import mock from xsslint.linters import JavaScriptLinter, MakoTemplateLinter, PythonLinter, UnderscoreTemplateLinter -from xsslint.main import _lint +from xsslint.main import _lint, _build_ruleset from xsslint.reporting import SummaryResults -from xsslint.rules import Rules class TestXSSLinter(TestCase): @@ -33,6 +32,11 @@ class TestXSSLinter(TestCase): patcher.start() self.addCleanup(patcher.stop) + self.out = StringIO() + self.template_linters = self._build_linters() + self.ruleset = _build_ruleset(self.template_linters) + self.summary_results = SummaryResults(self.ruleset) + def patch_is_valid_directory(self, linter_class): """ Creates a mock patch for _is_valid_directory on a Linter to always @@ -57,42 +61,39 @@ class TestXSSLinter(TestCase): """ Tests the top-level linting with default options. """ - out = StringIO() - summary_results = SummaryResults() - _lint( 'scripts/xsslint/tests/templates', - template_linters=self._build_linters(), + template_linters=self.template_linters, options={ 'list_files': False, 'verbose': False, 'rule_totals': False, 'skip_dirs': () }, - summary_results=summary_results, - out=out, + summary_results=self.summary_results, + out=self.out, ) - output = out.getvalue() + output = self.out.getvalue() # Assert violation details are displayed. - self.assertIsNotNone(re.search('test\.html.*{}'.format(Rules.mako_missing_default.rule_id), output)) - self.assertIsNotNone(re.search('test\.coffee.*{}'.format(Rules.javascript_concat_html.rule_id), output)) - self.assertIsNotNone(re.search('test\.coffee.*{}'.format(Rules.underscore_not_escaped.rule_id), output)) - self.assertIsNotNone(re.search('test\.js.*{}'.format(Rules.javascript_concat_html.rule_id), output)) - self.assertIsNotNone(re.search('test\.js.*{}'.format(Rules.underscore_not_escaped.rule_id), output)) + self.assertIsNotNone(re.search('test\.html.*{}'.format(self.ruleset.mako_missing_default.rule_id), output)) + self.assertIsNotNone(re.search('test\.coffee.*{}'.format(self.ruleset.javascript_concat_html.rule_id), output)) + self.assertIsNotNone(re.search('test\.coffee.*{}'.format(self.ruleset.underscore_not_escaped.rule_id), output)) + self.assertIsNotNone(re.search('test\.js.*{}'.format(self.ruleset.javascript_concat_html.rule_id), output)) + self.assertIsNotNone(re.search('test\.js.*{}'.format(self.ruleset.underscore_not_escaped.rule_id), output)) lines_with_rule = 0 lines_without_rule = 0 # Output with verbose setting only. for underscore_match in re.finditer('test\.underscore:.*\n', output): - if re.search(Rules.underscore_not_escaped.rule_id, underscore_match.group()) is not None: + if re.search(self.ruleset.underscore_not_escaped.rule_id, underscore_match.group()) is not None: lines_with_rule += 1 else: lines_without_rule += 1 self.assertGreaterEqual(lines_with_rule, 1) self.assertEquals(lines_without_rule, 0) - self.assertIsNone(re.search('test\.py.*{}'.format(Rules.python_parse_error.rule_id), output)) - self.assertIsNotNone(re.search('test\.py.*{}'.format(Rules.python_wrap_html.rule_id), output)) + self.assertIsNone(re.search('test\.py.*{}'.format(self.ruleset.python_parse_error.rule_id), output)) + self.assertIsNotNone(re.search('test\.py.*{}'.format(self.ruleset.python_wrap_html.rule_id), output)) # Assert no rule totals. - self.assertIsNone(re.search('{}:\s*{} violations'.format(Rules.python_parse_error.rule_id, 0), output)) + self.assertIsNone(re.search('{}:\s*{} violations'.format(self.ruleset.python_parse_error.rule_id, 0), output)) # Assert final total self.assertIsNotNone(re.search('{} violations total'.format(7), output)) @@ -100,34 +101,31 @@ class TestXSSLinter(TestCase): """ Tests the top-level linting with verbose option. """ - out = StringIO() - summary_results = SummaryResults() - _lint( 'scripts/xsslint/tests/templates', - template_linters=self._build_linters(), + template_linters=self.template_linters, options={ 'list_files': False, 'verbose': True, 'rule_totals': False, 'skip_dirs': () }, - summary_results=summary_results, - out=out, + summary_results=self.summary_results, + out=self.out, ) - output = out.getvalue() + output = self.out.getvalue() lines_with_rule = 0 lines_without_rule = 0 # Output with verbose setting only. for underscore_match in re.finditer('test\.underscore:.*\n', output): - if re.search(Rules.underscore_not_escaped.rule_id, underscore_match.group()) is not None: + if re.search(self.ruleset.underscore_not_escaped.rule_id, underscore_match.group()) is not None: lines_with_rule += 1 else: lines_without_rule += 1 self.assertGreaterEqual(lines_with_rule, 1) self.assertGreaterEqual(lines_without_rule, 1) # Assert no rule totals. - self.assertIsNone(re.search('{}:\s*{} violations'.format(Rules.python_parse_error.rule_id, 0), output)) + self.assertIsNone(re.search('{}:\s*{} violations'.format(self.ruleset.python_parse_error.rule_id, 0), output)) # Assert final total self.assertIsNotNone(re.search('{} violations total'.format(7), output)) @@ -135,55 +133,50 @@ class TestXSSLinter(TestCase): """ Tests the top-level linting with rule totals option. """ - out = StringIO() - summary_results = SummaryResults() - _lint( 'scripts/xsslint/tests/templates', - template_linters=self._build_linters(), + template_linters=self.template_linters, options={ 'list_files': False, 'verbose': False, 'rule_totals': True, 'skip_dirs': () }, - summary_results=summary_results, - out=out, + summary_results=self.summary_results, + out=self.out, ) - output = out.getvalue() - self.assertIsNotNone(re.search('test\.py.*{}'.format(Rules.python_wrap_html.rule_id), output)) + output = self.out.getvalue() + self.assertIsNotNone(re.search('test\.py.*{}'.format(self.ruleset.python_wrap_html.rule_id), output)) # Assert totals output. - self.assertIsNotNone(re.search('{}:\s*{} violations'.format(Rules.python_parse_error.rule_id, 0), output)) - self.assertIsNotNone(re.search('{}:\s*{} violations'.format(Rules.python_wrap_html.rule_id, 1), output)) + self.assertIsNotNone(re.search('{}:\s*{} violations'.format(self.ruleset.python_parse_error.rule_id, 0), output)) + self.assertIsNotNone(re.search('{}:\s*{} violations'.format(self.ruleset.python_wrap_html.rule_id, 1), output)) self.assertIsNotNone(re.search('{} violations total'.format(7), output)) def test_lint_with_list_files(self): """ Tests the top-level linting with list files option. """ - out = StringIO() - summary_results = SummaryResults() _lint( 'scripts/xsslint/tests/templates', - template_linters=self._build_linters(), + template_linters=self.template_linters, options={ 'list_files': True, 'verbose': False, 'rule_totals': False, 'skip_dirs': () }, - summary_results=summary_results, - out=out, + summary_results=self.summary_results, + out=self.out, ) - output = out.getvalue() + output = self.out.getvalue() # Assert file with rule is not output. - self.assertIsNone(re.search('test\.py.*{}'.format(Rules.python_wrap_html.rule_id), output)) + self.assertIsNone(re.search('test\.py.*{}'.format(self.ruleset.python_wrap_html.rule_id), output)) # Assert file is output. self.assertIsNotNone(re.search('test\.py', output)) # Assert no totals. - self.assertIsNone(re.search('{}:\s*{} violations'.format(Rules.python_parse_error.rule_id, 0), output)) + self.assertIsNone(re.search('{}:\s*{} violations'.format(self.ruleset.python_parse_error.rule_id, 0), output)) self.assertIsNone(re.search('{} violations total'.format(7), output)) diff --git a/scripts/xsslint/xsslint/linters.py b/scripts/xsslint/xsslint/linters.py index 62c158673a..35953a39a2 100644 --- a/scripts/xsslint/xsslint/linters.py +++ b/scripts/xsslint/xsslint/linters.py @@ -6,10 +6,10 @@ import os import re import textwrap +from xsslint import visitors from xsslint.reporting import ExpressionRuleViolation, FileResults, RuleViolation -from xsslint.rules import Rules +from xsslint.rules import RuleSet from xsslint.utils import Expression, ParseString, StringLines, is_skip_dir -from xsslint.visitors import AllNodeVisitor, HtmlStringVisitor, OuterFormatVisitor class BaseLinter(object): @@ -194,6 +194,11 @@ class UnderscoreTemplateLinter(BaseLinter): """ The linter for Underscore.js template files. """ + + ruleset = RuleSet( + underscore_not_escaped='underscore-not-escaped', + ) + def __init__(self, skip_dirs=None): """ Init method. @@ -250,7 +255,7 @@ class UnderscoreTemplateLinter(BaseLinter): for expression in expressions: if not self._is_safe_unescaped_expression(expression): results.violations.append(ExpressionRuleViolation( - Rules.underscore_not_escaped, expression + self.ruleset.underscore_not_escaped, expression )) def _is_safe_unescaped_expression(self, expression): @@ -309,12 +314,24 @@ class JavaScriptLinter(BaseLinter): LINE_COMMENT_DELIM = "//" + ruleset = RuleSet( + javascript_jquery_append='javascript-jquery-append', + javascript_jquery_prepend='javascript-jquery-prepend', + javascript_jquery_insertion='javascript-jquery-insertion', + javascript_jquery_insert_into_target='javascript-jquery-insert-into-target', + 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, coffeescript_skip_dirs=None): """ Init method. """ super(JavaScriptLinter, self).__init__() self.underscore_linter = underscore_linter + self.ruleset = self.ruleset + self.underscore_linter.ruleset self._skip_javascript_dirs = javascript_skip_dirs or () self._skip_coffeescript_dirs = coffeescript_skip_dirs or () @@ -361,28 +378,28 @@ class JavaScriptLinter(BaseLinter): no_caller_check = None no_argument_check = None self._check_jquery_function( - file_contents, "append", Rules.javascript_jquery_append, no_caller_check, + file_contents, "append", self.ruleset.javascript_jquery_append, no_caller_check, self._is_jquery_argument_safe, results ) self._check_jquery_function( - file_contents, "prepend", Rules.javascript_jquery_prepend, no_caller_check, + file_contents, "prepend", self.ruleset.javascript_jquery_prepend, no_caller_check, self._is_jquery_argument_safe, results ) self._check_jquery_function( file_contents, "unwrap|wrap|wrapAll|wrapInner|after|before|replaceAll|replaceWith", - Rules.javascript_jquery_insertion, no_caller_check, self._is_jquery_argument_safe, results + self.ruleset.javascript_jquery_insertion, no_caller_check, self._is_jquery_argument_safe, results ) self._check_jquery_function( file_contents, "appendTo|prependTo|insertAfter|insertBefore", - Rules.javascript_jquery_insert_into_target, self._is_jquery_insert_caller_safe, no_argument_check, results + self.ruleset.javascript_jquery_insert_into_target, self._is_jquery_insert_caller_safe, no_argument_check, results ) self._check_jquery_function( - file_contents, "html", Rules.javascript_jquery_html, no_caller_check, + 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, Rules.javascript_concat_html, 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) results.prepare_results(file_contents, line_comment_delim=self.LINE_COMMENT_DELIM) @@ -430,7 +447,7 @@ class JavaScriptLinter(BaseLinter): regex = re.compile(r"(?]*expression_filter=(?:"h"|\'h\')[^>]*/>') page_match = page_h_filter_regex.search(mako_template) if not page_match: - results.violations.append(RuleViolation(Rules.mako_missing_default)) + results.violations.append(RuleViolation(self.ruleset.mako_missing_default)) return page_match def _check_mako_expressions(self, mako_template, has_page_default, results): @@ -1019,7 +1064,7 @@ class MakoTemplateLinter(BaseLinter): for expression in expressions: if expression.end_index is None: results.violations.append(ExpressionRuleViolation( - Rules.mako_unparseable_expression, expression + self.ruleset.mako_unparseable_expression, expression )) continue @@ -1128,16 +1173,16 @@ class MakoTemplateLinter(BaseLinter): self.python_linter.check_python_code_is_safe(python_code, root_node, python_results) # Check mako expression specific Python rules. if root_node is not None: - visitor = HtmlStringVisitor(python_code, python_results, True) + visitor = visitors.HtmlStringVisitor(python_code, python_results, True) visitor.visit(root_node) for unsafe_html_string_node in visitor.unsafe_html_string_nodes: python_results.violations.append(ExpressionRuleViolation( - Rules.python_wrap_html, visitor.node_to_expression(unsafe_html_string_node) + self.ruleset.python_wrap_html, visitor.node_to_expression(unsafe_html_string_node) )) if has_page_default: for over_escaped_entity_string_node in visitor.over_escaped_entity_string_nodes: python_results.violations.append(ExpressionRuleViolation( - Rules.mako_html_entities, visitor.node_to_expression(over_escaped_entity_string_node) + self.ruleset.mako_html_entities, visitor.node_to_expression(over_escaped_entity_string_node) )) python_results.prepare_results(python_code, line_comment_delim=self.LINE_COMMENT_DELIM) self._shift_and_add_violations(python_results, start_offset, results) @@ -1183,7 +1228,7 @@ class MakoTemplateLinter(BaseLinter): """ if context == 'unknown': results.violations.append(ExpressionRuleViolation( - Rules.mako_unknown_context, expression + self.ruleset.mako_unknown_context, expression )) return @@ -1202,7 +1247,7 @@ class MakoTemplateLinter(BaseLinter): if filters_match is None: if context == 'javascript': results.violations.append(ExpressionRuleViolation( - Rules.mako_invalid_js_filter, expression + self.ruleset.mako_invalid_js_filter, expression )) return filters = filters_match.group(1).replace(" ", "").split(",") @@ -1215,14 +1260,14 @@ class MakoTemplateLinter(BaseLinter): # suppress this violation if the page default hasn't been set, # otherwise the template might get less safe results.violations.append(ExpressionRuleViolation( - Rules.mako_unwanted_html_filter, expression + self.ruleset.mako_unwanted_html_filter, expression )) elif filters == ['n', 'strip_all_tags_but_br']: # {x | n, strip_all_tags_but_br} is valid in html context pass else: results.violations.append(ExpressionRuleViolation( - Rules.mako_invalid_html_filter, expression + self.ruleset.mako_invalid_html_filter, expression )) elif context == 'javascript': self._check_js_expression_not_with_html(mako_template, expression, results) @@ -1234,7 +1279,7 @@ class MakoTemplateLinter(BaseLinter): self._check_js_string_expression_in_quotes(mako_template, expression, results) else: results.violations.append(ExpressionRuleViolation( - Rules.mako_invalid_js_filter, expression + self.ruleset.mako_invalid_js_filter, expression )) def _check_js_string_expression_in_quotes(self, mako_template, expression, results): @@ -1250,7 +1295,7 @@ class MakoTemplateLinter(BaseLinter): parse_string = self._find_string_wrapping_expression(mako_template, expression) if parse_string is None: results.violations.append(ExpressionRuleViolation( - Rules.mako_js_missing_quotes, expression + self.ruleset.mako_js_missing_quotes, expression )) def _check_js_expression_not_with_html(self, mako_template, expression, results): @@ -1266,7 +1311,7 @@ class MakoTemplateLinter(BaseLinter): parse_string = self._find_string_wrapping_expression(mako_template, expression) if parse_string is not None and re.search('[<>]', parse_string.string) is not None: results.violations.append(ExpressionRuleViolation( - Rules.mako_js_html_string, expression + self.ruleset.mako_js_html_string, expression )) def _find_string_wrapping_expression(self, mako_template, expression): diff --git a/scripts/xsslint/xsslint/main.py b/scripts/xsslint/xsslint/main.py index 40526d7cac..028d0b4c86 100644 --- a/scripts/xsslint/xsslint/main.py +++ b/scripts/xsslint/xsslint/main.py @@ -7,6 +7,7 @@ import os import sys from xsslint.reporting import SummaryResults +from xsslint.rules import RuleSet from xsslint.utils import is_skip_dir @@ -18,6 +19,23 @@ def _load_config_module(module_path): return importlib.import_module(module_path) +def _build_ruleset(template_linters): + """ + Combines the RuleSets from the provided template_linters into a single, aggregate RuleSet. + + Arguments: + template_linters: A list of linting objects. + + Returns: + The combined RuleSet. + """ + return reduce( + lambda combined, current: combined + current.ruleset, + template_linters, + RuleSet() + ) + + def _process_file(full_path, template_linters, options, summary_results, out): """ For each linter, lints the provided file. This means finding and printing @@ -148,9 +166,10 @@ def main(): 'verbose': args.verbose, 'skip_dirs': getattr(config, 'SKIP_DIRS', ()) } - summary_results = SummaryResults() template_linters = getattr(config, 'LINTERS', ()) if not template_linters: raise ValueError("LINTERS is empty or undefined in the config module ({}).".format(args.config)) + ruleset = _build_ruleset(template_linters) + summary_results = SummaryResults(ruleset) _lint(args.path, template_linters, options, summary_results, out=sys.stdout) diff --git a/scripts/xsslint/xsslint/reporting.py b/scripts/xsslint/xsslint/reporting.py index 1db02d29b4..210b69a62f 100644 --- a/scripts/xsslint/xsslint/reporting.py +++ b/scripts/xsslint/xsslint/reporting.py @@ -6,7 +6,6 @@ from __future__ import print_function import os import re -from xsslint.rules import Rules from xsslint.utils import StringLines @@ -238,13 +237,16 @@ class SummaryResults(object): Contains the summary results for all violations. """ - def __init__(self): + def __init__(self, ruleset): """ Init method. + + Arguments: + ruleset: A RuleSet object containing all of the possible Rules. """ self.total_violations = 0 self.totals_by_rule = dict.fromkeys( - [rule.rule_id for rule in Rules.__members__.values()], 0 + [rule.rule_id for rule in ruleset.rules.values()], 0 ) def add_violation(self, violation): diff --git a/scripts/xsslint/xsslint/rules.py b/scripts/xsslint/xsslint/rules.py index 699d92e872..350da77627 100644 --- a/scripts/xsslint/xsslint/rules.py +++ b/scripts/xsslint/xsslint/rules.py @@ -1,39 +1,22 @@ -from enum import Enum - - -class Rules(Enum): - """ - An Enum of each rule which the linter will check. - """ - # IMPORTANT: Do not edit without also updating the docs: - # - http://edx.readthedocs.org/projects/edx-developer-guide/en/latest/conventions/preventing_xss.html#xss-linter - mako_missing_default = 'mako-missing-default' - mako_multiple_page_tags = 'mako-multiple-page-tags' - mako_unparseable_expression = 'mako-unparseable-expression' - mako_unwanted_html_filter = 'mako-unwanted-html-filter' - mako_invalid_html_filter = 'mako-invalid-html-filter' - mako_invalid_js_filter = 'mako-invalid-js-filter' - mako_js_missing_quotes = 'mako-js-missing-quotes' - mako_js_html_string = 'mako-js-html-string' - mako_html_entities = 'mako-html-entities' - mako_unknown_context = 'mako-unknown-context' - underscore_not_escaped = 'underscore-not-escaped' - javascript_jquery_append = 'javascript-jquery-append' - javascript_jquery_prepend = 'javascript-jquery-prepend' - javascript_jquery_insertion = 'javascript-jquery-insertion' - javascript_jquery_insert_into_target = 'javascript-jquery-insert-into-target' - javascript_jquery_html = 'javascript-jquery-html' - javascript_concat_html = 'javascript-concat-html' - javascript_escape = 'javascript-escape' - javascript_interpolate = 'javascript-interpolate' - python_concat_html = 'python-concat-html' - python_custom_escape = 'python-custom-escape' - python_deprecated_display_name = 'python-deprecated-display-name' - python_requires_html_or_text = 'python-requires-html-or-text' - python_close_before_format = 'python-close-before-format' - python_wrap_html = 'python-wrap-html' - python_interpolate_html = 'python-interpolate-html' - python_parse_error = 'python-parse-error' - +class Rule(object): def __init__(self, rule_id): self.rule_id = rule_id + + def __eq__(self, other): + return self.rule_id == other.rule_id + + +class RuleSet(object): + def __init__(self, **kwargs): + self.rules = {} + for k, v in kwargs.items(): + self.rules[k] = Rule(v) + + def __getattr__(self, attr_name): + return self.rules[attr_name] + + def __add__(self, other): + result = self.__class__() + result.rules.update(self.rules) + result.rules.update(other.rules) + return result diff --git a/scripts/xsslint/xsslint/visitors.py b/scripts/xsslint/xsslint/visitors.py index 581a563cea..c1de930bf7 100644 --- a/scripts/xsslint/xsslint/visitors.py +++ b/scripts/xsslint/xsslint/visitors.py @@ -5,10 +5,20 @@ import ast import re from xsslint.reporting import ExpressionRuleViolation -from xsslint.rules import Rules +from xsslint.rules import RuleSet from xsslint.utils import Expression, ParseString, StringLines +ruleset = RuleSet( + python_concat_html='python-concat-html', + python_deprecated_display_name='python-deprecated-display-name', + python_requires_html_or_text='python-requires-html-or-text', + python_close_before_format='python-close-before-format', + python_wrap_html='python-wrap-html', + python_interpolate_html='python-interpolate-html', +) + + class BaseVisitor(ast.NodeVisitor): """ Base class for AST NodeVisitor used for Python xss linting. @@ -238,7 +248,7 @@ class OuterFormatVisitor(BaseVisitor): visitor.visit(node) for unsafe_html_string_node in visitor.unsafe_html_string_nodes: self.results.violations.append(ExpressionRuleViolation( - Rules.python_wrap_html, self.node_to_expression(unsafe_html_string_node) + ruleset.python_wrap_html, self.node_to_expression(unsafe_html_string_node) )) # Do not continue processing child nodes of this format() node. else: @@ -264,7 +274,7 @@ class AllNodeVisitor(BaseVisitor): """ if node.attr == 'display_name_with_default_escaped': self.results.violations.append(ExpressionRuleViolation( - Rules.python_deprecated_display_name, self.node_to_expression(node) + ruleset.python_deprecated_display_name, self.node_to_expression(node) )) self.generic_visit(node) @@ -293,14 +303,14 @@ class AllNodeVisitor(BaseVisitor): # Text() or HTML(). if is_caller_html_or_text is False: self.results.violations.append(ExpressionRuleViolation( - Rules.python_requires_html_or_text, self.node_to_expression(node.func) + ruleset.python_requires_html_or_text, self.node_to_expression(node.func) )) elif isinstance(node.func, ast.Name) and node.func.id in ['HTML', 'Text']: visitor = ContainsFormatVisitor(self.file_contents, self.results) visitor.visit(node) if visitor.contains_format_call: self.results.violations.append(ExpressionRuleViolation( - Rules.python_close_before_format, self.node_to_expression(node.func) + ruleset.python_close_before_format, self.node_to_expression(node.func) )) self.generic_visit(node) @@ -313,9 +323,9 @@ class AllNodeVisitor(BaseVisitor): """ rule = None if isinstance(node.op, ast.Mod): - rule = Rules.python_interpolate_html + rule = ruleset.python_interpolate_html elif isinstance(node.op, ast.Add): - rule = Rules.python_concat_html + rule = ruleset.python_concat_html if rule is not None: visitor = HtmlStringVisitor(self.file_contents, self.results) visitor.visit(node.left)