Skip to content

Python : Add Xpath injection query #3522

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
RasmusWL merged 4 commits into from Jun 25, 2020
Merged

Python : Add Xpath injection query #3522

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8c5a971
Python : Add Xpath injection query
May 13, 2020
424e88d
include sugestions from review
Jun 7, 2020
ce1f0a3
Python: Minor fixup of qhelp for XPath injection
RasmusWL Jun 10, 2020
a519132
add support for libxml2
Jun 21, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
include sugestions from review
  • Loading branch information
Porcupiney Hairs
Porcupiney Hairs committed Jun 7, 2020
commit 424e88d318b1a5fae565bae6a91be5f95b6789f9
34 changes: 16 additions & 18 deletions python/ql/src/experimental/CWE-643/xpath.qhelp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,32 +1,30 @@
<!DOCTYPE qhelp SYSTEM "qhelp.dtd">
<qhelp>
<overview>
Using user-supplied information to construct an XPath query for XML data can
<p>
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).
</p>
</overview>
<recommendation>
<p>
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.
</p>
<p>
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.
</p>
<example>

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

<sample src="xpath.py" />
</example>
<references>
<li>OWASP XPath injection : <a href="https://owasp.org/www-community/attacks/XPATH_Injection"></a>/>> </li>
</references>


</recommendation>


<example>
<p>In the example below, the xpath query is controlled by the user and hence leads to a vulnerability.</p>
<sample src="xpathBad.py" />
</example>
<p> This can be fixed by using a parameterized query as shown below.</p>
<sample src="xpathGood.py" />
Comment thread
RasmusWL marked this conversation as resolved.
Outdated
<references>
<li>OWASP XPath injection : <a href="https://owasp.org/www-community/attacks/XPATH_Injection"></a>/>> </li>
</references>
</qhelp>
4 changes: 2 additions & 2 deletions python/ql/src/experimental/CWE-643/xpath.py → ...n/ql/src/experimental/CWE-643/xpathBad.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@


def a(request):
xpathQuery = request.GET['xpath']
value = request.GET['xpath']
f = StringIO('<foo><bar></bar></foo>')
tree = etree.parse(f)
r = tree.xpath(xpathQuery)
r = tree.xpath("/tag[@id='%s']" % value)


urlpatterns = [
Expand Down
18 changes: 18 additions & 0 deletions python/ql/src/experimental/CWE-643/xpathGood.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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('<foo><bar></bar></foo>')
tree = etree.parse(f)
r = tree.xpath("/tag[@id=$tagid]", tagid=value)


urlpatterns = [
path('a', a)
]
34 changes: 12 additions & 22 deletions python/ql/src/experimental/semmle/python/security/injection/Xpath.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand All @@ -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
)
}
Expand All @@ -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
)
}
Expand All @@ -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
)
}
Comment thread
This conversation was marked as resolved.

Expand Down
1 change: 0 additions & 1 deletion python/ql/test/experimental/CWE-643/XpathLibTests/options
View file
Open in desktop

This file was deleted.

4 changes: 0 additions & 4 deletions python/ql/test/experimental/CWE-643/XpathLibTests/xpathSinks.expected
View file
Open in desktop

This file was deleted.

17 changes: 14 additions & 3 deletions python/ql/test/experimental/CWE-643/xpath.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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 |
Expand All @@ -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 |
0 ...perimental/CWE-643/XpathLibTests/xpath.py → python/ql/test/experimental/CWE-643/xpath.py
View file
Open in desktop
File renamed without changes.
18 changes: 18 additions & 0 deletions python/ql/test/experimental/CWE-643/xpathBad.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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('<foo><bar></bar></foo>')
tree = etree.parse(f)
r = tree.xpath("/tag[@id='%s']" % value)


urlpatterns = [
path('a', a)
]
12 changes: 6 additions & 6 deletions python/ql/test/experimental/CWE-643/xpathFlow.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,15 @@


@app.route("/xpath1")
def a():
def xpath1():
xpathQuery = request.args.get('xml', '')
f = StringIO('<foo><bar></bar></foo>')
tree = etree.parse(f)
r = tree.xpath(xpathQuery)


@app.route("/xpath2")
def b():
def xpath2():
xpathQuery = request.args.get('xml', '')

root = etree.XML("<root><a>TEXT</a></root>")
Expand All @@ -23,16 +23,16 @@ def b():


@app.route("/xpath3")
def c():
def xpath3():
xpathQuery = request.args.get('xml', '')
root = etree.XML("<root><a>TEXT</a></root>")
find_text = etree.XPath(xpathQuery, smart_strings=False)
text = find_text(root)[0]


@app.route("/xpath4")
def d():
def xpath4():
xpathQuery = request.args.get('xml', '')
root = etree.XML("<root><a>TEXT</a></root>")
find_text = find = etree.ETXPath(xpathQuery)
text = find_text(root)[0]
find_text = etree.ETXPath(xpathQuery)
text = find_text(root)[0]
18 changes: 18 additions & 0 deletions python/ql/test/experimental/CWE-643/xpathGood.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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('<foo><bar></bar></foo>')
tree = etree.parse(f)
r = tree.xpath("/tag[@id=$tagid]", tagid=value)


urlpatterns = [
path('a', a)
]
10 changes: 10 additions & 0 deletions python/ql/test/experimental/CWE-643/xpathSinks.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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 |
2 changes: 1 addition & 1 deletion ...ental/CWE-643/XpathLibTests/xpathSinks.ql → ...l/test/experimental/CWE-643/xpathSinks.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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