From 1ceb963d4c11c4cbabf3a9e621afd74f011158a7 Mon Sep 17 00:00:00 2001 From: Porcupiney Hairs Date: Wed, 20 May 2020 02:03:38 +0530 Subject: [PATCH 01/10] Python : Add support for detecting XSLT Injection This PR adds support for detecting XSLT injection in Python. I have included the ql files as well as the tests with this. --- python/ql/src/experimental/CWE-643/Xslt.qhelp | 16 +++ python/ql/src/experimental/CWE-643/Xslt.ql | 35 ++++++ python/ql/src/experimental/CWE-643/xslt.py | 14 +++ .../semmle/python/security/injection/XSLT.qll | 119 ++++++++++++++++++ .../test/experimental/CWE-074/Xslt.expected | 47 +++++++ .../ql/test/experimental/CWE-074/Xslt.qlref | 1 + .../experimental/CWE-074/XsltSinks.expected | 12 ++ .../ql/test/experimental/CWE-074/XsltSinks.ql | 6 + python/ql/test/experimental/CWE-074/options | 1 + python/ql/test/experimental/CWE-074/xslt.py | 14 +++ .../experimental/CWE-074/xsltInjection.py | 79 ++++++++++++ .../ql/test/experimental/CWE-074/xsltSinks.py | 56 +++++++++ .../query-tests/Security/lib/lxml/__init__.py | 0 .../query-tests/Security/lib/lxml/etree.py | 37 ++++++ 14 files changed, 437 insertions(+) create mode 100644 python/ql/src/experimental/CWE-643/Xslt.qhelp create mode 100644 python/ql/src/experimental/CWE-643/Xslt.ql create mode 100644 python/ql/src/experimental/CWE-643/xslt.py create mode 100644 python/ql/src/experimental/semmle/python/security/injection/XSLT.qll create mode 100644 python/ql/test/experimental/CWE-074/Xslt.expected create mode 100644 python/ql/test/experimental/CWE-074/Xslt.qlref create mode 100644 python/ql/test/experimental/CWE-074/XsltSinks.expected create mode 100644 python/ql/test/experimental/CWE-074/XsltSinks.ql create mode 100644 python/ql/test/experimental/CWE-074/options create mode 100644 python/ql/test/experimental/CWE-074/xslt.py create mode 100644 python/ql/test/experimental/CWE-074/xsltInjection.py create mode 100644 python/ql/test/experimental/CWE-074/xsltSinks.py create mode 100644 python/ql/test/query-tests/Security/lib/lxml/__init__.py create mode 100644 python/ql/test/query-tests/Security/lib/lxml/etree.py diff --git a/python/ql/src/experimental/CWE-643/Xslt.qhelp b/python/ql/src/experimental/CWE-643/Xslt.qhelp new file mode 100644 index 000000000000..cdcc7e4dada5 --- /dev/null +++ b/python/ql/src/experimental/CWE-643/Xslt.qhelp @@ -0,0 +1,16 @@ + + + + Processing an unvalidated XSL stylesheet can allow an attacker to change the structure and contents of the resultant XML, include arbitrary files from the file system, or execute arbitrary code. + + +

+ This vulnerability can be prevented by not allowing untrusted user input to be passed as a XSL stylesheet. + If the application logic necessiates processing untrusted XSL stylesheets, the input should be properly filtered and sanitized before use. +

+ +

In the example below, the XSL stylesheet is controlled by the user and hence leads to a vulnerability.

+ + + + \ No newline at end of file diff --git a/python/ql/src/experimental/CWE-643/Xslt.ql b/python/ql/src/experimental/CWE-643/Xslt.ql new file mode 100644 index 000000000000..09345858160e --- /dev/null +++ b/python/ql/src/experimental/CWE-643/Xslt.ql @@ -0,0 +1,35 @@ +/** + * @name Xslt query built from user-controlled sources + * @description Building a XSLT query from user-controlled sources is vulnerable to insertion of + * malicious XSLT code by the user. + * @kind path-problem + * @problem.severity error + * @precision high + * @id py/xslt-injection + * @tags security + * external/cwe/cwe-643 + */ + +import python +import semmle.python.security.Paths +/* Sources */ +import semmle.python.web.HttpRequest +/* Sinks */ +import experimental.semmle.python.security.injection.XSLT + +class XSLTInjectionConfiguration extends TaintTracking::Configuration { + XSLTInjectionConfiguration() { this = "XSLT injection configuration" } + + override predicate isSource(TaintTracking::Source source) { + source instanceof HttpRequestTaintSource + } + + override predicate isSink(TaintTracking::Sink sink) { + sink instanceof XSLTInjection::XSLTInjectionSink + } +} + +from XSLTInjectionConfiguration config, TaintedPathSource src, TaintedPathSink sink +where config.hasFlowPath(src, sink) +select sink.getSink(), src, sink, "This XSLT query depends on $@.", src.getSource(), + "a user-provided value" diff --git a/python/ql/src/experimental/CWE-643/xslt.py b/python/ql/src/experimental/CWE-643/xslt.py new file mode 100644 index 000000000000..1655916c7e06 --- /dev/null +++ b/python/ql/src/experimental/CWE-643/xslt.py @@ -0,0 +1,14 @@ +from lxml import etree +from io import StringIO +from flask import Flask, request + +app = Flask(__name__) + + +@app.route("/xslt") +def bad(): + xsltQuery = request.args.get('xml', '') + xslt_root = etree.XML(xsltQuery) + f = StringIO('') + tree = etree.parse(f) + result_tree = tree.xslt(xslt_root) # Not OK diff --git a/python/ql/src/experimental/semmle/python/security/injection/XSLT.qll b/python/ql/src/experimental/semmle/python/security/injection/XSLT.qll new file mode 100644 index 000000000000..795d439d83a4 --- /dev/null +++ b/python/ql/src/experimental/semmle/python/security/injection/XSLT.qll @@ -0,0 +1,119 @@ +/** + * Provides class and predicates to track external data that + * may represent malicious XSLT query objects. + * + * This module is intended to be imported into a taint-tracking query + * to extend `TaintKind` and `TaintSink`. + */ + +import python +import semmle.python.security.TaintTracking +import semmle.python.web.HttpRequest + +/** Models XSLT Injection related classes and functions */ +module XSLTInjection { + /** Returns a class value which refers to `lxml.etree` */ + Value etree() { result = Value::named("lxml.etree") } + + /** A generic taint sink that is vulnerable to XSLT injection. */ + abstract class XSLTInjectionSink extends TaintSink { } + + /** + * A kind of "taint", representing an untrusted XML string + */ + private class ExternalXmlStringKind extends ExternalStringKind { + ExternalXmlStringKind() { this = "etree.XML string" } + + override TaintKind getTaintForFlowStep(ControlFlowNode fromnode, ControlFlowNode tonode) { + etreeXML(fromnode, tonode) and result instanceof ExternalXmlKind + or + etreeFromStringList(fromnode, tonode) and result instanceof ExternalXmlKind + or + etreeFromString(fromnode, tonode) and result instanceof ExternalXmlKind + } + } + + /** + * A kind of "taint", representing a XML encoded string + */ + class ExternalXmlKind extends TaintKind { + ExternalXmlKind() { this = "lxml etree xml" } + } + + private predicate etreeXML(ControlFlowNode fromnode, CallNode tonode) { + exists(CallNode call, AttrNode atr | + atr = etree().getAReference().getASuccessor() and + // XML(text, parser=None, base_url=None) + atr.getName() = "XML" and + atr = call.getFunction() + | + call.getArg(0) = fromnode and + call = tonode + ) + } + + private predicate etreeFromString(ControlFlowNode fromnode, CallNode tonode) { + // fromstring(text, parser=None) + exists(CallNode call | call.getFunction().(AttrNode).getObject("fromstring").pointsTo(etree()) | + call.getArg(0) = fromnode and + call = tonode + ) + } + + private predicate etreeFromStringList(ControlFlowNode fromnode, CallNode tonode) { + // fromstringlist(strings, parser=None) + exists(CallNode call | + call.getFunction().(AttrNode).getObject("fromstringlist").pointsTo(etree()) + | + call.getArg(0) = fromnode and + call = tonode + ) + } + + /** + * A Sink representing an argument to the `etree.XSLT` call. + * + * from lxml import etree + * root = etree.XML("") + * find_text = etree.XSLT("`sink`") + */ + private class EtreeXSLTArgument extends XSLTInjectionSink { + override string toString() { result = "lxml.etree.XSLT" } + + EtreeXSLTArgument() { + exists(CallNode call | call.getFunction().(AttrNode).getObject("XSLT").pointsTo(etree()) | + call.getArg(0) = this + ) + } + + override predicate sinks(TaintKind kind) { kind instanceof ExternalXmlKind } + } + + /** + * A Sink representing an argument to the `XSLT` call to a parsed xml document. + * + * from lxml import etree + * from io import StringIO + * `sink` = etree.XML(xsltQuery) + * tree = etree.parse(f) + * result_tree = tree.xslt(`sink`) + */ + private class ParseXSLTArgument extends XSLTInjectionSink { + override string toString() { result = "lxml.etree.parse.xslt" } + + ParseXSLTArgument() { + exists( + CallNode parseCall, CallNode xsltCall, ControlFlowNode obj, Variable var, AssignStmt assign + | + parseCall.getFunction().(AttrNode).getObject("parse").pointsTo(etree()) and + assign.getValue().(Call).getAFlowNode() = parseCall and + xsltCall.getFunction().(AttrNode).getObject("xslt") = obj and + var.getAUse() = obj and + assign.getATarget() = var.getAStore() and + xsltCall.getArg(0) = this + ) + } + + override predicate sinks(TaintKind kind) { kind instanceof ExternalXmlKind } + } +} diff --git a/python/ql/test/experimental/CWE-074/Xslt.expected b/python/ql/test/experimental/CWE-074/Xslt.expected new file mode 100644 index 000000000000..89f19160f697 --- /dev/null +++ b/python/ql/test/experimental/CWE-074/Xslt.expected @@ -0,0 +1,47 @@ +edges +| xslt.py:10:17:10:28 | dict of etree.XML string | xslt.py:10:17:10:43 | etree.XML string | +| xslt.py:10:17:10:28 | dict of etree.XML string | xslt.py:10:17:10:43 | etree.XML string | +| xslt.py:10:17:10:43 | etree.XML string | xslt.py:11:27:11:35 | etree.XML string | +| xslt.py:10:17:10:43 | etree.XML string | xslt.py:11:27:11:35 | etree.XML string | +| xslt.py:11:17:11:36 | lxml etree xml | xslt.py:14:29:14:37 | lxml etree xml | +| xslt.py:11:17:11:36 | lxml etree xml | xslt.py:14:29:14:37 | lxml etree xml | +| xslt.py:11:27:11:35 | etree.XML string | xslt.py:11:17:11:36 | lxml etree xml | +| xslt.py:11:27:11:35 | etree.XML string | xslt.py:11:17:11:36 | lxml etree xml | +| xsltInjection.py:10:17:10:28 | dict of etree.XML string | xsltInjection.py:10:17:10:43 | etree.XML string | +| xsltInjection.py:10:17:10:28 | dict of etree.XML string | xsltInjection.py:10:17:10:43 | etree.XML string | +| xsltInjection.py:10:17:10:43 | etree.XML string | xsltInjection.py:11:27:11:35 | etree.XML string | +| xsltInjection.py:10:17:10:43 | etree.XML string | xsltInjection.py:11:27:11:35 | etree.XML string | +| xsltInjection.py:11:17:11:36 | lxml etree xml | xsltInjection.py:12:28:12:36 | lxml etree xml | +| xsltInjection.py:11:17:11:36 | lxml etree xml | xsltInjection.py:12:28:12:36 | lxml etree xml | +| xsltInjection.py:11:27:11:35 | etree.XML string | xsltInjection.py:11:17:11:36 | lxml etree xml | +| xsltInjection.py:11:27:11:35 | etree.XML string | xsltInjection.py:11:17:11:36 | lxml etree xml | +| xsltInjection.py:17:17:17:28 | dict of etree.XML string | xsltInjection.py:17:17:17:43 | etree.XML string | +| xsltInjection.py:17:17:17:28 | dict of etree.XML string | xsltInjection.py:17:17:17:43 | etree.XML string | +| xsltInjection.py:17:17:17:43 | etree.XML string | xsltInjection.py:18:27:18:35 | etree.XML string | +| xsltInjection.py:17:17:17:43 | etree.XML string | xsltInjection.py:18:27:18:35 | etree.XML string | +| xsltInjection.py:18:17:18:36 | lxml etree xml | xsltInjection.py:21:29:21:37 | lxml etree xml | +| xsltInjection.py:18:17:18:36 | lxml etree xml | xsltInjection.py:21:29:21:37 | lxml etree xml | +| xsltInjection.py:18:27:18:35 | etree.XML string | xsltInjection.py:18:17:18:36 | lxml etree xml | +| xsltInjection.py:18:27:18:35 | etree.XML string | xsltInjection.py:18:17:18:36 | lxml etree xml | +| xsltInjection.py:26:17:26:28 | dict of etree.XML string | xsltInjection.py:26:17:26:43 | etree.XML string | +| xsltInjection.py:26:17:26:28 | dict of etree.XML string | xsltInjection.py:26:17:26:43 | etree.XML string | +| xsltInjection.py:26:17:26:43 | etree.XML string | xsltInjection.py:27:27:27:35 | etree.XML string | +| xsltInjection.py:26:17:26:43 | etree.XML string | xsltInjection.py:27:27:27:35 | etree.XML string | +| xsltInjection.py:27:17:27:36 | lxml etree xml | xsltInjection.py:31:24:31:32 | lxml etree xml | +| xsltInjection.py:27:17:27:36 | lxml etree xml | xsltInjection.py:31:24:31:32 | lxml etree xml | +| xsltInjection.py:27:27:27:35 | etree.XML string | xsltInjection.py:27:17:27:36 | lxml etree xml | +| xsltInjection.py:27:27:27:35 | etree.XML string | xsltInjection.py:27:17:27:36 | lxml etree xml | +| xsltInjection.py:35:17:35:28 | dict of etree.XML string | xsltInjection.py:35:17:35:43 | etree.XML string | +| xsltInjection.py:35:17:35:28 | dict of etree.XML string | xsltInjection.py:35:17:35:43 | etree.XML string | +| xsltInjection.py:35:17:35:43 | etree.XML string | xsltInjection.py:36:34:36:42 | etree.XML string | +| xsltInjection.py:35:17:35:43 | etree.XML string | xsltInjection.py:36:34:36:42 | etree.XML string | +| xsltInjection.py:36:17:36:43 | lxml etree xml | xsltInjection.py:40:24:40:32 | lxml etree xml | +| xsltInjection.py:36:17:36:43 | lxml etree xml | xsltInjection.py:40:24:40:32 | lxml etree xml | +| xsltInjection.py:36:34:36:42 | etree.XML string | xsltInjection.py:36:17:36:43 | lxml etree xml | +| xsltInjection.py:36:34:36:42 | etree.XML string | xsltInjection.py:36:17:36:43 | lxml etree xml | +#select +| xslt.py:14:29:14:37 | xslt_root | xslt.py:10:17:10:28 | dict of etree.XML string | xslt.py:14:29:14:37 | lxml etree xml | This XSLT query depends on $@. | xslt.py:10:17:10:28 | Attribute | a user-provided value | +| xsltInjection.py:12:28:12:36 | xslt_root | xsltInjection.py:10:17:10:28 | dict of etree.XML string | xsltInjection.py:12:28:12:36 | lxml etree xml | This XSLT query depends on $@. | xsltInjection.py:10:17:10:28 | Attribute | a user-provided value | +| xsltInjection.py:21:29:21:37 | xslt_root | xsltInjection.py:17:17:17:28 | dict of etree.XML string | xsltInjection.py:21:29:21:37 | lxml etree xml | This XSLT query depends on $@. | xsltInjection.py:17:17:17:28 | Attribute | a user-provided value | +| xsltInjection.py:31:24:31:32 | xslt_root | xsltInjection.py:26:17:26:28 | dict of etree.XML string | xsltInjection.py:31:24:31:32 | lxml etree xml | This XSLT query depends on $@. | xsltInjection.py:26:17:26:28 | Attribute | a user-provided value | +| xsltInjection.py:40:24:40:32 | xslt_root | xsltInjection.py:35:17:35:28 | dict of etree.XML string | xsltInjection.py:40:24:40:32 | lxml etree xml | This XSLT query depends on $@. | xsltInjection.py:35:17:35:28 | Attribute | a user-provided value | diff --git a/python/ql/test/experimental/CWE-074/Xslt.qlref b/python/ql/test/experimental/CWE-074/Xslt.qlref new file mode 100644 index 000000000000..32605307db89 --- /dev/null +++ b/python/ql/test/experimental/CWE-074/Xslt.qlref @@ -0,0 +1 @@ +experimental/CWE-643/Xslt.ql diff --git a/python/ql/test/experimental/CWE-074/XsltSinks.expected b/python/ql/test/experimental/CWE-074/XsltSinks.expected new file mode 100644 index 000000000000..7150b3046e28 --- /dev/null +++ b/python/ql/test/experimental/CWE-074/XsltSinks.expected @@ -0,0 +1,12 @@ +| xslt.py:14:29:14:37 | lxml.etree.parse.xslt | lxml etree xml | +| xsltInjection.py:12:28:12:36 | lxml.etree.XSLT | lxml etree xml | +| xsltInjection.py:21:29:21:37 | lxml.etree.parse.xslt | lxml etree xml | +| xsltInjection.py:31:24:31:32 | lxml.etree.parse.xslt | lxml etree xml | +| xsltInjection.py:40:24:40:32 | lxml.etree.parse.xslt | lxml etree xml | +| xsltInjection.py:50:24:50:32 | lxml.etree.parse.xslt | lxml etree xml | +| xsltInjection.py:60:24:60:32 | lxml.etree.parse.xslt | lxml etree xml | +| xsltInjection.py:69:24:69:32 | lxml.etree.parse.xslt | lxml etree xml | +| xsltInjection.py:79:24:79:32 | lxml.etree.parse.xslt | lxml etree xml | +| xsltSinks.py:17:28:17:36 | lxml.etree.XSLT | lxml etree xml | +| xsltSinks.py:30:29:30:37 | lxml.etree.parse.xslt | lxml etree xml | +| xsltSinks.py:44:24:44:32 | lxml.etree.parse.xslt | lxml etree xml | diff --git a/python/ql/test/experimental/CWE-074/XsltSinks.ql b/python/ql/test/experimental/CWE-074/XsltSinks.ql new file mode 100644 index 000000000000..2149e6921d0b --- /dev/null +++ b/python/ql/test/experimental/CWE-074/XsltSinks.ql @@ -0,0 +1,6 @@ +import python +import experimental.semmle.python.security.injection.XSLT + +from XSLTInjection::XSLTInjectionSink sink, TaintKind kind +where sink.sinks(kind) +select sink, kind diff --git a/python/ql/test/experimental/CWE-074/options b/python/ql/test/experimental/CWE-074/options new file mode 100644 index 000000000000..79d9dcd31b66 --- /dev/null +++ b/python/ql/test/experimental/CWE-074/options @@ -0,0 +1 @@ +semmle-extractor-options: -p ../../query-tests/Security/lib/ --max-import-depth=3 diff --git a/python/ql/test/experimental/CWE-074/xslt.py b/python/ql/test/experimental/CWE-074/xslt.py new file mode 100644 index 000000000000..1655916c7e06 --- /dev/null +++ b/python/ql/test/experimental/CWE-074/xslt.py @@ -0,0 +1,14 @@ +from lxml import etree +from io import StringIO +from flask import Flask, request + +app = Flask(__name__) + + +@app.route("/xslt") +def bad(): + xsltQuery = request.args.get('xml', '') + xslt_root = etree.XML(xsltQuery) + f = StringIO('') + tree = etree.parse(f) + result_tree = tree.xslt(xslt_root) # Not OK diff --git a/python/ql/test/experimental/CWE-074/xsltInjection.py b/python/ql/test/experimental/CWE-074/xsltInjection.py new file mode 100644 index 000000000000..ddab954bbff8 --- /dev/null +++ b/python/ql/test/experimental/CWE-074/xsltInjection.py @@ -0,0 +1,79 @@ +from lxml import etree +from io import StringIO +from flask import Flask, request + +app = Flask(__name__) + + +@app.route("/xslt1") +def a(): + xsltQuery = request.args.get('xml', '') + xslt_root = etree.XML(xsltQuery) + transform = etree.XSLT(xslt_root) # Not OK + + +@app.route("/xslt2") +def b(): + xsltQuery = request.args.get('xml', '') + xslt_root = etree.XML(xsltQuery) + f = StringIO('') + tree = etree.parse(f) + result_tree = tree.xslt(xslt_root) # Not OK + + +@app.route("/xslt3") +def c(): + xsltQuery = request.args.get('xml', '') + xslt_root = etree.XML(xsltQuery) + + f = StringIO('') + tree = etree.parse(f) + result = tree.xslt(xslt_root, a="'A'") # Not OK + +@app.route("/xslt4") +def d(): + xsltQuery = request.args.get('xml', '') + xslt_root = etree.fromstring(xsltQuery) + + f = StringIO('') + tree = etree.parse(f) + result = tree.xslt(xslt_root, a="'A'") # Not OK + +@app.route("/xslt5") +def e(): + xsltQuery = request.args.get('xml', '') + xsltStrings = [xsltQuery,"asd","random"] + xslt_root = etree.fromstringlist(xsltStrings) + + f = StringIO('') + tree = etree.parse(f) + result = tree.xslt(xslt_root, a="'A'") # Not OK + + +@app.route("/xslt6") +def f(): + xsltQuery = '' + xslt_root = etree.XML(xsltQuery) + + f = StringIO('') + tree = etree.parse(f) + result = tree.xslt(xslt_root, a="'A'") # OK + +@app.route("/xslt7") +def g(): + xsltQuery = '' + xslt_root = etree.fromstring(xsltQuery) + + f = StringIO('') + tree = etree.parse(f) + result = tree.xslt(xslt_root, a="'A'") # OK + +@app.route("/xslt8") +def h(): + xsltQuery = '' + xsltStrings = [xsltQuery,"asd","random"] + xslt_root = etree.fromstringlist(xsltStrings) + + f = StringIO('') + tree = etree.parse(f) + result = tree.xslt(xslt_root, a="'A'") # OK \ No newline at end of file diff --git a/python/ql/test/experimental/CWE-074/xsltSinks.py b/python/ql/test/experimental/CWE-074/xsltSinks.py new file mode 100644 index 000000000000..a82fc0c6c5fc --- /dev/null +++ b/python/ql/test/experimental/CWE-074/xsltSinks.py @@ -0,0 +1,56 @@ +from lxml import etree +from io import StringIO + +from django.urls import path +from django.http import HttpResponse +from django.template import Template, Context, Engine, engines + + +def a(request): + xslt_root = etree.XML('''\ + + + + + ''') + transform = etree.XSLT(xslt_root) + + +def b(request): + xslt_root = etree.XML('''\ + + + + + ''') + f = StringIO('') + tree = etree.parse(f) + result_tree = tree.xslt(xslt_root) + + +def c(request): + xslt_root = etree.XML('''\ + + + + + ''') + + f = StringIO('') + tree = etree.parse(f) + result = tree.xslt(xslt_root, a="'A'") + + +urlpatterns = [ + path('a', a), + path('b', b), + path('c', c) +] + +if __name__ == "__main__": + a(None) + b(None) + c(None) diff --git a/python/ql/test/query-tests/Security/lib/lxml/__init__.py b/python/ql/test/query-tests/Security/lib/lxml/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/python/ql/test/query-tests/Security/lib/lxml/etree.py b/python/ql/test/query-tests/Security/lib/lxml/etree.py new file mode 100644 index 000000000000..b50046ba6f7c --- /dev/null +++ b/python/ql/test/query-tests/Security/lib/lxml/etree.py @@ -0,0 +1,37 @@ +class _ElementTree(object): + def xpath(self, _path, namespaces=None, extensions=None, smart_strings=True, **_variables): + pass + + def xslt(self, _xslt, extensions=None, access_control=None, **_kw): + pass + + +class ETXPath(object): + def __init__(self, path, extensions=None, regexp=True, smart_strings=True): + pass + + +class Xpath(object): + def __init__(self, path, namespaces=None, extensions=None, regexp=True, smart_strings=True): + pass + + +class XSLT(object): + def __init__(self, xslt_input, extensions=None, regexp=True, access_control=None): + pass + + +def parse(self, parser=None, base_url=None): + return _ElementTree() + + +def fromstring(self, text, parser=None, base_url=None): + pass + + +def fromstringlist(self, strings, parser=None): + pass + + +def XML(self, text, parser=None, base_url=None): + pass From 6dd9106301dcb23b6c7d3ed62e78d2819c548f21 Mon Sep 17 00:00:00 2001 From: porcupineyhairs <61983466+porcupineyhairs@users.noreply.github.com> Date: Mon, 8 Jun 2020 03:12:23 +0530 Subject: [PATCH 02/10] Update XSLT.qll --- .../src/experimental/semmle/python/security/injection/XSLT.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/ql/src/experimental/semmle/python/security/injection/XSLT.qll b/python/ql/src/experimental/semmle/python/security/injection/XSLT.qll index 795d439d83a4..a0680f91de33 100644 --- a/python/ql/src/experimental/semmle/python/security/injection/XSLT.qll +++ b/python/ql/src/experimental/semmle/python/security/injection/XSLT.qll @@ -7,7 +7,7 @@ */ import python -import semmle.python.security.TaintTracking +import semmle.python.dataflow.TaintTracking import semmle.python.web.HttpRequest /** Models XSLT Injection related classes and functions */ From 33a9fb6034b3a58bc273abe926d5f30d9667d12b Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Thu, 11 Jun 2020 11:30:54 +0200 Subject: [PATCH 03/10] Python: Reorder XSLT qhelp to be valid --- python/ql/src/experimental/CWE-643/Xslt.qhelp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/python/ql/src/experimental/CWE-643/Xslt.qhelp b/python/ql/src/experimental/CWE-643/Xslt.qhelp index cdcc7e4dada5..a3eb44f03c35 100644 --- a/python/ql/src/experimental/CWE-643/Xslt.qhelp +++ b/python/ql/src/experimental/CWE-643/Xslt.qhelp @@ -8,9 +8,9 @@ This vulnerability can be prevented by not allowing untrusted user input to be passed as a XSL stylesheet. If the application logic necessiates processing untrusted XSL stylesheets, the input should be properly filtered and sanitized before use.

- -

In the example below, the XSL stylesheet is controlled by the user and hence leads to a vulnerability.

- - - \ No newline at end of file + +

In the example below, the XSL stylesheet is controlled by the user and hence leads to a vulnerability.

+ + + From a24974b1946ab8e1ed72b02622f460b51d988173 Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Thu, 11 Jun 2020 11:45:38 +0200 Subject: [PATCH 04/10] Python: Add missing

to qhelp --- python/ql/src/experimental/CWE-643/Xslt.qhelp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/python/ql/src/experimental/CWE-643/Xslt.qhelp b/python/ql/src/experimental/CWE-643/Xslt.qhelp index a3eb44f03c35..d82dd8ab2c64 100644 --- a/python/ql/src/experimental/CWE-643/Xslt.qhelp +++ b/python/ql/src/experimental/CWE-643/Xslt.qhelp @@ -1,7 +1,9 @@ - Processing an unvalidated XSL stylesheet can allow an attacker to change the structure and contents of the resultant XML, include arbitrary files from the file system, or execute arbitrary code. +

+ Processing an unvalidated XSL stylesheet can allow an attacker to change the structure and contents of the resultant XML, include arbitrary files from the file system, or execute arbitrary code. +

From 994db060c70de5783da0f959b4e2cf53f6b8745d Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Thu, 25 Jun 2020 11:53:12 +0200 Subject: [PATCH 05/10] Python: Use CWE-091 for XSLT As indicated here https://www.zaproxy.org/docs/alerts/90017/ --- python/ql/src/experimental/{CWE-643 => CWE-091}/Xslt.qhelp | 0 python/ql/src/experimental/{CWE-643 => CWE-091}/Xslt.ql | 0 python/ql/src/experimental/{CWE-643 => CWE-091}/xslt.py | 0 python/ql/test/experimental/{CWE-074 => CWE-091}/Xslt.expected | 0 python/ql/test/experimental/{CWE-074 => CWE-091}/Xslt.qlref | 0 .../ql/test/experimental/{CWE-074 => CWE-091}/XsltSinks.expected | 0 python/ql/test/experimental/{CWE-074 => CWE-091}/XsltSinks.ql | 0 python/ql/test/experimental/{CWE-074 => CWE-091}/options | 0 python/ql/test/experimental/{CWE-074 => CWE-091}/xslt.py | 0 python/ql/test/experimental/{CWE-074 => CWE-091}/xsltInjection.py | 0 python/ql/test/experimental/{CWE-074 => CWE-091}/xsltSinks.py | 0 11 files changed, 0 insertions(+), 0 deletions(-) rename python/ql/src/experimental/{CWE-643 => CWE-091}/Xslt.qhelp (100%) rename python/ql/src/experimental/{CWE-643 => CWE-091}/Xslt.ql (100%) rename python/ql/src/experimental/{CWE-643 => CWE-091}/xslt.py (100%) rename python/ql/test/experimental/{CWE-074 => CWE-091}/Xslt.expected (100%) rename python/ql/test/experimental/{CWE-074 => CWE-091}/Xslt.qlref (100%) rename python/ql/test/experimental/{CWE-074 => CWE-091}/XsltSinks.expected (100%) rename python/ql/test/experimental/{CWE-074 => CWE-091}/XsltSinks.ql (100%) rename python/ql/test/experimental/{CWE-074 => CWE-091}/options (100%) rename python/ql/test/experimental/{CWE-074 => CWE-091}/xslt.py (100%) rename python/ql/test/experimental/{CWE-074 => CWE-091}/xsltInjection.py (100%) rename python/ql/test/experimental/{CWE-074 => CWE-091}/xsltSinks.py (100%) diff --git a/python/ql/src/experimental/CWE-643/Xslt.qhelp b/python/ql/src/experimental/CWE-091/Xslt.qhelp similarity index 100% rename from python/ql/src/experimental/CWE-643/Xslt.qhelp rename to python/ql/src/experimental/CWE-091/Xslt.qhelp diff --git a/python/ql/src/experimental/CWE-643/Xslt.ql b/python/ql/src/experimental/CWE-091/Xslt.ql similarity index 100% rename from python/ql/src/experimental/CWE-643/Xslt.ql rename to python/ql/src/experimental/CWE-091/Xslt.ql diff --git a/python/ql/src/experimental/CWE-643/xslt.py b/python/ql/src/experimental/CWE-091/xslt.py similarity index 100% rename from python/ql/src/experimental/CWE-643/xslt.py rename to python/ql/src/experimental/CWE-091/xslt.py diff --git a/python/ql/test/experimental/CWE-074/Xslt.expected b/python/ql/test/experimental/CWE-091/Xslt.expected similarity index 100% rename from python/ql/test/experimental/CWE-074/Xslt.expected rename to python/ql/test/experimental/CWE-091/Xslt.expected diff --git a/python/ql/test/experimental/CWE-074/Xslt.qlref b/python/ql/test/experimental/CWE-091/Xslt.qlref similarity index 100% rename from python/ql/test/experimental/CWE-074/Xslt.qlref rename to python/ql/test/experimental/CWE-091/Xslt.qlref diff --git a/python/ql/test/experimental/CWE-074/XsltSinks.expected b/python/ql/test/experimental/CWE-091/XsltSinks.expected similarity index 100% rename from python/ql/test/experimental/CWE-074/XsltSinks.expected rename to python/ql/test/experimental/CWE-091/XsltSinks.expected diff --git a/python/ql/test/experimental/CWE-074/XsltSinks.ql b/python/ql/test/experimental/CWE-091/XsltSinks.ql similarity index 100% rename from python/ql/test/experimental/CWE-074/XsltSinks.ql rename to python/ql/test/experimental/CWE-091/XsltSinks.ql diff --git a/python/ql/test/experimental/CWE-074/options b/python/ql/test/experimental/CWE-091/options similarity index 100% rename from python/ql/test/experimental/CWE-074/options rename to python/ql/test/experimental/CWE-091/options diff --git a/python/ql/test/experimental/CWE-074/xslt.py b/python/ql/test/experimental/CWE-091/xslt.py similarity index 100% rename from python/ql/test/experimental/CWE-074/xslt.py rename to python/ql/test/experimental/CWE-091/xslt.py diff --git a/python/ql/test/experimental/CWE-074/xsltInjection.py b/python/ql/test/experimental/CWE-091/xsltInjection.py similarity index 100% rename from python/ql/test/experimental/CWE-074/xsltInjection.py rename to python/ql/test/experimental/CWE-091/xsltInjection.py diff --git a/python/ql/test/experimental/CWE-074/xsltSinks.py b/python/ql/test/experimental/CWE-091/xsltSinks.py similarity index 100% rename from python/ql/test/experimental/CWE-074/xsltSinks.py rename to python/ql/test/experimental/CWE-091/xsltSinks.py From e60af68b293095da82fefbd8770734a595bc8571 Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Thu, 25 Jun 2020 11:54:34 +0200 Subject: [PATCH 06/10] Python: Move lxml.etree library stub (so merge is easy) --- .../query-tests/Security/lib/lxml/{etree.py => etree/__init__.py} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename python/ql/test/query-tests/Security/lib/lxml/{etree.py => etree/__init__.py} (100%) diff --git a/python/ql/test/query-tests/Security/lib/lxml/etree.py b/python/ql/test/query-tests/Security/lib/lxml/etree/__init__.py similarity index 100% rename from python/ql/test/query-tests/Security/lib/lxml/etree.py rename to python/ql/test/query-tests/Security/lib/lxml/etree/__init__.py From 1e5eeb80096145159b5c8ec3b573019be9d8f6e9 Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Thu, 25 Jun 2020 12:03:15 +0200 Subject: [PATCH 07/10] Python: Move lxml.etree library stub to reduce clutter --- .../query-tests/Security/lib/lxml/{etree/__init__.py => etree.py} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename python/ql/test/query-tests/Security/lib/lxml/{etree/__init__.py => etree.py} (100%) diff --git a/python/ql/test/query-tests/Security/lib/lxml/etree/__init__.py b/python/ql/test/query-tests/Security/lib/lxml/etree.py similarity index 100% rename from python/ql/test/query-tests/Security/lib/lxml/etree/__init__.py rename to python/ql/test/query-tests/Security/lib/lxml/etree.py From 22ad8f717fc7403d01a3fff4f666bb2f5c228653 Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Thu, 25 Jun 2020 12:06:52 +0200 Subject: [PATCH 08/10] Python: Remove usage of .getASuccessor() in XSLT.qll --- .../experimental/semmle/python/security/injection/XSLT.qll | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/python/ql/src/experimental/semmle/python/security/injection/XSLT.qll b/python/ql/src/experimental/semmle/python/security/injection/XSLT.qll index a0680f91de33..0fe6348b10a8 100644 --- a/python/ql/src/experimental/semmle/python/security/injection/XSLT.qll +++ b/python/ql/src/experimental/semmle/python/security/injection/XSLT.qll @@ -41,12 +41,7 @@ module XSLTInjection { } private predicate etreeXML(ControlFlowNode fromnode, CallNode tonode) { - exists(CallNode call, AttrNode atr | - atr = etree().getAReference().getASuccessor() and - // XML(text, parser=None, base_url=None) - atr.getName() = "XML" and - atr = call.getFunction() - | + exists(CallNode call | call.getFunction().(AttrNode).getObject("XML").pointsTo(etree()) | call.getArg(0) = fromnode and call = tonode ) From 08384e30af15e69ebfa1ae49e5e78d65f4752559 Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Fri, 26 Jun 2020 12:06:31 +0200 Subject: [PATCH 09/10] Python: Minor doc fixes from review --- python/ql/src/experimental/CWE-091/Xslt.qhelp | 2 +- .../experimental/semmle/python/security/injection/XSLT.qll | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/python/ql/src/experimental/CWE-091/Xslt.qhelp b/python/ql/src/experimental/CWE-091/Xslt.qhelp index d82dd8ab2c64..d35da4e82d49 100644 --- a/python/ql/src/experimental/CWE-091/Xslt.qhelp +++ b/python/ql/src/experimental/CWE-091/Xslt.qhelp @@ -7,7 +7,7 @@

- This vulnerability can be prevented by not allowing untrusted user input to be passed as a XSL stylesheet. + This vulnerability can be prevented by not allowing untrusted user input to be passed as an XSL stylesheet. If the application logic necessiates processing untrusted XSL stylesheets, the input should be properly filtered and sanitized before use.

diff --git a/python/ql/src/experimental/semmle/python/security/injection/XSLT.qll b/python/ql/src/experimental/semmle/python/security/injection/XSLT.qll index 0fe6348b10a8..d4f2814df97d 100644 --- a/python/ql/src/experimental/semmle/python/security/injection/XSLT.qll +++ b/python/ql/src/experimental/semmle/python/security/injection/XSLT.qll @@ -41,6 +41,7 @@ module XSLTInjection { } private predicate etreeXML(ControlFlowNode fromnode, CallNode tonode) { + // etree.XML("") exists(CallNode call | call.getFunction().(AttrNode).getObject("XML").pointsTo(etree()) | call.getArg(0) = fromnode and call = tonode @@ -48,7 +49,7 @@ module XSLTInjection { } private predicate etreeFromString(ControlFlowNode fromnode, CallNode tonode) { - // fromstring(text, parser=None) + // etree.fromstring(text, parser=None) exists(CallNode call | call.getFunction().(AttrNode).getObject("fromstring").pointsTo(etree()) | call.getArg(0) = fromnode and call = tonode @@ -56,7 +57,7 @@ module XSLTInjection { } private predicate etreeFromStringList(ControlFlowNode fromnode, CallNode tonode) { - // fromstringlist(strings, parser=None) + // etree.fromstringlist(strings, parser=None) exists(CallNode call | call.getFunction().(AttrNode).getObject("fromstringlist").pointsTo(etree()) | From b164f2695dd592530e07b49a94535a6cfc1acf0f Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Fri, 26 Jun 2020 12:08:12 +0200 Subject: [PATCH 10/10] Python: One more minor doc fix from review --- python/ql/src/experimental/CWE-091/Xslt.ql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/ql/src/experimental/CWE-091/Xslt.ql b/python/ql/src/experimental/CWE-091/Xslt.ql index 09345858160e..aa35d92c01a7 100644 --- a/python/ql/src/experimental/CWE-091/Xslt.ql +++ b/python/ql/src/experimental/CWE-091/Xslt.ql @@ -1,5 +1,5 @@ /** - * @name Xslt query built from user-controlled sources + * @name XSLT query built from user-controlled sources * @description Building a XSLT query from user-controlled sources is vulnerable to insertion of * malicious XSLT code by the user. * @kind path-problem