Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Define XSS-linter rules on Linter classes.

Browse Source
This commit is contained in:
Anthony Mangano
2018-03-22 12:07:12 -04:00
parent a576d682ff
commit edc98939b5
7 changed files with 292 additions and 229 deletions
Download Patch File Download Diff File Expand all files Collapse all files

223
scripts/xsslint/tests/test_linters.py
View File

@@ -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 = "<p>" + message + "</p>"', 'rule': Rules.javascript_concat_html},
{'template': 'var m = "<p>" + message + "</p>"', 'rule': ruleset.javascript_concat_html},
{
'template': r'var m = "<p>\"escaped quote\"" + message + "\"escaped quote\"</p>"',
'rule': Rules.javascript_concat_html
'rule': ruleset.javascript_concat_html
},
{'template': ' // var m = "<p>" + commentedOutMessage + "</p>"', 'rule': None},
{'template': 'var m = " <p> " + message + " </p> "', 'rule': Rules.javascript_concat_html},
{'template': 'var m = " <p> " + message + " broken string', 'rule': Rules.javascript_concat_html},
{'template': 'var m = " <p> " + message + " </p> "', 'rule': ruleset.javascript_concat_html},
{'template': 'var m = " <p> " + 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("<div/>")', 'rule': Rules.javascript_jquery_append},
{'template': 'test.append("<div/>")', 'rule': ruleset.javascript_jquery_append},
{'template': 'graph.svg.append("g")', 'rule': None},
{'template': 'test.append( $( "<div>" ) )', 'rule': None},
{'template': 'test.append($("<div>"))', 'rule': None},
{'template': 'test.append($("<div/>"))', '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($("<div/>"))', '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 = "<p>" + commentedOutMessage + "</p>"', 'rule': None},
{'template': 'm = "<p>" + message + "</p>"', 'rule': [Rules.python_concat_html, Rules.python_concat_html]},
{'template': 'm = " <p> " + message + " </p> "', 'rule': [Rules.python_concat_html, Rules.python_concat_html]},
{'template': 'm = " <p> " + message + " broken string', 'rule': Rules.python_parse_error},
{'template': 'm = "<p>" + message + "</p>"', 'rule': [ruleset.python_concat_html, ruleset.python_concat_html]},
{'template': 'm = " <p> " + message + " </p> "', 'rule': [ruleset.python_concat_html, ruleset.python_concat_html]},
{'template': 'm = " <p> " + 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("</span>"),
)
"""),
'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("</span>"),
)
"""),
'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("</a>"),
))
"""),
'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("</a>"),
)
"""),
'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="</span>",
)
"""),
'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 = '<a href="{}"'.format(url)""",
'rule': Rules.python_wrap_html
'rule': ruleset.python_wrap_html
},
{
'python': r"""msg = '{}</p>'.format(message)""",
'rule': Rules.python_wrap_html
'rule': ruleset.python_wrap_html
},
{
'python': r"""r'regex with {} and named group(?P<id>\d+)?$'.format(test)""",
@@ -593,7 +600,7 @@ class TestPythonLinter(TestLinter):
},
{
'python': r"""msg = '<a href="%s"' % url""",
'rule': Rules.python_interpolate_html
'rule': ruleset.python_interpolate_html
},
{
'python':
@@ -606,11 +613,11 @@ class TestPythonLinter(TestLinter):
},
{
'python': r"""msg = '%s</p>' % message""",
'rule': Rules.python_interpolate_html
'rule': ruleset.python_interpolate_html
},
{
'python': "msg = HTML('<span></span>'",
'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):
</div>
''').format(response=response_text)
""",
'rule': Rules.python_wrap_html,
'rule': ruleset.python_wrap_html,
'start_line': 2,
},
{
@@ -683,7 +690,7 @@ class TestPythonLinter(TestLinter):
</div>
''').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 = '''<h3 class="result">{response}</h3>'''.format(response=response_text)
""",
'rule': Rules.python_wrap_html,
'rule': ruleset.python_wrap_html,
'start_line': 6,
},
{
@@ -712,7 +719,7 @@ class TestPythonLinter(TestLinter):
</div>
''').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"/> <some-other-tag expression_filter="h" /> ',
'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("</span>"),
)}
"""),
'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("</span>"),
)}
"""),
'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("</a>"),
))}
"""),
'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("</a>"),
)}
"""),
'rule': Rules.python_close_before_format
'rule': ruleset.python_close_before_format
},
{
'expression':
@@ -928,7 +939,7 @@ class TestMakoTemplateLinter(TestLinter):
span_end="</span>",
)}
"""),
'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': "${'<span></span>'}",
'rule': Rules.python_wrap_html
'rule': ruleset.python_wrap_html
},
{
'expression': "${'Embedded HTML <strong></strong>'}",
'rule': Rules.python_wrap_html
'rule': ruleset.python_wrap_html
},
{
'expression': "${ HTML('<span></span>') }",
@@ -966,19 +977,19 @@ class TestMakoTemplateLinter(TestLinter):
},
{
'expression': "${ '<span></span>' + 'some other text' }",
'rule': [Rules.python_concat_html, Rules.python_wrap_html]
'rule': [ruleset.python_concat_html, ruleset.python_wrap_html]
},
{
'expression': "${ HTML('<span>missing closing parentheses.</span>' }",
'rule': Rules.python_parse_error
'rule': ruleset.python_parse_error
},
{
'expression': "${'Rock &amp; Roll'}",
'rule': Rules.mako_html_entities
'rule': ruleset.mako_html_entities
},
{
'expression': "${'Rock &#38; 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())

83
scripts/xsslint/tests/test_main.py
View File

@@ -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))

105
scripts/xsslint/xsslint/linters.py
View File

@@ -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"(?<!StringUtils).interpolate\(")
for function_match in regex.finditer(file_contents):
expression = self._get_expression_for_function(file_contents, function_match)
results.violations.append(ExpressionRuleViolation(Rules.javascript_interpolate, expression))
results.violations.append(ExpressionRuleViolation(self.ruleset.javascript_interpolate, expression))
def _check_javascript_escape(self, file_contents, results):
"""
@@ -447,7 +464,7 @@ class JavaScriptLinter(BaseLinter):
regex = regex = re.compile(r"(?<!_).escape\(")
for function_match in regex.finditer(file_contents):
expression = self._get_expression_for_function(file_contents, function_match)
results.violations.append(ExpressionRuleViolation(Rules.javascript_escape, expression))
results.violations.append(ExpressionRuleViolation(self.ruleset.javascript_escape, expression))
def _check_jquery_function(self, file_contents, function_names, rule, is_caller_safe, is_argument_safe, results):
"""
@@ -459,7 +476,7 @@ class JavaScriptLinter(BaseLinter):
function_names: A pipe delimited list of names of the functions
(e.g. "wrap|after|before").
rule: The name of the rule to use for validation errors (e.g.
Rules.javascript_jquery_append).
self.ruleset.javascript_jquery_append).
is_caller_safe: A function to test if caller of the JQuery function
is safe.
is_argument_safe: A function to test if the argument passed to the
@@ -697,6 +714,14 @@ class PythonLinter(BaseLinter):
LINE_COMMENT_DELIM = "#"
ruleset = RuleSet(
python_parse_error='python-parse-error',
python_custom_escape='python-custom-escape',
# The Visitor classes are python-specific and should be moved into the PythonLinter once they have
# been decoupled from the MakoTemplateLinter.
) + visitors.ruleset
def __init__(self, skip_dirs=None):
"""
Init method.
@@ -756,7 +781,7 @@ class PythonLinter(BaseLinter):
# different assumptions.
if root_node is not None:
# check format() rules that can be run on outer-most format() calls
visitor = OuterFormatVisitor(file_contents, results)
visitor = visitors.OuterFormatVisitor(file_contents, results)
visitor.visit(root_node)
results.prepare_results(file_contents, line_comment_delim=self.LINE_COMMENT_DELIM)
@@ -773,7 +798,7 @@ class PythonLinter(BaseLinter):
"""
if root_node is not None:
# check illegal concatenation and interpolation
visitor = AllNodeVisitor(python_code, results)
visitor = visitors.AllNodeVisitor(python_code, results)
visitor.visit(root_node)
# check rules parse with regex
self._check_custom_escape(python_code, results)
@@ -801,7 +826,7 @@ class PythonLinter(BaseLinter):
line_start_index = lines.line_number_to_start_index(e.lineno)
expression = Expression(line_start_index + e.offset)
results.violations.append(ExpressionRuleViolation(
Rules.python_parse_error, expression
self.ruleset.python_parse_error, expression
))
return None
@@ -846,7 +871,7 @@ class PythonLinter(BaseLinter):
for match in re.finditer("(<.*&lt;|&lt;.*<)", file_contents):
expression = Expression(match.start(), match.end())
results.violations.append(ExpressionRuleViolation(
Rules.python_custom_escape, expression
self.ruleset.python_custom_escape, expression
))
@@ -856,6 +881,25 @@ class MakoTemplateLinter(BaseLinter):
"""
LINE_COMMENT_DELIM = "##"
ruleset = RuleSet(
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',
# NOTE The MakoTemplateLinter directly checks for python_wrap_html and directly
# instantiates Visitor instances to check for python issues. This logic should
# be moved into the PythonLinter. The MakoTemplateLinter should only check for
# Mako-specific issues.
python_wrap_html='python-wrap-html',
) + visitors.ruleset
def __init__(self, javascript_linter, python_linter, skip_dirs=None):
"""
Init method.
@@ -863,6 +907,7 @@ class MakoTemplateLinter(BaseLinter):
super(MakoTemplateLinter, self).__init__()
self.javascript_linter = javascript_linter
self.python_linter = python_linter
self.ruleset = self.ruleset + self.javascript_linter.ruleset + self.python_linter.ruleset
self._skip_mako_dirs = skip_dirs or ()
def process_file(self, directory, file_name):
@@ -986,18 +1031,18 @@ class MakoTemplateLinter(BaseLinter):
page_tag_count = self._get_page_tag_count(mako_template)
# check if there are too many page expressions
if 2 <= page_tag_count:
results.violations.append(RuleViolation(Rules.mako_multiple_page_tags))
results.violations.append(RuleViolation(self.ruleset.mako_multiple_page_tags))
return False
# make sure there is exactly 1 page expression, excluding commented out
# page expressions, before proceeding
elif page_tag_count != 1:
results.violations.append(RuleViolation(Rules.mako_missing_default))
results.violations.append(RuleViolation(self.ruleset.mako_missing_default))
return False
# check that safe by default (h filter) is turned on
page_h_filter_regex = re.compile('<%page[^>]*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):

21
scripts/xsslint/xsslint/main.py
View File

@@ -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)

8
scripts/xsslint/xsslint/reporting.py
View File

@@ -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):

57
scripts/xsslint/xsslint/rules.py
View File

@@ -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

24
scripts/xsslint/xsslint/visitors.py
View File

@@ -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)