Merge branch 'main' into redsun82/rust-analyzer-update

Author: Paolo Tranquilli

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

@@ -18,6 +18,8 @@ private module Input implements InputSig<Location, RustDataFlow> {
     // 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()
+    or
+    n.(Node::PostUpdateNode).getPreUpdateNode().asExpr() = getPostUpdateReverseStep(_, _)
   }
 
   predicate missingLocationExclude(RustDataFlow::Node n) { not exists(n.asExpr().getLocation()) }

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

@@ -90,10 +90,10 @@ final class DataFlowCall extends TDataFlowCall {
 }
 
 /**
- * The position of a parameter or an argument in a function or call.
+ * The position of a parameter in a function.
  *
- * As there is a 1-to-1 correspondence between parameter positions and
- * arguments positions in Rust we use the same type for both.
+ * In Rust there is a 1-to-1 correspondence between parameter positions and
+ * arguments positions, so we use the same underlying type for both.
  */
 final class ParameterPosition extends TParameterPosition {
   /** Gets the underlying integer position, if any. */
@@ -126,6 +126,22 @@ final class ParameterPosition extends TParameterPosition {
   }
 }
 
+/**
+ * The position of an argument in a call.
+ *
+ * In Rust there is a 1-to-1 correspondence between parameter positions and
+ * arguments positions, so we use the same underlying type for both.
+ */
+final class ArgumentPosition extends ParameterPosition {
+  /** Gets the argument of `call` at this position, if any. */
+  Expr getArgument(CallExprBase call) {
+    result = call.getArgList().getArg(this.getPosition())
+    or
+    this.isSelf() and
+    result = call.(MethodCallExpr).getReceiver()
+  }
+}
+
 /** Holds if `call` invokes a qualified path that resolves to a method. */
 private predicate callToMethod(CallExpr call) {
   exists(Path path |
@@ -197,6 +213,28 @@ private ExprCfgNode getALastEvalNode(ExprCfgNode e) {
   result.(BreakExprCfgNode).getTarget() = e
 }
 
+/**
+ * Holds if a reverse local flow step should be added from the post-update node
+ * for `e` to the post-update node for the result.
+ *
+ * This is needed to allow for side-effects on compound expressions to propagate
+ * to sub components. For example, in
+ *
+ * ```rust
+ * ({ foo(); &mut a}).set_data(taint);
+ * ```
+ *
+ * we add a reverse flow step from `[post] { foo(); &mut a}` to `[post] &mut a`,
+ * in order for the side-effect of `set_data` to reach `&mut a`.
+ */
+ExprCfgNode getPostUpdateReverseStep(ExprCfgNode e, boolean preservesValue) {
+  result = getALastEvalNode(e) and
+  preservesValue = true
+  or
+  result = e.(CastExprCfgNode).getExpr() and
+  preservesValue = false
+}
+
 module LocalFlow {
   predicate flowSummaryLocalStep(Node nodeFrom, Node nodeTo, string model) {
     exists(FlowSummaryImpl::Public::SummarizedCallable c |
@@ -258,6 +296,9 @@ module LocalFlow {
     // 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)
   }
 }
 
@@ -432,6 +473,8 @@ private module Aliases {
 
   class ParameterPositionAlias = ParameterPosition;
 
+  class ArgumentPositionAlias = ArgumentPosition;
+
   class ContentAlias = Content;
 
   class ContentSetAlias = ContentSet;
@@ -550,7 +593,7 @@ module RustDataFlow implements InputSig<Location> {
 
   class ParameterPosition = ParameterPositionAlias;
 
-  class ArgumentPosition = ParameterPosition;
+  class ArgumentPosition = ArgumentPositionAlias;
 
   /**
    * Holds if the parameter position `ppos` matches the argument position

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

@@ -58,7 +58,9 @@ module Input implements InputSig<Location, RustDataFlow> {
 
   string encodeParameterPosition(ParameterPosition pos) { result = pos.toString() }
 
-  predicate encodeArgumentPosition = encodeParameterPosition/1;
+  string encodeArgumentPosition(RustDataFlow::ArgumentPosition pos) {
+    result = encodeParameterPosition(pos)
+  }
 
   string encodeContent(ContentSet cs, string arg) {
     exists(Content c | cs = TSingletonContentSet(c) |
@@ -146,17 +148,21 @@ private module StepsInput implements Impl::Private::StepsInputSig {
   RustDataFlow::Node getSourceNode(Input::SourceBase source, Impl::Private::SummaryComponent sc) {
     sc = Impl::Private::SummaryComponent::return(_) and
     result.asExpr().getExpr() = source.getCall()
+    or
+    exists(CallExprBase call, Expr arg, ArgumentPosition pos |
+      result.(RustDataFlow::PostUpdateNode).getPreUpdateNode().asExpr().getExpr() = arg and
+      sc = Impl::Private::SummaryComponent::argument(pos) and
+      call = source.getCall() and
+      arg = pos.getArgument(call)
+    )
   }
 
   RustDataFlow::Node getSinkNode(Input::SinkBase sink, Impl::Private::SummaryComponent sc) {
-    exists(CallExprBase call, Expr arg, ParameterPosition pos |
+    exists(CallExprBase call, Expr arg, ArgumentPosition pos |
       result.asExpr().getExpr() = arg and
       sc = Impl::Private::SummaryComponent::argument(pos) and
-      call = sink.getCall()
-    |
-      arg = call.getArgList().getArg(pos.getPosition())
-      or
-      arg = call.(MethodCallExpr).getReceiver() and pos.isSelf()
+      call = sink.getCall() and
+      arg = pos.getArgument(call)
     )
   }
 }

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

@@ -29,7 +29,7 @@ abstract class NodePublic extends TNode {
   /**
    * Gets the expression that corresponds to this node, if any.
    */
-  ExprCfgNode asExpr() { none() }
+  final ExprCfgNode asExpr() { this = TExprNode(result) }
 
   /**
    * Gets the parameter that corresponds to this node, if any.
@@ -39,7 +39,7 @@ abstract class NodePublic extends TNode {
   /**
    * Gets the pattern that corresponds to this node, if any.
    */
-  PatCfgNode asPat() { none() }
+  final PatCfgNode asPat() { this = TPatNode(result) }
 }
 
 abstract class Node extends NodePublic {
@@ -144,16 +144,12 @@ class ExprNode extends AstCfgFlowNode, TExprNode {
   override ExprCfgNode n;
 
   ExprNode() { this = TExprNode(n) }
-
-  override ExprCfgNode asExpr() { result = n }
 }
 
 final class PatNode extends AstCfgFlowNode, TPatNode {
   override PatCfgNode n;
 
   PatNode() { this = TPatNode(n) }
-
-  override PatCfgNode asPat() { result = n }
 }
 
 /** A data flow node that corresponds to a name node in the CFG. */
@@ -467,10 +463,12 @@ newtype TNode =
     or
     e =
       [
-        any(IndexExprCfgNode i).getBase(), any(FieldExprCfgNode access).getExpr(),
-        any(TryExprCfgNode try).getExpr(),
-        any(PrefixExprCfgNode pe | pe.getOperatorName() = "*").getExpr(),
-        any(AwaitExprCfgNode a).getExpr(), any(MethodCallExprCfgNode mc).getReceiver()
+        any(IndexExprCfgNode i).getBase(), //
+        any(FieldExprCfgNode access).getExpr(), //
+        any(TryExprCfgNode try).getExpr(), //
+        any(PrefixExprCfgNode pe | pe.getOperatorName() = "*").getExpr(), //
+        any(AwaitExprCfgNode a).getExpr(), any(MethodCallExprCfgNode mc).getReceiver(), //
+        getPostUpdateReverseStep(any(PostUpdateNode n).getPreUpdateNode().asExpr(), _)
       ]
   } or
   TReceiverNode(MethodCallExprCfgNode mc, Boolean isPost) or

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

@@ -332,6 +332,8 @@ import Cached
 private import codeql.rust.dataflow.Ssa
 
 private module DataFlowIntegrationInput implements Impl::DataFlowIntegrationInputSig {
+  private import codeql.rust.dataflow.internal.DataFlowImpl as DataFlowImpl
+
   class Expr extends CfgNodes::AstCfgNode {
     predicate hasCfgNode(SsaInput::BasicBlock bb, int i) { this = bb.getNode(i) }
   }
@@ -343,14 +345,22 @@ private module DataFlowIntegrationInput implements Impl::DataFlowIntegrationInpu
     none() // handled in `DataFlowImpl.qll` instead
   }
 
+  private predicate isArg(CfgNodes::CallExprBaseCfgNode call, CfgNodes::ExprCfgNode e) {
+    call.getArgument(_) = e
+    or
+    call.(CfgNodes::MethodCallExprCfgNode).getReceiver() = e
+    or
+    exists(CfgNodes::ExprCfgNode mid |
+      isArg(call, mid) and
+      e = DataFlowImpl::getPostUpdateReverseStep(mid, _)
+    )
+  }
+
   predicate allowFlowIntoUncertainDef(UncertainWriteDefinition def) {
     exists(CfgNodes::CallExprBaseCfgNode call, Variable v, BasicBlock bb, int i |
       def.definesAt(v, bb, i) and
-      mutablyBorrows(bb.getNode(i).getAstNode(), v)
-    |
-      call.getArgument(_) = bb.getNode(i)
-      or
-      call.(CfgNodes::MethodCallExprCfgNode).getReceiver() = bb.getNode(i)
+      mutablyBorrows(bb.getNode(i).getAstNode(), v) and
+      isArg(call, bb.getNode(i))
     )
   }
 

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

@@ -53,6 +53,9 @@ module RustTaintTracking implements InputSig<Location, RustDataFlow> {
       exists(FormatArgsExprCfgNode format | succ.asExpr() = format |
         pred.asExpr() = [format.getArgumentExpr(_), format.getFormatTemplateVariableAccess(_)]
       )
+      or
+      succ.(Node::PostUpdateNode).getPreUpdateNode().asExpr() =
+        getPostUpdateReverseStep(pred.(Node::PostUpdateNode).getPreUpdateNode().asExpr(), false)
     )
     or
     FlowSummaryImpl::Private::Steps::summaryLocalStep(pred.(Node::FlowSummaryNode).getSummaryNode(),