diff --git a/python/ql/src/Security/CWE-079/Jinja2WithoutEscaping.qhelp b/python/ql/src/Security/CWE-079/Jinja2WithoutEscaping.qhelp new file mode 100644 index 00000000000..9d81797efea --- /dev/null +++ b/python/ql/src/Security/CWE-079/Jinja2WithoutEscaping.qhelp @@ -0,0 +1,42 @@ + + + + +

+ +Cross-site scripting attacks can occur if untrusted input is not escaped. This applies to templates as well as code. +The jinja2 templates may be vulnerable to XSS if the environment has autoescape set to False. +Unfortunately, jinja2 sets autoescape to False by default. +Explicitly setting autoescape to True when creating an Environment object will prevent this. +

+
+ + +

+Avoid setting jinja2 autoescape to False. +Jinja2 provides the function select_autoescape to make sure that the correct auto-escaping is chosen. +For example, it can be used when creating an environment Environment(autoescape=select_autoescape(['html', 'xml']) +

+
+ + +

+The following example is a minimal flask app which shows a safe and unsafe way to render the given name back to the page. +The first view is unsafe as first_name is not escaped, leaving the page vulnerable to cross-site scripting attacks. +The second view is safe as first_name is escaped, so it is not vulnerable to cross-site scripting attacks. +

+ +
+ + +
  • +http://jinja.pocoo.org/docs/2.10/api/ +Jinja2: API. +
  • +
  • +Wikipedia: Cross-site scripting. +
  • +
    +
    diff --git a/python/ql/src/Security/CWE-079/Jinja2WithoutEscaping.ql b/python/ql/src/Security/CWE-079/Jinja2WithoutEscaping.ql new file mode 100644 index 00000000000..f726313f4c9 --- /dev/null +++ b/python/ql/src/Security/CWE-079/Jinja2WithoutEscaping.ql @@ -0,0 +1,56 @@ +/** + * @name Jinja2 templating with autoescape=False + * @description Using jinja2 templates with autoescape=False can + * cause a cross-site scripting vulnerability. + * @kind problem + * @problem.severity error + * @precision medium + * @id py/jinja2/autoescape-false + * @tags security + * external/cwe/cwe-079 + */ + +import python + +predicate jinja2Environment(Object callable, int autoescape) { + exists(ModuleObject jinja2 | + jinja2.getName() = "jinja2" | + jinja2.getAttribute("Environment") = callable and + callable.(ClassObject).getPyClass().getInitMethod().getArg(autoescape+1).asName().getId() = "autoescape" + or + exists(ModuleObject environment | + environment.getAttribute("Template") = callable and + callable.(ClassObject).lookupAttribute("__new__").(FunctionObject).getFunction().getArg(autoescape+1).asName().getId() = "autoescape" + ) + ) +} + +ControlFlowNode getAutoEscapeParameter(CallNode call) { + exists(Object callable | + call.getFunction().refersTo(callable) | + jinja2Environment(callable, _) and + result = call.getArgByName("autoescape") + or + exists(int arg | + jinja2Environment(callable, arg) and + result = call.getArg(arg) + ) + ) +} + +from CallNode call +where +not exists(call.getNode().getStarargs()) and +not exists(call.getNode().getKwargs()) and +( + not exists(getAutoEscapeParameter(call)) and + exists(Object env | + call.getFunction().refersTo(env) and + jinja2Environment(env, _) + ) + or + exists(Object isFalse | + getAutoEscapeParameter(call).refersTo(isFalse) and isFalse.booleanValue() = false + ) +) +select call, "Using jinja2 templates with autoescape=False can potentially allow XSS attacks." diff --git a/python/ql/src/Security/CWE-079/examples/jinja2.py b/python/ql/src/Security/CWE-079/examples/jinja2.py new file mode 100644 index 00000000000..2811f5e10e8 --- /dev/null +++ b/python/ql/src/Security/CWE-079/examples/jinja2.py @@ -0,0 +1,27 @@ +from flask import Flask, request, make_response, escape +from jinja2 import Environment, select_autoescape, FileSystemLoader + +app = Flask(__name__) +loader = FileSystemLoader( searchpath="templates/" ) + +unsafe_env = Environment(loader=loader) +safe1_env = Environment(loader=loader, autoescape=True) +safe2_env = Environment(loader=loader, autoescape=select_autoescape()) + +def render_response_from_env(env): + name = request.args.get('name', '') + template = env.get_template('template.html') + return make_response(template.render(name=name)) + +@app.route('/unsafe') +def unsafe(): + return render_response_from_env(unsafe_env) + +@app.route('/safe1') +def safe1(): + return render_response_from_env(safe1_env) + +@app.route('/safe2') +def safe2(): + return render_response_from_env(safe2_env) + diff --git a/python/ql/test/query-tests/Security/CWE-079/Jinja2WithoutEscaping.expected b/python/ql/test/query-tests/Security/CWE-079/Jinja2WithoutEscaping.expected new file mode 100644 index 00000000000..8640a75ca23 --- /dev/null +++ b/python/ql/test/query-tests/Security/CWE-079/Jinja2WithoutEscaping.expected @@ -0,0 +1,4 @@ +| jinja2_escaping.py:9:14:9:39 | ControlFlowNode for Environment() | Using jinja2 templates with autoescape=False can potentially allow XSS attacks. | +| jinja2_escaping.py:41:5:41:29 | ControlFlowNode for Environment() | Using jinja2 templates with autoescape=False can potentially allow XSS attacks. | +| jinja2_escaping.py:43:1:43:3 | ControlFlowNode for E() | Using jinja2 templates with autoescape=False can potentially allow XSS attacks. | +| jinja2_escaping.py:44:1:44:15 | ControlFlowNode for E() | Using jinja2 templates with autoescape=False can potentially allow XSS attacks. | diff --git a/python/ql/test/query-tests/Security/CWE-079/Jinja2WithoutEscaping.qlref b/python/ql/test/query-tests/Security/CWE-079/Jinja2WithoutEscaping.qlref new file mode 100644 index 00000000000..9fefcf4a030 --- /dev/null +++ b/python/ql/test/query-tests/Security/CWE-079/Jinja2WithoutEscaping.qlref @@ -0,0 +1 @@ +Security/CWE-079/Jinja2WithoutEscaping.ql diff --git a/python/ql/test/query-tests/Security/CWE-079/ReflectedXss.expected b/python/ql/test/query-tests/Security/CWE-079/ReflectedXss.expected index b60e97ff468..b305b84491e 100644 --- a/python/ql/test/query-tests/Security/CWE-079/ReflectedXss.expected +++ b/python/ql/test/query-tests/Security/CWE-079/ReflectedXss.expected @@ -1,6 +1,8 @@ edges | ../lib/flask/__init__.py:14:19:14:20 | externally controlled string | ../lib/flask/__init__.py:15:19:15:20 | externally controlled string | | ../lib/flask/__init__.py:14:19:14:20 | externally controlled string | ../lib/flask/__init__.py:16:25:16:26 | externally controlled string | +| jinja2_escaping.py:14:12:14:23 | dict of externally controlled string | jinja2_escaping.py:14:12:14:39 | externally controlled string | +| jinja2_escaping.py:14:12:14:39 | externally controlled string | jinja2_escaping.py:16:47:16:50 | externally controlled string | | reflected_xss.py:7:18:7:29 | dict of externally controlled string | reflected_xss.py:7:18:7:45 | externally controlled string | | reflected_xss.py:7:18:7:45 | externally controlled string | reflected_xss.py:8:44:8:53 | externally controlled string | | reflected_xss.py:8:26:8:53 | externally controlled string | ../lib/flask/__init__.py:14:19:14:20 | externally controlled string | diff --git a/python/ql/test/query-tests/Security/CWE-079/jinja2_escaping.py b/python/ql/test/query-tests/Security/CWE-079/jinja2_escaping.py new file mode 100644 index 00000000000..51e298d890a --- /dev/null +++ b/python/ql/test/query-tests/Security/CWE-079/jinja2_escaping.py @@ -0,0 +1,51 @@ + +Environment(loader=templateLoader, autoescape=fake_func()) +from flask import Flask, request, make_response, escape +from jinja2 import Environment, select_autoescape, FileSystemLoader + +app = Flask(__name__) +loader = FileSystemLoader( searchpath="templates/" ) + +unsafe_env = Environment(loader=loader) +safe1_env = Environment(loader=loader, autoescape=True) +safe2_env = Environment(loader=loader, autoescape=select_autoescape()) + +def render_response_from_env(env): + name = request.args.get('name', '') + template = env.get_template('template.html') + return make_response(template.render(name=name)) + +@app.route('/unsafe') +def unsafe(): + return render_response_from_env(unsafe_env) + +@app.route('/safe1') +def safe1(): + return render_response_from_env(safe1_env) + +@app.route('/safe2') +def safe2(): + return render_response_from_env(safe2_env) + +# Explicit autoescape + +e = Environment( + loader=loader, + autoescape=select_autoescape(['html', 'htm', 'xml']) +) # GOOD + +# Additional checks with flow. +auto = select_autoescape +e = Environment(autoescape=auto) # GOOD +z = 0 +e = Environment(autoescape=z) # BAD +E = Environment +E() # BAD +E(autoescape=z) # BAD +E(autoescape=auto) # GOOD +E(autoescape=0+1) # GOOD + +def checked(cond=False): + if cond: + e = Environment(autoescape=cond) # GOOD +