mirror of
https://github.com/github/codeql.git
synced 2025-12-20 10:46:30 +01:00
include sugestions from review
This commit is contained in:
@@ -1,11 +1,13 @@
|
||||
<!DOCTYPE qhelp SYSTEM "qhelp.dtd">
|
||||
<qhelp>
|
||||
<overview>
|
||||
<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
|
||||
is being used for authentication (such as an XML based user file).
|
||||
</p>
|
||||
</overview>
|
||||
<recommendation>
|
||||
<p>
|
||||
@@ -15,18 +17,14 @@
|
||||
<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>
|
||||
</recommendation>
|
||||
<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" />
|
||||
<sample src="xpathBad.py" />
|
||||
</example>
|
||||
<p> This can be fixed by using a parameterized query as shown below.</p>
|
||||
<sample src="xpathGood.py" />
|
||||
<references>
|
||||
<li>OWASP XPath injection : <a href="https://owasp.org/www-community/attacks/XPATH_Injection"></a>/>> </li>
|
||||
</references>
|
||||
|
||||
|
||||
</recommendation>
|
||||
|
||||
|
||||
</qhelp>
|
||||
@@ -7,10 +7,10 @@ from django.template import Template, Context, Engine, engines
|
||||
|
||||
|
||||
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 = [
|
||||
18
python/ql/src/experimental/CWE-643/xpathGood.py
Normal file
18
python/ql/src/experimental/CWE-643/xpathGood.py
Normal file
@@ -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)
|
||||
]
|
||||
@@ -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
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
@@ -1 +0,0 @@
|
||||
semmle-extractor-options: --max-import-depth=3 -p ../../../query-tests/Security/lib/
|
||||
@@ -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 |
|
||||
@@ -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 |
|
||||
|
||||
18
python/ql/test/experimental/CWE-643/xpathBad.py
Normal file
18
python/ql/test/experimental/CWE-643/xpathBad.py
Normal file
@@ -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)
|
||||
]
|
||||
@@ -6,7 +6,7 @@ app = Flask(__name__)
|
||||
|
||||
|
||||
@app.route("/xpath1")
|
||||
def a():
|
||||
def xpath1():
|
||||
xpathQuery = request.args.get('xml', '')
|
||||
f = StringIO('<foo><bar></bar></foo>')
|
||||
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("<root><a>TEXT</a></root>")
|
||||
@@ -23,7 +23,7 @@ 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)
|
||||
@@ -31,8 +31,8 @@ def c():
|
||||
|
||||
|
||||
@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)
|
||||
find_text = etree.ETXPath(xpathQuery)
|
||||
text = find_text(root)[0]
|
||||
18
python/ql/test/experimental/CWE-643/xpathGood.py
Normal file
18
python/ql/test/experimental/CWE-643/xpathGood.py
Normal file
@@ -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
python/ql/test/experimental/CWE-643/xpathSinks.expected
Normal file
10
python/ql/test/experimental/CWE-643/xpathSinks.expected
Normal file
@@ -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 |
|
||||
Reference in New Issue
Block a user