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

Java: Threat model implementation with priorities.
2026-04-25 08:45:14 +02:00 · 2023-10-27 09:33:53 -04:00
parent 4aed638066 e4276f7adb
commit b18a6d5e0b
21 changed files with 189 additions and 89 deletions
--- 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"
--- a/java/ql/lib/ext/threatmodels/supported-threat-models.model.yml
+++ b/java/ql/lib/ext/threatmodels/supported-threat-models.model.yml
@@ -1,7 +0,0 @@
-extensions:
-
-  - addsTo:
-      pack: codeql/java-all
-      extensible: supportedThreatModels
-    data:
-      - ["default"] # The "default" threat model is always included.
--- a/java/ql/lib/qlpack.yml
+++ b/java/ql/lib/qlpack.yml
@@ -10,6 +10,7 @@ dependencies:
  codeql/mad: ${workspace}
  codeql/rangeanalysis: ${workspace}
  codeql/regex: ${workspace}
+  codeql/threat-models: ${workspace}
  codeql/tutorial: ${workspace}
  codeql/typetracking: ${workspace}
  codeql/util: ${workspace}
@@ -17,5 +18,4 @@ dataExtensions:
  - ext/*.model.yml
  - ext/generated/*.model.yml
  - ext/experimental/*.model.yml
-  - ext/threatmodels/*.model.yml
 warnOnImplicitThis: true
--- a/java/ql/lib/semmle/code/java/dataflow/ExternalFlowConfiguration.qll
+++ b/java/ql/lib/semmle/code/java/dataflow/ExternalFlowConfiguration.qll
@@ -1,31 +0,0 @@
-/**
- * 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.
- */
-
-private import ExternalFlowExtensions
-
-/**
- * Holds if the specified kind of source model is supported for the current query.
- */
-extensible private predicate supportedThreatModels(string kind);
-
-/**
- * Holds if the specified kind of source model is containted within the specified group.
- */
-extensible private predicate threatModelGrouping(string kind, string group);
-
-/**
- * Gets the threat models that are direct descendants of the specified kind/group.
- */
-private string getChildThreatModel(string group) { threatModelGrouping(result, group) }
-
-/**
- * Holds if the source model kind `kind` is relevant for generic queries
- * under the current threat model configuration.
- */
-predicate currentThreatModel(string kind) {
-  exists(string group | supportedThreatModels(group) and kind = getChildThreatModel*(group))
-}
--- 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
--- 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
--- 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
--- 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
--- 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
--- 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(...) |
--- 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"]
--- 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
--- a/java/ql/test/library-tests/dataflow/threat-models/threat-models1.expected
+++ b/java/ql/test/library-tests/dataflow/threat-models/threat-models1.expected
@@ -1,4 +0,0 @@
-| default |
-| remote |
-| request |
-| response |
--- a/java/ql/test/library-tests/dataflow/threat-models/threat-models1.ql
+++ b/java/ql/test/library-tests/dataflow/threat-models/threat-models1.ql
@@ -1,5 +0,0 @@
-import semmle.code.java.dataflow.ExternalFlowConfiguration as ExternalFlowConfiguration
-
-query predicate supportedThreatModels(string kind) {
-  ExternalFlowConfiguration::currentThreatModel(kind)
-}
--- a/java/ql/test/library-tests/dataflow/threat-models/threat-models2.expected
+++ b/java/ql/test/library-tests/dataflow/threat-models/threat-models2.expected
@@ -1,9 +0,0 @@
-| commandargs |
-| database |
-| default |
-| environment |
-| file |
-| local |
-| remote |
-| request |
-| response |
--- 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
@@ -1,7 +0,0 @@
-extensions:
-
-  - addsTo:
-      pack: codeql/java-all
-      extensible: supportedThreatModels
-    data:
-      - ["local"] # Add the "local" group threat model.
--- a/java/ql/test/library-tests/dataflow/threat-models/threat-models2.ql
+++ b/java/ql/test/library-tests/dataflow/threat-models/threat-models2.ql
@@ -1,5 +0,0 @@
-import semmle.code.java.dataflow.ExternalFlowConfiguration as ExternalFlowConfiguration
-
-query predicate supportedThreatModels(string kind) {
-  ExternalFlowConfiguration::currentThreatModel(kind)
-}
--- 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")
+}
--- 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
--- a/java/ql/lib/ext/threatmodels/threat-model-grouping.model.yml
+++ b/java/ql/lib/ext/threatmodels/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
--- 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