Skip to content

Commit 6967b06

Browse files
atorralbaatorralba
committed
Decouple XsltInjection.qll to reuse the taint tracking configuration
1 parent fc58ada commit 6967b06

File tree

5 files changed

+43
-54
lines changed

5 files changed

+43
-54
lines changed

‎java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@ private module Frameworks {
110110
private import semmle.code.java.security.MvelInjection
111111
private import semmle.code.java.security.OgnlInjection
112112
private import semmle.code.java.security.XPath
113+
private import semmle.code.java.security.XsltInjectionSinkModels
113114
private import semmle.code.java.frameworks.android.Android
114115
private import semmle.code.java.frameworks.android.SQLite
115116
private import semmle.code.java.frameworks.Jdbc

‎java/ql/src/Security/CWE/CWE-074/XsltInjection.ql

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -11,29 +11,9 @@
1111
*/
1212

1313
import java
14-
import semmle.code.java.dataflow.FlowSources
15-
import semmle.code.java.security.XsltInjection
14+
import semmle.code.java.security.XsltInjectionQuery
1615
import DataFlow::PathGraph
1716

18-
/**
19-
* A taint-tracking configuration for unvalidated user input that is used in XSLT transformation.
20-
*/
21-
class XsltInjectionFlowConfig extends TaintTracking::Configuration {
22-
XsltInjectionFlowConfig() { this = "XsltInjectionFlowConfig" }
23-
24-
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
25-
26-
override predicate isSink(DataFlow::Node sink) { sink instanceof XsltInjectionSink }
27-
28-
override predicate isSanitizer(DataFlow::Node node) {
29-
node.getType() instanceof PrimitiveType or node.getType() instanceof BoxedType
30-
}
31-
32-
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
33-
any(XsltInjectionAdditionalTaintStep c).step(node1, node2)
34-
}
35-
}
36-
3717
from DataFlow::PathNode source, DataFlow::PathNode sink, XsltInjectionFlowConfig conf
3818
where conf.hasFlowPath(source, sink)
3919
select sink.getNode(), source, sink, "XSLT transformation might include stylesheet from $@.",

‎java/ql/src/semmle/code/java/security/XsltInjection.qll renamed to ‎java/ql/src/semmle/code/java/security/XsltInjectionQuery.qll

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/** Provides classes to reason about XSLT injection vulnerabilities. */
22

33
import java
4-
import semmle.code.java.dataflow.ExternalFlow
4+
import semmle.code.java.dataflow.FlowSources
55
import semmle.code.java.security.XmlParsers
66
import semmle.code.java.dataflow.DataFlow
77

@@ -11,20 +11,6 @@ import semmle.code.java.dataflow.DataFlow
1111
*/
1212
abstract class XsltInjectionSink extends DataFlow::Node { }
1313

14-
private class DefaultXsltInjectionSinkModel extends SinkModelCsv {
15-
override predicate row(string row) {
16-
row =
17-
[
18-
"javax.xml.transform;Transformer;false;transform;;;Argument[-1];xslt",
19-
"net.sf.saxon.s9api;XsltTransformer;false;transform;;;Argument[-1];xslt",
20-
"net.sf.saxon.s9api;Xslt30Transformer;false;transform;;;Argument[-1];xslt",
21-
"net.sf.saxon.s9api;Xslt30Transformer;false;applyTemplates;;;Argument[-1];xslt",
22-
"net.sf.saxon.s9api;Xslt30Transformer;false;callFunction;;;Argument[-1];xslt",
23-
"net.sf.saxon.s9api;Xslt30Transformer;false;callTemplate;;;Argument[-1];xslt"
24-
]
25-
}
26-
}
27-
2814
/** A default sink representing methods susceptible to XSLT Injection attacks. */
2915
private class DefaultXsltInjectionSink extends XsltInjectionSink {
3016
DefaultXsltInjectionSink() { sinkNode(this, "xslt") }
@@ -59,6 +45,25 @@ private class DefaultXsltInjectionAdditionalTaintStep extends XsltInjectionAddit
5945
}
6046
}
6147

48+
/**
49+
* A taint-tracking configuration for unvalidated user input that is used in XSLT transformation.
50+
*/
51+
class XsltInjectionFlowConfig extends TaintTracking::Configuration {
52+
XsltInjectionFlowConfig() { this = "XsltInjectionFlowConfig" }
53+
54+
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
55+
56+
override predicate isSink(DataFlow::Node sink) { sink instanceof XsltInjectionSink }
57+
58+
override predicate isSanitizer(DataFlow::Node node) {
59+
node.getType() instanceof PrimitiveType or node.getType() instanceof BoxedType
60+
}
61+
62+
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
63+
any(XsltInjectionAdditionalTaintStep c).step(node1, node2)
64+
}
65+
}
66+
6267
/**
6368
* Holds if `n1` to `n2` is a dataflow step that converts between `InputStream` or `Reader` and
6469
* `XMLStreamReader`, i.e. `XMLInputFactory.createXMLStreamReader(tainted)`.

‎java/ql/src/semmle/code/java/security/XsltInjectionSinkModels.qll

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
/** Provides sink models relating to XSLT injection vulnerabilities. */
2+
3+
import semmle.code.java.dataflow.ExternalFlow
4+
5+
private class DefaultXsltInjectionSinkModel extends SinkModelCsv {
6+
override predicate row(string row) {
7+
row =
8+
[
9+
"javax.xml.transform;Transformer;false;transform;;;Argument[-1];xslt",
10+
"net.sf.saxon.s9api;XsltTransformer;false;transform;;;Argument[-1];xslt",
11+
"net.sf.saxon.s9api;Xslt30Transformer;false;transform;;;Argument[-1];xslt",
12+
"net.sf.saxon.s9api;Xslt30Transformer;false;applyTemplates;;;Argument[-1];xslt",
13+
"net.sf.saxon.s9api;Xslt30Transformer;false;callFunction;;;Argument[-1];xslt",
14+
"net.sf.saxon.s9api;Xslt30Transformer;false;callTemplate;;;Argument[-1];xslt"
15+
]
16+
}
17+
}

‎java/ql/test/query-tests/security/CWE-074/XsltInjectionTest.ql

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,19 @@
11
import java
22
import semmle.code.java.dataflow.TaintTracking
33
import semmle.code.java.dataflow.FlowSources
4-
import semmle.code.java.security.XsltInjection
4+
import semmle.code.java.security.XsltInjectionQuery
55
import TestUtilities.InlineExpectationsTest
66

7-
class Conf extends TaintTracking::Configuration {
8-
Conf() { this = "test:cwe:xslt-injection" }
9-
10-
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
11-
12-
override predicate isSink(DataFlow::Node sink) { sink instanceof XsltInjectionSink }
13-
14-
override predicate isSanitizer(DataFlow::Node node) {
15-
node.getType() instanceof PrimitiveType or node.getType() instanceof BoxedType
16-
}
17-
18-
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
19-
any(XsltInjectionAdditionalTaintStep c).step(node1, node2)
20-
}
21-
}
22-
237
class HasXsltInjectionTest extends InlineExpectationsTest {
248
HasXsltInjectionTest() { this = "HasXsltInjectionTest" }
259

2610
override string getARelevantTag() { result = "hasXsltInjection" }
2711

2812
override predicate hasActualResult(Location location, string element, string tag, string value) {
2913
tag = "hasXsltInjection" and
30-
exists(DataFlow::Node src, DataFlow::Node sink, Conf conf | conf.hasFlow(src, sink) |
14+
exists(DataFlow::Node src, DataFlow::Node sink, XsltInjectionFlowConfig conf |
15+
conf.hasFlow(src, sink)
16+
|
3117
sink.getLocation() = location and
3218
element = sink.toString() and
3319
value = ""

0 commit comments

Comments
 (0)