Skip to content

Commit d8bb527

Browse files
atorralbaatorralba
committed
Refactor to use CSV sink models
1 parent c792567 commit d8bb527

File tree

2 files changed

+118
-133
lines changed

2 files changed

+118
-133
lines changed

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

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,28 @@
1212

1313
import java
1414
import semmle.code.java.dataflow.FlowSources
15-
import XsltInjectionLib
15+
import semmle.code.java.security.XsltInjection
1616
import DataFlow::PathGraph
1717

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+
1837
from DataFlow::PathNode source, DataFlow::PathNode sink, XsltInjectionFlowConfig conf
1938
where conf.hasFlowPath(source, sink)
2039
select sink.getNode(), source, sink, "XSLT transformation might include stylesheet from $@.",

‎java/ql/src/Security/CWE/CWE-074/XsltInjectionLib.qll renamed to ‎java/ql/src/semmle/code/java/security/XsltInjection.qll

Lines changed: 98 additions & 132 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,51 @@
1+
/** Provides classes to reason about XSLT injection vulnerabilities. */
2+
13
import java
2-
import semmle.code.java.dataflow.FlowSources
4+
import semmle.code.java.dataflow.ExternalFlow
35
import semmle.code.java.security.XmlParsers
4-
import DataFlow
6+
import semmle.code.java.dataflow.DataFlow
57

68
/**
7-
* A taint-tracking configuration for unvalidated user input that is used in XSLT transformation.
9+
* A data flow sink for unvalidated user input that is used in XSLT transformation.
10+
* Extend this class to add your own XSLT Injection sinks.
811
*/
9-
class XsltInjectionFlowConfig extends TaintTracking::Configuration {
10-
XsltInjectionFlowConfig() { this = "XsltInjectionFlowConfig" }
11-
12-
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
12+
abstract class XsltInjectionSink extends DataFlow::Node { }
13+
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+
}
1327

14-
override predicate isSink(DataFlow::Node sink) { sink instanceof XsltInjectionSink }
28+
/** A default sink representing methods susceptible to XSLT Injection attacks. */
29+
private class DefaultXsltInjectionSink extends XsltInjectionSink {
30+
DefaultXsltInjectionSink() { sinkNode(this, "xslt") }
31+
}
1532

16-
override predicate isSanitizer(DataFlow::Node node) {
17-
node.getType() instanceof PrimitiveType or node.getType() instanceof BoxedType
18-
}
33+
/**
34+
* A unit class for adding additional taint steps.
35+
*
36+
* Extend this class to add additional taint steps that should apply to the `XsltInjectionFlowConfig`.
37+
*/
38+
class XsltInjectionAdditionalTaintStep extends Unit {
39+
/**
40+
* Holds if the step from `node1` to `node2` should be considered a taint
41+
* step for the `XsltInjectionFlowConfig` configuration.
42+
*/
43+
abstract predicate step(DataFlow::Node node1, DataFlow::Node node2);
44+
}
1945

20-
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
46+
/** A set of additional taint steps to consider when taint tracking XSLT related data flows. */
47+
private class DefaultXsltInjectionAdditionalTaintStep extends XsltInjectionAdditionalTaintStep {
48+
override predicate step(DataFlow::Node node1, DataFlow::Node node2) {
2149
xmlStreamReaderStep(node1, node2) or
2250
xmlEventReaderStep(node1, node2) or
2351
staxSourceStep(node1, node2) or
@@ -31,95 +59,11 @@ class XsltInjectionFlowConfig extends TaintTracking::Configuration {
3159
}
3260
}
3361

34-
/** The class `javax.xml.transform.stax.StAXSource`. */
35-
class TypeStAXSource extends Class {
36-
TypeStAXSource() { this.hasQualifiedName("javax.xml.transform.stax", "StAXSource") }
37-
}
38-
39-
/** The class `javax.xml.transform.dom.DOMSource`. */
40-
class TypeDOMSource extends Class {
41-
TypeDOMSource() { this.hasQualifiedName("javax.xml.transform.dom", "DOMSource") }
42-
}
43-
44-
/** The interface `javax.xml.transform.Templates`. */
45-
class TypeTemplates extends Interface {
46-
TypeTemplates() { this.hasQualifiedName("javax.xml.transform", "Templates") }
47-
}
48-
49-
/** The method `net.sf.saxon.s9api.XsltTransformer.transform`. */
50-
class XsltTransformerTransformMethod extends Method {
51-
XsltTransformerTransformMethod() {
52-
this.getDeclaringType().hasQualifiedName("net.sf.saxon.s9api", "XsltTransformer") and
53-
this.hasName("transform")
54-
}
55-
}
56-
57-
/** The method `net.sf.saxon.s9api.Xslt30Transformer.transform`. */
58-
class Xslt30TransformerTransformMethod extends Method {
59-
Xslt30TransformerTransformMethod() {
60-
this.getDeclaringType().hasQualifiedName("net.sf.saxon.s9api", "Xslt30Transformer") and
61-
this.hasName("transform")
62-
}
63-
}
64-
65-
/** The method `net.sf.saxon.s9api.Xslt30Transformer.applyTemplates`. */
66-
class Xslt30TransformerApplyTemplatesMethod extends Method {
67-
Xslt30TransformerApplyTemplatesMethod() {
68-
this.getDeclaringType().hasQualifiedName("net.sf.saxon.s9api", "Xslt30Transformer") and
69-
this.hasName("applyTemplates")
70-
}
71-
}
72-
73-
/** The method `net.sf.saxon.s9api.Xslt30Transformer.callFunction`. */
74-
class Xslt30TransformerCallFunctionMethod extends Method {
75-
Xslt30TransformerCallFunctionMethod() {
76-
this.getDeclaringType().hasQualifiedName("net.sf.saxon.s9api", "Xslt30Transformer") and
77-
this.hasName("callFunction")
78-
}
79-
}
80-
81-
/** The method `net.sf.saxon.s9api.Xslt30Transformer.callTemplate`. */
82-
class Xslt30TransformerCallTemplateMethod extends Method {
83-
Xslt30TransformerCallTemplateMethod() {
84-
this.getDeclaringType().hasQualifiedName("net.sf.saxon.s9api", "Xslt30Transformer") and
85-
this.hasName("callTemplate")
86-
}
87-
}
88-
89-
/** The class `net.sf.saxon.s9api.XsltCompiler`. */
90-
class TypeXsltCompiler extends Class {
91-
TypeXsltCompiler() { this.hasQualifiedName("net.sf.saxon.s9api", "XsltCompiler") }
92-
}
93-
94-
/** The class `net.sf.saxon.s9api.XsltExecutable`. */
95-
class TypeXsltExecutable extends Class {
96-
TypeXsltExecutable() { this.hasQualifiedName("net.sf.saxon.s9api", "XsltExecutable") }
97-
}
98-
99-
/** The class `net.sf.saxon.s9api.XsltPackage`. */
100-
class TypeXsltPackage extends Class {
101-
TypeXsltPackage() { this.hasQualifiedName("net.sf.saxon.s9api", "XsltPackage") }
102-
}
103-
104-
/** A data flow sink for unvalidated user input that is used in XSLT transformation. */
105-
class XsltInjectionSink extends DataFlow::ExprNode {
106-
XsltInjectionSink() {
107-
exists(MethodAccess ma, Method m | m = ma.getMethod() and ma.getQualifier() = this.getExpr() |
108-
ma instanceof TransformerTransform or
109-
m instanceof XsltTransformerTransformMethod or
110-
m instanceof Xslt30TransformerTransformMethod or
111-
m instanceof Xslt30TransformerApplyTemplatesMethod or
112-
m instanceof Xslt30TransformerCallFunctionMethod or
113-
m instanceof Xslt30TransformerCallTemplateMethod
114-
)
115-
}
116-
}
117-
11862
/**
11963
* Holds if `n1` to `n2` is a dataflow step that converts between `InputStream` or `Reader` and
12064
* `XMLStreamReader`, i.e. `XMLInputFactory.createXMLStreamReader(tainted)`.
12165
*/
122-
predicate xmlStreamReaderStep(ExprNode n1, ExprNode n2) {
66+
private predicate xmlStreamReaderStep(DataFlow::Node n1, DataFlow::Node n2) {
12367
exists(XmlInputFactoryStreamReader xmlStreamReader |
12468
n1.asExpr() = xmlStreamReader.getSink() and
12569
n2.asExpr() = xmlStreamReader
@@ -130,7 +74,7 @@ predicate xmlStreamReaderStep(ExprNode n1, ExprNode n2) {
13074
* Holds if `n1` to `n2` is a dataflow step that converts between `InputStream` or `Reader` and
13175
* `XMLEventReader`, i.e. `XMLInputFactory.createXMLEventReader(tainted)`.
13276
*/
133-
predicate xmlEventReaderStep(ExprNode n1, ExprNode n2) {
77+
private predicate xmlEventReaderStep(DataFlow::Node n1, DataFlow::Node n2) {
13478
exists(XmlInputFactoryEventReader xmlEventReader |
13579
n1.asExpr() = xmlEventReader.getSink() and
13680
n2.asExpr() = xmlEventReader
@@ -141,7 +85,7 @@ predicate xmlEventReaderStep(ExprNode n1, ExprNode n2) {
14185
* Holds if `n1` to `n2` is a dataflow step that converts between `XMLStreamReader` or
14286
* `XMLEventReader` and `StAXSource`, i.e. `new StAXSource(tainted)`.
14387
*/
144-
predicate staxSourceStep(ExprNode n1, ExprNode n2) {
88+
private predicate staxSourceStep(DataFlow::Node n1, DataFlow::Node n2) {
14589
exists(ConstructorCall cc | cc.getConstructedType() instanceof TypeStAXSource |
14690
n1.asExpr() = cc.getAnArgument() and
14791
n2.asExpr() = cc
@@ -152,7 +96,7 @@ predicate staxSourceStep(ExprNode n1, ExprNode n2) {
15296
* Holds if `n1` to `n2` is a dataflow step that converts between `InputStream` and `Document`,
15397
* i.e. `DocumentBuilder.parse(tainted)`.
15498
*/
155-
predicate documentBuilderStep(ExprNode n1, ExprNode n2) {
99+
private predicate documentBuilderStep(DataFlow::Node n1, DataFlow::Node n2) {
156100
exists(DocumentBuilderParse documentBuilder |
157101
n1.asExpr() = documentBuilder.getSink() and
158102
n2.asExpr() = documentBuilder
@@ -163,13 +107,30 @@ predicate documentBuilderStep(ExprNode n1, ExprNode n2) {
163107
* Holds if `n1` to `n2` is a dataflow step that converts between `Document` and `DOMSource`, i.e.
164108
* `new DOMSource(tainted)`.
165109
*/
166-
predicate domSourceStep(ExprNode n1, ExprNode n2) {
110+
private predicate domSourceStep(DataFlow::Node n1, DataFlow::Node n2) {
167111
exists(ConstructorCall cc | cc.getConstructedType() instanceof TypeDOMSource |
168112
n1.asExpr() = cc.getAnArgument() and
169113
n2.asExpr() = cc
170114
)
171115
}
172116

117+
/**
118+
* Holds if `n1` to `n2` is a dataflow step that converts between `Source` and `Transformer` or
119+
* `Templates`, i.e. `TransformerFactory.newTransformer(tainted)` or
120+
* `TransformerFactory.newTemplates(tainted)`.
121+
*/
122+
private predicate newTransformerOrTemplatesStep(DataFlow::Node n1, DataFlow::Node n2) {
123+
exists(MethodAccess ma, Method m | ma.getMethod() = m |
124+
n1.asExpr() = ma.getAnArgument() and
125+
n2.asExpr() = ma and
126+
m.getDeclaringType() instanceof TransformerFactory and
127+
m.hasName(["newTransformer", "newTemplates"]) and
128+
not exists(TransformerFactoryWithSecureProcessingFeatureFlowConfig conf |
129+
conf.hasFlowToExpr(ma.getQualifier())
130+
)
131+
)
132+
}
133+
173134
/**
174135
* A data flow configuration for secure processing feature that is enabled on `TransformerFactory`.
175136
*/
@@ -207,31 +168,11 @@ private class TransformerFactoryFeatureConfig extends ParserConfig {
207168
}
208169
}
209170

210-
/**
211-
* Holds if `n1` to `n2` is a dataflow step that converts between `Source` and `Transformer` or
212-
* `Templates`, i.e. `TransformerFactory.newTransformer(tainted)` or
213-
* `TransformerFactory.newTemplates(tainted)`.
214-
*/
215-
predicate newTransformerOrTemplatesStep(ExprNode n1, ExprNode n2) {
216-
exists(MethodAccess ma, Method m | ma.getMethod() = m |
217-
n1.asExpr() = ma.getAnArgument() and
218-
n2.asExpr() = ma and
219-
(
220-
m.getDeclaringType() instanceof TransformerFactory and m.hasName("newTransformer")
221-
or
222-
m.getDeclaringType() instanceof TransformerFactory and m.hasName("newTemplates")
223-
) and
224-
not exists(TransformerFactoryWithSecureProcessingFeatureFlowConfig conf |
225-
conf.hasFlowToExpr(ma.getQualifier())
226-
)
227-
)
228-
}
229-
230171
/**
231172
* Holds if `n1` to `n2` is a dataflow step that converts between `Templates` and `Transformer`,
232173
* i.e. `tainted.newTransformer()`.
233174
*/
234-
predicate newTransformerFromTemplatesStep(ExprNode n1, ExprNode n2) {
175+
private predicate newTransformerFromTemplatesStep(DataFlow::Node n1, DataFlow::Node n2) {
235176
exists(MethodAccess ma, Method m | ma.getMethod() = m |
236177
n1.asExpr() = ma.getQualifier() and
237178
n2.asExpr() = ma and
@@ -246,17 +187,12 @@ predicate newTransformerFromTemplatesStep(ExprNode n1, ExprNode n2) {
246187
* `XsltCompiler.loadExecutablePackage(tainted)` or `XsltCompiler.compilePackage(tainted)` or
247188
* `XsltCompiler.loadLibraryPackage(tainted)`.
248189
*/
249-
predicate xsltCompilerStep(ExprNode n1, ExprNode n2) {
190+
private predicate xsltCompilerStep(DataFlow::Node n1, DataFlow::Node n2) {
250191
exists(MethodAccess ma, Method m | ma.getMethod() = m |
251192
n1.asExpr() = ma.getArgument(0) and
252193
n2.asExpr() = ma and
253194
m.getDeclaringType() instanceof TypeXsltCompiler and
254-
(
255-
m.hasName("compile") or
256-
m.hasName("loadExecutablePackage") or
257-
m.hasName("compilePackage") or
258-
m.hasName("loadLibraryPackage")
259-
)
195+
m.hasName(["compile", "loadExecutablePackage", "compilePackage", "loadLibraryPackage"])
260196
)
261197
}
262198

@@ -265,24 +201,54 @@ predicate xsltCompilerStep(ExprNode n1, ExprNode n2) {
265201
* `XsltTransformer` or `Xslt30Transformer`, i.e. `XsltExecutable.load()` or
266202
* `XsltExecutable.load30()`.
267203
*/
268-
predicate xsltExecutableStep(ExprNode n1, ExprNode n2) {
204+
private predicate xsltExecutableStep(DataFlow::Node n1, DataFlow::Node n2) {
269205
exists(MethodAccess ma, Method m | ma.getMethod() = m |
270206
n1.asExpr() = ma.getQualifier() and
271207
n2.asExpr() = ma and
272208
m.getDeclaringType() instanceof TypeXsltExecutable and
273-
(m.hasName("load") or m.hasName("load30"))
209+
m.hasName(["load", "load30"])
274210
)
275211
}
276212

277213
/**
278214
* Holds if `n1` to `n2` is a dataflow step that converts between `XsltPackage` and
279215
* `XsltExecutable`, i.e. `XsltPackage.link()`.
280216
*/
281-
predicate xsltPackageStep(ExprNode n1, ExprNode n2) {
217+
private predicate xsltPackageStep(DataFlow::Node n1, DataFlow::Node n2) {
282218
exists(MethodAccess ma, Method m | ma.getMethod() = m |
283219
n1.asExpr() = ma.getQualifier() and
284220
n2.asExpr() = ma and
285221
m.getDeclaringType() instanceof TypeXsltPackage and
286222
m.hasName("link")
287223
)
288224
}
225+
226+
/** The class `javax.xml.transform.stax.StAXSource`. */
227+
private class TypeStAXSource extends Class {
228+
TypeStAXSource() { this.hasQualifiedName("javax.xml.transform.stax", "StAXSource") }
229+
}
230+
231+
/** The class `javax.xml.transform.dom.DOMSource`. */
232+
private class TypeDOMSource extends Class {
233+
TypeDOMSource() { this.hasQualifiedName("javax.xml.transform.dom", "DOMSource") }
234+
}
235+
236+
/** The interface `javax.xml.transform.Templates`. */
237+
private class TypeTemplates extends Interface {
238+
TypeTemplates() { this.hasQualifiedName("javax.xml.transform", "Templates") }
239+
}
240+
241+
/** The class `net.sf.saxon.s9api.XsltCompiler`. */
242+
private class TypeXsltCompiler extends Class {
243+
TypeXsltCompiler() { this.hasQualifiedName("net.sf.saxon.s9api", "XsltCompiler") }
244+
}
245+
246+
/** The class `net.sf.saxon.s9api.XsltExecutable`. */
247+
private class TypeXsltExecutable extends Class {
248+
TypeXsltExecutable() { this.hasQualifiedName("net.sf.saxon.s9api", "XsltExecutable") }
249+
}
250+
251+
/** The class `net.sf.saxon.s9api.XsltPackage`. */
252+
private class TypeXsltPackage extends Class {
253+
TypeXsltPackage() { this.hasQualifiedName("net.sf.saxon.s9api", "XsltPackage") }
254+
}

0 commit comments

Comments
 (0)