Merge pull request github#14582 from github/dbartol/threat-models-2

Dave Bartolomeo · web-flow · commit b18a6d5e0bf0 · 2023-10-27T09:33:53.000-04:00
Java: Threat model implementation with priorities.
diff --git a/codeql-workspace.yml b/codeql-workspace.yml
@@ -6,7 +6,7 @@ provide:
   - "*/ql/consistency-queries/qlpack.yml"
   - "*/ql/automodel/src/qlpack.yml"
   - "*/ql/automodel/test/qlpack.yml"
-  - "shared/*/qlpack.yml"
+  - "shared/**/qlpack.yml"
   - "cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/tainted/qlpack.yml"
   - "go/ql/config/legacy-support/qlpack.yml"
   - "go/build/codeql-extractor-go/codeql-extractor.yml"
diff --git a/java/ql/lib/ext/threatmodels/supported-threat-models.model.yml b/java/ql/lib/ext/threatmodels/supported-threat-models.model.yml
diff --git a/java/ql/lib/qlpack.yml b/java/ql/lib/qlpack.yml
@@ -10,12 +10,12 @@ dependencies:
   codeql/mad: ${workspace}
   codeql/rangeanalysis: ${workspace}
   codeql/regex: ${workspace}
+  codeql/threat-models: ${workspace}
   codeql/tutorial: ${workspace}
   codeql/typetracking: ${workspace}
   codeql/util: ${workspace}
 dataExtensions:
   - ext/*.model.yml
   - ext/generated/*.model.yml
   - ext/experimental/*.model.yml
-  - ext/threatmodels/*.model.yml
 warnOnImplicitThis: true
diff --git a/java/ql/lib/semmle/code/java/dataflow/ExternalFlowConfiguration.qll b/java/ql/lib/semmle/code/java/dataflow/ExternalFlowConfiguration.qll
diff --git a/java/ql/lib/semmle/code/java/dataflow/FlowSources.qll b/java/ql/lib/semmle/code/java/dataflow/FlowSources.qll
@@ -29,7 +29,7 @@ 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
+private import codeql.threatmodels.ThreatModels
 
 /**
  * A data flow source.
@@ -47,10 +47,6 @@ abstract class SourceNode extends DataFlow::Node {
  */
 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
diff --git a/java/ql/test/library-tests/dataflow/threat-models/threat-models-flowtest2.ext.yml b/java/ql/test/library-tests/dataflow/threat-models/threat-models-flowtest2.ext.yml
@@ -1,10 +1,10 @@
 extensions:
 
   - addsTo:
-      pack: codeql/java-all
-      extensible: supportedThreatModels
+      pack: codeql/threat-models
+      extensible: threatModelConfiguration
     data:
-      - ["database"]
+      - ["database", true, 0]
 
   - addsTo:
       pack: codeql/java-all
diff --git a/java/ql/test/library-tests/dataflow/threat-models/threat-models-flowtest3.ext.yml b/java/ql/test/library-tests/dataflow/threat-models/threat-models-flowtest3.ext.yml
@@ -1,10 +1,10 @@
 extensions:
 
   - addsTo:
-      pack: codeql/java-all
-      extensible: supportedThreatModels
+      pack: codeql/threat-models
+      extensible: threatModelConfiguration
     data:
-      - ["local"]
+      - ["local", true, 0]
 
   - addsTo:
       pack: codeql/java-all
diff --git a/java/ql/test/library-tests/dataflow/threat-models/threat-models-flowtest4.ext.yml b/java/ql/test/library-tests/dataflow/threat-models/threat-models-flowtest4.ext.yml
@@ -1,10 +1,10 @@
 extensions:
 
   - addsTo:
-      pack: codeql/java-all
-      extensible: supportedThreatModels
+      pack: codeql/threat-models
+      extensible: threatModelConfiguration
     data:
-      - ["all"]
+      - ["all", true, 0]
 
   - addsTo:
       pack: codeql/java-all
diff --git a/java/ql/test/library-tests/dataflow/threat-models/threat-models-flowtest5.ext.yml b/java/ql/test/library-tests/dataflow/threat-models/threat-models-flowtest5.ext.yml
@@ -1,11 +1,11 @@
 extensions:
 
   - addsTo:
-      pack: codeql/java-all
-      extensible: supportedThreatModels
+      pack: codeql/threat-models
+      extensible: threatModelConfiguration
     data:
-      - ["environment"]
-      - ["commandargs"]
+      - ["environment", true, 0]
+      - ["commandargs", true, 0]
 
   - addsTo:
       pack: codeql/java-all
diff --git a/java/ql/test/library-tests/dataflow/threat-models/threat-models-flowtest6.expected b/java/ql/test/library-tests/dataflow/threat-models/threat-models-flowtest6.expected
@@ -0,0 +1,54 @@
+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 |
+| Test.java:30:21:30:61 | executeQuery(...) : String | Test.java:33:26:33:68 | ... + ... |
+| Test.java:30:21:30:61 | executeQuery(...) : String | Test.java:36:36:36:41 | result |
+| Test.java:64:5:64:13 | System.in : InputStream | Test.java:64:20:64:23 | data [post update] : byte[] |
+| Test.java:64:20:64:23 | data [post update] : byte[] | Test.java:67:69:67:72 | data : byte[] |
+| Test.java:64:20:64:23 | data [post update] : byte[] | Test.java:70:49:70:52 | data : byte[] |
+| Test.java:67:56:67:73 | byteToString(...) : String | Test.java:67:26:67:80 | ... + ... |
+| Test.java:67:69:67:72 | data : byte[] | Test.java:10:31:10:41 | data : byte[] |
+| Test.java:67:69:67:72 | data : byte[] | Test.java:67:56:67:73 | byteToString(...) : String |
+| Test.java:70:49:70:52 | data : byte[] | Test.java:10:31:10:41 | data : byte[] |
+| Test.java:70:49:70:52 | data : byte[] | Test.java:70:36:70:53 | byteToString(...) |
+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[] |
+| Test.java:30:21:30:61 | executeQuery(...) : String | semmle.label | executeQuery(...) : String |
+| Test.java:33:26:33:68 | ... + ... | semmle.label | ... + ... |
+| Test.java:36:36:36:41 | result | semmle.label | result |
+| Test.java:64:5:64:13 | System.in : InputStream | semmle.label | System.in : InputStream |
+| Test.java:64:20:64:23 | data [post update] : byte[] | semmle.label | data [post update] : byte[] |
+| Test.java:67:26:67:80 | ... + ... | semmle.label | ... + ... |
+| Test.java:67:56:67:73 | byteToString(...) : String | semmle.label | byteToString(...) : String |
+| Test.java:67:69:67:72 | data : byte[] | semmle.label | data : byte[] |
+| Test.java:70:36:70:53 | byteToString(...) | semmle.label | byteToString(...) |
+| Test.java:70:49:70:52 | 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 |
+| Test.java:67:69:67:72 | data : byte[] | Test.java:10:31:10:41 | data : byte[] | Test.java:11:12:11:51 | new String(...) : String | Test.java:67:56:67:73 | byteToString(...) : String |
+| Test.java:70:49:70:52 | data : byte[] | Test.java:10:31:10:41 | data : byte[] | Test.java:11:12:11:51 | new String(...) : String | Test.java:70:36:70:53 | byteToString(...) |
+#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 | ... + ... |
+| Test.java:30:21:30:61 | executeQuery(...) : String | Test.java:33:26:33:68 | ... + ... |
+| Test.java:30:21:30:61 | executeQuery(...) : String | Test.java:36:36:36:41 | result |
+| Test.java:64:5:64:13 | System.in : InputStream | Test.java:67:26:67:80 | ... + ... |
+| Test.java:64:5:64:13 | System.in : InputStream | Test.java:70:36:70:53 | byteToString(...) |
diff --git a/java/ql/test/library-tests/dataflow/threat-models/threat-models-flowtest6.ext.yml b/java/ql/test/library-tests/dataflow/threat-models/threat-models-flowtest6.ext.yml
@@ -0,0 +1,16 @@
+extensions:
+
+  - addsTo:
+      pack: codeql/threat-models
+      extensible: threatModelConfiguration
+    data:
+      - ["local", true, 0]
+      - ["environment", false, 1]
+
+  - 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"]
diff --git a/java/ql/test/library-tests/dataflow/threat-models/threat-models-flowtest6.ql b/java/ql/test/library-tests/dataflow/threat-models/threat-models-flowtest6.ql
@@ -0,0 +1,12 @@
+/**
+ * This is a dataflow test using the "default" threat model with the
+ * addition of the threat model group "local", but without the
+ * "environment" threat model.
+ */
+
+import Test
+import ThreatModel::PathGraph
+
+from ThreatModel::PathNode source, ThreatModel::PathNode sink
+where ThreatModel::flowPath(source, sink)
+select source, sink
diff --git a/java/ql/test/library-tests/dataflow/threat-models/threat-models1.expected b/java/ql/test/library-tests/dataflow/threat-models/threat-models1.expected
diff --git a/java/ql/test/library-tests/dataflow/threat-models/threat-models1.ql b/java/ql/test/library-tests/dataflow/threat-models/threat-models1.ql
diff --git a/java/ql/test/library-tests/dataflow/threat-models/threat-models2.expected b/java/ql/test/library-tests/dataflow/threat-models/threat-models2.expected
diff --git a/java/ql/test/library-tests/dataflow/threat-models/threat-models2.ext.yml b/java/ql/test/library-tests/dataflow/threat-models/threat-models2.ext.yml
diff --git a/java/ql/test/library-tests/dataflow/threat-models/threat-models2.ql b/java/ql/test/library-tests/dataflow/threat-models/threat-models2.ql
diff --git a/shared/threat-models/codeql/threatmodels/ThreatModels.qll b/shared/threat-models/codeql/threatmodels/ThreatModels.qll
@@ -0,0 +1,77 @@
+/**
+ * INTERNAL use only. This is an experimental API subject to change without notice.
+ *
+ * This module provides extensible predicates for configuring which kinds of MaD models
+ * are applicable to generic queries.
+ */
+
+/**
+ * Holds configuration entries to specify which threat models are enabled.
+ *
+ * - `kind` - Specifies the threat model to configure. This can be the name of a specific threat
+ *   model (for example, `environment`), a group (`local`), or `all`.
+ * - `enable` - `true` to enable the specified threat model (and its children), or `false` to disable it.
+ * - `priority` - The order in which the configuration should be applied. Lower values are applied first.
+ *
+ * The final configuration is the result of processing each row in ascending order of its `priority` column.
+ * For example:
+ * - `{ kind: "all", enable: true, priority: 0 }`
+ * - `{ kind: "remote", enable: false, priority: 1 }`
+ * - `{ kind: "environment", enable: true, priority: 2 }`
+ * This configuration first enables all threat models, then disables the `remote` group, and finally re-enables
+ * the `environment` threat model.
+ */
+extensible predicate threatModelConfiguration(string kind, boolean enable, int priority);
+
+/**
+ * Holds if the specified kind of source model is containted within the specified group.
+ */
+extensible private predicate threatModelGrouping(string kind, string group);
+
+/** Holds if the specified threat model kind is mentioned in either the configuration or grouping table. */
+private predicate knownThreatModel(string kind) {
+  threatModelConfiguration(kind, _, _) or
+  threatModelGrouping(kind, _) or
+  threatModelGrouping(_, kind) or
+  kind = "all"
+}
+
+/**
+ * Gets the threat model group that directly contains the specified threat model.
+ */
+private string getParentThreatModel(string child) {
+  threatModelGrouping(child, result)
+  or
+  knownThreatModel(child) and child != "all" and result = "all"
+}
+
+/**
+ * Holds if the `enabled` column is set to `true` of the highest-priority configuration row
+ * whose `kind` column includes the specified threat model kind.
+ */
+private predicate threatModelEnabled(string kind) {
+  // Find the highest-priority configuration row whose `kind` column includes the specified threat
+  // model kind. If such a row exists and its `enabled` column is `true`, then the threat model is
+  // enabled.
+  knownThreatModel(kind) and
+  max(boolean enabled, int priority |
+    exists(string configuredKind | configuredKind = getParentThreatModel*(kind) |
+      threatModelConfiguration(configuredKind, enabled, priority)
+    )
+  |
+    enabled order by priority
+  ) = true
+}
+
+/**
+ * Holds if the source model kind `kind` is relevant for generic queries
+ * under the current threat model configuration.
+ */
+bindingset[kind]
+predicate currentThreatModel(string kind) {
+  threatModelEnabled(kind)
+  or
+  // For any threat model kind not mentioned in the configuration or grouping tables, its state of
+  // enablement is controlled only by the entries that specifiy the "all" kind.
+  not knownThreatModel(kind) and threatModelEnabled("all")
+}
diff --git a/shared/threat-models/ext/supported-threat-models.model.yml b/shared/threat-models/ext/supported-threat-models.model.yml
@@ -0,0 +1,6 @@
+extensions:
+  - addsTo:
+      pack: codeql/threat-models
+      extensible: threatModelConfiguration
+    data:
+      - ["default", true, -2147483648] # The "default" threat model is included by default
diff --git a/shared/threat-models/ext/threat-model-grouping.model.yml b/shared/threat-models/ext/threat-model-grouping.model.yml
@@ -1,7 +1,7 @@
 extensions:
 
   - addsTo:
-      pack: codeql/java-all
+      pack: codeql/threat-models
       extensible: threatModelGrouping
     data:
       # Default threat model
diff --git a/shared/threat-models/qlpack.yml b/shared/threat-models/qlpack.yml
@@ -0,0 +1,7 @@
+name: codeql/threat-models
+version: 0.0.0-dev
+library: true
+groups: shared
+dataExtensions:
+  - ext/*.model.yml
+warnOnImplicitThis: true
