From 8c5a97170d98279a3b6944f202b2d8245c626141 Mon Sep 17 00:00:00 2001 From: Porcupiney Hairs Date: Wed, 13 May 2020 18:38:08 +0530 Subject: [PATCH 1/4] Python : Add Xpath injection query This PR adds support for detecting XPATH injection in Python. I have included the ql files as well as the tests with this. --- python/ql/src/experimental/CWE-643/xpath.py | 18 ++++ .../ql/src/experimental/CWE-643/xpath.qhelp | 32 +++++++ python/ql/src/experimental/CWE-643/xpath.ql | 35 +++++++ .../python/security/injection/Xpath.qll | 96 +++++++++++++++++++ .../CWE-643/XpathLibTests/options | 1 + .../CWE-643/XpathLibTests/xpath.py | 33 +++++++ .../CWE-643/XpathLibTests/xpathSinks.expected | 4 + .../CWE-643/XpathLibTests/xpathSinks.ql | 6 ++ python/ql/test/experimental/CWE-643/options | 1 + .../test/experimental/CWE-643/xpath.expected | 22 +++++ .../ql/test/experimental/CWE-643/xpath.qlref | 1 + .../ql/test/experimental/CWE-643/xpathFlow.py | 38 ++++++++ .../query-tests/Security/lib/lxml/__init__.py | 0 .../Security/lib/lxml/etree/__init__.py | 37 +++++++ 14 files changed, 324 insertions(+) create mode 100644 python/ql/src/experimental/CWE-643/xpath.py create mode 100644 python/ql/src/experimental/CWE-643/xpath.qhelp create mode 100644 python/ql/src/experimental/CWE-643/xpath.ql create mode 100644 python/ql/src/experimental/semmle/python/security/injection/Xpath.qll create mode 100644 python/ql/test/experimental/CWE-643/XpathLibTests/options create mode 100644 python/ql/test/experimental/CWE-643/XpathLibTests/xpath.py create mode 100644 python/ql/test/experimental/CWE-643/XpathLibTests/xpathSinks.expected create mode 100644 python/ql/test/experimental/CWE-643/XpathLibTests/xpathSinks.ql create mode 100644 python/ql/test/experimental/CWE-643/options create mode 100644 python/ql/test/experimental/CWE-643/xpath.expected create mode 100644 python/ql/test/experimental/CWE-643/xpath.qlref create mode 100644 python/ql/test/experimental/CWE-643/xpathFlow.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/__init__.py diff --git a/python/ql/src/experimental/CWE-643/xpath.py b/python/ql/src/experimental/CWE-643/xpath.py new file mode 100644 index 000000000000..69732a8f9a3d --- /dev/null +++ b/python/ql/src/experimental/CWE-643/xpath.py @@ -0,0 +1,18 @@ +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): + xpathQuery = request.GET['xpath'] + f = StringIO('') + tree = etree.parse(f) + r = tree.xpath(xpathQuery) + + +urlpatterns = [ + path('a', a) +] diff --git a/python/ql/src/experimental/CWE-643/xpath.qhelp b/python/ql/src/experimental/CWE-643/xpath.qhelp new file mode 100644 index 000000000000..434cdacd4d17 --- /dev/null +++ b/python/ql/src/experimental/CWE-643/xpath.qhelp @@ -0,0 +1,32 @@ + + + + Using user-supplied information to construct an XPath query for XML data can + result in an XPath injection flaw. By sending intentionally malformed information, + an attacker can access data that he may not normally have access to. + He/She may even be able to elevate his privileges on the web site if the XML data + is being used for authentication (such as an XML based user file). + + +

+ XPath injection can be prevented using parameterized XPath interface or escaping the user input to make it safe to include in a dynamically constructed query. + If you are using quotes to terminate untrusted input in a dynamically constructed XPath query, then you need to escape that quote in the untrusted input to ensure the untrusted data can’t try to break out of that quoted context. +

+

+ Another better mitigation option is to use a precompiled XPath query. Precompiled XPath queries are already preset before the program executes, rather than created on the fly after the user’s input has been added to the string. This is a better route because you don’t have to worry about missing a character that should have been escaped. +

+ + +

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

+ + + + +
  • OWASP XPath injection : />>
    • +     + + +     + + +     \ No newline at end of file diff --git a/python/ql/src/experimental/CWE-643/xpath.ql b/python/ql/src/experimental/CWE-643/xpath.ql new file mode 100644 index 000000000000..fbdf57d4f1ac --- /dev/null +++ b/python/ql/src/experimental/CWE-643/xpath.ql @@ -0,0 +1,35 @@ +/** + * @name XPath query built from user-controlled sources + * @description Building a XPath query from user-controlled sources is vulnerable to insertion of + * malicious Xpath code by the user. + * @kind path-problem + * @problem.severity error + * @precision high + * @id py/xpath-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.Xpath + +class XpathInjectionConfiguration extends TaintTracking::Configuration { + XpathInjectionConfiguration() { this = "Xpath injection configuration" } + + override predicate isSource(TaintTracking::Source source) { + source instanceof HttpRequestTaintSource + } + + override predicate isSink(TaintTracking::Sink sink) { + sink instanceof XpathInjection::XpathInjectionSink + } +} + +from XpathInjectionConfiguration config, TaintedPathSource src, TaintedPathSink sink +where config.hasFlowPath(src, sink) +select sink.getSink(), src, sink, "This Xpath query depends on $@.", src.getSource(), + "a user-provided value" diff --git a/python/ql/src/experimental/semmle/python/security/injection/Xpath.qll b/python/ql/src/experimental/semmle/python/security/injection/Xpath.qll new file mode 100644 index 000000000000..94276997e218 --- /dev/null +++ b/python/ql/src/experimental/semmle/python/security/injection/Xpath.qll @@ -0,0 +1,96 @@ +/** + * Provides class and predicates to track external data that + * may represent malicious xpath 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 Xpath Injection related classes and functions */ +module XpathInjection { + /** Returns a class value which refers to `lxml.etree` */ + Value etree() { result = Value::named("lxml.etree") } + + /** A generic taint sink that is vulnerable to Xpath injection. */ + abstract class XpathInjectionSink extends TaintSink { } + + /** + * A Sink representing an argument to the `etree.Xpath` call. + * + * from lxml import etree + * root = etree.XML("") + * find_text = etree.XPath("`sink`") + */ + private class EtreeXpathArgument extends XpathInjectionSink { + override string toString() { result = "lxml.etree.Xpath" } + + EtreeXpathArgument() { + exists(CallNode call, AttrNode atr | + atr = etree().getAReference().getASuccessor() and + atr.getName() = "XPath" and + atr = call.getFunction() + | + call.getArg(0) = this + ) + } + + override predicate sinks(TaintKind kind) { kind instanceof ExternalStringKind } + } + + /** + * A Sink representing an argument to the `etree.EtXpath` call. + * + * from lxml import etree + * root = etree.XML("") + * find_text = etree.EtXPath("`sink`") + */ + private class EtreeETXpathArgument extends XpathInjectionSink { + override string toString() { result = "lxml.etree.ETXpath" } + + EtreeETXpathArgument() { + exists(CallNode call, AttrNode atr | + atr = etree().getAReference().getASuccessor() and + atr.getName() = "ETXPath" and + atr = call.getFunction() + | + call.getArg(0) = this + ) + } + + override predicate sinks(TaintKind kind) { kind instanceof ExternalStringKind } + } + + /** + * A Sink representing an argument to the `xpath` call to a parsed xml document. + * + * from lxml import etree + * from io import StringIO + * f = StringIO('') + * tree = etree.parse(f) + * r = tree.xpath('`sink`') + */ + private class ParseXpathArgument extends XpathInjectionSink { + override string toString() { result = "lxml.etree.parse.xpath" } + + ParseXpathArgument() { + exists(CallNode parseCall, AttrNode parse, string s | + parse = etree().getAReference().getASuccessor() and + parse.getName() = "parse" and + parse = parseCall.getFunction() and + exists(CallNode xpathCall, AttrNode xpath | + xpath = parseCall.getASuccessor*() and + xpath.getName() = "xpath" and + xpath = xpathCall.getFunction() and + s = xpath.getName() and + this = xpathCall.getArg(0) + ) + ) + } + + override predicate sinks(TaintKind kind) { kind instanceof ExternalStringKind } + } +} diff --git a/python/ql/test/experimental/CWE-643/XpathLibTests/options b/python/ql/test/experimental/CWE-643/XpathLibTests/options new file mode 100644 index 000000000000..7fb713d59242 --- /dev/null +++ b/python/ql/test/experimental/CWE-643/XpathLibTests/options @@ -0,0 +1 @@ +semmle-extractor-options: --max-import-depth=3 -p ../../../query-tests/Security/lib/ diff --git a/python/ql/test/experimental/CWE-643/XpathLibTests/xpath.py b/python/ql/test/experimental/CWE-643/XpathLibTests/xpath.py new file mode 100644 index 000000000000..8b0e06d78be0 --- /dev/null +++ b/python/ql/test/experimental/CWE-643/XpathLibTests/xpath.py @@ -0,0 +1,33 @@ +from lxml import etree +from io import StringIO + + +def a(): + f = StringIO('') + tree = etree.parse(f) + r = tree.xpath('/foo/bar') + + +def b(): + root = etree.XML("TEXT") + find_text = etree.XPath("//text()") + text = find_text(root)[0] + + +def c(): + root = etree.XML("TEXT") + find_text = etree.XPath("//text()", smart_strings=False) + text = find_text(root)[0] + + +def d(): + root = etree.XML("TEXT") + find_text = find = etree.ETXPath("//{ns}b") + text = find_text(root)[0] + + +if __name__ == "__main__": + a() + b() + c() + d() diff --git a/python/ql/test/experimental/CWE-643/XpathLibTests/xpathSinks.expected b/python/ql/test/experimental/CWE-643/XpathLibTests/xpathSinks.expected new file mode 100644 index 000000000000..f40dd8ece3b0 --- /dev/null +++ b/python/ql/test/experimental/CWE-643/XpathLibTests/xpathSinks.expected @@ -0,0 +1,4 @@ +| xpath.py:8:20:8:29 | lxml.etree.parse.xpath | externally controlled string | +| xpath.py:13:29:13:38 | lxml.etree.Xpath | externally controlled string | +| xpath.py:19:29:19:38 | lxml.etree.Xpath | externally controlled string | +| xpath.py:25:38:25:46 | lxml.etree.ETXpath | externally controlled string | diff --git a/python/ql/test/experimental/CWE-643/XpathLibTests/xpathSinks.ql b/python/ql/test/experimental/CWE-643/XpathLibTests/xpathSinks.ql new file mode 100644 index 000000000000..8a96e90035cc --- /dev/null +++ b/python/ql/test/experimental/CWE-643/XpathLibTests/xpathSinks.ql @@ -0,0 +1,6 @@ +import python +import experimental.semmle.python.security.injection.Xpath + +from XpathInjection::XpathInjectionSink sink, TaintKind kind +where sink.sinks(kind) +select sink, kind diff --git a/python/ql/test/experimental/CWE-643/options b/python/ql/test/experimental/CWE-643/options new file mode 100644 index 000000000000..48b8916042aa --- /dev/null +++ b/python/ql/test/experimental/CWE-643/options @@ -0,0 +1 @@ +semmle-extractor-options: --max-import-depth=3 -p ../../query-tests/Security/lib/ diff --git a/python/ql/test/experimental/CWE-643/xpath.expected b/python/ql/test/experimental/CWE-643/xpath.expected new file mode 100644 index 000000000000..3a1ecc3888fd --- /dev/null +++ b/python/ql/test/experimental/CWE-643/xpath.expected @@ -0,0 +1,22 @@ +edges +| xpathFlow.py:10:18:10:29 | dict of externally controlled string | xpathFlow.py:10:18:10:44 | externally controlled string | +| xpathFlow.py:10:18:10:29 | dict of externally controlled string | xpathFlow.py:10:18:10:44 | externally controlled string | +| xpathFlow.py:10:18:10:44 | externally controlled string | xpathFlow.py:13:20:13:29 | externally controlled string | +| xpathFlow.py:10:18:10:44 | externally controlled string | xpathFlow.py:13:20:13:29 | externally controlled string | +| xpathFlow.py:18:18:18:29 | dict of externally controlled string | xpathFlow.py:18:18:18:44 | externally controlled string | +| xpathFlow.py:18:18:18:29 | dict of externally controlled string | xpathFlow.py:18:18:18:44 | externally controlled string | +| xpathFlow.py:18:18:18:44 | externally controlled string | xpathFlow.py:21:29:21:38 | externally controlled string | +| xpathFlow.py:18:18:18:44 | externally controlled string | xpathFlow.py:21:29:21:38 | externally controlled string | +| xpathFlow.py:27:18:27:29 | dict of externally controlled string | xpathFlow.py:27:18:27:44 | externally controlled string | +| xpathFlow.py:27:18:27:29 | dict of externally controlled string | xpathFlow.py:27:18:27:44 | externally controlled string | +| xpathFlow.py:27:18:27:44 | externally controlled string | xpathFlow.py:29:29:29:38 | externally controlled string | +| xpathFlow.py:27:18:27:44 | externally controlled string | xpathFlow.py:29:29:29:38 | externally controlled string | +| xpathFlow.py:35:18:35:29 | dict of externally controlled string | xpathFlow.py:35:18:35:44 | externally controlled string | +| xpathFlow.py:35:18:35:29 | dict of externally controlled string | xpathFlow.py:35:18:35:44 | externally controlled string | +| xpathFlow.py:35:18:35:44 | externally controlled string | xpathFlow.py:37:38:37:47 | externally controlled string | +| xpathFlow.py:35:18:35:44 | externally controlled string | xpathFlow.py:37:38:37:47 | externally controlled string | +#select +| xpathFlow.py:13:20:13:29 | xpathQuery | xpathFlow.py:10:18:10:29 | dict of externally controlled string | xpathFlow.py:13:20:13:29 | externally controlled string | This Xpath query depends on $@. | xpathFlow.py:10:18:10:29 | Attribute | a user-provided value | +| xpathFlow.py:21:29:21:38 | xpathQuery | xpathFlow.py:18:18:18:29 | dict of externally controlled string | xpathFlow.py:21:29:21:38 | externally controlled string | This Xpath query depends on $@. | xpathFlow.py:18:18:18:29 | Attribute | a user-provided value | +| xpathFlow.py:29:29:29:38 | xpathQuery | xpathFlow.py:27:18:27:29 | dict of externally controlled string | xpathFlow.py:29:29:29:38 | externally controlled string | This Xpath query depends on $@. | xpathFlow.py:27:18:27:29 | Attribute | a user-provided value | +| xpathFlow.py:37:38:37:47 | xpathQuery | xpathFlow.py:35:18:35:29 | dict of externally controlled string | xpathFlow.py:37:38:37:47 | externally controlled string | This Xpath query depends on $@. | xpathFlow.py:35:18:35:29 | Attribute | a user-provided value | diff --git a/python/ql/test/experimental/CWE-643/xpath.qlref b/python/ql/test/experimental/CWE-643/xpath.qlref new file mode 100644 index 000000000000..61dcb500e5e0 --- /dev/null +++ b/python/ql/test/experimental/CWE-643/xpath.qlref @@ -0,0 +1 @@ +experimental/CWE-643/xpath.ql \ No newline at end of file diff --git a/python/ql/test/experimental/CWE-643/xpathFlow.py b/python/ql/test/experimental/CWE-643/xpathFlow.py new file mode 100644 index 000000000000..714169d9bd68 --- /dev/null +++ b/python/ql/test/experimental/CWE-643/xpathFlow.py @@ -0,0 +1,38 @@ +from lxml import etree +from io import StringIO +from flask import Flask, request + +app = Flask(__name__) + + +@app.route("/xpath1") +def a(): + xpathQuery = request.args.get('xml', '') + f = StringIO('') + tree = etree.parse(f) + r = tree.xpath(xpathQuery) + + +@app.route("/xpath2") +def b(): + xpathQuery = request.args.get('xml', '') + + root = etree.XML("TEXT") + find_text = etree.XPath(xpathQuery) + text = find_text(root)[0] + + +@app.route("/xpath3") +def c(): + xpathQuery = request.args.get('xml', '') + root = etree.XML("TEXT") + find_text = etree.XPath(xpathQuery, smart_strings=False) + text = find_text(root)[0] + + +@app.route("/xpath4") +def d(): + xpathQuery = request.args.get('xml', '') + root = etree.XML("TEXT") + find_text = find = etree.ETXPath(xpathQuery) + text = find_text(root)[0] \ No newline at end of file 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/__init__.py b/python/ql/test/query-tests/Security/lib/lxml/etree/__init__.py new file mode 100644 index 000000000000..139553b0d6c7 --- /dev/null +++ b/python/ql/test/query-tests/Security/lib/lxml/etree/__init__.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 424e88d318b1a5fae565bae6a91be5f95b6789f9 Mon Sep 17 00:00:00 2001 From: Porcupiney Hairs Date: Mon, 8 Jun 2020 02:52:11 +0530 Subject: [PATCH 2/4] include sugestions from review --- .../ql/src/experimental/CWE-643/xpath.qhelp | 34 +++++++++---------- .../CWE-643/{xpath.py => xpathBad.py} | 4 +-- .../ql/src/experimental/CWE-643/xpathGood.py | 18 ++++++++++ .../python/security/injection/Xpath.qll | 34 +++++++------------ .../CWE-643/XpathLibTests/options | 1 - .../CWE-643/XpathLibTests/xpathSinks.expected | 4 --- .../test/experimental/CWE-643/xpath.expected | 17 ++++++++-- .../CWE-643/{XpathLibTests => }/xpath.py | 0 .../ql/test/experimental/CWE-643/xpathBad.py | 18 ++++++++++ .../ql/test/experimental/CWE-643/xpathFlow.py | 12 +++---- .../ql/test/experimental/CWE-643/xpathGood.py | 18 ++++++++++ .../experimental/CWE-643/xpathSinks.expected | 10 ++++++ .../CWE-643/{XpathLibTests => }/xpathSinks.ql | 2 +- 13 files changed, 115 insertions(+), 57 deletions(-) rename python/ql/src/experimental/CWE-643/{xpath.py => xpathBad.py} (79%) create mode 100644 python/ql/src/experimental/CWE-643/xpathGood.py delete mode 100644 python/ql/test/experimental/CWE-643/XpathLibTests/options delete mode 100644 python/ql/test/experimental/CWE-643/XpathLibTests/xpathSinks.expected rename python/ql/test/experimental/CWE-643/{XpathLibTests => }/xpath.py (100%) create mode 100644 python/ql/test/experimental/CWE-643/xpathBad.py create mode 100644 python/ql/test/experimental/CWE-643/xpathGood.py create mode 100644 python/ql/test/experimental/CWE-643/xpathSinks.expected rename python/ql/test/experimental/CWE-643/{XpathLibTests => }/xpathSinks.ql (89%) diff --git a/python/ql/src/experimental/CWE-643/xpath.qhelp b/python/ql/src/experimental/CWE-643/xpath.qhelp index 434cdacd4d17..11e33dc6a4dd 100644 --- a/python/ql/src/experimental/CWE-643/xpath.qhelp +++ b/python/ql/src/experimental/CWE-643/xpath.qhelp @@ -1,32 +1,30 @@ - Using user-supplied information to construct an XPath query for XML data can +

    + Using user-supplied information to construct an XPath query for XML data can result in an XPath injection flaw. By sending intentionally malformed information, - an attacker can access data that he may not normally have access to. - He/She may even be able to elevate his privileges on the web site if the XML data + an attacker can access data that he may not normally have access to. + He/She may even be able to elevate his privileges on the web site if the XML data is being used for authentication (such as an XML based user file). +

    - XPath injection can be prevented using parameterized XPath interface or escaping the user input to make it safe to include in a dynamically constructed query. - If you are using quotes to terminate untrusted input in a dynamically constructed XPath query, then you need to escape that quote in the untrusted input to ensure the untrusted data can’t try to break out of that quoted context. + XPath injection can be prevented using parameterized XPath interface or escaping the user input to make it safe to include in a dynamically constructed query. + If you are using quotes to terminate untrusted input in a dynamically constructed XPath query, then you need to escape that quote in the untrusted input to ensure the untrusted data can’t try to break out of that quoted context.

    Another better mitigation option is to use a precompiled XPath query. Precompiled XPath queries are already preset before the program executes, rather than created on the fly after the user’s input has been added to the string. This is a better route because you don’t have to worry about missing a character that should have been escaped.

    - - -

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

    - - -     - -
  • OWASP XPath injection : />>
    • -     - -     - - + +

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

    + +     +

    This can be fixed by using a parameterized query as shown below.

    + + +
  • OWASP XPath injection : />>
    • +     \ No newline at end of file diff --git a/python/ql/src/experimental/CWE-643/xpath.py b/python/ql/src/experimental/CWE-643/xpathBad.py similarity index 79% rename from python/ql/src/experimental/CWE-643/xpath.py rename to python/ql/src/experimental/CWE-643/xpathBad.py index 69732a8f9a3d..ee836dd385eb 100644 --- a/python/ql/src/experimental/CWE-643/xpath.py +++ b/python/ql/src/experimental/CWE-643/xpathBad.py @@ -7,10 +7,10 @@ def a(request): - xpathQuery = request.GET['xpath'] + value = request.GET['xpath'] f = StringIO('') tree = etree.parse(f) - r = tree.xpath(xpathQuery) + r = tree.xpath("/tag[@id='%s']" % value) urlpatterns = [ diff --git a/python/ql/src/experimental/CWE-643/xpathGood.py b/python/ql/src/experimental/CWE-643/xpathGood.py new file mode 100644 index 000000000000..d5a27ca83f4e --- /dev/null +++ b/python/ql/src/experimental/CWE-643/xpathGood.py @@ -0,0 +1,18 @@ +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): + value = request.GET['xpath'] + f = StringIO('') + tree = etree.parse(f) + r = tree.xpath("/tag[@id=$tagid]", tagid=value) + + +urlpatterns = [ + path('a', a) +] diff --git a/python/ql/src/experimental/semmle/python/security/injection/Xpath.qll b/python/ql/src/experimental/semmle/python/security/injection/Xpath.qll index 94276997e218..953d548f1765 100644 --- a/python/ql/src/experimental/semmle/python/security/injection/Xpath.qll +++ b/python/ql/src/experimental/semmle/python/security/injection/Xpath.qll @@ -7,7 +7,7 @@ */ import python -import semmle.python.security.TaintTracking +import semmle.python.dataflow.TaintTracking import semmle.python.web.HttpRequest /** Models Xpath Injection related classes and functions */ @@ -29,11 +29,7 @@ module XpathInjection { override string toString() { result = "lxml.etree.Xpath" } EtreeXpathArgument() { - exists(CallNode call, AttrNode atr | - atr = etree().getAReference().getASuccessor() and - atr.getName() = "XPath" and - atr = call.getFunction() - | + exists(CallNode call | call.getFunction().(AttrNode).getObject("XPath").pointsTo(etree()) | call.getArg(0) = this ) } @@ -52,11 +48,7 @@ module XpathInjection { override string toString() { result = "lxml.etree.ETXpath" } EtreeETXpathArgument() { - exists(CallNode call, AttrNode atr | - atr = etree().getAReference().getASuccessor() and - atr.getName() = "ETXPath" and - atr = call.getFunction() - | + exists(CallNode call | call.getFunction().(AttrNode).getObject("ETXPath").pointsTo(etree()) | call.getArg(0) = this ) } @@ -77,17 +69,15 @@ module XpathInjection { override string toString() { result = "lxml.etree.parse.xpath" } ParseXpathArgument() { - exists(CallNode parseCall, AttrNode parse, string s | - parse = etree().getAReference().getASuccessor() and - parse.getName() = "parse" and - parse = parseCall.getFunction() and - exists(CallNode xpathCall, AttrNode xpath | - xpath = parseCall.getASuccessor*() and - xpath.getName() = "xpath" and - xpath = xpathCall.getFunction() and - s = xpath.getName() and - this = xpathCall.getArg(0) - ) + exists( + CallNode parseCall, CallNode xpathCall, ControlFlowNode obj, Variable var, AssignStmt assign + | + parseCall.getFunction().(AttrNode).getObject("parse").pointsTo(etree()) and + assign.getValue().(Call).getAFlowNode() = parseCall and + xpathCall.getFunction().(AttrNode).getObject("xpath") = obj and + var.getAUse() = obj and + assign.getATarget() = var.getAStore() and + xpathCall.getArg(0) = this ) } diff --git a/python/ql/test/experimental/CWE-643/XpathLibTests/options b/python/ql/test/experimental/CWE-643/XpathLibTests/options deleted file mode 100644 index 7fb713d59242..000000000000 --- a/python/ql/test/experimental/CWE-643/XpathLibTests/options +++ /dev/null @@ -1 +0,0 @@ -semmle-extractor-options: --max-import-depth=3 -p ../../../query-tests/Security/lib/ diff --git a/python/ql/test/experimental/CWE-643/XpathLibTests/xpathSinks.expected b/python/ql/test/experimental/CWE-643/XpathLibTests/xpathSinks.expected deleted file mode 100644 index f40dd8ece3b0..000000000000 --- a/python/ql/test/experimental/CWE-643/XpathLibTests/xpathSinks.expected +++ /dev/null @@ -1,4 +0,0 @@ -| xpath.py:8:20:8:29 | lxml.etree.parse.xpath | externally controlled string | -| xpath.py:13:29:13:38 | lxml.etree.Xpath | externally controlled string | -| xpath.py:19:29:19:38 | lxml.etree.Xpath | externally controlled string | -| xpath.py:25:38:25:46 | lxml.etree.ETXpath | externally controlled string | diff --git a/python/ql/test/experimental/CWE-643/xpath.expected b/python/ql/test/experimental/CWE-643/xpath.expected index 3a1ecc3888fd..a4e5cc36eedf 100644 --- a/python/ql/test/experimental/CWE-643/xpath.expected +++ b/python/ql/test/experimental/CWE-643/xpath.expected @@ -1,4 +1,14 @@ edges +| xpathBad.py:9:7:9:13 | django.request.HttpRequest | xpathBad.py:10:13:10:19 | django.request.HttpRequest | +| xpathBad.py:9:7:9:13 | django.request.HttpRequest | xpathBad.py:10:13:10:19 | django.request.HttpRequest | +| xpathBad.py:10:13:10:19 | django.request.HttpRequest | xpathBad.py:10:13:10:23 | django.http.request.QueryDict | +| xpathBad.py:10:13:10:19 | django.request.HttpRequest | xpathBad.py:10:13:10:23 | django.http.request.QueryDict | +| xpathBad.py:10:13:10:23 | django.http.request.QueryDict | xpathBad.py:10:13:10:32 | externally controlled string | +| xpathBad.py:10:13:10:23 | django.http.request.QueryDict | xpathBad.py:10:13:10:32 | externally controlled string | +| xpathBad.py:10:13:10:32 | externally controlled string | xpathBad.py:13:39:13:43 | externally controlled string | +| xpathBad.py:10:13:10:32 | externally controlled string | xpathBad.py:13:39:13:43 | externally controlled string | +| xpathBad.py:13:39:13:43 | externally controlled string | xpathBad.py:13:20:13:43 | externally controlled string | +| xpathBad.py:13:39:13:43 | externally controlled string | xpathBad.py:13:20:13:43 | externally controlled string | | xpathFlow.py:10:18:10:29 | dict of externally controlled string | xpathFlow.py:10:18:10:44 | externally controlled string | | xpathFlow.py:10:18:10:29 | dict of externally controlled string | xpathFlow.py:10:18:10:44 | externally controlled string | | xpathFlow.py:10:18:10:44 | externally controlled string | xpathFlow.py:13:20:13:29 | externally controlled string | @@ -13,10 +23,11 @@ edges | xpathFlow.py:27:18:27:44 | externally controlled string | xpathFlow.py:29:29:29:38 | externally controlled string | | xpathFlow.py:35:18:35:29 | dict of externally controlled string | xpathFlow.py:35:18:35:44 | externally controlled string | | xpathFlow.py:35:18:35:29 | dict of externally controlled string | xpathFlow.py:35:18:35:44 | externally controlled string | -| xpathFlow.py:35:18:35:44 | externally controlled string | xpathFlow.py:37:38:37:47 | externally controlled string | -| xpathFlow.py:35:18:35:44 | externally controlled string | xpathFlow.py:37:38:37:47 | externally controlled string | +| xpathFlow.py:35:18:35:44 | externally controlled string | xpathFlow.py:37:31:37:40 | externally controlled string | +| xpathFlow.py:35:18:35:44 | externally controlled string | xpathFlow.py:37:31:37:40 | externally controlled string | #select +| xpathBad.py:13:20:13:43 | BinaryExpr | xpathBad.py:9:7:9:13 | django.request.HttpRequest | xpathBad.py:13:20:13:43 | externally controlled string | This Xpath query depends on $@. | xpathBad.py:9:7:9:13 | request | a user-provided value | | xpathFlow.py:13:20:13:29 | xpathQuery | xpathFlow.py:10:18:10:29 | dict of externally controlled string | xpathFlow.py:13:20:13:29 | externally controlled string | This Xpath query depends on $@. | xpathFlow.py:10:18:10:29 | Attribute | a user-provided value | | xpathFlow.py:21:29:21:38 | xpathQuery | xpathFlow.py:18:18:18:29 | dict of externally controlled string | xpathFlow.py:21:29:21:38 | externally controlled string | This Xpath query depends on $@. | xpathFlow.py:18:18:18:29 | Attribute | a user-provided value | | xpathFlow.py:29:29:29:38 | xpathQuery | xpathFlow.py:27:18:27:29 | dict of externally controlled string | xpathFlow.py:29:29:29:38 | externally controlled string | This Xpath query depends on $@. | xpathFlow.py:27:18:27:29 | Attribute | a user-provided value | -| xpathFlow.py:37:38:37:47 | xpathQuery | xpathFlow.py:35:18:35:29 | dict of externally controlled string | xpathFlow.py:37:38:37:47 | externally controlled string | This Xpath query depends on $@. | xpathFlow.py:35:18:35:29 | Attribute | a user-provided value | +| xpathFlow.py:37:31:37:40 | xpathQuery | xpathFlow.py:35:18:35:29 | dict of externally controlled string | xpathFlow.py:37:31:37:40 | externally controlled string | This Xpath query depends on $@. | xpathFlow.py:35:18:35:29 | Attribute | a user-provided value | diff --git a/python/ql/test/experimental/CWE-643/XpathLibTests/xpath.py b/python/ql/test/experimental/CWE-643/xpath.py similarity index 100% rename from python/ql/test/experimental/CWE-643/XpathLibTests/xpath.py rename to python/ql/test/experimental/CWE-643/xpath.py diff --git a/python/ql/test/experimental/CWE-643/xpathBad.py b/python/ql/test/experimental/CWE-643/xpathBad.py new file mode 100644 index 000000000000..ee836dd385eb --- /dev/null +++ b/python/ql/test/experimental/CWE-643/xpathBad.py @@ -0,0 +1,18 @@ +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): + value = request.GET['xpath'] + f = StringIO('') + tree = etree.parse(f) + r = tree.xpath("/tag[@id='%s']" % value) + + +urlpatterns = [ + path('a', a) +] diff --git a/python/ql/test/experimental/CWE-643/xpathFlow.py b/python/ql/test/experimental/CWE-643/xpathFlow.py index 714169d9bd68..7e3d82260772 100644 --- a/python/ql/test/experimental/CWE-643/xpathFlow.py +++ b/python/ql/test/experimental/CWE-643/xpathFlow.py @@ -6,7 +6,7 @@ @app.route("/xpath1") -def a(): +def xpath1(): xpathQuery = request.args.get('xml', '') f = StringIO('') tree = etree.parse(f) @@ -14,7 +14,7 @@ def a(): @app.route("/xpath2") -def b(): +def xpath2(): xpathQuery = request.args.get('xml', '') root = etree.XML("TEXT") @@ -23,7 +23,7 @@ def b(): @app.route("/xpath3") -def c(): +def xpath3(): xpathQuery = request.args.get('xml', '') root = etree.XML("TEXT") find_text = etree.XPath(xpathQuery, smart_strings=False) @@ -31,8 +31,8 @@ def c(): @app.route("/xpath4") -def d(): +def xpath4(): xpathQuery = request.args.get('xml', '') root = etree.XML("TEXT") - find_text = find = etree.ETXPath(xpathQuery) - text = find_text(root)[0] \ No newline at end of file + find_text = etree.ETXPath(xpathQuery) + text = find_text(root)[0] diff --git a/python/ql/test/experimental/CWE-643/xpathGood.py b/python/ql/test/experimental/CWE-643/xpathGood.py new file mode 100644 index 000000000000..d5a27ca83f4e --- /dev/null +++ b/python/ql/test/experimental/CWE-643/xpathGood.py @@ -0,0 +1,18 @@ +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): + value = request.GET['xpath'] + f = StringIO('') + tree = etree.parse(f) + r = tree.xpath("/tag[@id=$tagid]", tagid=value) + + +urlpatterns = [ + path('a', a) +] diff --git a/python/ql/test/experimental/CWE-643/xpathSinks.expected b/python/ql/test/experimental/CWE-643/xpathSinks.expected new file mode 100644 index 000000000000..87ce65b26c06 --- /dev/null +++ b/python/ql/test/experimental/CWE-643/xpathSinks.expected @@ -0,0 +1,10 @@ +| xpath.py:8:20:8:29 | lxml.etree.parse.xpath | externally controlled string | +| xpath.py:13:29:13:38 | lxml.etree.Xpath | externally controlled string | +| xpath.py:19:29:19:38 | lxml.etree.Xpath | externally controlled string | +| xpath.py:25:38:25:46 | lxml.etree.ETXpath | externally controlled string | +| xpathBad.py:13:20:13:43 | lxml.etree.parse.xpath | externally controlled string | +| xpathFlow.py:13:20:13:29 | lxml.etree.parse.xpath | externally controlled string | +| xpathFlow.py:21:29:21:38 | lxml.etree.Xpath | externally controlled string | +| xpathFlow.py:29:29:29:38 | lxml.etree.Xpath | externally controlled string | +| xpathFlow.py:37:31:37:40 | lxml.etree.ETXpath | externally controlled string | +| xpathGood.py:13:20:13:37 | lxml.etree.parse.xpath | externally controlled string | diff --git a/python/ql/test/experimental/CWE-643/XpathLibTests/xpathSinks.ql b/python/ql/test/experimental/CWE-643/xpathSinks.ql similarity index 89% rename from python/ql/test/experimental/CWE-643/XpathLibTests/xpathSinks.ql rename to python/ql/test/experimental/CWE-643/xpathSinks.ql index 8a96e90035cc..21f80d9641a2 100644 --- a/python/ql/test/experimental/CWE-643/XpathLibTests/xpathSinks.ql +++ b/python/ql/test/experimental/CWE-643/xpathSinks.ql @@ -3,4 +3,4 @@ import experimental.semmle.python.security.injection.Xpath from XpathInjection::XpathInjectionSink sink, TaintKind kind where sink.sinks(kind) -select sink, kind +select sink, kind \ No newline at end of file From ce1f0a39acd2f764930c0d901eb7f0d0a6189943 Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Wed, 10 Jun 2020 16:59:40 +0200 Subject: [PATCH 3/4] Python: Minor fixup of qhelp for XPath injection --- python/ql/src/experimental/CWE-643/xpath.qhelp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/ql/src/experimental/CWE-643/xpath.qhelp b/python/ql/src/experimental/CWE-643/xpath.qhelp index 11e33dc6a4dd..c16eafc6bec1 100644 --- a/python/ql/src/experimental/CWE-643/xpath.qhelp +++ b/python/ql/src/experimental/CWE-643/xpath.qhelp @@ -21,10 +21,10 @@

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

    -

    This can be fixed by using a parameterized query as shown below.

    +
  • OWASP XPath injection : />>
    • - \ No newline at end of file + From a519132407db1f89a3038b7479fa119b8e1fbc44 Mon Sep 17 00:00:00 2001 From: Porcupiney Hairs Date: Mon, 22 Jun 2020 01:59:08 +0530 Subject: [PATCH 4/4] add support for libxml2 --- .../python/security/injection/Xpath.qll | 29 ++++++++++++ .../test/experimental/CWE-643/xpath.expected | 45 ++++++++++--------- python/ql/test/experimental/CWE-643/xpath.py | 7 +++ .../ql/test/experimental/CWE-643/xpathFlow.py | 13 +++++- .../experimental/CWE-643/xpathSinks.expected | 10 +++-- .../test/experimental/CWE-643/xpathSinks.ql | 2 +- .../Security/lib/libxml2/__init__.py | 10 +++++ 7 files changed, 90 insertions(+), 26 deletions(-) create mode 100644 python/ql/test/query-tests/Security/lib/libxml2/__init__.py diff --git a/python/ql/src/experimental/semmle/python/security/injection/Xpath.qll b/python/ql/src/experimental/semmle/python/security/injection/Xpath.qll index 953d548f1765..01a3e6de38df 100644 --- a/python/ql/src/experimental/semmle/python/security/injection/Xpath.qll +++ b/python/ql/src/experimental/semmle/python/security/injection/Xpath.qll @@ -15,6 +15,9 @@ module XpathInjection { /** Returns a class value which refers to `lxml.etree` */ Value etree() { result = Value::named("lxml.etree") } + /** Returns a class value which refers to `lxml.etree` */ + Value libxml2parseFile() { result = Value::named("libxml2.parseFile") } + /** A generic taint sink that is vulnerable to Xpath injection. */ abstract class XpathInjectionSink extends TaintSink { } @@ -83,4 +86,30 @@ module XpathInjection { override predicate sinks(TaintKind kind) { kind instanceof ExternalStringKind } } + + /** + * A Sink representing an argument to the `xpathEval` call to a parsed libxml2 document. + * + * import libxml2 + * tree = libxml2.parseFile("file.xml") + * r = tree.xpathEval('`sink`') + */ + private class ParseFileXpathEvalArgument extends XpathInjectionSink { + override string toString() { result = "libxml2.parseFile.xpathEval" } + + ParseFileXpathEvalArgument() { + exists( + CallNode parseCall, CallNode xpathCall, ControlFlowNode obj, Variable var, AssignStmt assign + | + parseCall.getFunction().(AttrNode).pointsTo(libxml2parseFile()) and + assign.getValue().(Call).getAFlowNode() = parseCall and + xpathCall.getFunction().(AttrNode).getObject("xpathEval") = obj and + var.getAUse() = obj and + assign.getATarget() = var.getAStore() and + xpathCall.getArg(0) = this + ) + } + + override predicate sinks(TaintKind kind) { kind instanceof ExternalStringKind } + } } diff --git a/python/ql/test/experimental/CWE-643/xpath.expected b/python/ql/test/experimental/CWE-643/xpath.expected index a4e5cc36eedf..2f32859d6a98 100644 --- a/python/ql/test/experimental/CWE-643/xpath.expected +++ b/python/ql/test/experimental/CWE-643/xpath.expected @@ -9,25 +9,30 @@ edges | xpathBad.py:10:13:10:32 | externally controlled string | xpathBad.py:13:39:13:43 | externally controlled string | | xpathBad.py:13:39:13:43 | externally controlled string | xpathBad.py:13:20:13:43 | externally controlled string | | xpathBad.py:13:39:13:43 | externally controlled string | xpathBad.py:13:20:13:43 | externally controlled string | -| xpathFlow.py:10:18:10:29 | dict of externally controlled string | xpathFlow.py:10:18:10:44 | externally controlled string | -| xpathFlow.py:10:18:10:29 | dict of externally controlled string | xpathFlow.py:10:18:10:44 | externally controlled string | -| xpathFlow.py:10:18:10:44 | externally controlled string | xpathFlow.py:13:20:13:29 | externally controlled string | -| xpathFlow.py:10:18:10:44 | externally controlled string | xpathFlow.py:13:20:13:29 | externally controlled string | -| xpathFlow.py:18:18:18:29 | dict of externally controlled string | xpathFlow.py:18:18:18:44 | externally controlled string | -| xpathFlow.py:18:18:18:29 | dict of externally controlled string | xpathFlow.py:18:18:18:44 | externally controlled string | -| xpathFlow.py:18:18:18:44 | externally controlled string | xpathFlow.py:21:29:21:38 | externally controlled string | -| xpathFlow.py:18:18:18:44 | externally controlled string | xpathFlow.py:21:29:21:38 | externally controlled string | -| xpathFlow.py:27:18:27:29 | dict of externally controlled string | xpathFlow.py:27:18:27:44 | externally controlled string | -| xpathFlow.py:27:18:27:29 | dict of externally controlled string | xpathFlow.py:27:18:27:44 | externally controlled string | -| xpathFlow.py:27:18:27:44 | externally controlled string | xpathFlow.py:29:29:29:38 | externally controlled string | -| xpathFlow.py:27:18:27:44 | externally controlled string | xpathFlow.py:29:29:29:38 | externally controlled string | -| xpathFlow.py:35:18:35:29 | dict of externally controlled string | xpathFlow.py:35:18:35:44 | externally controlled string | -| xpathFlow.py:35:18:35:29 | dict of externally controlled string | xpathFlow.py:35:18:35:44 | externally controlled string | -| xpathFlow.py:35:18:35:44 | externally controlled string | xpathFlow.py:37:31:37:40 | externally controlled string | -| xpathFlow.py:35:18:35:44 | externally controlled string | xpathFlow.py:37:31:37:40 | externally controlled string | +| xpathFlow.py:11:18:11:29 | dict of externally controlled string | xpathFlow.py:11:18:11:44 | externally controlled string | +| xpathFlow.py:11:18:11:29 | dict of externally controlled string | xpathFlow.py:11:18:11:44 | externally controlled string | +| xpathFlow.py:11:18:11:44 | externally controlled string | xpathFlow.py:14:20:14:29 | externally controlled string | +| xpathFlow.py:11:18:11:44 | externally controlled string | xpathFlow.py:14:20:14:29 | externally controlled string | +| xpathFlow.py:20:18:20:29 | dict of externally controlled string | xpathFlow.py:20:18:20:44 | externally controlled string | +| xpathFlow.py:20:18:20:29 | dict of externally controlled string | xpathFlow.py:20:18:20:44 | externally controlled string | +| xpathFlow.py:20:18:20:44 | externally controlled string | xpathFlow.py:23:29:23:38 | externally controlled string | +| xpathFlow.py:20:18:20:44 | externally controlled string | xpathFlow.py:23:29:23:38 | externally controlled string | +| xpathFlow.py:30:18:30:29 | dict of externally controlled string | xpathFlow.py:30:18:30:44 | externally controlled string | +| xpathFlow.py:30:18:30:29 | dict of externally controlled string | xpathFlow.py:30:18:30:44 | externally controlled string | +| xpathFlow.py:30:18:30:44 | externally controlled string | xpathFlow.py:32:29:32:38 | externally controlled string | +| xpathFlow.py:30:18:30:44 | externally controlled string | xpathFlow.py:32:29:32:38 | externally controlled string | +| xpathFlow.py:39:18:39:29 | dict of externally controlled string | xpathFlow.py:39:18:39:44 | externally controlled string | +| xpathFlow.py:39:18:39:29 | dict of externally controlled string | xpathFlow.py:39:18:39:44 | externally controlled string | +| xpathFlow.py:39:18:39:44 | externally controlled string | xpathFlow.py:41:31:41:40 | externally controlled string | +| xpathFlow.py:39:18:39:44 | externally controlled string | xpathFlow.py:41:31:41:40 | externally controlled string | +| xpathFlow.py:47:18:47:29 | dict of externally controlled string | xpathFlow.py:47:18:47:44 | externally controlled string | +| xpathFlow.py:47:18:47:29 | dict of externally controlled string | xpathFlow.py:47:18:47:44 | externally controlled string | +| xpathFlow.py:47:18:47:44 | externally controlled string | xpathFlow.py:49:29:49:38 | externally controlled string | +| xpathFlow.py:47:18:47:44 | externally controlled string | xpathFlow.py:49:29:49:38 | externally controlled string | #select | xpathBad.py:13:20:13:43 | BinaryExpr | xpathBad.py:9:7:9:13 | django.request.HttpRequest | xpathBad.py:13:20:13:43 | externally controlled string | This Xpath query depends on $@. | xpathBad.py:9:7:9:13 | request | a user-provided value | -| xpathFlow.py:13:20:13:29 | xpathQuery | xpathFlow.py:10:18:10:29 | dict of externally controlled string | xpathFlow.py:13:20:13:29 | externally controlled string | This Xpath query depends on $@. | xpathFlow.py:10:18:10:29 | Attribute | a user-provided value | -| xpathFlow.py:21:29:21:38 | xpathQuery | xpathFlow.py:18:18:18:29 | dict of externally controlled string | xpathFlow.py:21:29:21:38 | externally controlled string | This Xpath query depends on $@. | xpathFlow.py:18:18:18:29 | Attribute | a user-provided value | -| xpathFlow.py:29:29:29:38 | xpathQuery | xpathFlow.py:27:18:27:29 | dict of externally controlled string | xpathFlow.py:29:29:29:38 | externally controlled string | This Xpath query depends on $@. | xpathFlow.py:27:18:27:29 | Attribute | a user-provided value | -| xpathFlow.py:37:31:37:40 | xpathQuery | xpathFlow.py:35:18:35:29 | dict of externally controlled string | xpathFlow.py:37:31:37:40 | externally controlled string | This Xpath query depends on $@. | xpathFlow.py:35:18:35:29 | Attribute | a user-provided value | +| xpathFlow.py:14:20:14:29 | xpathQuery | xpathFlow.py:11:18:11:29 | dict of externally controlled string | xpathFlow.py:14:20:14:29 | externally controlled string | This Xpath query depends on $@. | xpathFlow.py:11:18:11:29 | Attribute | a user-provided value | +| xpathFlow.py:23:29:23:38 | xpathQuery | xpathFlow.py:20:18:20:29 | dict of externally controlled string | xpathFlow.py:23:29:23:38 | externally controlled string | This Xpath query depends on $@. | xpathFlow.py:20:18:20:29 | Attribute | a user-provided value | +| xpathFlow.py:32:29:32:38 | xpathQuery | xpathFlow.py:30:18:30:29 | dict of externally controlled string | xpathFlow.py:32:29:32:38 | externally controlled string | This Xpath query depends on $@. | xpathFlow.py:30:18:30:29 | Attribute | a user-provided value | +| xpathFlow.py:41:31:41:40 | xpathQuery | xpathFlow.py:39:18:39:29 | dict of externally controlled string | xpathFlow.py:41:31:41:40 | externally controlled string | This Xpath query depends on $@. | xpathFlow.py:39:18:39:29 | Attribute | a user-provided value | +| xpathFlow.py:49:29:49:38 | xpathQuery | xpathFlow.py:47:18:47:29 | dict of externally controlled string | xpathFlow.py:49:29:49:38 | externally controlled string | This Xpath query depends on $@. | xpathFlow.py:47:18:47:29 | Attribute | a user-provided value | diff --git a/python/ql/test/experimental/CWE-643/xpath.py b/python/ql/test/experimental/CWE-643/xpath.py index 8b0e06d78be0..2c1ecd1c50aa 100644 --- a/python/ql/test/experimental/CWE-643/xpath.py +++ b/python/ql/test/experimental/CWE-643/xpath.py @@ -26,8 +26,15 @@ def d(): text = find_text(root)[0] +def e(): + import libxml2 + doc = libxml2.parseFile('xpath_injection/credential.xml') + results = doc.xpathEval('sink') + + if __name__ == "__main__": a() b() c() d() + e() diff --git a/python/ql/test/experimental/CWE-643/xpathFlow.py b/python/ql/test/experimental/CWE-643/xpathFlow.py index 7e3d82260772..c2fe2ce1edb3 100644 --- a/python/ql/test/experimental/CWE-643/xpathFlow.py +++ b/python/ql/test/experimental/CWE-643/xpathFlow.py @@ -1,4 +1,3 @@ -from lxml import etree from io import StringIO from flask import Flask, request @@ -7,6 +6,8 @@ @app.route("/xpath1") def xpath1(): + from lxml import etree + xpathQuery = request.args.get('xml', '') f = StringIO('') tree = etree.parse(f) @@ -15,6 +16,7 @@ def xpath1(): @app.route("/xpath2") def xpath2(): + from lxml import etree xpathQuery = request.args.get('xml', '') root = etree.XML("TEXT") @@ -24,6 +26,7 @@ def xpath2(): @app.route("/xpath3") def xpath3(): + from lxml import etree xpathQuery = request.args.get('xml', '') root = etree.XML("TEXT") find_text = etree.XPath(xpathQuery, smart_strings=False) @@ -32,7 +35,15 @@ def xpath3(): @app.route("/xpath4") def xpath4(): + from lxml import etree xpathQuery = request.args.get('xml', '') root = etree.XML("TEXT") find_text = etree.ETXPath(xpathQuery) text = find_text(root)[0] + +@app.route("/xpath5") +def xpath5(): + import libxml2 + xpathQuery = request.args.get('xml', '') + doc = libxml2.parseFile('xpath_injection/credential.xml') + results = doc.xpathEval(xpathQuery) diff --git a/python/ql/test/experimental/CWE-643/xpathSinks.expected b/python/ql/test/experimental/CWE-643/xpathSinks.expected index 87ce65b26c06..c5d2000ab529 100644 --- a/python/ql/test/experimental/CWE-643/xpathSinks.expected +++ b/python/ql/test/experimental/CWE-643/xpathSinks.expected @@ -2,9 +2,11 @@ | xpath.py:13:29:13:38 | lxml.etree.Xpath | externally controlled string | | xpath.py:19:29:19:38 | lxml.etree.Xpath | externally controlled string | | xpath.py:25:38:25:46 | lxml.etree.ETXpath | externally controlled string | +| xpath.py:32:29:32:34 | libxml2.parseFile.xpathEval | externally controlled string | | xpathBad.py:13:20:13:43 | lxml.etree.parse.xpath | externally controlled string | -| xpathFlow.py:13:20:13:29 | lxml.etree.parse.xpath | externally controlled string | -| xpathFlow.py:21:29:21:38 | lxml.etree.Xpath | externally controlled string | -| xpathFlow.py:29:29:29:38 | lxml.etree.Xpath | externally controlled string | -| xpathFlow.py:37:31:37:40 | lxml.etree.ETXpath | externally controlled string | +| xpathFlow.py:14:20:14:29 | lxml.etree.parse.xpath | externally controlled string | +| xpathFlow.py:23:29:23:38 | lxml.etree.Xpath | externally controlled string | +| xpathFlow.py:32:29:32:38 | lxml.etree.Xpath | externally controlled string | +| xpathFlow.py:41:31:41:40 | lxml.etree.ETXpath | externally controlled string | +| xpathFlow.py:49:29:49:38 | libxml2.parseFile.xpathEval | externally controlled string | | xpathGood.py:13:20:13:37 | lxml.etree.parse.xpath | externally controlled string | diff --git a/python/ql/test/experimental/CWE-643/xpathSinks.ql b/python/ql/test/experimental/CWE-643/xpathSinks.ql index 21f80d9641a2..8a96e90035cc 100644 --- a/python/ql/test/experimental/CWE-643/xpathSinks.ql +++ b/python/ql/test/experimental/CWE-643/xpathSinks.ql @@ -3,4 +3,4 @@ import experimental.semmle.python.security.injection.Xpath from XpathInjection::XpathInjectionSink sink, TaintKind kind where sink.sinks(kind) -select sink, kind \ No newline at end of file +select sink, kind diff --git a/python/ql/test/query-tests/Security/lib/libxml2/__init__.py b/python/ql/test/query-tests/Security/lib/libxml2/__init__.py new file mode 100644 index 000000000000..057488829f42 --- /dev/null +++ b/python/ql/test/query-tests/Security/lib/libxml2/__init__.py @@ -0,0 +1,10 @@ +def parseFile(filename): + return xmlDoc(_obj=None) + + +class xmlDoc(Object): + def __init__(self, _obj=None): + pass + + def xpathEval(self, expr): + pass