Rust: TaintedPath query

python/ql/lib/semmle/python/Concepts.qll

@@ -181,7 +181,7 @@ module Path {
     }
   }
 
-  /** A data-flow node that checks that a path is safe to access. */
+  /** A data-flow node that checks that a path is safe to access in some way, for example by having a controlled prefix. */
   class SafeAccessCheck extends DataFlow::ExprNode {
     SafeAccessCheck() { this = DataFlow::BarrierGuard<safeAccessCheck/3>::getABarrierNode() }
   }
@@ -192,7 +192,7 @@ module Path {
 
   /** Provides a class for modeling new path safety checks. */
   module SafeAccessCheck {
-    /** A data-flow node that checks that a path is safe to access. */
+    /** A data-flow node that checks that a path is safe to access in some way, for example by having a controlled prefix. */
     abstract class Range extends DataFlow::GuardNode {
       /** Holds if this guard validates `node` upon evaluating to `branch`. */
       abstract predicate checks(ControlFlowNode node, boolean branch);

rust/ql/integration-tests/hello-project/summary.expected

@@ -15,7 +15,7 @@
 | Macro calls - resolved | 2 |
 | Macro calls - total | 2 |
 | Macro calls - unresolved | 0 |
-| Taint edges - number of edges | 1671 |
+| Taint edges - number of edges | 1674 |
 | Taint reach - nodes tainted | 0 |
 | Taint reach - per million nodes | 0 |
 | Taint sinks - cryptographic operations | 0 |

rust/ql/integration-tests/hello-workspace/summary.cargo.expected

@@ -15,7 +15,7 @@
 | Macro calls - resolved | 2 |
 | Macro calls - total | 2 |
 | Macro calls - unresolved | 0 |
-| Taint edges - number of edges | 1671 |
+| Taint edges - number of edges | 1674 |
 | Taint reach - nodes tainted | 0 |
 | Taint reach - per million nodes | 0 |
 | Taint sinks - cryptographic operations | 0 |

rust/ql/integration-tests/hello-workspace/summary.rust-project.expected

@@ -15,7 +15,7 @@
 | Macro calls - resolved | 2 |
 | Macro calls - total | 2 |
 | Macro calls - unresolved | 0 |
-| Taint edges - number of edges | 1671 |
+| Taint edges - number of edges | 1674 |
 | Taint reach - nodes tainted | 0 |
 | Taint reach - per million nodes | 0 |
 | Taint sinks - cryptographic operations | 0 |

rust/ql/lib/codeql/rust/Concepts.qll

@@ -8,6 +8,8 @@ private import codeql.rust.dataflow.DataFlow
 private import codeql.threatmodels.ThreatModels
 private import codeql.rust.Frameworks
 private import codeql.rust.dataflow.FlowSource
+private import codeql.rust.controlflow.ControlFlowGraph as Cfg
+private import codeql.rust.controlflow.CfgNodes as CfgNodes
 
 /**
  * A data flow source for a specific threat-model.
@@ -264,3 +266,38 @@ module Cryptography {
 
   class CryptographicAlgorithm = SC::CryptographicAlgorithm;
 }
+
+/** Provides classes for modeling path-related APIs. */
+module Path {
+  final class PathNormalization = PathNormalization::Range;
+
+  /** Provides a class for modeling new path normalization APIs. */
+  module PathNormalization {
+    /**
+     * A data-flow node that performs path normalization. This is often needed in order
+     * to safely access paths.
+     */
+    abstract class Range extends DataFlow::Node {
+      /** Gets an argument to this path normalization that is interpreted as a path. */
+      abstract DataFlow::Node getPathArg();
+    }
+  }
+
+  /** A data-flow node that checks that a path is safe to access in some way, for example by having a controlled prefix. */
+  class SafeAccessCheck extends DataFlow::ExprNode {
+    SafeAccessCheck() { this = DataFlow::BarrierGuard<safeAccessCheck/3>::getABarrierNode() }
+  }
+
+  private predicate safeAccessCheck(CfgNodes::AstCfgNode g, Cfg::CfgNode node, boolean branch) {
+    g.(SafeAccessCheck::Range).checks(node, branch)
+  }
+
+  /** Provides a class for modeling new path safety checks. */
+  module SafeAccessCheck {
+    /** A data-flow node that checks that a path is safe to access in some way, for example by having a controlled prefix. */
+    abstract class Range extends CfgNodes::AstCfgNode {
+      /** Holds if this guard validates `node` upon evaluating to `branch`. */
+      abstract predicate checks(Cfg::CfgNode node, boolean branch);
+    }
+  }
+}

rust/ql/lib/codeql/rust/Frameworks.qll

@@ -3,5 +3,7 @@
  */
 
 private import codeql.rust.frameworks.rustcrypto.RustCrypto
+private import codeql.rust.frameworks.Poem
 private import codeql.rust.frameworks.Sqlx
 private import codeql.rust.frameworks.stdlib.Clone
+private import codeql.rust.frameworks.stdlib.Stdlib

rust/ql/lib/codeql/rust/dataflow/DataFlow.qll

@@ -8,6 +8,8 @@ private import codeql.dataflow.DataFlow
 private import internal.DataFlowImpl as DataFlowImpl
 private import internal.Node as Node
 private import internal.Content as Content
+private import codeql.rust.controlflow.ControlFlowGraph as Cfg
+private import codeql.rust.controlflow.CfgNodes as CfgNodes
 
 /**
  * Provides classes for performing local (intra-procedural) and global
@@ -16,6 +18,8 @@ private import internal.Content as Content
 module DataFlow {
   final class Node = Node::NodePublic;
 
+  final class ExprNode = Node::ExprNode;
+
   /**
    * The value of a parameter at function entry, viewed as a node in a data
    * flow graph.
@@ -56,4 +60,31 @@ module DataFlow {
   predicate localFlow(Node::Node source, Node::Node sink) { localFlowStep*(source, sink) }
 
   import DataFlowMake<Location, DataFlowImpl::RustDataFlow>
+
+  /**
+   * Holds if the guard `g` validates the expression `e` upon evaluating to `v`.
+   *
+   * The expression `e` is expected to be a syntactic part of the guard `g`.
+   * For example, the guard `g` might be a call `isSafe(x)` and the expression `e`
+   * the argument `x`.
+   */
+  signature predicate guardChecksSig(CfgNodes::AstCfgNode g, Cfg::CfgNode e, boolean branch);
+
+  /**
+   * Provides a set of barrier nodes for a guard that validates an expression.
+   *
+   * This is expected to be used in `isBarrier`/`isSanitizer` definitions
+   * in data flow and taint tracking.
+   */
+  module BarrierGuard<guardChecksSig/3 guardChecks> {
+    private import internal.DataFlowImpl::SsaFlow as SsaFlow
+    private import internal.SsaImpl as SsaImpl
+
+    /** Gets a node that is safely guarded by the given guard check. */
+    pragma[nomagic]
+    Node getABarrierNode() {
+      SsaFlow::asNode(result) =
+        SsaImpl::DataFlowIntegration::BarrierGuard<guardChecks/3>::getABarrierNode()
+    }
+  }
 }

rust/ql/lib/codeql/rust/dataflow/internal/Content.qll

@@ -214,6 +214,41 @@ final class SingletonContentSet extends ContentSet, TSingletonContentSet {
   override Content getAReadContent() { result = c }
 }
 
+/**
+ * A step in a flow summary defined using `OptionalStep[name]`. An `OptionalStep` is "opt-in", which means
+ * that by default the step is not present in the flow summary and needs to be explicitly enabled by defining
+ * an additional flow step.
+ */
+final class OptionalStep extends ContentSet, TOptionalStep {
+  override string toString() {
+    exists(string name |
+      this = TOptionalStep(name) and
+      result = "OptionalStep[" + name + "]"
+    )
+  }
+
+  override Content getAStoreContent() { none() }
+
+  override Content getAReadContent() { none() }
+}
+
+/**
+ * A step in a flow summary defined using `OptionalBarrier[name]`. An `OptionalBarrier` is "opt-out", by default
+ * data can flow freely through the step. Flow through the step can be explicity blocked by defining its node as a barrier.
+ */
+final class OptionalBarrier extends ContentSet, TOptionalBarrier {
+  override string toString() {
+    exists(string name |
+      this = TOptionalBarrier(name) and
+      result = "OptionalBarrier[" + name + "]"
+    )
+  }
+
+  override Content getAStoreContent() { none() }
+
+  override Content getAReadContent() { none() }
+}
+
 private import codeql.rust.internal.CachedStages
 
 cached

rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll

@@ -581,6 +581,12 @@ module RustDataFlow implements InputSig<Location> {
     model = ""
     or
     LocalFlow::flowSummaryLocalStep(nodeFrom, nodeTo, model)
+    or
+    // Add flow through optional barriers. This step is then blocked by the barrier for queries that choose to use the barrier.
+    FlowSummaryImpl::Private::Steps::summaryReadStep(nodeFrom
+          .(Node::FlowSummaryNode)
+          .getSummaryNode(), TOptionalBarrier(_), nodeTo.(Node::FlowSummaryNode).getSummaryNode()) and
+    model = ""
   }
 
   /**
@@ -710,7 +716,17 @@ module RustDataFlow implements InputSig<Location> {
     )
     or
     FlowSummaryImpl::Private::Steps::summaryReadStep(node1.(FlowSummaryNode).getSummaryNode(), cs,
-      node2.(FlowSummaryNode).getSummaryNode())
+      node2.(FlowSummaryNode).getSummaryNode()) and
+    not isSpecialContentSet(cs)
+  }
+
+  /**
+   * Holds if `cs` is used to encode a special operation as a content component, but should not
+   * be treated as an ordinary content component.
+   */
+  private predicate isSpecialContentSet(ContentSet cs) {
+    cs instanceof TOptionalStep or
+    cs instanceof TOptionalBarrier
   }
 
   pragma[nomagic]
@@ -807,7 +823,8 @@ module RustDataFlow implements InputSig<Location> {
     storeContentStep(node1, cs.(SingletonContentSet).getContent(), node2)
     or
     FlowSummaryImpl::Private::Steps::summaryStoreStep(node1.(FlowSummaryNode).getSummaryNode(), cs,
-      node2.(FlowSummaryNode).getSummaryNode())
+      node2.(FlowSummaryNode).getSummaryNode()) and
+    not isSpecialContentSet(cs)
   }
 
   /**
@@ -1093,7 +1110,14 @@ private module Cached {
   newtype TReturnKind = TNormalReturnKind()
 
   cached
-  newtype TContentSet = TSingletonContentSet(Content c)
+  newtype TContentSet =
+    TSingletonContentSet(Content c) or
+    TOptionalStep(string name) {
+      name = any(FlowSummaryImpl::Private::AccessPathToken tok).getAnArgument("OptionalStep")
+    } or
+    TOptionalBarrier(string name) {
+      name = any(FlowSummaryImpl::Private::AccessPathToken tok).getAnArgument("OptionalBarrier")
+    }
 
   /** Holds if `n` is a flow source of kind `kind`. */
   cached
@@ -1102,6 +1126,27 @@ private module Cached {
   /** Holds if `n` is a flow sink of kind `kind`. */
   cached
   predicate sinkNode(Node n, string kind) { n.(FlowSummaryNode).isSink(kind, _) }
+
+  /**
+   * A step in a flow summary defined using `OptionalStep[name]`. An `OptionalStep` is "opt-in", which means
+   * that by default the step is not present in the flow summary and needs to be explicitly enabled by defining
+   * an additional flow step.
+   */
+  cached
+  predicate optionalStep(Node node1, string name, Node node2) {
+    FlowSummaryImpl::Private::Steps::summaryReadStep(node1.(FlowSummaryNode).getSummaryNode(),
+      TOptionalStep(name), node2.(FlowSummaryNode).getSummaryNode())
+  }
+
+  /**
+   * A step in a flow summary defined using `OptionalBarrier[name]`. An `OptionalBarrier` is "opt-out", by default
+   * data can flow freely through the step. Flow through the step can be explicity blocked by defining its node as a barrier.
+   */
+  cached
+  predicate optionalBarrier(Node node, string name) {
+    FlowSummaryImpl::Private::Steps::summaryReadStep(_, TOptionalBarrier(name),
+      node.(FlowSummaryNode).getSummaryNode())
+  }
 }
 
 import Cached

rust/ql/lib/codeql/rust/dataflow/internal/FlowSummaryImpl.qll

@@ -105,6 +105,10 @@ module Input implements InputSig<Location, RustDataFlow> {
       c = TFutureContent() and
       arg = ""
     )
+    or
+    cs = TOptionalStep(arg) and result = "OptionalStep"
+    or
+    cs = TOptionalBarrier(arg) and result = "OptionalBarrier"
   }
 
   string encodeReturn(ReturnKind rk, string arg) { none() }

rust/ql/lib/codeql/rust/dataflow/internal/TaintTrackingImpl.qll

@@ -69,7 +69,9 @@ module RustTaintTracking implements InputSig<Location, RustDataFlow> {
     exists(Content c | c = cs.(SingletonContentSet).getContent() |
       c instanceof ElementContent or
       c instanceof ReferenceContent
-    )
+    ) and
+    // Optional steps are added through isAdditionalFlowStep but we don't want the implicit reads
+    not optionalStep(node, _, _)
   }
 
   /**

rust/ql/lib/codeql/rust/frameworks/Poem.qll

@@ -0,0 +1,20 @@
+/**
+ * Provides modeling for the `Poem` library.
+ */
+
+private import rust
+private import codeql.rust.Concepts
+private import codeql.rust.dataflow.DataFlow
+
+/**
+ * Parameters of a handler function
+ */
+private class PoemHandlerParam extends RemoteSource::Range {
+  PoemHandlerParam() {
+    exists(TupleStructPat param |
+      param.getResolvedPath() = ["crate::web::query::Query", "crate::web::path::Path"]
+    |
+      this.asPat().getPat() = param.getAField()
+    )
+  }
+}

rust/ql/lib/codeql/rust/frameworks/stdlib/Stdlib.qll

@@ -0,0 +1,23 @@
+/**
+ * Provides classes modeling security-relevant aspects of the standard libraries.
+ */
+
+private import rust
+private import codeql.rust.Concepts
+private import codeql.rust.controlflow.ControlFlowGraph as Cfg
+private import codeql.rust.controlflow.CfgNodes as CfgNodes
+private import codeql.rust.dataflow.DataFlow
+
+/**
+ * A call to the `starts_with` method on a `Path`.
+ */
+private class StartswithCall extends Path::SafeAccessCheck::Range, CfgNodes::MethodCallExprCfgNode {
+  StartswithCall() {
+    this.getAstNode().(Resolvable).getResolvedPath() = "<crate::path::Path>::starts_with"
+  }
+
+  override predicate checks(Cfg::CfgNode e, boolean branch) {
+    e = this.getReceiver() and
+    branch = true
+  }
+}

rust/ql/lib/codeql/rust/frameworks/stdlib/fs.model.yml

@@ -0,0 +1,46 @@
+extensions:
+  - addsTo:
+      pack: codeql/rust-all
+      extensible: sourceModel
+    data: []
+  - addsTo:
+      pack: codeql/rust-all
+      extensible: sinkModel
+    data:
+      - ["lang:std", "crate::fs::copy", "Argument[0]", "path-injection", "manual"]
+      - ["lang:std", "crate::fs::copy", "Argument[1]", "path-injection", "manual"]
+      - ["lang:std", "crate::fs::create_dir", "Argument[0]", "path-injection", "manual"]
+      - ["lang:std", "crate::fs::create_dir_all", "Argument[0]", "path-injection", "manual"]
+      - ["lang:std", "crate::fs::hard_link", "Argument[0]", "path-injection", "manual"]
+      - ["lang:std", "crate::fs::hard_link", "Argument[1]", "path-injection", "manual"]
+      - ["lang:std", "crate::fs::metadata", "Argument[0]", "path-injection", "manual"]
+      - ["lang:std", "crate::fs::read", "Argument[0]", "path-injection", "manual"]
+      - ["lang:std", "crate::fs::read_dir", "Argument[0]", "path-injection", "manual"]
+      - ["lang:std", "crate::fs::read_link", "Argument[0]", "path-injection", "manual"]
+      - ["lang:std", "crate::fs::read_to_string", "Argument[0]", "path-injection", "manual"]
+      - ["lang:std", "crate::fs::remove_dir", "Argument[0]", "path-injection", "manual"]
+      - ["lang:std", "crate::fs::remove_dir_all", "Argument[0]", "path-injection", "manual"]
+      - ["lang:std", "crate::fs::remove_file", "Argument[0]", "path-injection", "manual"]
+      - ["lang:std", "crate::fs::rename", "Argument[0]", "path-injection", "manual"]
+      - ["lang:std", "crate::fs::rename", "Argument[1]", "path-injection", "manual"]
+      - ["lang:std", "crate::fs::set_permissions", "Argument[0]", "path-injection", "manual"]
+      - ["lang:std", "crate::fs::soft_link", "Argument[0]", "path-injection", "manual"]
+      - ["lang:std", "crate::fs::soft_link", "Argument[1]", "path-injection", "manual"]
+      - ["lang:std", "crate::fs::symlink_metadata", "Argument[0]", "path-injection", "manual"]
+      - ["lang:std", "crate::fs::write", "Argument[0]", "path-injection", "manual"]
+      - ["lang:std", "<crate::fs::DirBuilder>::create", "Argument[0]", "path-injection", "manual"]
+      - ["lang:std", "<crate::fs::File>::create", "Argument[0]", "path-injection", "manual"]
+      - ["lang:std", "<crate::fs::File>::create_buffered", "Argument[0]", "path-injection", "manual"]
+      - ["lang:std", "<crate::fs::File>::create_new", "Argument[0]", "path-injection", "manual"]
+      - ["lang:std", "<crate::fs::File>::open", "Argument[0]", "path-injection", "manual"]
+      - ["lang:std", "<crate::fs::File>::open_buffered", "Argument[0]", "path-injection", "manual"]
+
+  - addsTo:
+      pack: codeql/rust-all
+      extensible: summaryModel
+    data:
+      - ["lang:std", "<crate::path::PathBuf as crate::convert::From>::from", "Argument[0]", "ReturnValue", "taint", "manual"]
+      - ["lang:std", "<crate::path::Path>::join", "Argument[self]", "ReturnValue", "taint", "manual"]
+      - ["lang:std", "<crate::path::Path>::join", "Argument[0]", "ReturnValue", "taint", "manual"]
+      - ["lang:std", "<crate::path::Path>::canonicalize", "Argument[self].OptionalStep[normalize-path]", "ReturnValue.Field[crate::result::Result::Ok(0)]", "taint", "manual"]
+      - ["lang:std", "<crate::path::Path>::canonicalize", "Argument[self].OptionalBarrier[normalize-path]", "ReturnValue.Field[crate::result::Result::Ok(0)]", "taint", "manual"]