Skip to content

Commit 547cbb6

Browse files
authored
Merge pull request #6331 from porcupineyhairs/pythonXpath
Python : Improve Xpath Injection Query
2 parents 3e1bc66 + d39df18 commit 547cbb6

File tree

13 files changed

+248
-210
lines changed

13 files changed

+248
-210
lines changed
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/**
2+
* @name XPath query built from user-controlled sources
3+
* @description Building a XPath query from user-controlled sources is vulnerable to insertion of
4+
* malicious Xpath code by the user.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @precision high
8+
* @id py/xpath-injection
9+
* @tags security
10+
* external/cwe/cwe-643
11+
*/
12+
13+
private import python
14+
private import semmle.python.Concepts
15+
private import semmle.python.dataflow.new.TaintTracking
16+
private import semmle.python.Concepts
17+
private import semmle.python.ApiGraphs
18+
private import semmle.python.dataflow.new.RemoteFlowSources
19+
private import semmle.python.dataflow.new.BarrierGuards
20+
import XpathInjection::XpathInjection
21+
import DataFlow::PathGraph
22+
23+
class XpathInjectionConfiguration extends TaintTracking::Configuration {
24+
XpathInjectionConfiguration() { this = "PathNotNormalizedConfiguration" }
25+
26+
override predicate isSource(DataFlow::Node source) { source instanceof Source }
27+
28+
override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
29+
}
30+
31+
from XpathInjectionConfiguration config, DataFlow::PathNode source, DataFlow::PathNode sink
32+
where config.hasFlowPath(source, sink)
33+
select sink, source, sink, "This Xpath query depends on $@.", source, "a user-provided value"
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
/**
2+
* Provides a taint-tracking configuration for detecting "Xpath Injection" vulnerabilities.
3+
*
4+
* Note, for performance reasons: only import this file if
5+
* `XpathInjection::Configuration` is needed, otherwise
6+
* `XpathInjectionCustomizations` should be imported instead.
7+
*/
8+
9+
private import python
10+
import semmle.python.dataflow.new.DataFlow
11+
import semmle.python.dataflow.new.TaintTracking
12+
13+
/**
14+
* Provides a taint-tracking configuration for detecting "Xpath Injection" vulnerabilities.
15+
*/
16+
module XpathInjection {
17+
import XpathInjectionCustomizations::XpathInjection
18+
19+
/**
20+
* A taint-tracking configuration for detecting "Xpath Injection" vulnerabilities.
21+
*/
22+
class Configuration extends TaintTracking::Configuration {
23+
Configuration() { this = "Xpath Injection" }
24+
25+
override predicate isSource(DataFlow::Node source) { source instanceof Source }
26+
27+
override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
28+
29+
override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
30+
31+
override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
32+
guard instanceof SanitizerGuard
33+
}
34+
}
35+
}
Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
/**
2+
* Provides class and predicates to track external data that
3+
* may represent malicious xpath query objects.
4+
*
5+
* This module is intended to be imported into a taint-tracking query.
6+
*/
7+
8+
private import python
9+
private import semmle.python.Concepts
10+
private import semmle.python.dataflow.new.TaintTracking
11+
private import semmle.python.Concepts
12+
private import semmle.python.ApiGraphs
13+
private import semmle.python.dataflow.new.RemoteFlowSources
14+
private import semmle.python.dataflow.new.BarrierGuards
15+
16+
/** Models Xpath Injection related classes and functions */
17+
module XpathInjection {
18+
/**
19+
* A data flow source for "XPath injection" vulnerabilities.
20+
*/
21+
abstract class Source extends DataFlow::Node { }
22+
23+
/**
24+
* A data flow sink for "XPath injection" vulnerabilities.
25+
*/
26+
abstract class Sink extends DataFlow::Node { }
27+
28+
/**
29+
* A sanitizer for "XPath injection" vulnerabilities.
30+
*/
31+
abstract class Sanitizer extends DataFlow::Node { }
32+
33+
/**
34+
* A sanitizer guard for "XPath injection" vulnerabilities.
35+
*/
36+
abstract class SanitizerGuard extends DataFlow::BarrierGuard { }
37+
38+
/**
39+
* A source of remote user input, considered as a flow source.
40+
*/
41+
class RemoteFlowSourceAsSource extends Source, RemoteFlowSource { }
42+
43+
/** Returns an API node referring to `lxml.etree` */
44+
API::Node etree() { result = API::moduleImport("lxml").getMember("etree") }
45+
46+
/** Returns an API node referring to `lxml.etree` */
47+
API::Node etreeFromString() { result = etree().getMember("fromstring") }
48+
49+
/** Returns an API node referring to `lxml.etree.parse` */
50+
API::Node etreeParse() { result = etree().getMember("parse") }
51+
52+
/** Returns an API node referring to `lxml.etree.parse` */
53+
API::Node libxml2parseFile() { result = API::moduleImport("libxml2").getMember("parseFile") }
54+
55+
/**
56+
* A Sink representing an argument to `etree.XPath` or `etree.ETXPath` call.
57+
*
58+
* from lxml import etree
59+
* root = etree.XML("<xmlContent>")
60+
* find_text = etree.XPath("`sink`")
61+
* find_text = etree.ETXPath("`sink`")
62+
*/
63+
private class EtreeXpathArgument extends Sink {
64+
EtreeXpathArgument() { this = etree().getMember(["XPath", "ETXPath"]).getACall().getArg(0) }
65+
}
66+
67+
/**
68+
* A Sink representing an argument to the `etree.XPath` call.
69+
*
70+
* from lxml import etree
71+
* root = etree.fromstring(file(XML_DB).read(), XMLParser())
72+
* find_text = root.xpath("`sink`")
73+
*/
74+
private class EtreeFromstringXpathArgument extends Sink {
75+
EtreeFromstringXpathArgument() {
76+
this = etreeFromString().getReturn().getMember("xpath").getACall().getArg(0)
77+
}
78+
}
79+
80+
/**
81+
* A Sink representing an argument to the `xpath` call to a parsed xml document.
82+
*
83+
* from lxml import etree
84+
* from io import StringIO
85+
* f = StringIO('<foo><bar></bar></foo>')
86+
* tree = etree.parse(f)
87+
* r = tree.xpath('`sink`')
88+
*/
89+
private class ParseXpathArgument extends Sink {
90+
ParseXpathArgument() { this = etreeParse().getReturn().getMember("xpath").getACall().getArg(0) }
91+
}
92+
93+
/**
94+
* A Sink representing an argument to the `xpathEval` call to a parsed libxml2 document.
95+
*
96+
* import libxml2
97+
* tree = libxml2.parseFile("file.xml")
98+
* r = tree.xpathEval('`sink`')
99+
*/
100+
private class ParseFileXpathEvalArgument extends Sink {
101+
ParseFileXpathEvalArgument() {
102+
this = libxml2parseFile().getReturn().getMember("xpathEval").getACall().getArg(0)
103+
}
104+
}
105+
}

python/ql/src/experimental/Security/CWE-643/xpath.ql

Lines changed: 0 additions & 36 deletions
This file was deleted.

python/ql/src/experimental/semmle/python/security/injection/Xpath.qll

Lines changed: 0 additions & 115 deletions
This file was deleted.

0 commit comments

Comments
 (0)