From 8ed3f3c86520a37e0b392d1202f0cfb797b73ff5 Mon Sep 17 00:00:00 2001
From: Ed Minnix <egregius313@github.com>
Date: Thu, 2 Nov 2023 10:25:47 -0400
Subject: [PATCH] Move to library

---
 .../TaintedEnvironmentVariableQuery.qll       | 36 +++++++++++++++
 .../CWE/CWE-078/ExecTaintedEnvironment.ql     | 45 +++----------------
 2 files changed, 42 insertions(+), 39 deletions(-)
 create mode 100644 java/ql/lib/semmle/code/java/security/TaintedEnvironmentVariableQuery.qll

diff --git a/java/ql/lib/semmle/code/java/security/TaintedEnvironmentVariableQuery.qll b/java/ql/lib/semmle/code/java/security/TaintedEnvironmentVariableQuery.qll
new file mode 100644
index 00000000000..0714bebf5ba
--- /dev/null
+++ b/java/ql/lib/semmle/code/java/security/TaintedEnvironmentVariableQuery.qll
@@ -0,0 +1,36 @@
+/** Modules to reason about the tainting of environment variables */
+
+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 module ProcessBuilderEnvironmentConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source.getType() instanceof TypeProcessBuilder }
+
+  predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
+    exists(MethodCall mc | mc.getQualifier() = node1.asExpr() and mc = node2.asExpr() |
+      mc.getMethod().hasQualifiedName("java.lang", "ProcessBuilder", "environment")
+    )
+  }
+
+  predicate isSink(DataFlow::Node sink) { sink.asExpr() = any(MapPutCall mpc).getQualifier() }
+}
+
+private module ProcessBuilderEnvironmentFlow =
+  TaintTracking::Global<ProcessBuilderEnvironmentConfig>;
+
+module ExecTaintedEnvironmentConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof ThreatModelFlowSource }
+
+  predicate isSink(DataFlow::Node sink) {
+    sinkNode(sink, "environment-injection")
+    or
+    exists(MapPutCall mpc | mpc.getAnArgument() = sink.asExpr() |
+      ProcessBuilderEnvironmentFlow::flow(_, DataFlow::exprNode(mpc.getQualifier()))
+    )
+  }
+}
+
+module ExecTaintedEnvironmentFlow = TaintTracking::Global<ExecTaintedEnvironmentConfig>;
diff --git a/java/ql/src/Security/CWE/CWE-078/ExecTaintedEnvironment.ql b/java/ql/src/Security/CWE/CWE-078/ExecTaintedEnvironment.ql
index d1a1f05daee..96eb013d952 100644
--- a/java/ql/src/Security/CWE/CWE-078/ExecTaintedEnvironment.ql
+++ b/java/ql/src/Security/CWE/CWE-078/ExecTaintedEnvironment.ql
@@ -11,43 +11,10 @@
  */
 
 import java
-import semmle.code.java.dataflow.TaintTracking
-import semmle.code.java.dataflow.DataFlow
-import semmle.code.java.dataflow.FlowSources
-import semmle.code.java.dataflow.ExternalFlow
+import semmle.code.java.security.TaintedEnvironmentVariableQuery
+import ExecTaintedEnvironmentFlow::PathGraph
 
-class ExecMethod extends Method {
-  ExecMethod() {
-    this.hasName("exec") and
-    this.getDeclaringType().hasQualifiedName("java.lang", "Runtime")
-  }
-}
-
-module ProcessBuilderEnvironmentFlow implements DataFlow::ConfigSig {
-  predicate isSource(DataFlow::Node source) {
-    source.getType().(RefType).hasQualifiedName("java.lang", "ProcessBuilder")
-  }
-
-  predicate isSink(DataFlow::Node sink) {
-    exists(MethodAccess ma | ma.getQualifier() = sink.asExpr() |
-      ma.getMethod().hasName("environment")
-    )
-  }
-}
-
-module ExecTaintedEnvironmentConfig implements DataFlow::ConfigSig {
-  predicate isSource(DataFlow::Node source) { source instanceof ThreatModelFlowSource }
-
-  predicate isSink(DataFlow::Node sink) { sinkNode(sink, "environment-injection") }
-}
-
-module ExecTaintedEnvironmentFlow = TaintTracking::Global<ExecTaintedEnvironmentConfig>;
-
-from Flow::PathNode source, Flow::PathNode sink, string label
-where
-  ExecTaintedCommandFlow::flowPath(source.asPathNode1(), sink.asPathNode1()) and label = "argument"
-  or
-  ExecTaintedEnvironmentFlow::flowPath(source.asPathNode2(), sink.asPathNode2()) and
-  label = "environment"
-select sink.getNode(), sink, source, "This command will be execute with a tainted $@.",
-  sink.getNode(), label
+from ExecTaintedEnvironmentFlow::PathNode source, ExecTaintedEnvironmentFlow::PathNode sink
+where ExecTaintedEnvironmentFlow::flowPath(source, sink)
+select sink.getNode(), source, sink, "This command will be execute with a tainted $@.",
+  sink.getNode(), "environment variable"