From 8f1b50b5cce2ea241dbee334c5f58234b8656849 Mon Sep 17 00:00:00 2001 From: Rajath Agasthya Date: Sat, 29 Jul 2017 01:33:26 -0700 Subject: [PATCH] Do not flag new way of escaping in jinja2 plugin Makes escaping using select_autoescape function valid by checking for ast.Call instance and if func id == select_autoescape. Example: from jinja2 import Environment, select_autoescape env = Environment(autoescape=select_autoescape(['html', 'htm', 'xml']), loader=PackageLoader('mypackage')) Change-Id: I47c6b346332a6d9f7c4c57dd45ab7636c78996a1 Closes-Bug: #1684249 --- bandit/plugins/jinja2_templates.py | 25 ++++++++++++++++++------- examples/jinja2_templating.py | 12 +++++++++++- tests/functional/test_functional.py | 4 ++-- 3 files changed, 31 insertions(+), 10 deletions(-) diff --git a/bandit/plugins/jinja2_templates.py b/bandit/plugins/jinja2_templates.py index fc3a2bc3..2adc77a6 100644 --- a/bandit/plugins/jinja2_templates.py +++ b/bandit/plugins/jinja2_templates.py @@ -47,13 +47,16 @@ false. A HIGH severity warning is generated in either of these scenarios. 14 >> Issue: By default, jinja2 sets autoescape to False. Consider using - autoescape=True to mitigate XSS vulnerabilities. + autoescape=True or use the select_autoescape function to mitigate XSS + vulnerabilities. Severity: High Confidence: High Location: ./examples/jinja2_templating.py:15 14 15 Environment(loader=templateLoader, 16 load=templateLoader) 17 + 18 Environment(autoescape=select_autoescape(['html', 'htm', 'xml']), + 19 loader=templateLoader) .. seealso:: @@ -93,13 +96,19 @@ def jinja2_autoescape_false(context): confidence=bandit.HIGH, text="Using jinja2 templates with autoescape=" "False is dangerous and can lead to XSS. " - "Use autoescape=True to mitigate XSS " + "Use autoescape=True or use the " + "select_autoescape function to mitigate XSS " "vulnerabilities." ) # found autoescape if getattr(node, 'arg', None) == 'autoescape': - if (getattr(node.value, 'id', None) == 'True' or - getattr(node.value, 'value', None) is True): + value = getattr(node, 'value', None) + if (getattr(value, 'id', None) == 'True' or + getattr(value, 'value', None) is True): + return + # Check if select_autoescape function is used. + elif isinstance(value, ast.Call) and getattr( + value.func, 'id', None) == 'select_autoescape': return else: return bandit.Issue( @@ -107,8 +116,9 @@ def jinja2_autoescape_false(context): confidence=bandit.MEDIUM, text="Using jinja2 templates with autoescape=" "False is dangerous and can lead to XSS. " - "Ensure autoescape=True to mitigate XSS " - "vulnerabilities." + "Ensure autoescape=True or use the " + "select_autoescape function to mitigate " + "XSS vulnerabilities." ) # We haven't found a keyword named autoescape, indicating default # behavior @@ -116,5 +126,6 @@ def jinja2_autoescape_false(context): severity=bandit.HIGH, confidence=bandit.HIGH, text="By default, jinja2 sets autoescape to False. Consider " - "using autoescape=True to mitigate XSS vulnerabilities." + "using autoescape=True or use the select_autoescape " + "function to mitigate XSS vulnerabilities." ) diff --git a/examples/jinja2_templating.py b/examples/jinja2_templating.py index d79617fb..d5aaa2dd 100644 --- a/examples/jinja2_templating.py +++ b/examples/jinja2_templating.py @@ -1,5 +1,5 @@ import jinja2 -from jinja2 import Environment +from jinja2 import Environment, select_autoescape templateLoader = jinja2.FileSystemLoader( searchpath="/" ) something = '' @@ -14,3 +14,13 @@ Environment(loader=templateLoader, Environment(loader=templateLoader, load=templateLoader) + +Environment(loader=templateLoader, autoescape=select_autoescape()) + +Environment(loader=templateLoader, + autoescape=select_autoescape(['html', 'htm', 'xml'])) + + +def fake_func(): + return 'foobar' +Environment(loader=templateLoader, autoescape=fake_func()) diff --git a/tests/functional/test_functional.py b/tests/functional/test_functional.py index 45224817..5ea82202 100644 --- a/tests/functional/test_functional.py +++ b/tests/functional/test_functional.py @@ -441,8 +441,8 @@ class FunctionalTests(testtools.TestCase): def test_jinja2_templating(self): '''Test jinja templating for potential XSS bugs.''' expect = { - 'SEVERITY': {'UNDEFINED': 0, 'LOW': 0, 'MEDIUM': 0, 'HIGH': 4}, - 'CONFIDENCE': {'UNDEFINED': 0, 'LOW': 0, 'MEDIUM': 1, 'HIGH': 3} + 'SEVERITY': {'UNDEFINED': 0, 'LOW': 0, 'MEDIUM': 0, 'HIGH': 5}, + 'CONFIDENCE': {'UNDEFINED': 0, 'LOW': 0, 'MEDIUM': 2, 'HIGH': 3} } self.check_example('jinja2_templating.py', expect)