Merge pull request #20502 from hvitved/rust/path-resolution-check-arity

Rust: Check call arities in path resolution

Author: hvitved

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

@@ -77,7 +77,7 @@ class CallableScopeTree extends StandardTree, PreOrderTree, PostOrderTree, Scope
 
   override AstNode getChildNode(int i) {
     i = 0 and
-    result = this.getParamList().getSelfParam()
+    result = this.getSelfParam()
     or
     result = this.getParam(i - 1)
     or

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

@@ -102,7 +102,7 @@ module Impl {
       f = resolvePath(path) and
       path.getSegment().getIdentifier().getText() = methodName and
       exists(SelfParam self |
-        self = f.getParamList().getSelfParam() and
+        self = f.getSelfParam() and
         if self.isRef() then selfIsRef = true else selfIsRef = false
       )
     )

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

@@ -17,5 +17,25 @@ module Impl {
    */
   class Callable extends Generated::Callable {
     override Param getParam(int index) { result = this.getParamList().getParam(index) }
+
+    /**
+     * Gets the self parameter of this callable, if it exists.
+     */
+    SelfParam getSelfParam() { result = this.getParamList().getSelfParam() }
+
+    /**
+     * Holds if `getSelfParam()` exists.
+     */
+    predicate hasSelfParam() { exists(this.getSelfParam()) }
+
+    /**
+     * Gets the number of parameters of this callable, including `self` if it exists.
+     */
+    int getNumberOfParamsInclSelf() {
+      exists(int arr |
+        arr = this.getNumberOfParams() and
+        if this.hasSelfParam() then result = arr + 1 else result = arr
+      )
+    }
   }
 }

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

@@ -109,7 +109,7 @@ module Impl {
       text = name.getText() and
       // exclude self parameters from functions without a body as these are
       // trait method declarations without implementations
-      not exists(Function f | not f.hasBody() and f.getParamList().getSelfParam() = sp)
+      not exists(Function f | not f.hasBody() and f.getSelfParam() = sp)
     )
     or
     exists(IdentPat pat |

rust/ql/lib/codeql/rust/internal/PathResolution.qll

@@ -4,6 +4,7 @@
 
 private import rust
 private import codeql.rust.elements.internal.generated.ParentChild
+private import codeql.rust.elements.internal.CallExprImpl::Impl as CallExprImpl
 private import codeql.rust.internal.CachedStages
 private import codeql.rust.frameworks.stdlib.Builtins as Builtins
 private import codeql.util.Option
@@ -604,7 +605,13 @@ private class EnumItemNode extends TypeItemNode instanceof Enum {
   }
 }
 
-private class VariantItemNode extends ItemNode instanceof Variant {
+/** An item that can be referenced with arguments. */
+abstract class ParameterizableItemNode extends ItemNode {
+  /** Gets the arity this item. */
+  abstract int getArity();
+}
+
+private class VariantItemNode extends ParameterizableItemNode instanceof Variant {
   override string getName() { result = Variant.super.getName().getText() }
 
   override Namespace getNamespace() {
@@ -617,6 +624,8 @@ private class VariantItemNode extends ItemNode instanceof Variant {
 
   override Visibility getVisibility() { result = super.getEnum().getVisibility() }
 
+  override int getArity() { result = super.getFieldList().(TupleFieldList).getNumberOfFields() }
+
   override predicate hasCanonicalPath(Crate c) { this.hasCanonicalPathPrefix(c) }
 
   bindingset[c]
@@ -638,7 +647,7 @@ private class VariantItemNode extends ItemNode instanceof Variant {
   }
 }
 
-class FunctionItemNode extends AssocItemNode instanceof Function {
+class FunctionItemNode extends AssocItemNode, ParameterizableItemNode instanceof Function {
   override string getName() { result = Function.super.getName().getText() }
 
   override predicate hasImplementation() { Function.super.hasImplementation() }
@@ -648,6 +657,8 @@ class FunctionItemNode extends AssocItemNode instanceof Function {
   override TypeParam getTypeParam(int i) { result = super.getGenericParamList().getTypeParam(i) }
 
   override Visibility getVisibility() { result = Function.super.getVisibility() }
+
+  override int getArity() { result = super.getNumberOfParamsInclSelf() }
 }
 
 abstract class ImplOrTraitItemNode extends ItemNode {
@@ -712,8 +723,10 @@ final class ImplItemNode extends ImplOrTraitItemNode instanceof Impl {
   TypeParamItemNode getBlanketImplementationTypeParam() { result = this.resolveSelfTy() }
 
   /**
-   * Holds if this impl block is a blanket implementation. That is, the
+   * Holds if this impl block is a [blanket implementation][1]. That is, the
    * implementation targets a generic parameter of the impl block.
+   *
+   * [1]: https://doc.rust-lang.org/book/ch10-02-traits.html#using-trait-bounds-to-conditionally-implement-methods
    */
   predicate isBlanketImplementation() { exists(this.getBlanketImplementationTypeParam()) }
 
@@ -865,7 +878,7 @@ private class ImplItemNodeImpl extends ImplItemNode {
   TraitItemNode resolveTraitTyCand() { result = resolvePathCand(this.getTraitPath()) }
 }
 
-private class StructItemNode extends TypeItemNode instanceof Struct {
+private class StructItemNode extends TypeItemNode, ParameterizableItemNode instanceof Struct {
   override string getName() { result = Struct.super.getName().getText() }
 
   override Namespace getNamespace() {
@@ -877,6 +890,8 @@ private class StructItemNode extends TypeItemNode instanceof Struct {
 
   override Visibility getVisibility() { result = Struct.super.getVisibility() }
 
+  override int getArity() { result = super.getFieldList().(TupleFieldList).getNumberOfFields() }
+
   override TypeParam getTypeParam(int i) { result = super.getGenericParamList().getTypeParam(i) }
 
   override predicate hasCanonicalPath(Crate c) { this.hasCanonicalPathPrefix(c) }
@@ -1687,6 +1702,14 @@ private ItemNode resolvePathCand(RelevantPath path) {
     or
     not pathUsesNamespace(path, _) and
     not path = any(MacroCall mc).getPath()
+  ) and
+  (
+    not path = CallExprImpl::getFunctionPath(_)
+    or
+    exists(CallExpr ce |
+      path = CallExprImpl::getFunctionPath(ce) and
+      result.(ParameterizableItemNode).getArity() = ce.getNumberOfArgs()
+    )
   )
 }
 

rust/ql/lib/codeql/rust/internal/TypeInference.qll

@@ -235,7 +235,7 @@ module Consistency {
     // Suppress the inconsistency if `n` is a self parameter and the type
     // mention for the self type has multiple types for a path.
     not exists(ImplItemNode impl, TypePath selfTypePath |
-      n = impl.getAnAssocItem().(Function).getParamList().getSelfParam() and
+      n = impl.getAnAssocItem().(Function).getSelfParam() and
       strictcount(impl.(Impl).getSelfTy().(TypeMention).resolveTypeAt(selfTypePath)) > 1
     )
   }
@@ -953,7 +953,7 @@ private module CallExprBaseMatchingInput implements MatchingInputSig {
       )
       or
       exists(SelfParam self |
-        self = pragma[only_bind_into](this.getParamList().getSelfParam()) and
+        self = pragma[only_bind_into](this.getSelfParam()) and
         dpos.isSelf() and
         result = inferAnnotatedType(self, path) // `self` parameter with type annotation
       )
@@ -977,7 +977,7 @@ private module CallExprBaseMatchingInput implements MatchingInputSig {
       exists(ImplOrTraitItemNode i |
         this = i.getAnAssocItem() and
         dpos.isSelf() and
-        not this.getParamList().hasSelfParam()
+        not this.hasSelfParam()
       |
         result = TSelfTypeParameter(i) and
         path.isEmpty()
@@ -1905,7 +1905,7 @@ private predicate methodCandidate(Type type, string name, int arity, Impl impl)
   type = impl.getSelfTy().(TypeMention).resolveType() and
   exists(Function f |
     f = impl.(ImplItemNode).getASuccessor(name) and
-    f.getParamList().hasSelfParam() and
+    f.hasSelfParam() and
     arity = f.getParamList().getNumberOfParams()
   )
 }
@@ -2227,7 +2227,7 @@ private module BlanketImplementation {
   ) {
     isCanonicalImpl(impl) and
     blanketImplementationTraitBound(impl, traitBound) and
-    f.getParamList().hasSelfParam() and
+    f.hasSelfParam() and
     arity = f.getParamList().getNumberOfParams() and
     (
       f = impl.getAssocItem(name)
@@ -2337,7 +2337,7 @@ private Function resolveMethodCallTarget(MethodCall mc) {
 pragma[nomagic]
 private predicate assocFuncResolutionDependsOnArgument(Function f, Impl impl, int pos) {
   functionResolutionDependsOnArgument(impl, _, f, pos, _, _) and
-  not f.getParamList().hasSelfParam()
+  not f.hasSelfParam()
 }
 
 private class FunctionCallExpr extends CallImpl::CallExprCall {

rust/ql/test/query-tests/security/CWE-327/CONSISTENCY/PathResolutionConsistency.expected

@@ -1,7 +1,2 @@
 multipleCallTargets
-| test_cipher.rs:20:27:20:48 | ...::new(...) |
-| test_cipher.rs:26:27:26:48 | ...::new(...) |
-| test_cipher.rs:29:27:29:48 | ...::new(...) |
-| test_cipher.rs:36:30:36:59 | ...::new(...) |
-| test_cipher.rs:39:30:39:63 | ...::new(...) |
 | test_cipher.rs:114:23:114:50 | ...::new(...) |