Merge pull request github#18291 from paldepind/rust-data-flow-models

Rust: Data flow improvements to unlock flow in sqlx test

Author: paldepind

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

@@ -712,6 +712,11 @@ private class CapturedVariableContent extends Content, TCapturedVariableContent
   override string toString() { result = "captured " + v }
 }
 
+/** A value referred to by a reference. */
+final class ReferenceContent extends Content, TReferenceContent {
+  override string toString() { result = "&ref" }
+}
+
 /**
  * An element in an array.
  */
@@ -1051,6 +1056,13 @@ module RustDataFlow implements InputSig<Location> {
           ["crate::option::Option::Some", "crate::result::Result::Ok"]
       )
       or
+      exists(PrefixExprCfgNode deref |
+        c instanceof ReferenceContent and
+        deref.getOperatorName() = "*" and
+        node1.asExpr() = deref.getExpr() and
+        node2.asExpr() = deref
+      )
+      or
       VariableCapture::readStep(node1, c, node2)
     )
     or
@@ -1128,6 +1140,12 @@ module RustDataFlow implements InputSig<Location> {
       node2.(PostUpdateNode).getPreUpdateNode().asExpr() = index.getBase()
     )
     or
+    exists(RefExprCfgNode ref |
+      c instanceof ReferenceContent and
+      node1.asExpr() = ref.getExpr() and
+      node2.asExpr() = ref
+    )
+    or
     VariableCapture::storeStep(node1, c, node2)
   }
 
@@ -1395,7 +1413,8 @@ private module Cached {
       e =
         [
           any(IndexExprCfgNode i).getBase(), any(FieldExprCfgNode access).getExpr(),
-          any(TryExprCfgNode try).getExpr()
+          any(TryExprCfgNode try).getExpr(),
+          any(PrefixExprCfgNode pe | pe.getOperatorName() = "*").getExpr()
         ]
     } or
     TSsaNode(SsaImpl::DataFlowIntegration::SsaNode node) or
@@ -1495,7 +1514,8 @@ private module Cached {
     TStructFieldContent(StructCanonicalPath s, string field) {
       field = s.getStruct().getFieldList().(RecordFieldList).getAField().getName().getText()
     } or
-    TCapturedVariableContent(VariableCapture::CapturedVariable v)
+    TCapturedVariableContent(VariableCapture::CapturedVariable v) or
+    TReferenceContent()
 
   cached
   newtype TContentSet = TSingletonContentSet(Content c)

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

@@ -88,22 +88,16 @@ module SsaInput implements SsaImplCommon::InputSig<Location> {
     |
       va instanceof VariableReadAccess
       or
+      // For immutable variables, we model a read when they are borrowed
+      // (although the actual read happens later, if at all).
+      va = any(RefExpr re).getExpr()
+      or
       // Although compound assignments, like `x += y`, may in fact not read `x`,
       // it makes sense to treat them as such
       va = any(CompoundAssignmentExpr cae).getLhs()
     ) and
     certain = true
     or
-    // For immutable variables, we model a read when they are borrowed (although the
-    // actual read happens later, if at all). This only affects the SSA liveness
-    // analysis.
-    exists(VariableAccess va |
-      va = any(RefExpr re).getExpr() and
-      va = bb.getNode(i).getAstNode() and
-      v = va.getVariable() and
-      certain = false
-    )
-    or
     capturedCallRead(_, bb, i, v) and certain = false
     or
     capturedExitRead(bb, i, v) and certain = false
@@ -146,7 +140,9 @@ private predicate adjacentDefReadExt(
 
 /** Holds if `v` is read at index `i` in basic block `bb`. */
 private predicate variableReadActual(BasicBlock bb, int i, Variable v) {
-  exists(VariableReadAccess read |
+  exists(VariableAccess read |
+    read instanceof VariableReadAccess or read = any(RefExpr re).getExpr()
+  |
     read.getVariable() = v and
     read = bb.getNode(i).getAstNode()
   )

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

@@ -59,7 +59,10 @@ module RustTaintTracking implements InputSig<Location, RustDataFlow> {
   bindingset[node]
   predicate defaultImplicitTaintRead(Node::Node node, ContentSet cs) {
     exists(node) and
-    cs.(SingletonContentSet).getContent() instanceof ArrayElementContent
+    exists(Content c | c = cs.(SingletonContentSet).getContent() |
+      c instanceof ArrayElementContent or
+      c instanceof ReferenceContent
+    )
   }
 
   /**

rust/ql/lib/codeql/rust/frameworks/reqwest.model.yml

@@ -0,0 +1,6 @@
+extensions:
+  - addsTo:
+      pack: codeql/rust-all
+      extensible: summaryModel
+    data:
+      - ["repo:https://github.com/seanmonstar/reqwest:reqwest", "<crate::blocking::response::Response>::text", "Argument[self]", "ReturnValue.Variant[crate::result::Result::Ok(0)]", "taint", "manual"]

rust/ql/lib/codeql/rust/frameworks/stdlib/lang-core.model.yml

@@ -3,4 +3,13 @@ extensions:
       pack: codeql/rust-all
       extensible: summaryModel
     data:
+      # Option
       - ["lang:core", "<crate::option::Option>::unwrap", "Argument[self].Variant[crate::option::Option::Some(0)]", "ReturnValue", "value", "manual"]
+      - ["lang:core", "<crate::option::Option>::unwrap_or", "Argument[self].Variant[crate::option::Option::Some(0)]", "ReturnValue", "value", "manual"]
+      - ["lang:core", "<crate::option::Option>::unwrap_or", "Argument[0]", "ReturnValue", "value", "manual"]
+      # Result
+      - ["lang:core", "<crate::result::Result>::unwrap", "Argument[self].Variant[crate::result::Result::Ok(0)]", "ReturnValue", "value", "manual"]
+      - ["lang:core", "<crate::result::Result>::unwrap_or", "Argument[self].Variant[crate::result::Result::Ok(0)]", "ReturnValue", "value", "manual"]
+      - ["lang:core", "<crate::result::Result>::unwrap_or", "Argument[0]", "ReturnValue", "value", "manual"]
+      # String
+      - ["lang:alloc", "<crate::string::String>::as_str", "Argument[self]", "ReturnValue", "taint", "manual"]

rust/ql/lib/utils/test/InlineFlowTest.qll

@@ -12,9 +12,13 @@ private import codeql.rust.dataflow.internal.TaintTrackingImpl
 private import codeql.rust.dataflow.internal.ModelsAsData as MaD
 private import internal.InlineExpectationsTestImpl as InlineExpectationsTestImpl
 
-// Holds if the target expression of `call` is a path and the string representation of the path is `name`.
+/**
+ * Holds if the target expression of `call` is a path and the string
+ * representation of the path has `name` as a prefix.
+ */
+bindingset[name]
 private predicate callTargetName(CallExprCfgNode call, string name) {
-  call.getFunction().(PathExprCfgNode).toString() = name
+  call.getFunction().(PathExprCfgNode).toString().matches(name + "%")
 }
 
 private module FlowTestImpl implements InputSig<Location, RustDataFlow> {

rust/ql/test/library-tests/dataflow/local/CONSISTENCY/DataFlowConsistency.expected

@@ -1,2 +1,2 @@
 identityLocalStep
-| main.rs:404:7:404:18 | phi(default_name) | Node steps to itself |
+| main.rs:394:7:394:18 | phi(default_name) | Node steps to itself |