Rust: Refactor classes representing calls

Author: hvitved

rust/ql/examples/snippets/simple_constant_password.ql

@@ -30,11 +30,11 @@ module ConstantPasswordConfig implements DataFlow::ConfigSig {
 
   predicate isSink(DataFlow::Node node) {
     // `node` is an argument whose corresponding parameter name matches the pattern "pass%"
-    exists(CallExpr call, Function target, int argIndex, Variable v |
+    exists(Call call, Function target, int argIndex, Variable v |
       call.getStaticTarget() = target and
       v.getParameter() = target.getParam(argIndex) and
       v.getText().matches("pass%") and
-      call.getArg(argIndex) = node.asExpr()
+      call.getArgument(argIndex) = node.asExpr()
     )
   }
 }

rust/ql/examples/snippets/simple_sql_injection.ql

@@ -23,9 +23,9 @@ module SqlInjectionConfig implements DataFlow::ConfigSig {
 
   predicate isSink(DataFlow::Node node) {
     // `node` is the first argument of a call to `sqlx_core::query::query`
-    exists(CallExpr call |
+    exists(Call call |
       call.getStaticTarget().getCanonicalPath() = "sqlx_core::query::query" and
-      call.getArg(0) = node.asExpr()
+      call.getArgument(0) = node.asExpr()
     )
   }
 }

rust/ql/lib/codeql/rust/controlflow/CfgNodes.qll

@@ -4,7 +4,6 @@
  */
 
 private import rust
-private import codeql.rust.elements.Call
 private import ControlFlowGraph
 private import internal.ControlFlowGraphImpl as CfgImpl
 private import internal.CfgNodes
@@ -201,28 +200,23 @@ final class BreakExprCfgNode extends Nodes::BreakExprCfgNode {
 }
 
 /**
- * A function or method call expression. See `CallExpr` and `MethodCallExpr` for further details.
+ * A method call expression. For example:
+ * ```rust
+ * x.foo(42);
+ * x.foo::<u32, u64>(42);
+ * ```
  */
-final class CallExprBaseCfgNode extends Nodes::CallExprBaseCfgNode {
-  private CallExprBaseChildMapping node;
+final class MethodCallExprCfgNode extends Nodes::MethodCallExprCfgNode {
+  private MethodCallExprChildMapping node;
 
-  CallExprBaseCfgNode() { node = this.getAstNode() }
+  MethodCallExprCfgNode() { node = this.getAstNode() }
 
   /** Gets the `i`th argument of this call. */
   ExprCfgNode getArgument(int i) {
     any(ChildMapping mapping).hasCfgChild(node, node.getArgList().getArg(i), this, result)
   }
 }
 
-/**
- * A method call expression. For example:
- * ```rust
- * x.foo(42);
- * x.foo::<u32, u64>(42);
- * ```
- */
-final class MethodCallExprCfgNode extends CallExprBaseCfgNode, Nodes::MethodCallExprCfgNode { }
-
 /**
  * A CFG node that calls a function.
  *
@@ -242,21 +236,31 @@ final class CallCfgNode extends ExprCfgNode {
   }
 
   /** Gets the `i`th argument of this call, if any. */
-  ExprCfgNode getPositionalArgument(int i) {
-    any(ChildMapping mapping).hasCfgChild(node, node.getPositionalArgument(i), this, result)
+  ExprCfgNode getArgument(int i) {
+    any(ChildMapping mapping).hasCfgChild(node, node.getArgument(i), this, result)
   }
 }
 
 /**
- * A function call expression. For example:
+ * An expression with parenthesized arguments. For example:
  * ```rust
  * foo(42);
  * foo::<u32, u64>(42);
  * foo[0](42);
  * foo(1) = 4;
+ * Option::Some(42);
  * ```
  */
-final class CallExprCfgNode extends CallExprBaseCfgNode, Nodes::CallExprCfgNode { }
+final class CallExprCfgNode extends Nodes::CallExprCfgNode {
+  private CallExprChildMapping node;
+
+  CallExprCfgNode() { node = this.getAstNode() }
+
+  /** Gets the `i`th argument of this call. */
+  ExprCfgNode getArgument(int i) {
+    any(ChildMapping mapping).hasCfgChild(node, node.getArgList().getArg(i), this, result)
+  }
+}
 
 /**
  * A FormatArgsExpr. For example:

rust/ql/lib/codeql/rust/controlflow/internal/CfgNodes.qll

@@ -57,8 +57,12 @@ class BreakExprTargetChildMapping extends ParentAstNode, Expr {
   override predicate relevantChild(AstNode child) { child.(BreakExpr).getTarget() = this }
 }
 
-class CallExprBaseChildMapping extends ParentAstNode, CallExprBase {
-  override predicate relevantChild(AstNode child) { child = this.getAnArg() }
+class CallExprChildMapping extends ParentAstNode, CallExpr {
+  override predicate relevantChild(AstNode child) { child = this.getArgList().getAnArg() }
+}
+
+class MethodCallExprChildMapping extends ParentAstNode, MethodCallExpr {
+  override predicate relevantChild(AstNode child) { child = this.getArgList().getAnArg() }
 }
 
 class StructExprChildMapping extends ParentAstNode, StructExpr {

rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImpl.qll

@@ -512,7 +512,7 @@ module ExprTrees {
 
   class MethodCallExprTree extends StandardPostOrderTree, MethodCallExpr {
     override AstNode getChildNode(int i) {
-      if i = 0 then result = this.getReceiver() else result = this.getArg(i - 1)
+      if i = 0 then result = this.getReceiver() else result = this.getArgList().getArg(i - 1)
     }
   }
 

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

@@ -269,8 +269,6 @@ newtype TContent =
         )]
   } or
   TFunctionCallReturnContent() or
-  TFunctionCallArgumentContent(int pos) {
-    pos in [0 .. any(CallExpr c).getArgList().getNumberOfArgs() - 1]
-  } or
+  TFunctionCallArgumentContent(int pos) { pos in [0 .. any(CallExpr c).getNumberOfArguments() - 1] } or
   TCapturedVariableContent(VariableCapture::CapturedVariable v) or
   TReferenceContent()

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

@@ -8,7 +8,6 @@ private import codeql.util.Boolean
 private import codeql.dataflow.DataFlow
 private import codeql.dataflow.internal.DataFlowImpl
 private import rust
-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
@@ -57,7 +56,7 @@ final class DataFlowCallable extends TDataFlowCallable {
 }
 
 final class DataFlowCall extends TDataFlowCall {
-  /** Gets the underlying call in the CFG, if any. */
+  /** Gets the underlying call, if any. */
   Call asCall() { this = TCall(result) }
 
   predicate isSummaryCall(
@@ -132,7 +131,7 @@ final class ParameterPosition extends TParameterPosition {
 final class ArgumentPosition extends ParameterPosition {
   /** Gets the argument of `call` at this position, if any. */
   Expr getArgument(Call call) {
-    result = call.getPositionalArgument(this.getPosition())
+    result = call.getArgument(this.getPosition())
     or
     result = call.getReceiver() and this.isSelf()
   }
@@ -142,8 +141,6 @@ final class ArgumentPosition extends ParameterPosition {
  * Holds if `arg` is an argument of `call` at the position `pos`.
  */
 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)
 }
 
@@ -293,10 +290,8 @@ predicate lambdaCreationExpr(Expr creation) {
  * Holds if `call` is a lambda call of kind `kind` where `receiver` is the
  * invoked expression.
  */
-predicate lambdaCallExpr(CallExpr call, LambdaCallKind kind, Expr receiver) {
-  receiver = call.getFunction() and
-  // All calls to complex expressions and local variable accesses are lambda call.
-  (receiver instanceof PathExpr implies receiver = any(Variable v).getAnAccess()) and
+predicate lambdaCallExpr(ClosureCallExpr call, LambdaCallKind kind, Expr receiver) {
+  receiver = call.getClosureExpr() and
   exists(kind)
 }
 
@@ -664,12 +659,19 @@ module RustDataFlow implements InputSig<Location> {
     )
   }
 
+  pragma[nomagic]
+  private TupleField getCallExprTupleField(CallExpr call, int pos) {
+    result = call.(TupleStructExpr).getStruct().getTupleField(pos)
+    or
+    result = call.(TupleVariantExpr).getVariant().getTupleField(pos)
+  }
+
   pragma[nomagic]
   additional predicate storeContentStep(Node node1, Content c, Node node2) {
     exists(CallExpr call, int pos |
-      node1.asExpr() = call.getArg(pragma[only_bind_into](pos)) and
+      node1.asExpr() = call.getArgument(pragma[only_bind_into](pos)) and
       node2.asExpr() = call and
-      c = TTupleFieldContent(call.getTupleField(pragma[only_bind_into](pos)))
+      c = TTupleFieldContent(getCallExprTupleField(call, pragma[only_bind_into](pos)))
     )
     or
     exists(StructExpr re, string field |
@@ -824,11 +826,7 @@ module RustDataFlow implements InputSig<Location> {
    */
   predicate lambdaCall(DataFlowCall call, LambdaCallKind kind, Node receiver) {
     (
-      receiver.asExpr() = call.asCall().(CallExpr).getFunction() and
-      // All calls to complex expressions and local variable accesses are lambda call.
-      exists(Expr f | f = receiver.asExpr() |
-        f instanceof PathExpr implies f = any(Variable v).getAnAccess()
-      )
+      receiver.asExpr() = call.asCall().(ClosureCallExpr).getClosureExpr()
       or
       call.isSummaryCall(_, receiver.(FlowSummaryNode).getSummaryNode())
     ) and
@@ -992,9 +990,7 @@ private module Cached {
   newtype TDataFlowCall =
     TCall(Call call) {
       Stages::DataFlowStage::ref() and
-      call.hasEnclosingCfgScope() and
-      // TODO: Handle index expressions as calls in data flow.
-      not call instanceof IndexExpr
+      call.hasEnclosingCfgScope()
     } or
     TSummaryCall(
       FlowSummaryImpl::Public::SummarizedCallable c, FlowSummaryImpl::Private::SummaryNode receiver

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

@@ -12,18 +12,17 @@ private import codeql.rust.dataflow.Ssa
 private import Content
 
 module Input implements InputSig<Location, RustDataFlow> {
-  private import codeql.rust.elements.internal.CallExprBaseImpl::Impl as CallExprBaseImpl
   private import codeql.rust.frameworks.stdlib.Stdlib
 
   class SummarizedCallableBase = Function;
 
   abstract private class SourceSinkBase extends AstNode {
     /** Gets the associated call. */
-    abstract CallExprBase getCall();
+    abstract ArgsExpr getCall();
 
     /** Holds if the associated call resolves to `path`. */
     final predicate callResolvesTo(string path) {
-      path = this.getCall().getStaticTarget().(Addressable).getCanonicalPath()
+      path = this.getCall().getResolvedTarget().getCanonicalPath()
     }
   }
 
@@ -44,7 +43,7 @@ module Input implements InputSig<Location, RustDataFlow> {
 
     MethodCallExprNameRef() { this = call.getIdentifier() }
 
-    override MethodCallExpr getCall() { result = call }
+    override ArgsExpr getCall() { result = call }
   }
 
   RustDataFlow::ArgumentPosition callbackSelfParameterPosition() { result.isClosureSelf() }
@@ -53,9 +52,7 @@ module Input implements InputSig<Location, RustDataFlow> {
 
   string encodeParameterPosition(ParameterPosition pos) { result = pos.toString() }
 
-  string encodeArgumentPosition(RustDataFlow::ArgumentPosition pos) {
-    result = encodeParameterPosition(pos)
-  }
+  string encodeArgumentPosition(RustDataFlow::ArgumentPosition pos) { result = pos.toString() }
 
   string encodeContent(ContentSet cs, string arg) {
     exists(Content c | cs = TSingletonContentSet(c) |
@@ -173,7 +170,7 @@ private module StepsInput implements Impl::Private::StepsInputSig {
   }
 
   RustDataFlow::Node getSinkNode(Input::SinkBase sink, Impl::Private::SummaryComponent sc) {
-    exists(CallExprBase call, Expr arg, ArgumentPosition pos |
+    exists(ArgsExpr call, Expr arg, ArgumentPosition pos |
       result.asExpr() = arg and
       sc = Impl::Private::SummaryComponent::argument(pos) and
       call = sink.getCall() and

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

@@ -47,7 +47,6 @@ private import rust
 private import codeql.rust.dataflow.FlowSummary
 private import codeql.rust.dataflow.FlowSource
 private import codeql.rust.dataflow.FlowSink
-private import codeql.rust.elements.internal.CallExprBaseImpl::Impl as CallExprBaseImpl
 
 /**
  * Holds if in a call to the function with canonical path `path`, the value referred

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

@@ -294,13 +294,12 @@ final class SummaryArgumentNode extends FlowSummaryNode, ArgumentNode {
  * passed into the closure body at an invocation.
  */
 final class ClosureArgumentNode extends ArgumentNode, ExprNode {
-  private CallExpr call_;
+  private Call call_;
 
   ClosureArgumentNode() { lambdaCallExpr(call_, _, this.asExpr()) }
 
   override predicate isArgumentOf(DataFlowCall call, RustDataFlow::ArgumentPosition pos) {
-    call.asCall() = call_ and
-    pos.isClosureSelf()
+    call.asCall() = call_ and pos.isClosureSelf()
   }
 }
 
@@ -527,7 +526,6 @@ newtype TNode =
       or
       e =
         [
-          any(IndexExpr i).getBase(), //
           any(FieldExpr access).getContainer(), //
           any(TryExpr try).getExpr(), //
           any(AwaitExpr a).getExpr(), //