python: add machinery for MaD barriers

yoff · yoff · commit 3f2e314065ac · 2025-12-11T11:47:28.000+01:00
and reinstate previously removed barrier
now as a MaD row
diff --git a/python/ql/lib/semmle/python/frameworks/Django.model.yml b/python/ql/lib/semmle/python/frameworks/Django.model.yml
@@ -0,0 +1,6 @@
+extensions:
+  - addsTo:
+      pack: codeql/python-all
+      extensible: barrierGuardModel
+    data:
+      - ['django', 'Member[utils].Member[http].Member[url_has_allowed_host_and_scheme].Argument[0,url:]', "true", 'url-redirection']
diff --git a/python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModels.qll b/python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModels.qll
@@ -344,6 +344,26 @@ private predicate sinkModel(string type, string path, string kind, string model)
   )
 }
 
+/** Holds if a barrier model exists for the given parameters. */
+private predicate barrierModel(string type, string path, string kind, string model) {
+  // No deprecation adapter for barrier models, they were not around back then.
+  exists(QlBuiltins::ExtensionId madId |
+    Extensions::barrierModel(type, path, kind, madId) and
+    model = "MaD:" + madId.toString()
+  )
+}
+
+/** Holds if a barrier guard model exists for the given parameters. */
+private predicate barrierGuardModel(
+  string type, string path, string branch, string kind, string model
+) {
+  // No deprecation adapter for barrier models, they were not around back then.
+  exists(QlBuiltins::ExtensionId madId |
+    Extensions::barrierGuardModel(type, path, branch, kind, madId) and
+    model = "MaD:" + madId.toString()
+  )
+}
+
 /** Holds if a summary model `row` exists for the given parameters. */
 private predicate summaryModel(
   string type, string path, string input, string output, string kind, string model
@@ -400,6 +420,8 @@ predicate isRelevantType(string type) {
   (
     sourceModel(type, _, _, _) or
     sinkModel(type, _, _, _) or
+    barrierModel(type, _, _, _) or
+    barrierGuardModel(type, _, _, _, _) or
     summaryModel(type, _, _, _, _, _) or
     typeModel(_, type, _)
   ) and
@@ -427,6 +449,8 @@ predicate isRelevantFullPath(string type, string path) {
   (
     sourceModel(type, path, _, _) or
     sinkModel(type, path, _, _) or
+    barrierModel(type, path, _, _) or
+    barrierGuardModel(type, path, _, _, _) or
     summaryModel(type, path, _, _, _, _) or
     typeModel(_, type, path)
   )
@@ -745,6 +769,32 @@ module ModelOutput {
       )
     }
 
+    /**
+     * Holds if a barrier model contributed `barrier` with the given `kind`.
+     */
+    cached
+    API::Node getABarrierNode(string kind, string model) {
+      exists(string type, string path |
+        barrierModel(type, path, kind, model) and
+        result = getNodeFromPath(type, path)
+      )
+    }
+
+    /**
+     * Holds if a barrier model contributed `barrier` with the given `kind`.
+     */
+    cached
+    API::Node getABarrierGuardNode(string kind, boolean branch, string model) {
+      exists(string type, string path, string branch_str |
+        branch = true and branch_str = "true"
+        or
+        branch = false and branch_str = "false"
+      |
+        barrierGuardModel(type, path, branch_str, kind, model) and
+        result = getNodeFromPath(type, path)
+      )
+    }
+
     /**
      * Holds if a relevant summary exists for these parameters.
      */
@@ -787,15 +837,27 @@ module ModelOutput {
   private import codeql.mad.ModelValidation as SharedModelVal
 
   /**
-   * Holds if a CSV source model contributed `source` with the given `kind`.
+   * Holds if an external model contributed `source` with the given `kind`.
    */
   API::Node getASourceNode(string kind) { result = getASourceNode(kind, _) }
 
   /**
-   * Holds if a CSV sink model contributed `sink` with the given `kind`.
+   * Holds if an external model contributed `sink` with the given `kind`.
    */
   API::Node getASinkNode(string kind) { result = getASinkNode(kind, _) }
 
+  /**
+   * Holds if an external model contributed `barrier` with the given `kind`.
+   */
+  API::Node getABarrierNode(string kind) { result = getABarrierNode(kind, _) }
+
+  /**
+   * Holds if an external model contributed `barrier-guard` with the given `kind` and `branch`.
+   */
+  API::Node getABarrierGuardNode(string kind, boolean branch) {
+    result = getABarrierGuardNode(kind, branch, _)
+  }
+
   private module KindValConfig implements SharedModelVal::KindValidationConfigSig {
     predicate summaryKind(string kind) { summaryModel(_, _, _, _, kind, _) }
 
diff --git a/python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModelsExtensions.qll b/python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModelsExtensions.qll
@@ -20,6 +20,26 @@ extensible predicate sourceModel(
  */
 extensible predicate sinkModel(string type, string path, string kind, QlBuiltins::ExtensionId madId);
 
+/**
+ * Holds if the value at `(type, path)` should be seen as a barrier
+ * of the given `kind` and `madId` is the data extension row number.
+ */
+extensible predicate barrierModel(
+  string type, string path, string kind, QlBuiltins::ExtensionId madId
+);
+
+/**
+ * Holds if the value at `(type, path)` should be seen as a barrier guard
+ * of the given `kind` and `madId` is the data extension row number.
+ * `path` is assumed to lead to a parameter of a call (possibly `self`), and
+ * the call is guarding the parameter.
+ * `branch` is either `true` or `false`, indicating which branch of the guard
+ * is protecting the parameter.
+ */
+extensible predicate barrierGuardModel(
+  string type, string path, string branch, string kind, QlBuiltins::ExtensionId madId
+);
+
 /**
  * Holds if in calls to `(type, path)`, the value referred to by `input`
  * can flow to the value referred to by `output` and `madId` is the data
diff --git a/python/ql/lib/semmle/python/frameworks/data/internal/empty.model.yml b/python/ql/lib/semmle/python/frameworks/data/internal/empty.model.yml
@@ -11,6 +11,16 @@ extensions:
       extensible: sinkModel
     data: []
 
+  - addsTo:
+      pack: codeql/python-all
+      extensible: barrierModel
+    data: []
+
+  - addsTo:
+      pack: codeql/python-all
+      extensible: barrierGuardModel
+    data: []
+
   - addsTo:
       pack: codeql/python-all
       extensible: summaryModel
diff --git a/python/ql/lib/semmle/python/security/dataflow/UrlRedirectCustomizations.qll b/python/ql/lib/semmle/python/security/dataflow/UrlRedirectCustomizations.qll
@@ -7,6 +7,7 @@
 private import python
 private import semmle.python.dataflow.new.DataFlow
 private import semmle.python.Concepts
+private import semmle.python.ApiGraphs
 private import semmle.python.dataflow.new.RemoteFlowSources
 private import semmle.python.dataflow.new.BarrierGuards
 private import semmle.python.frameworks.data.ModelsAsData
@@ -156,4 +157,23 @@ module UrlRedirect {
 
   /** DEPRECATED: Use ConstCompareAsSanitizerGuard instead. */
   deprecated class StringConstCompareAsSanitizerGuard = ConstCompareAsSanitizerGuard;
+
+  private predicate urlCheck(DataFlow::GuardNode g, ControlFlowNode node, boolean branch) {
+    exists(API::CallNode call, API::Node parameter |
+      parameter = call.getAParameter() and
+      parameter = ModelOutput::getABarrierGuardNode("url-redirection", branch)
+    |
+      g = call.asCfgNode() and
+      node = parameter.asSink().asCfgNode()
+    )
+  }
+
+  class SanitizerFromModel extends Sanitizer {
+    SanitizerFromModel() { this = DataFlow::BarrierGuard<urlCheck/3>::getABarrierNode() }
+
+    override predicate sanitizes(FlowState state) {
+      // sanitize all flow states
+      any()
+    }
+  }
 }
diff --git a/python/ql/test/query-tests/Security/CWE-601-UrlRedirect/UrlRedirect.expected b/python/ql/test/query-tests/Security/CWE-601-UrlRedirect/UrlRedirect.expected
@@ -52,7 +52,6 @@ edges
 | test.py:81:17:81:46 | ControlFlowNode for Attribute() | test.py:81:5:81:13 | ControlFlowNode for untrusted | provenance |  |
 | test.py:82:5:82:10 | ControlFlowNode for unsafe | test.py:83:21:83:26 | ControlFlowNode for unsafe | provenance |  |
 | test.py:90:5:90:13 | ControlFlowNode for untrusted | test.py:93:18:93:26 | ControlFlowNode for untrusted | provenance |  |
-| test.py:90:5:90:13 | ControlFlowNode for untrusted | test.py:95:25:95:33 | ControlFlowNode for untrusted | provenance |  |
 | test.py:90:17:90:23 | ControlFlowNode for request | test.py:90:17:90:28 | ControlFlowNode for Attribute | provenance | AdditionalTaintStep |
 | test.py:90:17:90:28 | ControlFlowNode for Attribute | test.py:90:17:90:46 | ControlFlowNode for Attribute() | provenance | dict.get |
 | test.py:90:17:90:46 | ControlFlowNode for Attribute() | test.py:90:5:90:13 | ControlFlowNode for untrusted | provenance |  |
@@ -123,7 +122,6 @@ nodes
 | test.py:90:17:90:28 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
 | test.py:90:17:90:46 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
 | test.py:93:18:93:26 | ControlFlowNode for untrusted | semmle.label | ControlFlowNode for untrusted |
-| test.py:95:25:95:33 | ControlFlowNode for untrusted | semmle.label | ControlFlowNode for untrusted |
 | test.py:111:5:111:13 | ControlFlowNode for untrusted | semmle.label | ControlFlowNode for untrusted |
 | test.py:111:17:111:23 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
 | test.py:111:17:111:28 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
@@ -150,7 +148,6 @@ subpaths
 | test.py:76:21:76:26 | ControlFlowNode for unsafe | test.py:1:26:1:32 | ControlFlowNode for ImportMember | test.py:76:21:76:26 | ControlFlowNode for unsafe | Untrusted URL redirection depends on a $@. | test.py:1:26:1:32 | ControlFlowNode for ImportMember | user-provided value |
 | test.py:83:21:83:26 | ControlFlowNode for unsafe | test.py:1:26:1:32 | ControlFlowNode for ImportMember | test.py:83:21:83:26 | ControlFlowNode for unsafe | Untrusted URL redirection depends on a $@. | test.py:1:26:1:32 | ControlFlowNode for ImportMember | user-provided value |
 | test.py:93:18:93:26 | ControlFlowNode for untrusted | test.py:1:26:1:32 | ControlFlowNode for ImportMember | test.py:93:18:93:26 | ControlFlowNode for untrusted | Untrusted URL redirection depends on a $@. | test.py:1:26:1:32 | ControlFlowNode for ImportMember | user-provided value |
-| test.py:95:25:95:33 | ControlFlowNode for untrusted | test.py:1:26:1:32 | ControlFlowNode for ImportMember | test.py:95:25:95:33 | ControlFlowNode for untrusted | Untrusted URL redirection depends on a $@. | test.py:1:26:1:32 | ControlFlowNode for ImportMember | user-provided value |
 | test.py:114:25:114:33 | ControlFlowNode for untrusted | test.py:1:26:1:32 | ControlFlowNode for ImportMember | test.py:114:25:114:33 | ControlFlowNode for untrusted | Untrusted URL redirection depends on a $@. | test.py:1:26:1:32 | ControlFlowNode for ImportMember | user-provided value |
 | test.py:140:25:140:33 | ControlFlowNode for untrusted | test.py:1:26:1:32 | ControlFlowNode for ImportMember | test.py:140:25:140:33 | ControlFlowNode for untrusted | Untrusted URL redirection depends on a $@. | test.py:1:26:1:32 | ControlFlowNode for ImportMember | user-provided value |
 | test.py:148:25:148:33 | ControlFlowNode for untrusted | test.py:1:26:1:32 | ControlFlowNode for ImportMember | test.py:148:25:148:33 | ControlFlowNode for untrusted | Untrusted URL redirection depends on a $@. | test.py:1:26:1:32 | ControlFlowNode for ImportMember | user-provided value |
