Skip to content

Commit 87d42b0

Browse files
tamasvajktamasvajk
committed
Java: Convert other sinks
1 parent 3e53484 commit 87d42b0

File tree

17 files changed

+302
-354
lines changed

17 files changed

+302
-354
lines changed

‎java/ql/src/Security/CWE/CWE-209/StackTraceExposure.ql

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import java
1616
import semmle.code.java.dataflow.DataFlow
1717
import semmle.code.java.dataflow.TaintTracking
1818
import semmle.code.java.security.XSS
19+
private import semmle.code.java.dataflow.ExternalFlow
1920

2021
/**
2122
* One of the `printStackTrace()` overloads on `Throwable`.
@@ -37,10 +38,12 @@ class ServletWriterSourceToPrintStackTraceMethodFlowConfig extends TaintTracking
3738

3839
override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof ServletWriterSource }
3940

40-
override predicate isSink(DataFlow::Node sink) {
41-
exists(MethodAccess ma |
42-
sink.asExpr() = ma.getAnArgument() and ma.getMethod() instanceof PrintStackTraceMethod
43-
)
41+
override predicate isSink(DataFlow::Node sink) { sinkNode(sink, "print-stack-trace") }
42+
}
43+
44+
private class PrintStackTraceSinkModel extends SinkModelCsv {
45+
override predicate row(string row) {
46+
row = ["java.lang;Throwable;true;printStackTrace;;;Argument;print-stack-trace"]
4447
}
4548
}
4649

‎java/ql/src/experimental/Security/CWE/CWE-036/OpenStream.ql

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import semmle.code.java.dataflow.TaintTracking
1515
import semmle.code.java.dataflow.FlowSources
1616
import semmle.code.java.dataflow.ExternalFlow
1717
import DataFlow::PathGraph
18+
private import semmle.code.java.dataflow.ExternalFlow
1819

1920
class URLConstructor extends ClassInstanceExpr {
2021
URLConstructor() { this.getConstructor().getDeclaringType() instanceof TypeUrl }
@@ -39,13 +40,7 @@ class RemoteURLToOpenStreamFlowConfig extends TaintTracking::Configuration {
3940

4041
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
4142

42-
override predicate isSink(DataFlow::Node sink) {
43-
exists(MethodAccess m |
44-
sink.asExpr() = m.getQualifier() and m.getMethod() instanceof URLOpenStreamMethod
45-
)
46-
or
47-
sinkNode(sink, "url-open-stream")
48-
}
43+
override predicate isSink(DataFlow::Node sink) { sinkNode(sink, "url-open-stream") }
4944

5045
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
5146
exists(URLConstructor u |
@@ -55,6 +50,12 @@ class RemoteURLToOpenStreamFlowConfig extends TaintTracking::Configuration {
5550
}
5651
}
5752

53+
private class URLOpenStreamSinkModel extends SinkModelCsv {
54+
override predicate row(string row) {
55+
row = ["java.net;URL;false;openStream;;;Argument[-1];url-open-stream"]
56+
}
57+
}
58+
5859
from DataFlow::PathNode source, DataFlow::PathNode sink, MethodAccess call
5960
where
6061
sink.getNode().asExpr() = call.getQualifier() and

‎java/ql/src/experimental/Security/CWE/CWE-074/XsltInjectionLib.qll

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import java
22
import semmle.code.java.dataflow.FlowSources
33
import semmle.code.java.security.XmlParsers
44
import DataFlow
5+
private import semmle.code.java.dataflow.ExternalFlow
56

67
/**
78
* A taint-tracking configuration for unvalidated user input that is used in XSLT transformation.
@@ -103,15 +104,20 @@ class TypeXsltPackage extends Class {
103104

104105
/** A data flow sink for unvalidated user input that is used in XSLT transformation. */
105106
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-
)
107+
XsltInjectionSink() { sinkNode(this, "xslt") }
108+
}
109+
110+
private class XsltInjectionSinkModel extends SinkModelCsv {
111+
override predicate row(string row) {
112+
row =
113+
[
114+
"net.sf.saxon.s9api;XsltTransformer;false;transform;;;Argument[-1];xslt",
115+
"net.sf.saxon.s9api;Xslt30Transformer;false;transform;;;Argument[-1];xslt",
116+
"net.sf.saxon.s9api;Xslt30Transformer;false;applyTemplates;;;Argument[-1];xslt",
117+
"net.sf.saxon.s9api;Xslt30Transformer;false;callFunction;;;Argument[-1];xslt",
118+
"net.sf.saxon.s9api;Xslt30Transformer;false;callTemplate;;;Argument[-1];xslt",
119+
"javax.xml.transform;Transformer;false;transform;;;Argument[-1];xslt"
120+
]
115121
}
116122
}
117123

@@ -186,16 +192,21 @@ private class TransformerFactoryWithSecureProcessingFeatureFlowConfig extends Da
186192
)
187193
}
188194

189-
override predicate isSink(DataFlow::Node sink) {
190-
exists(MethodAccess ma |
191-
sink.asExpr() = ma.getQualifier() and
192-
ma.getMethod().getDeclaringType() instanceof TransformerFactory
193-
)
194-
}
195+
override predicate isSink(DataFlow::Node sink) { sinkNode(sink, "xslt-transformer") }
195196

196197
override int fieldFlowBranchLimit() { result = 0 }
197198
}
198199

200+
private class TransformerFactorySinkModel extends SinkModelCsv {
201+
override predicate row(string row) {
202+
row =
203+
[
204+
"javax.xml.transform;TransformerFactory;false;;;;Argument[-1];xslt-transformer",
205+
"javax.xml.transform.sax;SAXTransformerFactory;false;;;;Argument[-1];xslt-transformer"
206+
]
207+
}
208+
}
209+
199210
/** A `ParserConfig` specific to `TransformerFactory`. */
200211
private class TransformerFactoryFeatureConfig extends ParserConfig {
201212
TransformerFactoryFeatureConfig() {

‎java/ql/src/experimental/Security/CWE/CWE-094/JexlInjectionLib.qll

Lines changed: 61 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import java
22
import semmle.code.java.dataflow.FlowSources
33
import semmle.code.java.dataflow.TaintTracking
4+
private import semmle.code.java.dataflow.ExternalFlow
45

56
/**
67
* A taint-tracking configuration for unsafe user input
@@ -21,7 +22,7 @@ class JexlInjectionConfig extends TaintTracking::Configuration {
2122
}
2223

2324
/**
24-
* A sink for Expresssion Language injection vulnerabilities via Jexl,
25+
* A sink for Expression Language injection vulnerabilities via Jexl,
2526
* i.e. method calls that run evaluation of a JEXL expression.
2627
*
2728
* Creating a `Callable` from a tainted JEXL expression or script is considered as a sink
@@ -30,18 +31,41 @@ class JexlInjectionConfig extends TaintTracking::Configuration {
3031
* maybe stored in an object field and then reached by a different flow.
3132
*/
3233
private class JexlEvaluationSink extends DataFlow::ExprNode {
33-
JexlEvaluationSink() {
34-
exists(MethodAccess ma, Method m, Expr taintFrom |
35-
ma.getMethod() = m and taintFrom = this.asExpr()
36-
|
37-
m instanceof DirectJexlEvaluationMethod and ma.getQualifier() = taintFrom
38-
or
39-
m instanceof CreateJexlCallableMethod and ma.getQualifier() = taintFrom
40-
or
41-
m instanceof JexlEngineGetSetPropertyMethod and
42-
taintFrom.getType() instanceof TypeString and
43-
ma.getAnArgument() = taintFrom
44-
)
34+
JexlEvaluationSink() { sinkNode(this, "jexl") }
35+
}
36+
37+
private class JexlEvaluationSinkModel extends SinkModelCsv {
38+
override predicate row(string row) {
39+
row =
40+
[
41+
// Direct JEXL evaluation
42+
"org.apache.commons.jexl2;Expression;false;evaluate;;;Argument[-1];jexl",
43+
"org.apache.commons.jexl3;JexlExpression;false;evaluate;;;Argument[-1];jexl",
44+
"org.apache.commons.jexl2;Script;false;execute;;;Argument[-1];jexl",
45+
"org.apache.commons.jexl3;JexlScript;false;execute;;;Argument[-1];jexl",
46+
"org.apache.commons.jexl2;JxltEngine$Expression;false;evaluate;;;Argument[-1];jexl",
47+
"org.apache.commons.jexl3;JxltEngine$Expression;false;evaluate;;;Argument[-1];jexl",
48+
"org.apache.commons.jexl2;JxltEngine$Expression;false;prepare;;;Argument[-1];jexl",
49+
"org.apache.commons.jexl3;JxltEngine$Expression;false;prepare;;;Argument[-1];jexl",
50+
"org.apache.commons.jexl2;JxltEngine$Template;false;evaluate;;;Argument[-1];jexl",
51+
"org.apache.commons.jexl3;JxltEngine$Template;false;evaluate;;;Argument[-1];jexl",
52+
"org.apache.commons.jexl2;UnifiedJEXL$Expression;false;evaluate;;;Argument[-1];jexl",
53+
"org.apache.commons.jexl3;UnifiedJEXL$Expression;false;evaluate;;;Argument[-1];jexl",
54+
"org.apache.commons.jexl2;UnifiedJEXL$Expression;false;prepare;;;Argument[-1];jexl",
55+
"org.apache.commons.jexl3;UnifiedJEXL$Expression;false;prepare;;;Argument[-1];jexl",
56+
"org.apache.commons.jexl2;UnifiedJEXL$Template;false;evaluate;;;Argument[-1];jexl",
57+
"org.apache.commons.jexl3;UnifiedJEXL$Template;false;evaluate;;;Argument[-1];jexl",
58+
// JEXL callable
59+
"org.apache.commons.jexl2;Expression;false;callable;;;Argument[-1];jexl",
60+
"org.apache.commons.jexl3;JexlExpression;false;callable;;;Argument[-1];jexl",
61+
"org.apache.commons.jexl2;Script;false;callable;;;Argument[-1];jexl",
62+
"org.apache.commons.jexl3;JexlScript;false;callable;;;Argument[-1];jexl",
63+
// Methods in the `JexlEngine` class that gets or sets a property with a JEXL expression.
64+
"org.apache.commons.jexl2;JexlEngine;false;getProperty;;;Argument[1..2];jexl",
65+
"org.apache.commons.jexl3;JexlEngine;false;getProperty;;;Argument[1..2];jexl",
66+
"org.apache.commons.jexl2;JexlEngine;false;setProperty;;;Argument[1];jexl",
67+
"org.apache.commons.jexl3;JexlEngine;false;setProperty;;;Argument[1];jexl"
68+
]
4569
}
4670
}
4771

@@ -98,22 +122,36 @@ private class SandboxedJexlFlowConfig extends DataFlow2::Configuration {
98122

99123
override predicate isSource(DataFlow::Node node) { node instanceof SandboxedJexlSource }
100124

101-
override predicate isSink(DataFlow::Node node) {
102-
exists(MethodAccess ma, Method m | ma.getMethod() = m |
103-
(
104-
m instanceof CreateJexlScriptMethod or
105-
m instanceof CreateJexlExpressionMethod or
106-
m instanceof CreateJexlTemplateMethod
107-
) and
108-
ma.getQualifier() = node.asExpr()
109-
)
110-
}
125+
override predicate isSink(DataFlow::Node node) { sinkNode(node, "sandboxed-jexl") }
111126

112127
override predicate isAdditionalFlowStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
113128
createsJexlEngine(fromNode, toNode)
114129
}
115130
}
116131

132+
private class SandboxedJexlEvaluationSinkModel extends SinkModelCsv {
133+
override predicate row(string row) {
134+
row =
135+
[
136+
// CreateJexlScriptMethod
137+
"org.apache.commons.jexl2;JexlEngine;false;createScript;;;Argument[-1];sandboxed-jexl",
138+
"org.apache.commons.jexl3;JexlEngine;false;createScript;;;Argument[-1];sandboxed-jexl",
139+
// CreateJexlExpressionMethod
140+
"org.apache.commons.jexl2;UnifiedJEXL;false;parse;;;Argument[-1];sandboxed-jexl",
141+
"org.apache.commons.jexl3;UnifiedJEXL;false;parse;;;Argument[-1];sandboxed-jexl",
142+
"org.apache.commons.jexl2;JxltEngine;false;createExpression;;;Argument[-1];sandboxed-jexl",
143+
"org.apache.commons.jexl3;JxltEngine;false;createExpression;;;Argument[-1];sandboxed-jexl",
144+
"org.apache.commons.jexl2;JexlEngine;false;createExpression;;;Argument[-1];sandboxed-jexl",
145+
"org.apache.commons.jexl3;JexlEngine;false;createExpression;;;Argument[-1];sandboxed-jexl",
146+
// CreateJexlTemplateMethod
147+
"org.apache.commons.jexl2;JxltEngine;false;createTemplate;;;Argument[-1];sandboxed-jexl",
148+
"org.apache.commons.jexl3;JxltEngine;false;createTemplate;;;Argument[-1];sandboxed-jexl",
149+
"org.apache.commons.jexl2;UnifiedJEXL;false;createTemplate;;;Argument[-1];sandboxed-jexl",
150+
"org.apache.commons.jexl3;UnifiedJEXL;false;createTemplate;;;Argument[-1];sandboxed-jexl"
151+
]
152+
}
153+
}
154+
117155
/**
118156
* Defines a data flow source for JEXL engines configured with a sandbox.
119157
*/
@@ -164,52 +202,13 @@ private predicate returnsDataFromBean(DataFlow::Node fromNode, DataFlow::Node to
164202
)
165203
}
166204

167-
/**
168-
* A methods in the `JexlEngine` class that gets or sets a property with a JEXL expression.
169-
*/
170-
private class JexlEngineGetSetPropertyMethod extends Method {
171-
JexlEngineGetSetPropertyMethod() {
172-
getDeclaringType() instanceof JexlEngine and
173-
hasName(["getProperty", "setProperty"])
174-
}
175-
}
176-
177-
/**
178-
* A method that triggers direct evaluation of JEXL expressions.
179-
*/
180-
private class DirectJexlEvaluationMethod extends Method {
181-
DirectJexlEvaluationMethod() {
182-
getDeclaringType() instanceof JexlExpression and hasName("evaluate")
183-
or
184-
getDeclaringType() instanceof JexlScript and hasName("execute")
185-
or
186-
getDeclaringType() instanceof JxltEngineExpression and hasName(["evaluate", "prepare"])
187-
or
188-
getDeclaringType() instanceof JxltEngineTemplate and hasName("evaluate")
189-
or
190-
getDeclaringType() instanceof UnifiedJexlExpression and hasName(["evaluate", "prepare"])
191-
or
192-
getDeclaringType() instanceof UnifiedJexlTemplate and hasName("evaluate")
193-
}
194-
}
195-
196205
/**
197206
* A method that creates a JEXL script.
198207
*/
199208
private class CreateJexlScriptMethod extends Method {
200209
CreateJexlScriptMethod() { getDeclaringType() instanceof JexlEngine and hasName("createScript") }
201210
}
202211

203-
/**
204-
* A method that creates a `Callable` for a JEXL expression or script.
205-
*/
206-
private class CreateJexlCallableMethod extends Method {
207-
CreateJexlCallableMethod() {
208-
(getDeclaringType() instanceof JexlExpression or getDeclaringType() instanceof JexlScript) and
209-
hasName("callable")
210-
}
211-
}
212-
213212
/**
214213
* A method that creates a JEXL template.
215214
*/
@@ -267,22 +266,6 @@ private class JexlUberspect extends Interface {
267266
}
268267
}
269268

270-
private class JxltEngineExpression extends NestedType {
271-
JxltEngineExpression() { getEnclosingType() instanceof JxltEngine and hasName("Expression") }
272-
}
273-
274-
private class JxltEngineTemplate extends NestedType {
275-
JxltEngineTemplate() { getEnclosingType() instanceof JxltEngine and hasName("Template") }
276-
}
277-
278-
private class UnifiedJexlExpression extends NestedType {
279-
UnifiedJexlExpression() { getEnclosingType() instanceof UnifiedJexl and hasName("Expression") }
280-
}
281-
282-
private class UnifiedJexlTemplate extends NestedType {
283-
UnifiedJexlTemplate() { getEnclosingType() instanceof UnifiedJexl and hasName("Template") }
284-
}
285-
286269
private class Reader extends RefType {
287270
Reader() { hasQualifiedName("java.io", "Reader") }
288271
}

‎java/ql/src/experimental/Security/CWE/CWE-094/MvelInjectionLib.qll

Lines changed: 26 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import java
22
import semmle.code.java.dataflow.FlowSources
33
import semmle.code.java.dataflow.TaintTracking
4+
private import semmle.code.java.dataflow.ExternalFlow
45

56
/**
67
* A taint-tracking configuration for unsafe user input
@@ -30,36 +31,31 @@ class MvelInjectionConfig extends TaintTracking::Configuration {
3031
* i.e. methods that run evaluation of a MVEL expression.
3132
*/
3233
class MvelEvaluationSink extends DataFlow::ExprNode {
33-
MvelEvaluationSink() {
34-
exists(StaticMethodAccess ma, Method m | m = ma.getMethod() |
35-
(
36-
m instanceof MvelEvalMethod or
37-
m instanceof TemplateRuntimeEvaluationMethod
38-
) and
39-
ma.getArgument(0) = asExpr()
40-
)
41-
or
42-
exists(MethodAccess ma, Method m | m = ma.getMethod() |
43-
m instanceof MvelScriptEngineEvaluationMethod and
44-
ma.getArgument(0) = asExpr()
45-
)
46-
or
47-
exists(MethodAccess ma, Method m | m = ma.getMethod() |
48-
(
49-
m instanceof ExecutableStatementEvaluationMethod or
50-
m instanceof CompiledExpressionEvaluationMethod or
51-
m instanceof CompiledAccExpressionEvaluationMethod or
52-
m instanceof AccessorEvaluationMethod or
53-
m instanceof CompiledScriptEvaluationMethod or
54-
m instanceof MvelCompiledScriptEvaluationMethod
55-
) and
56-
ma.getQualifier() = asExpr()
57-
)
58-
or
59-
exists(StaticMethodAccess ma, Method m | m = ma.getMethod() |
60-
m instanceof MvelRuntimeEvaluationMethod and
61-
ma.getArgument(1) = asExpr()
62-
)
34+
MvelEvaluationSink() { sinkNode(this, "mvel") }
35+
}
36+
37+
private class MvelEvaluationSinkModel extends SinkModelCsv {
38+
override predicate row(string row) {
39+
row =
40+
[
41+
"org.mvel2.jsr223;MvelScriptEngine;false;evaluate;;;Argument[0];mvel",
42+
"org.mvel2.jsr223;MvelScriptEngine;false;eval;;;Argument[0];mvel",
43+
"org.mvel2.compiler;ExecutableStatement;false;getValue;;;Argument[-1];mvel",
44+
"org.mvel2.compiler;CompiledExpression;false;getDirectValue;;;Argument[-1];mvel",
45+
"org.mvel2.compiler;CompiledAccExpression;false;getValue;;;Argument[-1];mvel",
46+
"org.mvel2.compiler;Accessor;false;getValue;;;Argument[-1];mvel",
47+
"javax.script;CompiledScript;false;eval;;;Argument[-1];mvel",
48+
"org.mvel2.jsr223;MvelCompiledScript;false;eval;;;Argument[-1];mvel",
49+
"org.mvel2;MVEL;false;eval;;;Argument[0];mvel",
50+
"org.mvel2;MVEL;false;executeExpression;;;Argument[0];mvel",
51+
"org.mvel2;MVEL;false;evalToBoolean;;;Argument[0];mvel",
52+
"org.mvel2;MVEL;false;evalToString;;;Argument[0];mvel",
53+
"org.mvel2;MVEL;false;executeAllExpression;;;Argument[0];mvel",
54+
"org.mvel2;MVEL;false;executeSetExpression;;;Argument[0];mvel",
55+
"org.mvel2.templates;TemplateRuntime;false;eval;;;Argument[0];mvel",
56+
"org.mvel2.templates;TemplateRuntime;false;execute;;;Argument[0];mvel",
57+
"org.mvel2;MVELRuntime;false;execute;;;Argument[1];mvel"
58+
]
6359
}
6460
}
6561

0 commit comments

Comments
 (0)