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
@@ -200,28 +199,23 @@ final class BreakExprCfgNode extends Nodes::BreakExprCfgNode {
   }
 }
 
-/**
- * A function or method call expression. See `CallExpr` and `MethodCallExpr` for further details.
- */
-final class CallExprBaseCfgNode extends Nodes::CallExprBaseCfgNode {
-  private CallExprBaseChildMapping node;
-
-  CallExprBaseCfgNode() { 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 { }
+final class MethodCallExprCfgNode extends Nodes::MethodCallExprCfgNode {
+  private MethodCallExprChildMapping node;
+
+  MethodCallExprCfgNode() { node = this.getAstNode() }
+
+  /** Gets the `i`th argument of this call. */
+  ExprCfgNode getPositionalArgument(int i) {
+    any(ChildMapping mapping).hasCfgChild(node, node.getPositionalArgument(i), this, result)
+  }
+}
 
 /**
  * A CFG node that calls a function.
@@ -238,7 +232,7 @@ final class CallCfgNode extends ExprCfgNode {
 
   /** Gets the receiver of this call if it is a method call. */
   ExprCfgNode getReceiver() {
-    any(ChildMapping mapping).hasCfgChild(node, node.getReceiver(), this, result)
+    any(ChildMapping mapping).hasCfgChild(node, node.(MethodCall).getReceiver(), this, result)
   }
 
   /** Gets the `i`th argument of this call, if any. */
@@ -248,15 +242,25 @@ final class CallCfgNode extends ExprCfgNode {
 }
 
 /**
- * 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 getSyntacticArgument(int i) {
+    any(ChildMapping mapping).hasCfgChild(node, node.getSyntacticArgument(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

@@ -210,14 +210,13 @@ module ExprTrees {
     override AstNode getChildNode(int i) { i = 0 and result = super.getExpr() }
   }
 
-  class BinaryOpExprTree extends StandardPostOrderTree instanceof BinaryExpr {
-    BinaryOpExprTree() { not this instanceof BinaryLogicalOperation }
-
-    override AstNode getChildNode(int i) {
-      i = 0 and result = super.getLhs()
-      or
-      i = 1 and result = super.getRhs()
+  class ArgsExprTree extends StandardPostOrderTree instanceof ArgsExpr {
+    ArgsExprTree() {
+      not this instanceof CallExpr and
+      not this instanceof BinaryLogicalOperation
     }
+
+    override AstNode getChildNode(int i) { result = super.getSyntacticArgument(i) }
   }
 
   class LogicalOrExprTree extends PostOrderTree, LogicalOrExpr {
@@ -296,7 +295,7 @@ module ExprTrees {
     override AstNode getChildNode(int i) {
       i = 0 and result = super.getFunction()
       or
-      result = super.getArgList().getArg(i - 1)
+      result = super.getSyntacticArgument(i - 1)
     }
   }
 
@@ -371,14 +370,6 @@ module ExprTrees {
     }
   }
 
-  class IndexExprTree extends StandardPostOrderTree instanceof IndexExpr {
-    override AstNode getChildNode(int i) {
-      i = 0 and result = super.getBase()
-      or
-      i = 1 and result = super.getIndex()
-    }
-  }
-
   class LetExprTree extends StandardPostOrderTree, LetExpr {
     override AstNode getChildNode(int i) {
       i = 0 and
@@ -510,12 +501,6 @@ 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)
-    }
-  }
-
   class OffsetOfExprTree extends LeafTree instanceof OffsetOfExpr { }
 
   class ParenExprTree extends ControlFlowTree, ParenExpr {
@@ -534,10 +519,6 @@ module ExprTrees {
 
   class PathExprTree extends LeafTree instanceof PathExpr { }
 
-  class PrefixExprTree extends StandardPostOrderTree instanceof PrefixExpr {
-    override AstNode getChildNode(int i) { i = 0 and result = super.getExpr() }
-  }
-
   class RangeExprTree extends StandardPostOrderTree instanceof RangeExpr {
     override AstNode getChildNode(int i) {
       i = 0 and result = super.getStart()

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

@@ -163,15 +163,15 @@ final class TuplePositionContent extends FieldContent, TTuplePositionContent {
 }
 
 /**
- * A content for the index of an argument to at function call.
+ * A content for the index of an argument to at closure call.
  *
  * Used by the model generator to create flow summaries for higher-order
  * functions.
  */
-final class FunctionCallArgumentContent extends Content, TFunctionCallArgumentContent {
+final class ClosureCallArgumentContent extends Content, TClosureCallArgumentContent {
   private int pos;
 
-  FunctionCallArgumentContent() { this = TFunctionCallArgumentContent(pos) }
+  ClosureCallArgumentContent() { this = TClosureCallArgumentContent(pos) }
 
   int getPosition() { result = pos }
 
@@ -269,8 +269,8 @@ newtype TContent =
         )]
   } or
   TFunctionCallReturnContent() or
-  TFunctionCallArgumentContent(int pos) {
-    pos in [0 .. any(CallExpr c).getArgList().getNumberOfArgs() - 1]
+  TClosureCallArgumentContent(int pos) {
+    pos in [0 .. any(ClosureCallExpr c).getNumberOfPositionalArguments()]
   } 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(
@@ -134,16 +133,14 @@ final class ArgumentPosition extends ParameterPosition {
   Expr getArgument(Call call) {
     result = call.getPositionalArgument(this.getPosition())
     or
-    result = call.getReceiver() and this.isSelf()
+    result = call.(MethodCall).getReceiver() and this.isSelf()
   }
 }
 
 /**
  * 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)
 }
 
@@ -666,10 +661,14 @@ 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
-      node2.asExpr() = call and
-      c = TTupleFieldContent(call.getTupleField(pragma[only_bind_into](pos)))
+    exists(CallExpr ce, TupleField tf, int pos |
+      node1.asExpr() = ce.getSyntacticArgument(pos) and
+      node2.asExpr() = ce and
+      c = TTupleFieldContent(tf)
+    |
+      tf = ce.(TupleStructExpr).getTupleField(pos)
+      or
+      tf = ce.(TupleVariantExpr).getTupleField(pos)
     )
     or
     exists(StructExpr re, string field |
@@ -715,7 +714,7 @@ module RustDataFlow implements InputSig<Location> {
     exists(DataFlowCall call, int i |
       isArgumentNode(node1, call, TPositionalParameterPosition(i)) and
       lambdaCall(call, _, node2.(PostUpdateNode).getPreUpdateNode()) and
-      c.(FunctionCallArgumentContent).getPosition() = i
+      c.(ClosureCallArgumentContent).getPosition() = i
     )
     or
     VariableCapture::storeStep(node1, c, node2)
@@ -824,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().(ClosureCallExpr).getClosureExpr()
       or
       call.isSummaryCall(_, receiver.(FlowSummaryNode).getSummaryNode())
     ) and
@@ -992,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/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 Call 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()
     }
   }
 
@@ -36,7 +35,7 @@ module Input implements InputSig<Location, RustDataFlow> {
 
     CallExprFunction() { this = call.getFunction() }
 
-    override CallExpr getCall() { result = call }
+    override Call getCall() { result = call }
   }
 
   private class MethodCallExprNameRef extends SourceBase, SinkBase {
@@ -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