Skip to content

Commit 351f35d

Browse files
tamasvajktamasvajk
committed
Revert "Java: Convert other sinks"
This reverts commit 87d42b0.
1 parent 87d42b0 commit 351f35d

File tree

17 files changed

+354
-302
lines changed

17 files changed

+354
-302
lines changed

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

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ 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
2019

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

3938
override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof ServletWriterSource }
4039

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"]
40+
override predicate isSink(DataFlow::Node sink) {
41+
exists(MethodAccess ma |
42+
sink.asExpr() = ma.getAnArgument() and ma.getMethod() instanceof PrintStackTraceMethod
43+
)
4744
}
4845
}
4946

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

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ 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
1918

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

4140
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
4241

43-
override predicate isSink(DataFlow::Node sink) { sinkNode(sink, "url-open-stream") }
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+
}
4449

4550
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
4651
exists(URLConstructor u |
@@ -50,12 +55,6 @@ class RemoteURLToOpenStreamFlowConfig extends TaintTracking::Configuration {
5055
}
5156
}
5257

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-
5958
from DataFlow::PathNode source, DataFlow::PathNode sink, MethodAccess call
6059
where
6160
sink.getNode().asExpr() = call.getQualifier() and

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

Lines changed: 15 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ 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
65

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

105104
/** A data flow sink for unvalidated user input that is used in XSLT transformation. */
106105
class XsltInjectionSink extends DataFlow::ExprNode {
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-
]
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+
)
121115
}
122116
}
123117

@@ -192,21 +186,16 @@ private class TransformerFactoryWithSecureProcessingFeatureFlowConfig extends Da
192186
)
193187
}
194188

195-
override predicate isSink(DataFlow::Node sink) { sinkNode(sink, "xslt-transformer") }
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+
}
196195

197196
override int fieldFlowBranchLimit() { result = 0 }
198197
}
199198

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-
210199
/** A `ParserConfig` specific to `TransformerFactory`. */
211200
private class TransformerFactoryFeatureConfig extends ParserConfig {
212201
TransformerFactoryFeatureConfig() {

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

Lines changed: 78 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
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
54

65
/**
76
* A taint-tracking configuration for unsafe user input
@@ -22,7 +21,7 @@ class JexlInjectionConfig extends TaintTracking::Configuration {
2221
}
2322

2423
/**
25-
* A sink for Expression Language injection vulnerabilities via Jexl,
24+
* A sink for Expresssion Language injection vulnerabilities via Jexl,
2625
* i.e. method calls that run evaluation of a JEXL expression.
2726
*
2827
* Creating a `Callable` from a tainted JEXL expression or script is considered as a sink
@@ -31,41 +30,18 @@ class JexlInjectionConfig extends TaintTracking::Configuration {
3130
* maybe stored in an object field and then reached by a different flow.
3231
*/
3332
private class JexlEvaluationSink extends DataFlow::ExprNode {
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-
]
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+
)
6945
}
7046
}
7147

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

12399
override predicate isSource(DataFlow::Node node) { node instanceof SandboxedJexlSource }
124100

125-
override predicate isSink(DataFlow::Node node) { sinkNode(node, "sandboxed-jexl") }
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+
}
126111

127112
override predicate isAdditionalFlowStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
128113
createsJexlEngine(fromNode, toNode)
129114
}
130115
}
131116

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-
155117
/**
156118
* Defines a data flow source for JEXL engines configured with a sandbox.
157119
*/
@@ -202,13 +164,52 @@ private predicate returnsDataFromBean(DataFlow::Node fromNode, DataFlow::Node to
202164
)
203165
}
204166

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+
205196
/**
206197
* A method that creates a JEXL script.
207198
*/
208199
private class CreateJexlScriptMethod extends Method {
209200
CreateJexlScriptMethod() { getDeclaringType() instanceof JexlEngine and hasName("createScript") }
210201
}
211202

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+
212213
/**
213214
* A method that creates a JEXL template.
214215
*/
@@ -266,6 +267,22 @@ private class JexlUberspect extends Interface {
266267
}
267268
}
268269

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+
269286
private class Reader extends RefType {
270287
Reader() { hasQualifiedName("java.io", "Reader") }
271288
}

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

Lines changed: 30 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
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
54

65
/**
76
* A taint-tracking configuration for unsafe user input
@@ -31,31 +30,36 @@ class MvelInjectionConfig extends TaintTracking::Configuration {
3130
* i.e. methods that run evaluation of a MVEL expression.
3231
*/
3332
class MvelEvaluationSink extends DataFlow::ExprNode {
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-
]
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+
)
5963
}
6064
}
6165

0 commit comments

Comments
 (0)