Skip to content

Commit 117a983

Browse files
egregius313egregius313
authored
Merge pull request github#12639 from egregius313/egregius313/java/refactor-injection-queries
Java: Refactor injection queries to new dataflow API
2 parents edfd871 + c8579d8 commit 117a983

30 files changed

+329
-91
lines changed

‎java/ql/lib/semmle/code/java/security/GroovyInjectionQuery.qll‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,12 @@ import semmle.code.java.dataflow.TaintTracking
66
import semmle.code.java.security.GroovyInjection
77

88
/**
9+
* DEPRECATED: Use `GroovyInjectionFlow` instead.
10+
*
911
* A taint-tracking configuration for unsafe user input
1012
* that is used to evaluate a Groovy expression.
1113
*/
12-
class GroovyInjectionConfig extends TaintTracking::Configuration {
14+
deprecated class GroovyInjectionConfig extends TaintTracking::Configuration {
1315
GroovyInjectionConfig() { this = "GroovyInjectionConfig" }
1416

1517
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
@@ -20,3 +22,23 @@ class GroovyInjectionConfig extends TaintTracking::Configuration {
2022
any(GroovyInjectionAdditionalTaintStep c).step(fromNode, toNode)
2123
}
2224
}
25+
26+
/**
27+
* A taint-tracking configuration for unsafe user input
28+
* that is used to evaluate a Groovy expression.
29+
*/
30+
module GroovyInjectionConfig implements DataFlow::ConfigSig {
31+
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
32+
33+
predicate isSink(DataFlow::Node sink) { sink instanceof GroovyInjectionSink }
34+
35+
predicate isAdditionalFlowStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
36+
any(GroovyInjectionAdditionalTaintStep c).step(fromNode, toNode)
37+
}
38+
}
39+
40+
/**
41+
* Detect taint flow of unsafe user input
42+
* that is used to evaluate a Groovy expression.
43+
*/
44+
module GroovyInjectionFlow = TaintTracking::Global<GroovyInjectionConfig>;

‎java/ql/lib/semmle/code/java/security/JexlInjectionQuery.qll‎

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,13 @@ private class DefaultJexlInjectionAdditionalTaintStep extends JexlInjectionAddit
3939
}
4040

4141
/**
42+
* DEPRECATED: Use `JexlInjectionFlow` instead.
43+
*
4244
* A taint-tracking configuration for unsafe user input
4345
* that is used to construct and evaluate a JEXL expression.
4446
* It supports both JEXL 2 and 3.
4547
*/
46-
class JexlInjectionConfig extends TaintTracking::Configuration {
48+
deprecated class JexlInjectionConfig extends TaintTracking::Configuration {
4749
JexlInjectionConfig() { this = "JexlInjectionConfig" }
4850

4951
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
@@ -55,6 +57,27 @@ class JexlInjectionConfig extends TaintTracking::Configuration {
5557
}
5658
}
5759

60+
/**
61+
* A taint-tracking configuration for unsafe user input
62+
* that is used to construct and evaluate a JEXL expression.
63+
* It supports both JEXL 2 and 3.
64+
*/
65+
module JexlInjectionConfig implements DataFlow::ConfigSig {
66+
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
67+
68+
predicate isSink(DataFlow::Node sink) { sink instanceof JexlEvaluationSink }
69+
70+
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
71+
any(JexlInjectionAdditionalTaintStep c).step(node1, node2)
72+
}
73+
}
74+
75+
/**
76+
* Tracks unsafe user input that is used to construct and evaluate a JEXL expression.
77+
* It supports both JEXL 2 and 3.
78+
*/
79+
module JexlInjectionFlow = TaintTracking::Global<JexlInjectionConfig>;
80+
5881
/**
5982
* Holds if `n1` to `n2` is a dataflow step that creates a JEXL script using an unsafe engine
6083
* by calling `tainted.createScript(jexlExpr)`.
@@ -99,19 +122,15 @@ private predicate createJexlTemplateStep(DataFlow::Node n1, DataFlow::Node n2) {
99122
/**
100123
* Holds if `expr` is a JEXL engine that is configured with a sandbox.
101124
*/
102-
private predicate isSafeEngine(Expr expr) {
103-
exists(SandboxedJexlFlowConfig config | config.hasFlowTo(DataFlow::exprNode(expr)))
104-
}
125+
private predicate isSafeEngine(Expr expr) { SandboxedJexlFlow::flowToExpr(expr) }
105126

106127
/**
107128
* A configuration for tracking sandboxed JEXL engines.
108129
*/
109-
private class SandboxedJexlFlowConfig extends DataFlow2::Configuration {
110-
SandboxedJexlFlowConfig() { this = "JexlInjection::SandboxedJexlFlowConfig" }
130+
private module SandboxedJexlFlowConfig implements DataFlow::ConfigSig {
131+
predicate isSource(DataFlow::Node node) { node instanceof SandboxedJexlSource }
111132

112-
override predicate isSource(DataFlow::Node node) { node instanceof SandboxedJexlSource }
113-
114-
override predicate isSink(DataFlow::Node node) {
133+
predicate isSink(DataFlow::Node node) {
115134
exists(MethodAccess ma, Method m |
116135
m instanceof CreateJexlScriptMethod or
117136
m instanceof CreateJexlExpressionMethod or
@@ -121,11 +140,13 @@ private class SandboxedJexlFlowConfig extends DataFlow2::Configuration {
121140
)
122141
}
123142

124-
override predicate isAdditionalFlowStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
143+
predicate isAdditionalFlowStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
125144
createJexlEngineStep(fromNode, toNode)
126145
}
127146
}
128147

148+
private module SandboxedJexlFlow = DataFlow::Global<SandboxedJexlFlowConfig>;
149+
129150
/**
130151
* Defines a data flow source for JEXL engines configured with a sandbox.
131152
*/

‎java/ql/lib/semmle/code/java/security/JndiInjectionQuery.qll‎

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,11 @@ import semmle.code.java.frameworks.SpringLdap
77
import semmle.code.java.security.JndiInjection
88

99
/**
10+
* DEPRECATED: Use `JndiInjectionFlow` instead.
11+
*
1012
* A taint-tracking configuration for unvalidated user input that is used in JNDI lookup.
1113
*/
12-
class JndiInjectionFlowConfig extends TaintTracking::Configuration {
14+
deprecated class JndiInjectionFlowConfig extends TaintTracking::Configuration {
1315
JndiInjectionFlowConfig() { this = "JndiInjectionFlowConfig" }
1416

1517
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
@@ -27,14 +29,34 @@ class JndiInjectionFlowConfig extends TaintTracking::Configuration {
2729
}
2830
}
2931

32+
/**
33+
* A taint-tracking configuration for unvalidated user input that is used in JNDI lookup.
34+
*/
35+
module JndiInjectionFlowConfig implements DataFlow::ConfigSig {
36+
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
37+
38+
predicate isSink(DataFlow::Node sink) { sink instanceof JndiInjectionSink }
39+
40+
predicate isBarrier(DataFlow::Node node) {
41+
node.getType() instanceof PrimitiveType or
42+
node.getType() instanceof BoxedType or
43+
node instanceof JndiInjectionSanitizer
44+
}
45+
46+
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
47+
any(JndiInjectionAdditionalTaintStep c).step(node1, node2)
48+
}
49+
}
50+
51+
/** Tracks flow of unvalidated user input that is used in JNDI lookup */
52+
module JndiInjectionFlow = TaintTracking::Global<JndiInjectionFlowConfig>;
53+
3054
/**
3155
* A method that does a JNDI lookup when it receives a `SearchControls` argument with `setReturningObjFlag` = `true`
3256
*/
3357
private class UnsafeSearchControlsSink extends JndiInjectionSink {
3458
UnsafeSearchControlsSink() {
35-
exists(UnsafeSearchControlsConf conf, MethodAccess ma |
36-
conf.hasFlowTo(DataFlow::exprNode(ma.getAnArgument()))
37-
|
59+
exists(MethodAccess ma | UnsafeSearchControlsFlow::flowToExpr(ma.getAnArgument()) |
3860
this.asExpr() = ma.getArgument(0)
3961
)
4062
}
@@ -44,14 +66,14 @@ private class UnsafeSearchControlsSink extends JndiInjectionSink {
4466
* Find flows between a `SearchControls` object with `setReturningObjFlag` = `true`
4567
* and an argument of an `LdapOperations.search` or `DirContext.search` call.
4668
*/
47-
private class UnsafeSearchControlsConf extends DataFlow2::Configuration {
48-
UnsafeSearchControlsConf() { this = "UnsafeSearchControlsConf" }
69+
private module UnsafeSearchControlsConfig implements DataFlow::ConfigSig {
70+
predicate isSource(DataFlow::Node source) { source instanceof UnsafeSearchControls }
4971

50-
override predicate isSource(DataFlow::Node source) { source instanceof UnsafeSearchControls }
51-
52-
override predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeSearchControlsArgument }
72+
predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeSearchControlsArgument }
5373
}
5474

75+
private module UnsafeSearchControlsFlow = DataFlow::Global<UnsafeSearchControlsConfig>;
76+
5577
/**
5678
* An argument of type `SearchControls` of an `LdapOperations.search` or `DirContext.search` call.
5779
*/

‎java/ql/lib/semmle/code/java/security/MvelInjectionQuery.qll‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,12 @@ import semmle.code.java.dataflow.TaintTracking
66
import semmle.code.java.security.MvelInjection
77

88
/**
9+
* DEPRECATED: Use `MvelInjectionFlow` instead.
10+
*
911
* A taint-tracking configuration for unsafe user input
1012
* that is used to construct and evaluate a MVEL expression.
1113
*/
12-
class MvelInjectionFlowConfig extends TaintTracking::Configuration {
14+
deprecated class MvelInjectionFlowConfig extends TaintTracking::Configuration {
1315
MvelInjectionFlowConfig() { this = "MvelInjectionFlowConfig" }
1416

1517
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
@@ -24,3 +26,22 @@ class MvelInjectionFlowConfig extends TaintTracking::Configuration {
2426
any(MvelInjectionAdditionalTaintStep c).step(node1, node2)
2527
}
2628
}
29+
30+
/**
31+
* A taint-tracking configuration for unsafe user input
32+
* that is used to construct and evaluate a MVEL expression.
33+
*/
34+
module MvelInjectionFlowConfig implements DataFlow::ConfigSig {
35+
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
36+
37+
predicate isSink(DataFlow::Node sink) { sink instanceof MvelEvaluationSink }
38+
39+
predicate isBarrier(DataFlow::Node sanitizer) { sanitizer instanceof MvelInjectionSanitizer }
40+
41+
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
42+
any(MvelInjectionAdditionalTaintStep c).step(node1, node2)
43+
}
44+
}
45+
46+
/** Tracks flow of unsafe user input that is used to construct and evaluate a MVEL expression. */
47+
module MvelInjectionFlow = TaintTracking::Global<MvelInjectionFlowConfig>;

‎java/ql/lib/semmle/code/java/security/OgnlInjectionQuery.qll‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,11 @@ import semmle.code.java.dataflow.FlowSources
55
import semmle.code.java.security.OgnlInjection
66

77
/**
8+
* DEPRECATED: Use `OgnlInjectionFlow` instead.
9+
*
810
* A taint-tracking configuration for unvalidated user input that is used in OGNL EL evaluation.
911
*/
10-
class OgnlInjectionFlowConfig extends TaintTracking::Configuration {
12+
deprecated class OgnlInjectionFlowConfig extends TaintTracking::Configuration {
1113
OgnlInjectionFlowConfig() { this = "OgnlInjectionFlowConfig" }
1214

1315
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
@@ -22,3 +24,23 @@ class OgnlInjectionFlowConfig extends TaintTracking::Configuration {
2224
any(OgnlInjectionAdditionalTaintStep c).step(node1, node2)
2325
}
2426
}
27+
28+
/**
29+
* A taint-tracking configuration for unvalidated user input that is used in OGNL EL evaluation.
30+
*/
31+
module OgnlInjectionFlowConfig implements DataFlow::ConfigSig {
32+
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
33+
34+
predicate isSink(DataFlow::Node sink) { sink instanceof OgnlInjectionSink }
35+
36+
predicate isBarrier(DataFlow::Node node) {
37+
node.getType() instanceof PrimitiveType or node.getType() instanceof BoxedType
38+
}
39+
40+
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
41+
any(OgnlInjectionAdditionalTaintStep c).step(node1, node2)
42+
}
43+
}
44+
45+
/** Tracks flow of unvalidated user input that is used in OGNL EL evaluation. */
46+
module OgnlInjectionFlow = TaintTracking::Global<OgnlInjectionFlowConfig>;

‎java/ql/lib/semmle/code/java/security/SpelInjectionQuery.qll‎

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,12 @@ private import semmle.code.java.frameworks.spring.SpringExpression
77
private import semmle.code.java.security.SpelInjection
88

99
/**
10+
* DEPRECATED: Use `SpelInjectionFlow` instead.
11+
*
1012
* A taint-tracking configuration for unsafe user input
1113
* that is used to construct and evaluate a SpEL expression.
1214
*/
13-
class SpelInjectionConfig extends TaintTracking::Configuration {
15+
deprecated class SpelInjectionConfig extends TaintTracking::Configuration {
1416
SpelInjectionConfig() { this = "SpelInjectionConfig" }
1517

1618
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
@@ -22,37 +24,52 @@ class SpelInjectionConfig extends TaintTracking::Configuration {
2224
}
2325
}
2426

27+
/**
28+
* A taint-tracking configuration for unsafe user input
29+
* that is used to construct and evaluate a SpEL expression.
30+
*/
31+
module SpelInjectionConfig implements DataFlow::ConfigSig {
32+
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
33+
34+
predicate isSink(DataFlow::Node sink) { sink instanceof SpelExpressionEvaluationSink }
35+
36+
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
37+
any(SpelExpressionInjectionAdditionalTaintStep c).step(node1, node2)
38+
}
39+
}
40+
41+
/** Tracks flow of unsafe user input that is used to construct and evaluate a SpEL expression. */
42+
module SpelInjectionFlow = TaintTracking::Global<SpelInjectionConfig>;
43+
2544
/** Default sink for SpEL injection vulnerabilities. */
2645
private class DefaultSpelExpressionEvaluationSink extends SpelExpressionEvaluationSink {
2746
DefaultSpelExpressionEvaluationSink() {
2847
exists(MethodAccess ma |
2948
ma.getMethod() instanceof ExpressionEvaluationMethod and
3049
ma.getQualifier() = this.asExpr() and
31-
not exists(SafeEvaluationContextFlowConfig config |
32-
config.hasFlowTo(DataFlow::exprNode(ma.getArgument(0)))
33-
)
50+
not SafeEvaluationContextFlow::flowToExpr(ma.getArgument(0))
3451
)
3552
}
3653
}
3754

3855
/**
3956
* A configuration for safe evaluation context that may be used in expression evaluation.
4057
*/
41-
private class SafeEvaluationContextFlowConfig extends DataFlow2::Configuration {
42-
SafeEvaluationContextFlowConfig() { this = "SpelInjection::SafeEvaluationContextFlowConfig" }
43-
44-
override predicate isSource(DataFlow::Node source) { source instanceof SafeContextSource }
58+
private module SafeEvaluationContextFlowConfig implements DataFlow::ConfigSig {
59+
predicate isSource(DataFlow::Node source) { source instanceof SafeContextSource }
4560

46-
override predicate isSink(DataFlow::Node sink) {
61+
predicate isSink(DataFlow::Node sink) {
4762
exists(MethodAccess ma |
4863
ma.getMethod() instanceof ExpressionEvaluationMethod and
4964
ma.getArgument(0) = sink.asExpr()
5065
)
5166
}
5267

53-
override int fieldFlowBranchLimit() { result = 0 }
68+
int fieldFlowBranchLimit() { result = 0 }
5469
}
5570

71+
private module SafeEvaluationContextFlow = DataFlow::Global<SafeEvaluationContextFlowConfig>;
72+
5673
/**
5774
* A `ContextSource` that is safe from SpEL injection.
5875
*/

0 commit comments

Comments
 (0)