diff --git a/java/ql/lib/semmle/code/java/security/TaintedEnvironmentVariableQuery.qll b/java/ql/lib/semmle/code/java/security/TaintedEnvironmentVariableQuery.qll index ca55f52f0a3..f76e92a49bd 100644 --- a/java/ql/lib/semmle/code/java/security/TaintedEnvironmentVariableQuery.qll +++ b/java/ql/lib/semmle/code/java/security/TaintedEnvironmentVariableQuery.qll @@ -2,10 +2,16 @@ private import semmle.code.java.dataflow.ExternalFlow private import semmle.code.java.dataflow.FlowSources -private import semmle.code.java.dataflow.TaintTracking private import semmle.code.java.Maps private import semmle.code.java.JDK +private class MapPutOrRemove extends MethodCall { + MapPutOrRemove() { + this.getMethod() instanceof MapMutator and + this.getMethod().getName().matches(["put%", "remove"]) + } +} + private module ProcessBuilderEnvironmentConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { exists(MethodCall mc | mc = source.asExpr() | @@ -13,22 +19,29 @@ private module ProcessBuilderEnvironmentConfig implements DataFlow::ConfigSig { ) } - predicate isSink(DataFlow::Node sink) { sink.asExpr() = any(MapMutation mm).getQualifier() } + predicate isSink(DataFlow::Node sink) { sink.asExpr() = any(MapPutOrRemove mm).getQualifier() } } private module ProcessBuilderEnvironmentFlow = DataFlow::Global; +/** + * A node that acts as a sanitizer in configurations related to environment variable injection. + */ +abstract class ExecTaintedEnvironmentSanitizer extends DataFlow::Node { } + /** * A taint-tracking configuration that tracks flow from unvalidated data to an environment variable for a subprocess. */ module ExecTaintedEnvironmentConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { source instanceof ThreatModelFlowSource } + predicate isBarrier(DataFlow::Node barrier) { barrier instanceof ExecTaintedEnvironmentSanitizer } + predicate isSink(DataFlow::Node sink) { sinkNode(sink, "environment-injection") or // sink is a key or value added to a `ProcessBuilder::environment` map. - exists(MapMutation mm | mm.getAnArgument() = sink.asExpr() | + exists(MapPutOrRemove mm | mm.getAnArgument() = sink.asExpr() | ProcessBuilderEnvironmentFlow::flowToExpr(mm.getQualifier()) ) }