Merge pull request #14257 from michaelnebel/java/threatmodelsources

Java: Introduce a class of dataflow nodes for the threat modeling.

Author: michaelnebel

java/ql/lib/ext/threatmodels/threat-model-grouping.model.yml

@@ -6,18 +6,17 @@ extensions:
     data:
       # Default threat model
       - ["remote", "default"]
-      - ["uri-path", "default"]
-
-      # Android threat models
-      - ["android-external-storage-dir", "android"]
-      - ["contentprovider", "android"]
 
       # Remote threat models
       - ["request", "remote"]
       - ["response", "remote"]
 
       # Local threat models
       - ["database", "local"]
-      - ["cli", "local"]
+      - ["commandargs", "local"]
       - ["environment", "local"]
       - ["file", "local"]
+
+      # Android threat models
+      - ["android-external-storage-dir", "android"]
+      - ["contentprovider", "android"]

java/ql/lib/semmle/code/java/dataflow/ExternalFlowConfiguration.qll

@@ -26,6 +26,6 @@ private string getChildThreatModel(string group) { threatModelGrouping(result, g
  * Holds if the source model kind `kind` is relevant for generic queries
  * under the current threat model configuration.
  */
-predicate sourceModelKindConfig(string kind) {
+predicate currentThreatModel(string kind) {
   exists(string group | supportedThreatModels(group) and kind = getChildThreatModel*(group))
 }

java/ql/lib/semmle/code/java/dataflow/FlowSources.qll

@@ -29,11 +29,42 @@ import semmle.code.java.frameworks.struts.StrutsActions
 import semmle.code.java.frameworks.Thrift
 import semmle.code.java.frameworks.javaee.jsf.JSFRenderer
 private import semmle.code.java.dataflow.ExternalFlow
+private import semmle.code.java.dataflow.ExternalFlowConfiguration
+
+/**
+ * A data flow source.
+ */
+abstract class SourceNode extends DataFlow::Node {
+  /**
+   * Gets a string that represents the source kind with respect to threat modeling.
+   */
+  abstract string getThreatModel();
+}
+
+/**
+ * A class of data flow sources that respects the
+ * current threat model configuration.
+ */
+class ThreatModelFlowSource extends DataFlow::Node {
+  ThreatModelFlowSource() {
+    // Expansive threat model.
+    currentThreatModel("all") and
+    (this instanceof SourceNode or sourceNode(this, _))
+    or
+    exists(string kind |
+      // Specific threat model.
+      currentThreatModel(kind) and
+      (this.(SourceNode).getThreatModel() = kind or sourceNode(this, kind))
+    )
+  }
+}
 
 /** A data flow source of remote user input. */
-abstract class RemoteFlowSource extends DataFlow::Node {
+abstract class RemoteFlowSource extends SourceNode {
   /** Gets a string that describes the type of this remote flow source. */
   abstract string getSourceType();
+
+  override string getThreatModel() { result = "remote" }
 }
 
 /**
@@ -175,14 +206,47 @@ abstract class UserInput extends DataFlow::Node { }
 private class RemoteUserInput extends UserInput instanceof RemoteFlowSource { }
 
 /** A node with input that may be controlled by a local user. */
-abstract class LocalUserInput extends UserInput { }
+abstract class LocalUserInput extends UserInput, SourceNode {
+  override string getThreatModel() { result = "local" }
+}
 
 /**
+ * DEPRECATED: Use the threat models feature.
+ * That is, use `ThreatModelFlowSource` as the class of nodes for sources
+ * and set up the threat model configuration to filter source nodes.
+ * Alternatively, use `getThreatModel` to filter nodes to create the
+ * class of nodes you need.
+ *
  * A node with input from the local environment, such as files, standard in,
  * environment variables, and main method parameters.
  */
-class EnvInput extends LocalUserInput {
+deprecated class EnvInput extends DataFlow::Node {
   EnvInput() {
+    this instanceof EnvironmentInput or
+    this instanceof CliInput or
+    this instanceof FileInput
+  }
+}
+
+/**
+ * A node with input from the local environment, such as
+ * environment variables.
+ */
+private class EnvironmentInput extends LocalUserInput {
+  EnvironmentInput() {
+    // Results from various specific methods.
+    this.asExpr().(MethodAccess).getMethod() instanceof EnvReadMethod
+  }
+
+  override string getThreatModel() { result = "environment" }
+}
+
+/**
+ * A node with input from the command line, such as standard in
+ * and main method parameters.
+ */
+private class CliInput extends LocalUserInput {
+  CliInput() {
     // Parameters to a main method.
     exists(MainMethod main | this.asParameter() = main.getParameter(0))
     or
@@ -191,23 +255,46 @@ class EnvInput extends LocalUserInput {
       f.getAnAnnotation().getType().getQualifiedName() = "org.kohsuke.args4j.Argument"
     )
     or
-    // Results from various specific methods.
-    this.asExpr().(MethodAccess).getMethod() instanceof EnvReadMethod
-    or
     // Access to `System.in`.
     exists(Field f | this.asExpr() = f.getAnAccess() | f instanceof SystemIn)
-    or
+  }
+
+  override string getThreatModel() { result = "commandargs" }
+}
+
+/**
+ * A node with input from the local environment, such as files.
+ */
+private class FileInput extends LocalUserInput {
+  FileInput() {
     // Access to files.
     this.asExpr()
         .(ConstructorCall)
         .getConstructedType()
         .hasQualifiedName("java.io", "FileInputStream")
   }
+
+  override string getThreatModel() { result = "file" }
 }
 
-/** A node with input from a database. */
-class DatabaseInput extends LocalUserInput {
-  DatabaseInput() { this.asExpr().(MethodAccess).getMethod() instanceof ResultSetGetStringMethod }
+/**
+ * DEPRECATED: Use the threat models feature.
+ * That is, use `ThreatModelFlowSource` as the class of nodes for sources
+ * and set up the threat model configuration to filter source nodes.
+ * Alternatively, use `getThreatModel` to filter nodes to create the
+ * class of nodes you need.
+ *
+ * A node with input from a database.
+ */
+deprecated class DatabaseInput = DbInput;
+
+/**
+ * A node with input from a database.
+ */
+private class DbInput extends LocalUserInput {
+  DbInput() { this.asExpr().(MethodAccess).getMethod() instanceof ResultSetGetStringMethod }
+
+  override string getThreatModel() { result = "database" }
 }
 
 /** A method that reads from the environment, such as `System.getProperty` or `System.getenv`. */

java/ql/lib/semmle/code/java/frameworks/hudson/Hudson.qll

@@ -24,6 +24,8 @@ private class FilePathRead extends LocalUserInput {
             "readToString"
           ])
   }
+
+  override string getThreatModel() { result = "file" }
 }
 
 private class HudsonUtilXssSanitizer extends XssSanitizer {

java/ql/test/library-tests/dataflow/threat-models/Empty.java

@@ -0,0 +1,72 @@
+import java.sql.*;
+import java.net.*;
+import java.util.logging.*;
+import java.nio.charset.StandardCharsets;
+import testlib.TestSources;
+
+class Test {
+  private TestSources sources = new TestSources();
+
+  private String byteToString(byte[] data) {
+    return new String(data, StandardCharsets.UTF_8);
+  }
+
+  public void M1(Statement handle) throws Exception {
+    // Only a source if "remote" is a selected threat model.
+    // This is included in the "default" threat model.
+    Socket sock = new Socket("localhost", 1234);
+    byte[] data = new byte[1024];
+    sock.getInputStream().read(data);
+
+    // Logging sink
+    Logger.getLogger("foo").severe(byteToString(data));
+
+    // SQL sink
+    handle.executeUpdate("INSERT INTO foo VALUES ('" + byteToString(data) + "')");
+  }
+
+  public void M2(Statement handle) throws Exception {
+    // Only a source if "database" is a selected threat model.
+    String result = sources.executeQuery("SELECT * FROM foo");
+
+    // SQL sink
+    handle.executeUpdate("INSERT INTO foo VALUES ('" + result + "')");
+
+    // Logging sink
+    Logger.getLogger("foo").severe(result);
+  }
+
+  public void M3(Statement handle) throws Exception {
+    // Only a source if "environment" is a selected threat model.
+    String result = sources.readEnv("MY_ENV_VAR");
+
+    // SQL sink
+    handle.executeUpdate("INSERT INTO foo VALUES ('" + result + "')");
+
+    // Logging sink
+    Logger.getLogger("foo").severe(result);
+  }
+
+  public void M4(Statement handle) throws Exception {
+    // Only a source if "custom" is a selected threat model.
+    String result = sources.getCustom("custom");
+
+    // SQL sink
+    handle.executeUpdate("INSERT INTO foo VALUES ('" + result + "')");
+
+    // Logging sink
+    Logger.getLogger("foo").severe(result);
+  }
+
+  public void M5(Statement handle) throws Exception {
+    // Only a source if "commandargs" is a selected threat model.
+    byte[] data = new byte[1024];
+    System.in.read(data);
+
+    // SQL sink
+    handle.executeUpdate("INSERT INTO foo VALUES ('" + byteToString(data) + "')");
+
+    // Logging sink
+    Logger.getLogger("foo").severe(byteToString(data));
+  }
+}

java/ql/test/library-tests/dataflow/threat-models/Test.java

@@ -0,0 +1,13 @@
+private import java
+private import semmle.code.java.dataflow.DataFlow
+private import semmle.code.java.dataflow.ExternalFlow
+private import semmle.code.java.dataflow.FlowSources
+private import semmle.code.java.dataflow.TaintTracking
+
+private module ThreatModelConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof ThreatModelFlowSource }
+
+  predicate isSink(DataFlow::Node sink) { sinkNode(sink, _) }
+}
+
+module ThreatModel = TaintTracking::Global<ThreatModelConfig>;

java/ql/test/library-tests/dataflow/threat-models/Test.qll

@@ -0,0 +1,9 @@
+package testlib;
+
+public class TestSources {
+    public String executeQuery(String query) { return null; }
+
+    public String readEnv(String env) { return null; }
+
+    public String getCustom(String s) { return null;}
+}

java/ql/test/library-tests/dataflow/threat-models/TestSources.java

@@ -0,0 +1,28 @@
+edges
+| Test.java:10:31:10:41 | data : byte[] | Test.java:11:23:11:26 | data : byte[] |
+| Test.java:11:23:11:26 | data : byte[] | Test.java:11:12:11:51 | new String(...) : String |
+| Test.java:19:5:19:25 | getInputStream(...) : InputStream | Test.java:19:32:19:35 | data [post update] : byte[] |
+| Test.java:19:32:19:35 | data [post update] : byte[] | Test.java:22:49:22:52 | data : byte[] |
+| Test.java:19:32:19:35 | data [post update] : byte[] | Test.java:25:69:25:72 | data : byte[] |
+| Test.java:22:49:22:52 | data : byte[] | Test.java:10:31:10:41 | data : byte[] |
+| Test.java:22:49:22:52 | data : byte[] | Test.java:22:36:22:53 | byteToString(...) |
+| Test.java:25:56:25:73 | byteToString(...) : String | Test.java:25:26:25:80 | ... + ... |
+| Test.java:25:69:25:72 | data : byte[] | Test.java:10:31:10:41 | data : byte[] |
+| Test.java:25:69:25:72 | data : byte[] | Test.java:25:56:25:73 | byteToString(...) : String |
+nodes
+| Test.java:10:31:10:41 | data : byte[] | semmle.label | data : byte[] |
+| Test.java:11:12:11:51 | new String(...) : String | semmle.label | new String(...) : String |
+| Test.java:11:23:11:26 | data : byte[] | semmle.label | data : byte[] |
+| Test.java:19:5:19:25 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
+| Test.java:19:32:19:35 | data [post update] : byte[] | semmle.label | data [post update] : byte[] |
+| Test.java:22:36:22:53 | byteToString(...) | semmle.label | byteToString(...) |
+| Test.java:22:49:22:52 | data : byte[] | semmle.label | data : byte[] |
+| Test.java:25:26:25:80 | ... + ... | semmle.label | ... + ... |
+| Test.java:25:56:25:73 | byteToString(...) : String | semmle.label | byteToString(...) : String |
+| Test.java:25:69:25:72 | data : byte[] | semmle.label | data : byte[] |
+subpaths
+| Test.java:22:49:22:52 | data : byte[] | Test.java:10:31:10:41 | data : byte[] | Test.java:11:12:11:51 | new String(...) : String | Test.java:22:36:22:53 | byteToString(...) |
+| Test.java:25:69:25:72 | data : byte[] | Test.java:10:31:10:41 | data : byte[] | Test.java:11:12:11:51 | new String(...) : String | Test.java:25:56:25:73 | byteToString(...) : String |
+#select
+| Test.java:19:5:19:25 | getInputStream(...) : InputStream | Test.java:22:36:22:53 | byteToString(...) |
+| Test.java:19:5:19:25 | getInputStream(...) : InputStream | Test.java:25:26:25:80 | ... + ... |

java/ql/test/library-tests/dataflow/threat-models/threat-models-flowtest1.expected

@@ -0,0 +1,14 @@
+extensions:
+
+  - addsTo:
+      pack: codeql/java-all
+      extensible: supportedThreatModels
+    data: []
+
+  - addsTo:
+      pack: codeql/java-all
+      extensible: sourceModel
+    data:
+      - ["testlib", "TestSources", False, "executeQuery", "(String)", "", "ReturnValue", "database", "manual"]
+      - ["testlib", "TestSources", False, "readEnv", "(String)", "", "ReturnValue", "environment", "manual"]
+      - ["testlib", "TestSources", False, "getCustom", "(String)", "", "ReturnValue", "custom", "manual"]