Rust: Remove borrow logic from Call classes

Author: hvitved

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

@@ -21,7 +21,7 @@ private module Input implements InputSig<Location, RustDataFlow> {
     or
     // We allow flow into post-update node for receiver expressions (from the
     // synthetic post receiever node).
-    n.(Node::PostUpdateNode).getPreUpdateNode().asExpr() = any(Node::ReceiverNode r).getReceiver()
+    n.(Node::PostUpdateNode).getPreUpdateNode().asExpr() = any(Node::DerefBorrowNode r).getNode()
     or
     n.(Node::PostUpdateNode).getPreUpdateNode().asExpr() = getPostUpdateReverseStep(_, _)
     or

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

@@ -12,7 +12,6 @@ private import codeql.rust.elements.Call
 private import SsaImpl as SsaImpl
 private import codeql.rust.controlflow.internal.Scope as Scope
 private import codeql.rust.internal.PathResolution
-private import codeql.rust.internal.TypeInference as TypeInference
 private import codeql.rust.controlflow.ControlFlowGraph
 private import codeql.rust.dataflow.Ssa
 private import codeql.rust.dataflow.FlowSummary
@@ -157,7 +156,7 @@ final class ArgumentPosition extends TArgumentPosition {
       inMethodCall = true
       or
       result = call.(IndexExpr).getIndex() and
-      pos = 1 and
+      pos = 0 and
       inMethodCall = true
     )
   }
@@ -201,8 +200,7 @@ final class ArgumentPosition extends TArgumentPosition {
 predicate isArgumentForCall(Expr arg, Call call, ArgumentPosition pos) {
   // TODO: Handle index expressions as calls in data flow.
   not call instanceof IndexExpr and
-  arg = pos.getArgument(call) and
-  not (pos.isReceiver() and call.receiverImplicitlyBorrowed())
+  arg = pos.getArgument(call)
 }
 
 /** Provides logic related to SSA. */
@@ -333,14 +331,6 @@ module LocalFlow {
     or
     nodeFrom.asPat().(OrPat).getAPat() = nodeTo.asPat()
     or
-    // Simple value step from receiver expression to receiver node, in case
-    // there is no implicit deref or borrow operation.
-    nodeFrom.asExpr() = nodeTo.(ReceiverNode).getReceiver()
-    or
-    // The dual step of the above, for the post-update nodes.
-    nodeFrom.(PostUpdateNode).getPreUpdateNode().(ReceiverNode).getReceiver() =
-      nodeTo.(PostUpdateNode).getPreUpdateNode().asExpr()
-    or
     nodeTo.(PostUpdateNode).getPreUpdateNode().asExpr() =
       getPostUpdateReverseStep(nodeFrom.(PostUpdateNode).getPreUpdateNode().asExpr(), true)
   }
@@ -430,7 +420,7 @@ module RustDataFlow implements InputSig<Location> {
     node.(FlowSummaryNode).getSummaryNode().isHidden() or
     node instanceof CaptureNode or
     node instanceof ClosureParameterNode or
-    node instanceof ReceiverNode or
+    node instanceof DerefBorrowNode or
     node.asExpr() instanceof ParenExpr or
     nodeIsHidden(node.(PostUpdateNode).getPreUpdateNode())
   }
@@ -584,16 +574,16 @@ module RustDataFlow implements InputSig<Location> {
   }
 
   pragma[nomagic]
-  private predicate implicitDerefToReceiver(Node node1, ReceiverNode node2, ReferenceContent c) {
-    TypeInference::receiverHasImplicitDeref(node1.asExpr()) and
-    node1.asExpr() = node2.getReceiver() and
+  private predicate implicitDeref(Node node1, DerefBorrowNode node2, ReferenceContent c) {
+    not node2.isBorrow() and
+    node1.asExpr() = node2.getNode() and
     exists(c)
   }
 
   pragma[nomagic]
-  private predicate implicitBorrowToReceiver(Node node1, ReceiverNode node2, ReferenceContent c) {
-    TypeInference::receiverHasImplicitBorrow(node1.asExpr()) and
-    node1.asExpr() = node2.getReceiver() and
+  private predicate implicitBorrow(Node node1, DerefBorrowNode node2, ReferenceContent c) {
+    node2.isBorrow() and
+    node1.asExpr() = node2.getNode() and
     exists(c)
   }
 
@@ -603,6 +593,15 @@ module RustDataFlow implements InputSig<Location> {
     exists(c)
   }
 
+  private Node getFieldExprContainerNode(FieldExpr fe) {
+    exists(Expr container | container = fe.getContainer() |
+      not any(DerefBorrowNode n).getNode() = container and
+      result.asExpr() = container
+      or
+      result.(DerefBorrowNode).getNode() = container
+    )
+  }
+
   pragma[nomagic]
   additional predicate readContentStep(Node node1, Content c, Node node2) {
     exists(TupleStructPat pat, int pos |
@@ -627,9 +626,9 @@ module RustDataFlow implements InputSig<Location> {
     node1.asPat().(RefPat).getPat() = node2.asPat()
     or
     exists(FieldExpr access |
-      node1.asExpr() = access.getContainer() and
       node2.asExpr() = access and
-      access = c.(FieldContent).getAnAccess()
+      access = c.(FieldContent).getAnAccess() and
+      node1 = getFieldExprContainerNode(access)
     )
     or
     exists(IndexExpr arr |
@@ -680,12 +679,10 @@ module RustDataFlow implements InputSig<Location> {
     referenceExprToExpr(node2.(PostUpdateNode).getPreUpdateNode(),
       node1.(PostUpdateNode).getPreUpdateNode(), c)
     or
-    // Step from receiver expression to receiver node, in case of an implicit
-    // dereference.
-    implicitDerefToReceiver(node1, node2, c)
+    implicitDeref(node1, node2, c)
     or
     // A read step dual to the store step for implicit borrows.
-    implicitBorrowToReceiver(node2.(PostUpdateNode).getPreUpdateNode(),
+    implicitBorrow(node2.(PostUpdateNode).getPreUpdateNode(),
       node1.(PostUpdateNode).getPreUpdateNode(), c)
     or
     VariableCapture::readStep(node1, c, node2)
@@ -721,7 +718,7 @@ module RustDataFlow implements InputSig<Location> {
     exists(AssignmentExpr assignment, FieldExpr access |
       assignment.getLhs() = access and
       node1.asExpr() = assignment.getRhs() and
-      node2.asExpr() = access.getContainer() and
+      node2 = getFieldExprContainerNode(access) and
       access = c.getAnAccess()
     )
   }
@@ -793,9 +790,11 @@ module RustDataFlow implements InputSig<Location> {
     or
     VariableCapture::storeStep(node1, c, node2)
     or
-    // Step from receiver expression to receiver node, in case of an implicit
-    // borrow.
-    implicitBorrowToReceiver(node1, node2, c)
+    implicitBorrow(node1, node2, c)
+    or
+    // A store step dual to the read step for implicit dereferences.
+    implicitDeref(node2.(PostUpdateNode).getPreUpdateNode(),
+      node1.(PostUpdateNode).getPreUpdateNode(), c)
   }
 
   /**

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

@@ -27,8 +27,9 @@ module Input implements InputSig<Location, RustDataFlow> {
     }
 
     Expr getArgument(ParameterPosition pos) {
-      exists(ParameterPosition pos0, ArgumentPosition apos |
-        result = apos.getArgument(this.getCall()) and
+      exists(ParameterPosition pos0, ArgumentPosition apos, CallExprBase call |
+        call = this.getCall() and
+        result = apos.getArgument(call) and
         RustDataFlow::parameterMatch(pos0, apos)
       |
         not pos.hasPosition(_) and

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

@@ -14,6 +14,7 @@ private import codeql.rust.controlflow.ControlFlowGraph
 private import codeql.rust.controlflow.CfgNodes
 private import codeql.rust.dataflow.Ssa
 private import codeql.rust.dataflow.FlowSummary
+private import codeql.rust.internal.TypeInference as TypeInference
 private import Node as Node
 private import DataFlowImpl
 private import FlowSummaryImpl as FlowSummaryImpl
@@ -238,35 +239,52 @@ final class ExprArgumentNode extends ArgumentNode, ExprNode {
   private Call call_;
   private RustDataFlow::ArgumentPosition pos_;
 
-  ExprArgumentNode() { isArgumentForCall(n, call_, pos_) }
+  ExprArgumentNode() {
+    isArgumentForCall(n, call_, pos_) and
+    not TypeInference::implicitDeref(n) and
+    not TypeInference::implicitBorrow(n)
+  }
 
   override predicate isArgumentOf(DataFlowCall call, RustDataFlow::ArgumentPosition pos) {
     call.asCall() = call_ and pos = pos_
   }
 }
 
 /**
- * The receiver of a method call _after_ any implicit borrow or dereferencing
- * has taken place.
+ * A node after implicit dereference or borrow.
  */
-final class ReceiverNode extends ArgumentNode, TReceiverNode {
-  private Call n;
+class DerefBorrowNode extends Node, TDerefBorrowNode {
+  AstNode n;
+  boolean isBorrow;
 
-  ReceiverNode() { this = TReceiverNode(n, false) }
+  DerefBorrowNode() { this = TDerefBorrowNode(n, isBorrow, false) }
 
-  Expr getReceiver() { result = n.getReceiver() }
+  AstNode getNode() { result = n }
 
-  MethodCallExpr getMethodCall() { result = n }
+  predicate isBorrow() { isBorrow = true }
 
-  override predicate isArgumentOf(DataFlowCall call, RustDataFlow::ArgumentPosition pos) {
-    call.asCall() = n and pos = TReceiverArgumentPosition()
+  override CfgScope getCfgScope() { result = n.getEnclosingCfgScope() }
+
+  override Location getLocation() { result = n.getLocation() }
+
+  override string toString() {
+    if isBorrow = true then result = n + " [borrowed]" else result = n + " [dereferenced]"
   }
+}
 
-  override CfgScope getCfgScope() { result = n.getEnclosingCfgScope() }
+/**
+ * The argument of a call _after_ any implicit borrow or dereferencing
+ * has taken place.
+ */
+final class DerefBorrowArgNode extends DerefBorrowNode, ArgumentNode {
+  private DataFlowCall call_;
+  private RustDataFlow::ArgumentPosition pos_;
 
-  override Location getLocation() { result = this.getReceiver().getLocation() }
+  DerefBorrowArgNode() { isArgumentForCall(n, call_.asCall(), pos_) }
 
-  override string toString() { result = "receiver for " + this.getReceiver() }
+  override predicate isArgumentOf(DataFlowCall call, RustDataFlow::ArgumentPosition pos) {
+    call = call_ and pos = pos_
+  }
 }
 
 final class SummaryArgumentNode extends FlowSummaryNode, ArgumentNode {
@@ -414,16 +432,17 @@ final class ExprPostUpdateNode extends PostUpdateNode, TExprPostUpdateNode {
   override Location getLocation() { result = e.getLocation() }
 }
 
-final class ReceiverPostUpdateNode extends PostUpdateNode, TReceiverNode {
-  private Call call;
+final class DerefBorrowPostUpdateNode extends PostUpdateNode, TDerefBorrowNode {
+  private Expr arg;
+  private boolean isBorrow;
 
-  ReceiverPostUpdateNode() { this = TReceiverNode(call, true) }
+  DerefBorrowPostUpdateNode() { this = TDerefBorrowNode(arg, isBorrow, true) }
 
-  override Node getPreUpdateNode() { result = TReceiverNode(call, false) }
+  override DerefBorrowNode getPreUpdateNode() { result = TDerefBorrowNode(arg, isBorrow, false) }
 
-  override CfgScope getCfgScope() { result = call.getEnclosingCfgScope() }
+  override CfgScope getCfgScope() { result = arg.getEnclosingCfgScope() }
 
-  override Location getLocation() { result = call.getReceiver().getLocation() }
+  override Location getLocation() { result = arg.getLocation() }
 }
 
 final class SummaryPostUpdateNode extends FlowSummaryNode, PostUpdateNode {
@@ -486,11 +505,16 @@ newtype TNode =
         ]
     )
   } or
-  TReceiverNode(Call call, Boolean isPost) {
-    call.hasEnclosingCfgScope() and
-    call.receiverImplicitlyBorrowed() and
+  TDerefBorrowNode(AstNode n, boolean borrow, Boolean isPost) {
     // TODO: Handle index expressions as calls in data flow.
-    not call instanceof IndexExpr
+    not n = any(IndexExpr ie).getBase() and
+    (
+      TypeInference::implicitDeref(n) and
+      borrow = false
+      or
+      TypeInference::implicitBorrow(n) and
+      borrow = true
+    )
   } or
   TSsaNode(SsaImpl::DataFlowIntegration::SsaNode node) or
   TFlowSummaryNode(FlowSummaryImpl::Private::SummaryNode sn) {

rust/ql/lib/codeql/rust/elements/internal/CallImpl.qll

@@ -34,9 +34,6 @@ module Impl {
    * called in Rust.
    */
   abstract class Call extends ExprImpl::Expr {
-    /** Holds if the receiver of this call is implicitly borrowed. */
-    predicate receiverImplicitlyBorrowed() { this.implicitBorrowAt(TSelfArgumentPosition(), _) }
-
     /** Gets the trait targeted by this call, if any. */
     abstract Trait getTrait();
 
@@ -49,9 +46,6 @@ module Impl {
     /** Gets the argument at the given position, if any. */
     abstract Expr getArgument(ArgumentPosition pos);
 
-    /** Holds if the argument at `pos` might be implicitly borrowed. */
-    abstract predicate implicitBorrowAt(ArgumentPosition pos, boolean certain);
-
     /** Gets the number of arguments _excluding_ any `self` argument. */
     int getNumberOfArguments() { result = count(this.getArgument(TPositionalArgumentPosition(_))) }
 
@@ -115,8 +109,6 @@ module Impl {
 
     override Trait getTrait() { callHasTraitQualifier(this, result) }
 
-    override predicate implicitBorrowAt(ArgumentPosition pos, boolean certain) { none() }
-
     override Expr getArgument(ArgumentPosition pos) {
       result = super.getArgList().getArg(pos.asPosition())
     }
@@ -139,8 +131,6 @@ module Impl {
 
     override Trait getTrait() { callHasTraitQualifier(this, result) }
 
-    override predicate implicitBorrowAt(ArgumentPosition pos, boolean certain) { none() }
-
     override Expr getArgument(ArgumentPosition pos) {
       pos.isSelf() and result = super.getArgList().getArg(0)
       or
@@ -153,10 +143,6 @@ module Impl {
 
     override Trait getTrait() { none() }
 
-    override predicate implicitBorrowAt(ArgumentPosition pos, boolean certain) {
-      pos.isSelf() and certain = false
-    }
-
     override Expr getArgument(ArgumentPosition pos) {
       pos.isSelf() and result = this.(MethodCallExpr).getReceiver()
       or
@@ -175,15 +161,6 @@ module Impl {
 
     override Trait getTrait() { result = trait }
 
-    override predicate implicitBorrowAt(ArgumentPosition pos, boolean certain) {
-      (
-        pos.isSelf() and borrows >= 1
-        or
-        pos.asPosition() = 0 and borrows = 2
-      ) and
-      certain = true
-    }
-
     override Expr getArgument(ArgumentPosition pos) {
       pos.isSelf() and result = super.getOperand(0)
       or
@@ -196,10 +173,6 @@ module Impl {
 
     override Trait getTrait() { result.getCanonicalPath() = "core::ops::index::Index" }
 
-    override predicate implicitBorrowAt(ArgumentPosition pos, boolean certain) {
-      pos.isSelf() and certain = true
-    }
-
     override Expr getArgument(ArgumentPosition pos) {
       pos.isSelf() and result = super.getBase()
       or

rust/ql/lib/codeql/rust/elements/internal/FieldExprImpl.qll

@@ -23,10 +23,10 @@ module Impl {
    */
   class FieldExpr extends Generated::FieldExpr {
     /** Gets the record field that this access references, if any. */
-    StructField getStructField() { result = TypeInference::resolveStructFieldExpr(this) }
+    StructField getStructField() { result = TypeInference::resolveStructFieldExpr(this, _) }
 
     /** Gets the tuple field that this access references, if any. */
-    TupleField getTupleField() { result = TypeInference::resolveTupleFieldExpr(this) }
+    TupleField getTupleField() { result = TypeInference::resolveTupleFieldExpr(this, _) }
 
     override string toStringImpl() {
       exists(string abbr, string name |

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

@@ -7,13 +7,13 @@ extensions:
       - ["<futures_util::io::buf_reader::BufReader>::new", "Argument[0]", "ReturnValue", "taint", "manual"]
       - ["<_ as futures_util::io::AsyncReadExt>::read", "Argument[self]", "Argument[0].Reference", "taint", "manual"]
       - ["<_ as futures_util::io::AsyncReadExt>::read", "Argument[self].Reference", "Argument[0].Reference", "taint", "manual"]
-      - ["<_ as futures_util::io::AsyncReadExt>::read_to_end", "Argument[self]", "Argument[0].Reference", "taint", "manual"]
+      #- ["<_ as futures_util::io::AsyncReadExt>::read_to_end", "Argument[self]", "Argument[0].Reference", "taint", "manual"]
       - ["<_ as futures_util::io::AsyncReadExt>::read_to_end", "Argument[self].Reference", "Argument[0].Reference", "taint", "manual"]
-      - ["<_ as futures_util::io::AsyncBufReadExt>::read_line", "Argument[self]", "Argument[0].Reference", "taint", "manual"]
+      #- ["<_ as futures_util::io::AsyncBufReadExt>::read_line", "Argument[self]", "Argument[0].Reference", "taint", "manual"]
       - ["<_ as futures_util::io::AsyncBufReadExt>::read_line", "Argument[self].Reference", "Argument[0].Reference", "taint", "manual"]
-      - ["<_ as futures_util::io::AsyncBufReadExt>::read_until", "Argument[self]", "Argument[1].Reference", "taint", "manual"]
+      #- ["<_ as futures_util::io::AsyncBufReadExt>::read_until", "Argument[self]", "Argument[1].Reference", "taint", "manual"]
       - ["<_ as futures_util::io::AsyncBufReadExt>::read_until", "Argument[self].Reference", "Argument[1].Reference", "taint", "manual"]
-      - ["<_ as futures_util::io::AsyncBufReadExt>::fill_buf", "Argument[self]", "ReturnValue.Future.Field[core::result::Result::Ok(0)]", "taint", "manual"]
+      - ["<_ as futures_util::io::AsyncBufReadExt>::fill_buf", "Argument[self].Reference", "ReturnValue.Future.Field[core::result::Result::Ok(0)]", "taint", "manual"]
       - ["<_ as futures_util::io::AsyncBufReadExt>::lines", "Argument[self]", "ReturnValue", "taint", "manual"]
       - ["<_ as futures_io::if_std::AsyncBufRead>::poll_fill_buf", "Argument[self].Reference", "ReturnValue.Field[core::task::poll::Poll::Ready(0)].Field[core::result::Result::Ok(0)]", "taint", "manual"]
       - ["<_ as futures_io::if_std::AsyncRead>::poll_read", "Argument[self].Reference", "Argument[1].Reference", "taint", "manual"]

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

@@ -48,6 +48,6 @@ extensions:
       # String
       - ["<alloc::string::String>::as_str", "Argument[self]", "ReturnValue", "value", "manual"]
       - ["<alloc::string::String>::as_bytes", "Argument[self]", "ReturnValue", "value", "manual"]
-      - ["<_ as alloc::string::ToString>::to_string", "Argument[self]", "ReturnValue", "taint", "manual"]
+      - ["<_ as alloc::string::ToString>::to_string", "Argument[self].Reference", "ReturnValue", "taint", "manual"]
       # Vec
       - ["alloc::vec::from_elem", "Argument[0]", "ReturnValue.Element", "value", "manual"]