Merge branch 'main' into fix/update-gson-model

java/ql/lib/ext/hudson.model.yml

@@ -32,8 +32,11 @@ extensions:
       - ["hudson", "FilePath", True, "write", "(String,String)", "", "Argument[0]", "file-content-store", "manual"]
       - ["hudson", "Launcher$ProcStarter", False, "cmds", "", "", "Argument[0]", "command-injection", "manual"]
       - ["hudson", "Launcher$ProcStarter", False, "cmdAsSingleString", "", "", "Argument[0]", "command-injection", "manual"]
+      - ["hudson", "Launcher$ProcStarter", False, "envs", "(String[])", "", "Argument[0]", "environment-injection", "manual"]
       - ["hudson", "Launcher", True, "launch", "", "", "Argument[0]", "command-injection", "manual"]
+      - ["hudson", "Launcher", True, "decorateByEnv", "(String[])", "", "Argument[0]", "environment-injection", "manual"]
       - ["hudson", "Launcher", True, "launchChannel", "", "", "Argument[0]", "command-injection", "manual"]
+      - ["hudson", "Launcher", True, "launchChannel", "", "", "Argument[3]", "environment-injection", "manual"]
       - ["hudson", "XmlFile", False, "XmlFile", "(XStream,File)", "", "Argument[1]", "path-injection", "ai-manual"]
   - addsTo:
       pack: codeql/java-all

java/ql/lib/ext/java.lang.model.yml

@@ -22,6 +22,8 @@ extensions:
       - ["java.lang", "Runtime", True, "exec", "(String,String[])", "", "Argument[0]", "command-injection", "ai-manual"]
       - ["java.lang", "Runtime", True, "exec", "(String,String[],File)", "", "Argument[0]", "command-injection", "ai-manual"]
       - ["java.lang", "Runtime", True, "exec", "(String,String[],File)", "", "Argument[2]", "command-injection", "ai-manual"]
+      # All implementations of `java.lang.Runtime::exec` take the environment variables as their second argument.
+      - ["java.lang", "Runtime", True, "exec", "", "", "Argument[1]", "environment-injection", "manual"]
       # These are potential vulnerabilities, but not for command-injection. No query for this kind of vulnerability currently exists.
       # - ["java.lang", "Runtime", False, "load", "(String)", "", "Argument[0]", "command-injection", "ai-manual"]
       # - ["java.lang", "Runtime", False, "loadLibrary", "(String)", "", "Argument[0]", "command-injection", "ai-manual"]

java/ql/lib/ext/org.apache.commons.exec.environment.model.yml

@@ -0,0 +1,6 @@
+extensions:
+  - addsTo:
+      pack: codeql/java-all
+      extensible: sinkModel
+    data:
+      - ["org.apache.commons.exec.environment", "EnvironmentUtils", True, "addVariableToEnvironment", "(Map,String)", "", "Argument[0]", "environment-injection", "manual"]

java/ql/lib/ext/org.apache.commons.exec.launcher.model.yml

@@ -0,0 +1,6 @@
+extensions:
+  - addsTo:
+      pack: codeql/java-all
+      extensible: sinkModel
+    data:
+      - ["org.apache.commons.exec.launcher", "CommandLauncher", True, "exec", "", "", "Argument[1]", "environment-injection", "manual"]

java/ql/lib/ext/org.apache.commons.exec.model.yml

@@ -9,3 +9,5 @@ extensions:
       - ["org.apache.commons.exec", "CommandLine", True, "addArguments", "(String,boolean)", "", "Argument[0]", "command-injection", "manual"]
       - ["org.apache.commons.exec", "CommandLine", True, "addArguments", "(String[])", "", "Argument[0]", "command-injection", "manual"]
       - ["org.apache.commons.exec", "CommandLine", True, "addArguments", "(String[],boolean)", "", "Argument[0]", "command-injection", "manual"]
+      - ["org.apache.commons.exec", "Executor", True, "execute", "(CommandLine,Map)", "", "Argument[1]", "environment-injection", "manual"]
+      - ["org.apache.commons.exec", "Executor", True, "execute", "(CommandLine,Map,ExecuteResultHandler)", "", "Argument[1]", "environment-injection", "manual"]

java/ql/lib/ext/org.pac4j.jwt.config.encryption.model.yml

@@ -0,0 +1,9 @@
+extensions:
+  - addsTo:
+      pack: codeql/java-all
+      extensible: sinkModel
+    data:
+      - ["org.pac4j.jwt.config.encryption", "SecretEncryptionConfiguration", True, "SecretEncryptionConfiguration", "", "", "Argument[0]", "credentials-key", "manual"]
+      - ["org.pac4j.jwt.config.encryption", "SecretEncryptionConfiguration", True, "setSecret", "", "", "Argument[0]", "credentials-key", "manual"]
+      - ["org.pac4j.jwt.config.encryption", "SecretEncryptionConfiguration", True, "setSecretBase64", "", "", "Argument[0]", "credentials-key", "manual"]
+      - ["org.pac4j.jwt.config.encryption", "SecretEncryptionConfiguration", True, "setSecretBytes", "", "", "Argument[0]", "credentials-key", "manual"]

java/ql/lib/ext/org.pac4j.jwt.config.signature.model.yml

@@ -0,0 +1,9 @@
+extensions:
+  - addsTo:
+      pack: codeql/java-all
+      extensible: sinkModel
+    data:
+      - ["org.pac4j.jwt.config.signature", "SecretSignatureConfiguration", True, "SecretEncryptionConfiguration", "", "", "Argument[0]", "credentials-key", "manual"]
+      - ["org.pac4j.jwt.config.signature", "SecretSignatureConfiguration", True, "setSecret", "", "", "Argument[0]", "credentials-key", "manual"]
+      - ["org.pac4j.jwt.config.signature", "SecretSignatureConfiguration", True, "setSecretBase64", "", "", "Argument[0]", "credentials-key", "manual"]
+      - ["org.pac4j.jwt.config.signature", "SecretSignatureConfiguration", True, "setSecretBytes", "", "", "Argument[0]", "credentials-key", "manual"]

java/ql/lib/semmle/code/java/frameworks/Netty.qll

@@ -0,0 +1,23 @@
+/** Provides definitions related to the Netty framework. */
+
+import java
+
+/** The interface `Cookie` in the packages `io.netty.handler.codec.http` and `io.netty.handler.codec.http.cookie`. */
+class NettyCookie extends Interface {
+  NettyCookie() { this.hasQualifiedName("io.netty.handler.codec.http" + [".cookie", ""], "Cookie") }
+}
+
+/** The class `DefaultCookie` in the packages `io.netty.handler.codec.http` and `io.netty.handler.codec.http.cookie`. */
+class NettyDefaultCookie extends Class {
+  NettyDefaultCookie() {
+    this.hasQualifiedName("io.netty.handler.codec.http" + [".cookie", ""], "DefaultCookie")
+  }
+}
+
+/** The method `setValue` of the interface `Cookie` or a class implementing it. */
+class NettySetCookieValueMethod extends Method {
+  NettySetCookieValueMethod() {
+    this.getDeclaringType*() instanceof NettyCookie and
+    this.hasName("setValue")
+  }
+}

java/ql/lib/semmle/code/java/frameworks/OpenSaml.qll

@@ -0,0 +1,29 @@
+/**
+ * Provides classes and predicates for working with the OpenSAML libraries.
+ */
+
+import java
+private import semmle.code.java.security.InsecureRandomnessQuery
+
+/** The interface  `org.opensaml.saml.saml2.core.RequestAbstractType`. */
+class SamlRequestAbstractType extends Interface {
+  SamlRequestAbstractType() {
+    this.hasQualifiedName("org.opensaml.saml.saml2.core", "RequestAbstractType")
+  }
+}
+
+/** The method `setID` of  the interface `RequestAbstractType`. */
+class SamlRequestSetIdMethod extends Method {
+  SamlRequestSetIdMethod() {
+    this.getDeclaringType() instanceof SamlRequestAbstractType and
+    this.hasName("setID")
+  }
+}
+
+private class SamlRequestSetIdSink extends InsecureRandomnessSink {
+  SamlRequestSetIdSink() {
+    exists(MethodCall c | c.getMethod() instanceof SamlRequestSetIdMethod |
+      c.getArgument(0) = this.asExpr()
+    )
+  }
+}

java/ql/lib/semmle/code/java/frameworks/Servlets.qll

@@ -244,7 +244,7 @@ class TypeCookie extends Class {
 }
 
 /**
- * The method `getValue(String)` declared in `javax.servlet.http.Cookie`.
+ * The method `getValue()` declared in `javax.servlet.http.Cookie`.
  */
 class CookieGetValueMethod extends Method {
   CookieGetValueMethod() {
@@ -254,6 +254,16 @@ class CookieGetValueMethod extends Method {
   }
 }
 
+/**
+ * The method `setValue(String)` declared in `javax.servlet.http.Cookie`.
+ */
+class CookieSetValueMethod extends Method {
+  CookieSetValueMethod() {
+    this.getDeclaringType() instanceof TypeCookie and
+    this.hasName("setValue")
+  }
+}
+
 /**
  * The method `getName()` declared in `javax.servlet.http.Cookie`.
  */

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

@@ -0,0 +1,32 @@
+/** Provides definitions to reason about HTTP cookies. */
+
+import java
+private import semmle.code.java.frameworks.Netty
+private import semmle.code.java.frameworks.Servlets
+
+/** An expression setting the value of a cookie. */
+abstract class SetCookieValue extends Expr { }
+
+private class ServletSetCookieValue extends SetCookieValue {
+  ServletSetCookieValue() {
+    exists(Call c |
+      c.(ClassInstanceExpr).getConstructedType() instanceof TypeCookie and
+      this = c.getArgument(1)
+      or
+      c.(MethodCall).getMethod() instanceof CookieSetValueMethod and
+      this = c.getArgument(0)
+    )
+  }
+}
+
+private class NettySetCookieValue extends SetCookieValue {
+  NettySetCookieValue() {
+    exists(Call c |
+      c.(ClassInstanceExpr).getConstructedType() instanceof NettyDefaultCookie and
+      this = c.getArgument(1)
+      or
+      c.(MethodCall).getMethod() instanceof NettySetCookieValueMethod and
+      this = c.getArgument(0)
+    )
+  }
+}

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

@@ -1,12 +1,14 @@
 /** Provides classes and predicates for reasoning about insecure randomness. */
 
 import java
+private import semmle.code.java.frameworks.OpenSaml
 private import semmle.code.java.frameworks.Servlets
+private import semmle.code.java.dataflow.ExternalFlow
+private import semmle.code.java.dataflow.TaintTracking
+private import semmle.code.java.security.Cookies
+private import semmle.code.java.security.RandomQuery
 private import semmle.code.java.security.SensitiveActions
 private import semmle.code.java.security.SensitiveApi
-private import semmle.code.java.dataflow.TaintTracking
-private import semmle.code.java.dataflow.ExternalFlow
-private import semmle.code.java.security.RandomQuery
 
 /**
  * A node representing a source of insecure randomness.
@@ -18,7 +20,7 @@ abstract class InsecureRandomnessSource extends DataFlow::Node { }
 private class RandomMethodSource extends InsecureRandomnessSource {
   RandomMethodSource() {
     exists(RandomDataSource s | this.asExpr() = s.getOutput() |
-      not s.getQualifier().getType() instanceof SafeRandomImplementation
+      not s.getSourceOfRandomness() instanceof SafeRandomImplementation
     )
   }
 }
@@ -40,7 +42,7 @@ private class TypeHadoopOsSecureRandom extends SafeRandomImplementation {
 }
 
 /**
- * A node representing an operation which should not use a Insecurely random value.
+ * A node representing an operation which should not use an insecurely random value.
  */
 abstract class InsecureRandomnessSink extends DataFlow::Node { }
 
@@ -48,16 +50,7 @@ abstract class InsecureRandomnessSink extends DataFlow::Node { }
  * A node which sets the value of a cookie.
  */
 private class CookieSink extends InsecureRandomnessSink {
-  CookieSink() {
-    exists(Call c |
-      c.(ClassInstanceExpr).getConstructedType() instanceof TypeCookie and
-      this.asExpr() = c.getArgument(1)
-      or
-      c.(MethodCall).getMethod().getDeclaringType() instanceof TypeCookie and
-      c.(MethodCall).getMethod().hasName("setValue") and
-      this.asExpr() = c.getArgument(0)
-    )
-  }
+  CookieSink() { this.asExpr() instanceof SetCookieValue }
 }
 
 private class SensitiveActionSink extends InsecureRandomnessSink {
@@ -76,6 +69,8 @@ module InsecureRandomnessConfig implements DataFlow::ConfigSig {
 
   predicate isBarrierIn(DataFlow::Node n) { isSource(n) }
 
+  predicate isBarrierOut(DataFlow::Node n) { isSink(n) }
+
   predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) {
     n1.asExpr() = n2.asExpr().(BinaryExpr).getAnOperand()
     or
@@ -88,6 +83,17 @@ module InsecureRandomnessConfig implements DataFlow::ConfigSig {
       n1.asExpr() = mc.getArgument(0) and
       n2.asExpr() = mc
     )
+    or
+    // TODO: Once we have a default sanitizer for UUIDs, we can convert these to global summaries.
+    exists(Call c |
+      c.(ClassInstanceExpr).getConstructedType().hasQualifiedName("java.util", "UUID") and
+      n1.asExpr() = c.getAnArgument() and
+      n2.asExpr() = c
+      or
+      c.(MethodCall).getMethod().hasQualifiedName("java.util", "UUID", "toString") and
+      n1.asExpr() = c.getQualifier() and
+      n2.asExpr() = c
+    )
   }
 }
 

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

@@ -3,6 +3,7 @@
  */
 
 import java
+private import semmle.code.java.dataflow.TypeFlow
 
 /**
  * A method access that returns random data or writes random data to an argument.
@@ -43,6 +44,9 @@ abstract class RandomDataSource extends MethodCall {
    * in the case where it writes random data to that argument.
    */
   abstract Expr getOutput();
+
+  /** Gets the type of the source of randomness used by this call. */
+  RefType getSourceOfRandomness() { boundOrStaticType(this.getQualifier(), result) }
 }
 
 /**
@@ -167,4 +171,18 @@ class ApacheCommonsRandomStringSource extends RandomDataSource {
   }
 
   override Expr getOutput() { result = this }
+
+  override RefType getSourceOfRandomness() {
+    if
+      this.getMethod().hasStringSignature("random(int, int, int, boolean, boolean, char[], Random)")
+    then boundOrStaticType(this.getArgument(6), result)
+    else result.hasQualifiedName("java.util", "Random")
+  }
+}
+
+/** Holds if `t` is the static type of `e`, or an upper bound of the runtime type of `e`. */
+private predicate boundOrStaticType(Expr e, RefType t) {
+  exprTypeFlow(e, t, false)
+  or
+  t = e.getType()
 }

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

@@ -0,0 +1,46 @@
+/** 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.Maps
+private import semmle.code.java.JDK
+
+private module ProcessBuilderEnvironmentConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) {
+    exists(MethodCall mc | mc = source.asExpr() |
+      mc.getMethod().hasQualifiedName("java.lang", "ProcessBuilder", "environment")
+    )
+  }
+
+  predicate isSink(DataFlow::Node sink) { sink.asExpr() = any(MapMutation mm).getQualifier() }
+}
+
+private module ProcessBuilderEnvironmentFlow = DataFlow::Global<ProcessBuilderEnvironmentConfig>;
+
+/**
+ * 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() |
+      ProcessBuilderEnvironmentFlow::flowToExpr(mm.getQualifier())
+    )
+  }
+}
+
+/**
+ * Taint-tracking flow for unvalidated data to an environment variable for a subprocess.
+ */
+module ExecTaintedEnvironmentFlow = TaintTracking::Global<ExecTaintedEnvironmentConfig>;