Skip to content

Commit 4c693b4

Browse files
RasmusWLRasmusWL
committed
Python: Port py/xslt-injection to new data-flow
1 parent ef139f2 commit 4c693b4

File tree

6 files changed

+286
-67
lines changed

6 files changed

+286
-67
lines changed

‎python/ql/src/experimental/Security/CWE-091/XsltConcept.qll

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
private import python
2+
private import semmle.python.dataflow.new.DataFlow
3+
private import semmle.python.ApiGraphs
4+
5+
/**
6+
* A data-flow node that constructs a XSLT transformer.
7+
*
8+
* Extend this class to refine existing API models. If you want to model new APIs,
9+
* extend `TemplateConstruction::Range` instead.
10+
*/
11+
class XsltConstruction extends DataFlow::Node instanceof XsltConstruction::Range {
12+
/** Gets the argument that specifies the XSLT transformer. */
13+
DataFlow::Node getXsltArg() { result = super.getXsltArg() }
14+
}
15+
16+
/** Provides a class for modeling new system-command execution APIs. */
17+
module XsltConstruction {
18+
/**
19+
* A data-flow node that constructs a XSLT transformer.
20+
*
21+
* Extend this class to model new APIs. If you want to refine existing API models,
22+
* extend `XsltConstruction` instead.
23+
*/
24+
abstract class Range extends DataFlow::Node {
25+
/** Gets the argument that specifies the XSLT transformer. */
26+
abstract DataFlow::Node getXsltArg();
27+
}
28+
}
29+
30+
/**
31+
* A data-flow node that executes a XSLT transformer.
32+
*
33+
* Extend this class to refine existing API models. If you want to model new APIs,
34+
* extend `TemplateConstruction::Range` instead.
35+
*/
36+
class XsltExecution extends DataFlow::Node instanceof XsltExecution::Range {
37+
/** Gets the argument that specifies the XSLT transformer. */
38+
DataFlow::Node getXsltArg() { result = super.getXsltArg() }
39+
}
40+
41+
/** Provides a class for modeling new system-command execution APIs. */
42+
module XsltExecution {
43+
/**
44+
* A data-flow node that executes a XSLT transformer.
45+
*
46+
* Extend this class to model new APIs. If you want to refine existing API models,
47+
* extend `XsltExecution` instead.
48+
*/
49+
abstract class Range extends DataFlow::Node {
50+
/** Gets the argument that specifies the XSLT transformer. */
51+
abstract DataFlow::Node getXsltArg();
52+
}
53+
}
54+
55+
// -----------------------------------------------------------------------------
56+
/**
57+
* A call to `lxml.etree.XSLT`.
58+
*
59+
* ```py
60+
* from lxml import etree
61+
* xslt_tree = etree.parse(...)
62+
* doc = etree.parse(...)
63+
* transform = etree.XSLT(xslt_tree)
64+
* result = transform(doc)
65+
* ```
66+
*/
67+
class LxmlEtreeXsltCall extends XsltConstruction::Range, API::CallNode {
68+
LxmlEtreeXsltCall() {
69+
this = API::moduleImport("lxml").getMember("etree").getMember("XSLT").getACall()
70+
}
71+
72+
override DataFlow::Node getXsltArg() { result = this.getParameter(0, "xslt_input").asSink() }
73+
}
74+
75+
/**
76+
* A call to `.xslt` on an lxml ElementTree object.
77+
*
78+
* ```py
79+
* from lxml import etree
80+
* xslt_tree = etree.parse(...)
81+
* doc = etree.parse(...)
82+
* result = doc.xslt(xslt_tree)
83+
* ```
84+
*/
85+
class XsltAttributeCall extends XsltExecution::Range, API::CallNode {
86+
XsltAttributeCall() { this = elementTreeConstruction(_).getReturn().getMember("xslt").getACall() }
87+
88+
override DataFlow::Node getXsltArg() { result = this.getParameter(0, "_xslt").asSink() }
89+
}
90+
91+
// -----------------------------------------------------------------------------
92+
API::CallNode elementTreeConstruction(DataFlow::Node inputArg) {
93+
// TODO: If we could, would be nice to model this as flow-summaries. But I'm not sure if we actually can :thinking:
94+
// see https://lxml.de/api/lxml.etree-module.html#fromstring
95+
result = API::moduleImport("lxml").getMember("etree").getMember("fromstring").getACall() and
96+
inputArg = result.getParameter(0, "text").asSink()
97+
or
98+
// see https://lxml.de/api/lxml.etree-module.html#fromstringlist
99+
result = API::moduleImport("lxml").getMember("etree").getMember("fromstringlist").getACall() and
100+
inputArg = result.getParameter(0, "strings").asSink()
101+
or
102+
// TODO: technically we should treat parse differently, since it takes a file as argument
103+
// see https://lxml.de/api/lxml.etree-module.html#parse
104+
result = API::moduleImport("lxml").getMember("etree").getMember("parse").getACall() and
105+
inputArg = result.getParameter(0, "source").asSink()
106+
or
107+
// see https://lxml.de/api/lxml.etree-module.html#XML
108+
result = API::moduleImport("lxml").getMember("etree").getMember("XML").getACall() and
109+
inputArg = result.getParameter(0, "text").asSink()
110+
}

‎python/ql/src/experimental/Security/CWE-091/XsltInjection.ql

Lines changed: 6 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -12,25 +12,10 @@
1212
*/
1313

1414
import python
15-
import semmle.python.security.Paths
16-
/* Sources */
17-
import semmle.python.web.HttpRequest
18-
/* Sinks */
19-
import experimental.semmle.python.security.injection.XSLT
15+
import XsltInjectionQuery
16+
import XsltInjectionFlow::PathGraph
2017

21-
class XsltInjectionConfiguration extends TaintTracking::Configuration {
22-
XsltInjectionConfiguration() { this = "XSLT injection configuration" }
23-
24-
deprecated override predicate isSource(TaintTracking::Source source) {
25-
source instanceof HttpRequestTaintSource
26-
}
27-
28-
deprecated override predicate isSink(TaintTracking::Sink sink) {
29-
sink instanceof XSLTInjection::XSLTInjectionSink
30-
}
31-
}
32-
33-
from XsltInjectionConfiguration config, TaintedPathSource src, TaintedPathSink sink
34-
where config.hasFlowPath(src, sink)
35-
select sink.getSink(), src, sink, "This XSLT query depends on $@.", src.getSource(),
36-
"a user-provided value"
18+
from XsltInjectionFlow::PathNode source, XsltInjectionFlow::PathNode sink
19+
where XsltInjectionFlow::flowPath(source, sink)
20+
select sink.getNode(), source, sink, "This XSLT query depends on $@.", source.getNode(),
21+
"user-provided value"

‎python/ql/src/experimental/Security/CWE-091/XsltInjectionCustomizations.qll

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
/**
2+
* Provides default sources, sinks and sanitizers for detecting
3+
* "XSLT injection"
4+
* vulnerabilities, as well as extension points for adding your own.
5+
*/
6+
7+
private import python
8+
private import semmle.python.dataflow.new.DataFlow
9+
private import semmle.python.Concepts
10+
private import semmle.python.dataflow.new.RemoteFlowSources
11+
private import semmle.python.dataflow.new.BarrierGuards
12+
private import XsltConcept
13+
14+
/**
15+
* Provides default sources, sinks and sanitizers for detecting
16+
* "XSLT injection"
17+
* vulnerabilities, as well as extension points for adding your own.
18+
*/
19+
module XsltInjection {
20+
/**
21+
* A data flow source for "XSLT injection" vulnerabilities.
22+
*/
23+
abstract class Source extends DataFlow::Node { }
24+
25+
/**
26+
* A data flow sink for "XSLT injection" vulnerabilities.
27+
*/
28+
abstract class Sink extends DataFlow::Node { }
29+
30+
/**
31+
* A sanitizer for "XSLT injection" vulnerabilities.
32+
*/
33+
abstract class Sanitizer extends DataFlow::Node { }
34+
35+
/**
36+
* A source of remote user input, considered as a flow source.
37+
*/
38+
class RemoteFlowSourceAsSource extends Source, RemoteFlowSource { }
39+
40+
/**
41+
* An XSLT construction, considered as a flow sink.
42+
*/
43+
class XsltConstructionAsSink extends Sink {
44+
XsltConstructionAsSink() { this = any(XsltConstruction c).getXsltArg() }
45+
}
46+
47+
/**
48+
* An XSLT execution, considered as a flow sink.
49+
*/
50+
class XsltExecutionAsSink extends Sink {
51+
XsltExecutionAsSink() { this = any(XsltExecution c).getXsltArg() }
52+
}
53+
54+
/**
55+
* A comparison with a constant string, considered as a sanitizer-guard.
56+
*/
57+
class StringConstCompareAsSanitizerGuard extends Sanitizer, StringConstCompareBarrier { }
58+
}

‎python/ql/src/experimental/Security/CWE-091/XsltInjectionQuery.qll

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/**
2+
* Provides a taint-tracking configuration for detecting "XSLT injection" vulnerabilities.
3+
*/
4+
5+
private import python
6+
import semmle.python.dataflow.new.DataFlow
7+
import semmle.python.dataflow.new.TaintTracking
8+
import XsltInjectionCustomizations::XsltInjection
9+
import XsltConcept
10+
11+
module XsltInjectionConfig implements DataFlow::ConfigSig {
12+
predicate isSource(DataFlow::Node node) { node instanceof Source }
13+
14+
predicate isSink(DataFlow::Node node) { node instanceof Sink }
15+
16+
predicate isAdditionalFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
17+
// I considered using a FlowState of (raw-string, ElementTree), but in all honesty
18+
// valid code would never have direct flow from a string to a sink anyway... so I
19+
// opted for the more simple approach.
20+
nodeTo = elementTreeConstruction(nodeFrom)
21+
}
22+
}
23+
24+
module XsltInjectionFlow = TaintTracking::Global<XsltInjectionConfig>;

0 commit comments

Comments
 (0)