Rust: Refactor classes representing calls

Author: hvitved

rust/ql/.generated.list

@@ -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/.gitattributes

@@ -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/examples/snippets/simple_constant_password.ql

@@ -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 ParenArgsExprCfgNode extends Nodes::ParenArgsExprCfgNode {
+  private ParenArgsExprChildMapping node;
+
+  ParenArgsExprCfgNode() { 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/examples/snippets/simple_sql_injection.ql

@@ -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 ParenArgsExprChildMapping extends ParentAstNode, ParenArgsExpr {
+  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/CfgNodes.qll

@@ -292,9 +292,9 @@ module ExprTrees {
     }
   }
 
-  class CallExprTree extends StandardPostOrderTree instanceof CallExpr {
+  class ParenArgsExprTree extends StandardPostOrderTree instanceof ParenArgsExpr {
     override AstNode getChildNode(int i) {
-      i = 0 and result = super.getFunction()
+      i = 0 and result = super.getBase()
       or
       result = super.getArgList().getArg(i - 1)
     }
@@ -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/controlflow/internal/CfgNodes.qll

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

rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImpl.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(ClosureCall call, LambdaCallKind kind, Expr receiver) {
+  receiver = call.getClosureExpr() and
   exists(kind)
 }
 
@@ -666,8 +661,8 @@ module RustDataFlow implements InputSig<Location> {
 
   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
+    exists(ParenArgsExpr call, int pos |
+      node1.asExpr() = call.getArgument(pragma[only_bind_into](pos)) and
       node2.asExpr() = call and
       c = TTupleFieldContent(call.getTupleField(pragma[only_bind_into](pos)))
     )
@@ -818,7 +813,7 @@ module RustDataFlow implements InputSig<Location> {
       // pointer. Except if the path occurs directly in a call, then it's just a
       // call to the function and not a function being passed as data.
       resolvePath(e.(PathExpr).getPath()) = c.asCfgScope() and
-      not any(CallExpr call).getFunction() = e
+      not any(ParenArgsExpr call).getBase() = e
     )
   }
 
@@ -828,11 +823,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().(ClosureCall).getClosureExpr()
       or
       call.isSummaryCall(_, receiver.(FlowSummaryNode).getSummaryNode())
     ) and
@@ -996,9 +987,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/Content.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()
     }
   }
 
@@ -32,19 +31,19 @@ module Input implements InputSig<Location, RustDataFlow> {
   abstract class SinkBase extends SourceSinkBase { }
 
   private class CallExprFunction extends SourceBase, SinkBase {
-    private CallExpr call;
+    private ParenArgsExpr call;
 
-    CallExprFunction() { this = call.getFunction() }
+    CallExprFunction() { this = call.getBase() }
 
-    override CallExpr getCall() { result = call }
+    override ParenArgsExpr getCall() { result = call }
   }
 
   private class MethodCallExprNameRef extends SourceBase, SinkBase {
     private MethodCallExpr call;
 
     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