data flow wip

Author: hvitved

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

@@ -145,13 +145,9 @@ final class ArgumentPosition extends ParameterPosition {
  * as the synthetic `ReceiverNode` is the argument for the `self` parameter.
  */
 predicate isArgumentForCall(ExprCfgNode arg, CallCfgNode call, ParameterPosition pos) {
-  // TODO: Handle index expressions as calls in data flow.
-  not call.getCall() instanceof IndexExpr and
-  (
-    call.getPositionalArgument(pos.getPosition()) = arg
-    or
-    call.getReceiver() = arg and pos.isSelf() and not call.getCall().receiverImplicitlyBorrowed()
-  )
+  call.getPositionalArgument(pos.getPosition()) = arg
+  or
+  call.getReceiver() = arg and pos.isSelf() and not call.getCall().receiverImplicitlyBorrowed()
 }
 
 /** Provides logic related to SSA. */
@@ -557,12 +553,6 @@ module RustDataFlow implements InputSig<Location> {
       access = c.(FieldContent).getAnAccess()
     )
     or
-    exists(IndexExprCfgNode arr |
-      c instanceof ElementContent and
-      node1.asExpr() = arr.getBase() and
-      node2.asExpr() = arr
-    )
-    or
     exists(ForExprCfgNode for |
       c instanceof ElementContent and
       node1.asExpr() = for.getIterable() and
@@ -582,13 +572,6 @@ module RustDataFlow implements InputSig<Location> {
           .isVariantField([any(OptionEnum o).getSome(), any(ResultEnum r).getOk()], 0)
     )
     or
-    exists(PrefixExprCfgNode deref |
-      c instanceof ReferenceContent and
-      deref.getOperatorName() = "*" and
-      node1.asExpr() = deref.getExpr() and
-      node2.asExpr() = deref
-    )
-    or
     // Read from function return
     exists(DataFlowCall call |
       lambdaCall(call, _, node1) and
@@ -700,6 +683,7 @@ module RustDataFlow implements InputSig<Location> {
     or
     referenceAssignment(node1, node2.(PostUpdateNode).getPreUpdateNode(), c)
     or
+    // todo: rely on flow summary
     exists(AssignmentExprCfgNode assignment, IndexExprCfgNode index |
       c instanceof ElementContent and
       assignment.getLhs() = index and
@@ -991,11 +975,7 @@ private module Cached {
 
   cached
   newtype TDataFlowCall =
-    TCall(CallCfgNode c) {
-      Stages::DataFlowStage::ref() and
-      // TODO: Handle index expressions as calls in data flow.
-      not c.getCall() instanceof IndexExpr
-    } or
+    TCall(CallCfgNode c) { Stages::DataFlowStage::ref() } or
     TSummaryCall(
       FlowSummaryImpl::Public::SummarizedCallable c, FlowSummaryImpl::Private::SummaryNode receiver
     ) {

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

@@ -467,15 +467,11 @@ newtype TNode =
         any(TryExprCfgNode try).getExpr(), //
         any(PrefixExprCfgNode pe | pe.getOperatorName() = "*").getExpr(), //
         any(AwaitExprCfgNode a).getExpr(), //
-        any(MethodCallExprCfgNode mc).getReceiver(), //
+        any(CallCfgNode call | call.getCall().receiverImplicitlyBorrowed()).getReceiver(), //
         getPostUpdateReverseStep(any(PostUpdateNode n).getPreUpdateNode().asExpr(), _)
       ]
   } or
-  TReceiverNode(CallCfgNode mc, Boolean isPost) {
-    mc.getCall().receiverImplicitlyBorrowed() and
-    // TODO: Handle index expressions as calls in data flow.
-    not mc.getCall() instanceof IndexExpr
-  } or
+  TReceiverNode(CallCfgNode mc, Boolean isPost) { mc.getCall().receiverImplicitlyBorrowed() } or
   TSsaNode(SsaImpl::DataFlowIntegration::SsaNode node) or
   TFlowSummaryNode(FlowSummaryImpl::Private::SummaryNode sn) or
   TClosureSelfReferenceNode(CfgScope c) { lambdaCreationExpr(c, _) } or

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

@@ -35,19 +35,14 @@ module RustTaintTracking implements InputSig<Location, RustDataFlow> {
       or
       pred.asExpr() = succ.asExpr().(CastExprCfgNode).getExpr()
       or
-      exists(IndexExprCfgNode index |
-        index.getIndex() instanceof RangeExprCfgNode and
-        pred.asExpr() = index.getBase() and
-        succ.asExpr() = index
-      )
-      or
-      // Although data flow through collections is modeled using stores/reads,
-      // we also allow taint to flow out of a tainted collection. This is
-      // needed in order to support taint-tracking configurations where the
-      // source is a collection.
-      exists(SingletonContentSet cs |
-        RustDataFlow::readStep(pred, cs, succ) and
+      // Although data flow through collections and references is modeled using
+      // stores/reads, we also allow taint to flow out of a tainted collection.
+      // This is needed in order to support taint-tracking configurations where
+      // the source is a collection or reference.
+      exists(SingletonContentSet cs | RustDataFlow::readStep(pred, cs, succ) |
         cs.getContent() instanceof ElementContent
+        or
+        cs.getContent() instanceof ReferenceContent
       )
       or
       exists(FormatArgsExprCfgNode format | succ.asExpr() = format |

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

@@ -7,7 +7,11 @@ 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
+private import codeql.rust.dataflow.FlowSummary
 private import codeql.rust.internal.PathResolution
+private import codeql.rust.internal.Type
+private import codeql.rust.internal.TypeInference
+private import codeql.rust.internal.TypeMention
 
 /**
  * A call to the `starts_with` method on a `Path`.
@@ -213,3 +217,111 @@ class StringStruct extends Struct {
   pragma[nomagic]
   StringStruct() { this.getCanonicalPath() = "alloc::string::String" }
 }
+
+/**
+ * The [`Deref` trait][1].
+ *
+ * [1]: https://doc.rust-lang.org/core/ops/trait.Deref.html
+ */
+class DerefTrait extends Trait {
+  pragma[nomagic]
+  DerefTrait() { this.getCanonicalPath() = "core::ops::deref::Deref" }
+
+  /** Gets the `Target` associated type. */
+  pragma[nomagic]
+  TypeAlias getTargetType() {
+    result = this.getAssocItemList().getAnAssocItem() and
+    result.getName().getText() = "Target"
+  }
+}
+
+/**
+ * One of the two special
+ *
+ * ```rust
+ * impl<T: ?Sized> const Deref for &T
+ * impl<T: ?Sized> const Deref for &mut T
+ * ```
+ *
+ * implementations.
+ */
+private class CoreDerefImpl extends ImplItemNode {
+  pragma[nomagic]
+  CoreDerefImpl() {
+    this.resolveTraitTy() instanceof DerefTrait and
+    this.(Impl)
+        .getSelfTy()
+        .(TypeMention)
+        .resolveTypeAt(TypePath::singleton(TRefTypeParameter()))
+        .(TypeParamTypeParameter)
+        .getTypeParam() = this.getTypeParam(_)
+  }
+
+  Function getDeref() { result = this.getASuccessor("deref") }
+}
+
+// TODO: Use MaD when `&(mut) T` is assigned an appropriate canonical path
+private class CoreDerefSummarizedCallable extends SummarizedCallable::Range {
+  CoreDerefSummarizedCallable() { this = any(CoreDerefImpl i).getDeref() }
+
+  override predicate propagatesFlow(string input, string output, boolean preservesValue) {
+    input = "Argument[self].Reference" and
+    output = "ReturnValue" and
+    preservesValue = true
+  }
+}
+
+/**
+ * The [`Index` trait][1].
+ *
+ * [1]: https://doc.rust-lang.org/std/ops/trait.Index.html
+ */
+class IndexTrait extends Trait {
+  pragma[nomagic]
+  IndexTrait() { this.getCanonicalPath() = "core::ops::index::Index" }
+
+  /** Gets the `Output` associated type. */
+  pragma[nomagic]
+  TypeAlias getOutputType() {
+    result = this.getAssocItemList().getAnAssocItem() and
+    result.getName().getText() = "Output"
+  }
+}
+
+/**
+ * One of the two special
+ *
+ * ```rust
+ * impl<T, I, const N: usize> Index<I> for [T; N]
+ * impl<T, I> ops::Index<I> for [T]
+ * ```
+ *
+ * implementations.
+ */
+private class CoreIndexImpl extends ImplItemNode {
+  pragma[nomagic]
+  CoreIndexImpl() {
+    this.resolveTraitTy() instanceof IndexTrait and
+    exists(TypeParameter tp | tp = TArrayTypeParameter() or tp = TSliceTypeParameter() |
+      this.(Impl)
+          .getSelfTy()
+          .(TypeMention)
+          .resolveTypeAt(TypePath::singleton(tp))
+          .(TypeParamTypeParameter)
+          .getTypeParam() = this.getTypeParam(_)
+    )
+  }
+
+  Function getIndex() { result = this.getASuccessor("index") }
+}
+
+// TODO: Use MaD when `[T(; N)]` is assigned an appropriate canonical path
+private class CoreIndexSummarizedCallable extends SummarizedCallable::Range {
+  CoreIndexSummarizedCallable() { this = any(CoreIndexImpl i).getIndex() }
+
+  override predicate propagatesFlow(string input, string output, boolean preservesValue) {
+    input = "Argument[self].Reference.Element" and
+    output = "ReturnValue" and
+    preservesValue = true
+  }
+}

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

@@ -35,15 +35,10 @@ extensions:
       - ["<alloc::boxed::Box>::pin", "Argument[0]", "ReturnValue.Reference", "value", "manual"]
       - ["<alloc::boxed::Box>::new", "Argument[0]", "ReturnValue.Reference", "value", "manual"]
       - ["<alloc::boxed::Box>::into_pin", "Argument[0]", "ReturnValue", "value", "manual"]
+      - ["<alloc::boxed::Box as core::ops::deref::Deref>::deref", "Argument[self].Reference", "ReturnValue", "value", "manual"]
       # Fmt
       - ["alloc::fmt::format", "Argument[0]", "ReturnValue", "taint", "manual"]
       # String
-      - ["<core::str>::as_str", "Argument[self]", "ReturnValue", "value", "manual"]
-      - ["<core::str>::as_bytes", "Argument[self]", "ReturnValue", "value", "manual"]
-      - ["<alloc::string::String>::as_str", "Argument[self]", "ReturnValue", "value", "manual"]
-      - ["<alloc::string::String>::as_bytes", "Argument[self]", "ReturnValue", "value", "manual"]
-      - ["<alloc::str as alloc::string::ToString>::to_string", "Argument[self]", "ReturnValue", "taint", "manual"]
-      - ["<alloc::string::String as alloc::string::ToString>::to_string", "Argument[self]", "ReturnValue", "taint", "manual"]
-      - ["<core::str>::parse", "Argument[self]", "ReturnValue.Field[core::result::Result::Ok(0)]", "taint", "manual"]
-      - ["<core::str>::trim", "Argument[self]", "ReturnValue.Reference", "taint", "manual"]
-      - ["<alloc::string::String as core::convert::From>::from", "Argument[0]", "ReturnValue", "value", "manual"]
+      - ["<alloc::string::String>::as_str", "Argument[self].Reference", "ReturnValue.Reference", "taint", "manual"]
+      - ["<alloc::string::String>::as_bytes", "Argument[self].Reference", "ReturnValue.Reference", "taint", "manual"]
+      - ["<alloc::string::String as core::convert::From>::from", "Argument[0].Reference", "ReturnValue", "taint", "manual"]

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

@@ -26,6 +26,9 @@ extensions:
       - ["<core::slice::iter::Iter as core::iter::traits::iterator::Iterator>::collect", "Argument[self].Element", "ReturnValue.Element", "value", "manual"]
       - ["<core::slice::iter::Iter as core::iter::traits::iterator::Iterator>::map", "Argument[self].Element", "Argument[0].Parameter[0]", "value", "manual"]
       - ["<_ as core::iter::traits::iterator::Iterator>::for_each", "Argument[self].Element", "Argument[0].Parameter[0]", "value", "manual"]
+      # Index
+      - ["<alloc::vec::Vec as core::ops::index::Index>::index", "Argument[self].Reference.Element", "ReturnValue", "value", "manual"]
+      - ["<alloc::string::String as core::ops::index::Index>::index", "Argument[self].Reference", "ReturnValue", "taint", "manual"]
       # Layout
       - ["<core::alloc::layout::Layout>::from_size_align", "Argument[0]", "ReturnValue.Field[core::result::Result::Ok(0)]", "taint", "manual"]
       - ["<core::alloc::layout::Layout>::from_size_align_unchecked", "Argument[0]", "ReturnValue", "taint", "manual"]
@@ -48,6 +51,7 @@ extensions:
       - ["<core::pin::Pin>::into_inner", "Argument[0]", "ReturnValue", "value", "manual"]
       - ["<core::pin::Pin>::into_inner_unchecked", "Argument[0]", "ReturnValue", "value", "manual"]
       - ["<core::pin::Pin>::set", "Argument[0]", "Argument[self]", "value", "manual"]
+      - ["<core::pin::Pin as core::ops::deref::Deref>::deref", "Argument[self].Reference", "ReturnValue", "value", "manual"]
       # Ptr
       - ["core::ptr::read", "Argument[0].Reference", "ReturnValue", "value", "manual"]
       - ["core::ptr::read_unaligned", "Argument[0].Reference", "ReturnValue", "value", "manual"]
@@ -56,13 +60,10 @@ extensions:
       - ["core::ptr::write_unaligned", "Argument[1]", "Argument[0].Reference", "value", "manual"]
       - ["core::ptr::write_volatile", "Argument[1]", "Argument[0].Reference", "value", "manual"]
       # Str
-      - ["<core::str>::as_str", "Argument[self]", "ReturnValue", "taint", "value"]
-      - ["<alloc::string::String>::as_str", "Argument[self]", "ReturnValue", "taint", "value"]
-      - ["<core::str>::as_bytes", "Argument[self]", "ReturnValue", "taint", "value"]
-      - ["<alloc::string::String>::as_bytes", "Argument[self]", "ReturnValue", "taint", "value"]
-      - ["<core::str>::to_string", "Argument[self]", "ReturnValue", "taint", "manual"]
-      - ["<core::str>::parse", "Argument[self]", "ReturnValue.Field[core::result::Result::Ok(0)]", "taint", "manual"]
-      - ["<core::str>::trim", "Argument[self]", "ReturnValue.Reference", "taint", "manual"]
+      - ["<core::str>::as_str", "Argument[self].Reference", "ReturnValue.Reference", "taint", "value"]
+      - ["<core::str>::as_bytes", "Argument[self].Reference", "ReturnValue.Reference", "taint", "value"]
+      - ["<core::str>::parse", "Argument[self].Reference", "ReturnValue.Field[core::result::Result::Ok(0)]", "taint", "manual"]
+      - ["<core::str>::trim", "Argument[self].Reference", "ReturnValue.Reference", "taint", "manual"]
   - addsTo:
       pack: codeql/rust-all
       extensible: sourceModel

rust/ql/lib/codeql/rust/internal/TypeInference.qll

@@ -3264,7 +3264,15 @@ private module Cached {
   /** Holds if `receiver` is the receiver of a method call with an implicit dereference. */
   cached
   predicate receiverHasImplicitDeref(AstNode receiver) {
-    none() // todo
+    exists(MethodCall mc |
+      exists(resolveMethodCallTarget(MkMethodCallDerefChainRef(mc, ".ref;"))) and
+      receiver = mc.getArgument(CallImpl::TSelfArgumentPosition())
+    )
+    // or
+    // exists(Op op |
+    //   op.(Call).implicitBorrowAt(CallImpl::TSelfArgumentPosition(), true) and
+    //   receiver = op.getOperand(0)
+    // )
     // exists(MethodCallExprMatchingInput::Access a, MethodCallExprMatchingInput::AccessPosition apos |
     //   apos.getArgumentPosition().isSelf() and
     //   apos.isBorrowed(_) and
@@ -3277,7 +3285,15 @@ private module Cached {
   /** Holds if `receiver` is the receiver of a method call with an implicit borrow. */
   cached
   predicate receiverHasImplicitBorrow(AstNode receiver) {
-    none() // todo
+    exists(MethodCall mc |
+      exists(resolveMethodCallTarget(MkMethodCallDerefChainRef(mc, ";ref"))) and
+      receiver = mc.getArgument(CallImpl::TSelfArgumentPosition())
+    )
+    or
+    exists(Op op |
+      op.(Call).implicitBorrowAt(CallImpl::TSelfArgumentPosition(), true) and
+      receiver = op.getOperand(0)
+    )
     // exists(MethodCallExprMatchingInput::Access a, MethodCallExprMatchingInput::AccessPosition apos |
     //   apos.getArgumentPosition().isSelf() and
     //   apos.isBorrowed(_) and

rust/ql/test/library-tests/dataflow/global/inline-flow.expected

@@ -400,6 +400,7 @@ subpaths
 | main.rs:301:50:301:50 | a [MyInt] | main.rs:289:18:289:21 | SelfParam [MyInt] | main.rs:289:48:291:5 | { ... } [MyInt] | main.rs:301:30:301:54 | ...::take_self(...) [MyInt] |
 | main.rs:306:55:306:55 | b [MyInt] | main.rs:293:26:293:37 | ...: MyInt [MyInt] | main.rs:293:49:295:5 | { ... } [MyInt] | main.rs:306:30:306:56 | ...::take_second(...) [MyInt] |
 testFailures
+| main.rs:277:14:277:58 | //... | Missing result: hasTaintFlow=28 |
 #select
 | main.rs:18:10:18:10 | a | main.rs:13:5:13:13 | source(...) | main.rs:18:10:18:10 | a | $@ | main.rs:13:5:13:13 | source(...) | source(...) |
 | main.rs:39:10:39:21 | a.get_data() | main.rs:38:23:38:31 | source(...) | main.rs:39:10:39:21 | a.get_data() | $@ | main.rs:38:23:38:31 | source(...) | source(...) |

rust/ql/test/library-tests/dataflow/global/viableCallable.expected

@@ -59,8 +59,6 @@
 | main.rs:212:13:212:34 | ...::new(...) | main.rs:205:5:208:5 | fn new |
 | main.rs:212:24:212:33 | source(...) | main.rs:1:1:3:1 | fn source |
 | main.rs:214:5:214:11 | sink(...) | main.rs:5:1:7:1 | fn sink |
-| main.rs:228:10:228:14 | * ... | main.rs:235:5:237:5 | fn deref |
-| main.rs:236:11:236:15 | * ... | main.rs:235:5:237:5 | fn deref |
 | main.rs:242:28:242:36 | source(...) | main.rs:1:1:3:1 | fn source |
 | main.rs:244:13:244:17 | ... + ... | main.rs:220:5:223:5 | fn add |
 | main.rs:245:5:245:17 | sink(...) | main.rs:5:1:7:1 | fn sink |