Skip to content

Commit 1544330

Browse files
committed
Minor fixes for code review
1 parent 4b9b27c commit 1544330

File tree

1 file changed

+16
-3
lines changed

1 file changed

+16
-3
lines changed

java/ql/lib/semmle/code/java/security/TaintedEnvironmentVariableQuery.qll

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,33 +2,46 @@
22

33
private import semmle.code.java.dataflow.ExternalFlow
44
private import semmle.code.java.dataflow.FlowSources
5-
private import semmle.code.java.dataflow.TaintTracking
65
private import semmle.code.java.Maps
76
private import semmle.code.java.JDK
87

8+
private class MapPutOrRemove extends MethodCall {
9+
MapPutOrRemove() {
10+
this.getMethod() instanceof MapMutator and
11+
this.getMethod().getName().matches(["put%", "remove"])
12+
}
13+
}
14+
915
private module ProcessBuilderEnvironmentConfig implements DataFlow::ConfigSig {
1016
predicate isSource(DataFlow::Node source) {
1117
exists(MethodCall mc | mc = source.asExpr() |
1218
mc.getMethod().hasQualifiedName("java.lang", "ProcessBuilder", "environment")
1319
)
1420
}
1521

16-
predicate isSink(DataFlow::Node sink) { sink.asExpr() = any(MapMutation mm).getQualifier() }
22+
predicate isSink(DataFlow::Node sink) { sink.asExpr() = any(MapPutOrRemove mm).getQualifier() }
1723
}
1824

1925
private module ProcessBuilderEnvironmentFlow = DataFlow::Global<ProcessBuilderEnvironmentConfig>;
2026

27+
/**
28+
* A node that acts as a sanitizer in configurations related to environment variable injection.
29+
*/
30+
abstract class ExecTaintedEnvironmentSanitizer extends DataFlow::Node { }
31+
2132
/**
2233
* A taint-tracking configuration that tracks flow from unvalidated data to an environment variable for a subprocess.
2334
*/
2435
module ExecTaintedEnvironmentConfig implements DataFlow::ConfigSig {
2536
predicate isSource(DataFlow::Node source) { source instanceof ThreatModelFlowSource }
2637

38+
predicate isBarrier(DataFlow::Node barrier) { barrier instanceof ExecTaintedEnvironmentSanitizer }
39+
2740
predicate isSink(DataFlow::Node sink) {
2841
sinkNode(sink, "environment-injection")
2942
or
3043
// sink is a key or value added to a `ProcessBuilder::environment` map.
31-
exists(MapMutation mm | mm.getAnArgument() = sink.asExpr() |
44+
exists(MapPutOrRemove mm | mm.getAnArgument() = sink.asExpr() |
3245
ProcessBuilderEnvironmentFlow::flowToExpr(mm.getQualifier())
3346
)
3447
}

0 commit comments

Comments
 (0)