wip

Author: hvitved

rust/ql/lib/codeql/rust/elements/Call.qll

@@ -8,34 +8,7 @@ private import internal.CallExprImpl::Impl as CallExprImpl
 
 final class Call = Impl::Call;
 
-private predicate isGuaranteedMethodCall(ArgsExpr call) {
-  call instanceof MethodCallExpr
-  or
-  call.(Operation).isOverloaded(_, _, _)
-  or
-  call instanceof IndexExpr
-}
-
-/**
- * A call expression that targets a method.
- *
- * Either
- *
- * - a `CallExpr` where we can resolve the target as a method,
- * - a `MethodCallExpr`,
- * - an `Operation` that targets an overloadable operator, or
- * - an `IndexExpr`.
- */
-final class MethodCall extends Call {
-  MethodCall() {
-    this.getStaticTarget() instanceof Method
-    or
-    isGuaranteedMethodCall(this)
-  }
-
-  /** Gets the static target of this method call, if any. */
-  Method getStaticTarget() { result = super.getStaticTarget() }
-}
+final class MethodCall = Impl::MethodCall;
 
 /**
  * A call expression that targets a closure.

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

@@ -72,15 +72,15 @@ module Impl {
       )
     }
 
-    private predicate isMethodCall() { this.getResolvedTarget() instanceof Method }
+    override Expr getPositionalArgument(int i) { result = super.getSyntacticArgument(i) }
+  }
 
-    override Expr getPositionalArgument(int i) {
-      if this.isMethodCall()
-      then result = this.getSyntacticArgument(i + 1)
-      else result = super.getSyntacticArgument(i)
-    }
+  class CallExprMethodCall extends CallExprCall, CallImpl::MethodCall {
+    CallExprMethodCall() { this.getResolvedTarget() instanceof Method }
+
+    override Expr getPositionalArgument(int i) { result = this.getSyntacticArgument(i + 1) }
 
-    override Expr getReceiver() { this.isMethodCall() and result = super.getSyntacticArgument(0) }
+    override Expr getReceiver() { result = super.getSyntacticArgument(0) }
   }
 
   /**

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

@@ -69,4 +69,18 @@ module Impl {
       )
     }
   }
+
+  /**
+   * A call expression that targets a method.
+   *
+   * Either
+   *
+   * - a `CallExpr` where we can resolve the target as a method,
+   * - a `MethodCallExpr`,
+   * - an `Operation` that targets an overloadable operator, or
+   * - an `IndexExpr`.
+   */
+  abstract class MethodCall extends Call {
+    override Method getStaticTarget() { result = super.getStaticTarget() }
+  }
 }

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

@@ -22,7 +22,7 @@ module Impl {
    * list[42] = 1;
    * ```
    */
-  class IndexExpr extends Generated::IndexExpr, CallImpl::Call {
+  class IndexExpr extends Generated::IndexExpr, CallImpl::MethodCall {
     override string toStringImpl() {
       result =
         this.getBase().toAbbreviatedString() + "[" + this.getIndex().toAbbreviatedString() + "]"

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

@@ -27,7 +27,7 @@ module Impl {
    * x.foo::<u32, u64>(42);
    * ```
    */
-  class MethodCallExpr extends Generated::MethodCallExpr, CallImpl::Call {
+  class MethodCallExpr extends Generated::MethodCallExpr, CallImpl::MethodCall {
     private string toStringPart(int index) {
       index = 0 and
       result = this.getReceiver().toAbbreviatedString()

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

@@ -137,8 +137,8 @@ module Impl {
     }
   }
 
-  private class CallOperation extends CallImpl::Call instanceof Operation {
-    CallOperation() { super.isOverloaded(_, _, _) }
+  private class OperationMethodCall extends CallImpl::MethodCall instanceof Operation {
+    OperationMethodCall() { super.isOverloaded(_, _, _) }
 
     override Expr getPositionalArgument(int i) { result = super.getOperand(i + 1) and i >= 0 }
 

rust/ql/lib/codeql/rust/security/XssExtensions.qll

@@ -55,7 +55,7 @@ module Xss {
     HeuristicHtmlEncodingBarrier() {
       exists(Call fc |
         fc.getStaticTarget().getName().getText().regexpMatch(".*(escape|encode).*") and
-        fc.getArgument(_) = this.asExpr()
+        fc.getAPositionalArgument() = this.asExpr()
       )
     }
   }

rust/ql/test/query-tests/security/CWE-079/actix/XSS.expected

@@ -2,8 +2,9 @@
 | main.rs:25:5:25:13 | ...::new | main.rs:8:1:8:18 | to | main.rs:25:5:25:13 | ...::new | Cross-site scripting vulnerability due to a $@. | main.rs:8:1:8:18 | to | user-provided value |
 edges
 | main.rs:8:1:8:18 | to | main.rs:9:29:9:51 | ...: ...::Path::<...> | provenance | Src:MaD:2  |
-| main.rs:9:29:9:51 | ...: ...::Path::<...> | main.rs:10:22:10:38 | path.into_inner() | provenance | MaD:3 |
+| main.rs:9:29:9:51 | ...: ...::Path::<...> | main.rs:10:22:10:25 | path | provenance |  |
 | main.rs:10:9:10:18 | user_input | main.rs:13:9:22:18 | MacroExpr | provenance |  |
+| main.rs:10:22:10:25 | path | main.rs:10:22:10:38 | path.into_inner() | provenance | MaD:3 |
 | main.rs:10:22:10:38 | path.into_inner() | main.rs:10:9:10:18 | user_input | provenance |  |
 | main.rs:12:9:12:12 | html | main.rs:25:15:25:18 | html | provenance |  |
 | main.rs:13:9:22:18 | ...::format(...) | main.rs:13:9:22:18 | { ... } | provenance |  |
@@ -21,6 +22,7 @@ nodes
 | main.rs:8:1:8:18 | to | semmle.label | to |
 | main.rs:9:29:9:51 | ...: ...::Path::<...> | semmle.label | ...: ...::Path::<...> |
 | main.rs:10:9:10:18 | user_input | semmle.label | user_input |
+| main.rs:10:22:10:25 | path | semmle.label | path |
 | main.rs:10:22:10:38 | path.into_inner() | semmle.label | path.into_inner() |
 | main.rs:12:9:12:12 | html | semmle.label | html |
 | main.rs:13:9:22:18 | ...::format(...) | semmle.label | ...::format(...) |