Rust: Fallback crate resolution

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

@@ -28,18 +28,6 @@ module Impl {
     /** Gets the position of this type parameter. */
     int getPosition() { this = any(GenericParamList l).getTypeParam(result) }
 
-    private TypeBound getTypeBoundAt(int i, int j) {
-      exists(TypeBoundList tbl | result = tbl.getBound(j) |
-        tbl = this.getTypeBoundList() and i = 0
-        or
-        exists(WherePred wp |
-          wp = this.(TypeParamItemNode).getAWherePred() and
-          tbl = wp.getTypeBoundList() and
-          wp = any(WhereClause wc).getPredicate(i)
-        )
-      )
-    }
-
     /**
      * Gets the `index`th type bound of this type parameter, if any.
      *
@@ -47,7 +35,8 @@ module Impl {
      * any `where` clauses for this type parameter.
      */
     TypeBound getTypeBound(int index) {
-      result = rank[index + 1](int i, int j | | this.getTypeBoundAt(i, j) order by i, j)
+      result =
+        rank[index + 1](int i, int j | | this.(TypeParamItemNode).getTypeBoundAt(i, j) order by i, j)
     }
 
     /**
@@ -56,12 +45,7 @@ module Impl {
      * This includes type bounds directly on this type parameter and bounds from
      * any `where` clauses for this type parameter.
      */
-    TypeBound getATypeBound() {
-      // NOTE: This predicate is used in path resolution, so it can not be
-      // defined using `getTypeBound` as that would cause non-monotonic
-      // recursion due to the `rank`.
-      result = this.getTypeBoundAt(_, _)
-    }
+    TypeBound getATypeBound() { result = this.getTypeBound(_) }
 
     override string toAbbreviatedString() { result = this.getName().getText() }
 

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

@@ -232,7 +232,7 @@ abstract class ItemNode extends Locatable {
     or
     // a trait has access to the associated items of its supertraits
     this =
-      any(TraitItemNode trait |
+      any(TraitItemNodeImpl trait |
         result = trait.resolveABound().getASuccessor(name, kind) and
         kind.isExternalOrBoth() and
         result instanceof AssocItemNode and
@@ -241,14 +241,14 @@ abstract class ItemNode extends Locatable {
     or
     // items made available by an implementation where `this` is the implementing type
     exists(ItemNode node |
-      this = node.(ImplItemNode).resolveSelfTy() and
+      this = node.(ImplItemNodeImpl).resolveSelfTy() and
       result = node.getASuccessor(name, kind) and
       kind.isExternalOrBoth() and
       result instanceof AssocItemNode
     )
     or
     // trait items with default implementations made available in an implementation
-    exists(ImplItemNode impl, ItemNode trait |
+    exists(ImplItemNodeImpl impl, ItemNode trait |
       this = impl and
       trait = impl.resolveTraitTy() and
       result = trait.getASuccessor(name, kind) and
@@ -258,14 +258,14 @@ abstract class ItemNode extends Locatable {
     )
     or
     // type parameters have access to the associated items of its bounds
-    result = this.(TypeParamItemNode).resolveABound().getASuccessor(name, kind).(AssocItemNode) and
+    result = this.(TypeParamItemNodeImpl).resolveABound().getASuccessor(name, kind).(AssocItemNode) and
     kind.isExternalOrBoth()
     or
     result =
-      this.(ImplTraitTypeReprItemNode).resolveABound().getASuccessor(name, kind).(AssocItemNode) and
+      this.(ImplTraitTypeReprItemNodeImpl).resolveABound().getASuccessor(name, kind).(AssocItemNode) and
     kind.isExternalOrBoth()
     or
-    result = this.(TypeAliasItemNode).resolveAlias().getASuccessor(name, kind) and
+    result = this.(TypeAliasItemNodeImpl).resolveAlias().getASuccessor(name, kind) and
     kind.isExternalOrBoth()
     or
     name = "super" and
@@ -311,6 +311,7 @@ abstract class ItemNode extends Locatable {
   }
 
   /** Gets an _external_ successor named `name`, if any. */
+  pragma[nomagic]
   ItemNode getASuccessor(string name) {
     exists(SuccessorKind kind |
       result = this.getASuccessor(name, kind) and
@@ -627,12 +628,7 @@ abstract class ImplOrTraitItemNode extends ItemNode {
   predicate hasAssocItem(string name) { name = this.getAnAssocItem().getName() }
 }
 
-pragma[nomagic]
-private TypeParamItemNode resolveTypeParamPathTypeRepr(PathTypeRepr ptr) {
-  result = resolvePath(ptr.getPath())
-}
-
-class ImplItemNode extends ImplOrTraitItemNode instanceof Impl {
+final class ImplItemNode extends ImplOrTraitItemNode instanceof Impl {
   Path getSelfPath() { result = super.getSelfTy().(PathTypeRepr).getPath() }
 
   Path getTraitPath() { result = super.getTrait().(PathTypeRepr).getPath() }
@@ -726,7 +722,7 @@ class ImplItemNode extends ImplOrTraitItemNode instanceof Impl {
   }
 }
 
-private class ImplTraitTypeReprItemNode extends TypeItemNode instanceof ImplTraitTypeRepr {
+final private class ImplTraitTypeReprItemNode extends TypeItemNode instanceof ImplTraitTypeRepr {
   pragma[nomagic]
   Path getABoundPath() {
     result = super.getTypeBoundList().getABound().getTypeRepr().(PathTypeRepr).getPath()
@@ -748,6 +744,11 @@ private class ImplTraitTypeReprItemNode extends TypeItemNode instanceof ImplTrai
   override string getCanonicalPath(Crate c) { none() }
 }
 
+private class ImplTraitTypeReprItemNodeImpl extends ImplTraitTypeReprItemNode {
+  pragma[nomagic]
+  ItemNode resolveABound() { result = resolvePathCand(this.getABoundPath()) }
+}
+
 private class ModuleItemNode extends ModuleLikeNode instanceof Module {
   override string getName() { result = Module.super.getName().getText() }
 
@@ -790,6 +791,12 @@ private class ModuleItemNode extends ModuleLikeNode instanceof Module {
   }
 }
 
+private class ImplItemNodeImpl extends ImplItemNode {
+  TypeItemNode resolveSelfTy() { result = resolvePathCand(this.getSelfPath()) }
+
+  TraitItemNode resolveTraitTy() { result = resolvePathCand(this.getTraitPath()) }
+}
+
 private class StructItemNode extends TypeItemNode instanceof Struct {
   override string getName() { result = Struct.super.getName().getText() }
 
@@ -825,7 +832,7 @@ private class StructItemNode extends TypeItemNode instanceof Struct {
   }
 }
 
-class TraitItemNode extends ImplOrTraitItemNode, TypeItemNode instanceof Trait {
+final class TraitItemNode extends ImplOrTraitItemNode, TypeItemNode instanceof Trait {
   pragma[nomagic]
   Path getABoundPath() { result = super.getATypeBound().getTypeRepr().(PathTypeRepr).getPath() }
 
@@ -880,7 +887,12 @@ class TraitItemNode extends ImplOrTraitItemNode, TypeItemNode instanceof Trait {
   }
 }
 
-class TypeAliasItemNode extends TypeItemNode, AssocItemNode instanceof TypeAlias {
+final private class TraitItemNodeImpl extends TraitItemNode {
+  pragma[nomagic]
+  ItemNode resolveABound() { result = resolvePathCand(this.getABoundPath()) }
+}
+
+final class TypeAliasItemNode extends TypeItemNode, AssocItemNode instanceof TypeAlias {
   pragma[nomagic]
   ItemNode resolveAlias() { result = resolvePath(super.getTypeRepr().(PathTypeRepr).getPath()) }
 
@@ -899,6 +911,11 @@ class TypeAliasItemNode extends TypeItemNode, AssocItemNode instanceof TypeAlias
   override string getCanonicalPath(Crate c) { none() }
 }
 
+private class TypeAliasItemNodeImpl extends TypeAliasItemNode instanceof TypeAlias {
+  pragma[nomagic]
+  ItemNode resolveAlias() { result = resolvePathCand(super.getTypeRepr().(PathTypeRepr).getPath()) }
+}
+
 private class UnionItemNode extends TypeItemNode instanceof Union {
   override string getName() { result = Union.super.getName().getText() }
 
@@ -957,18 +974,35 @@ private class BlockExprItemNode extends ItemNode instanceof BlockExpr {
   override string getCanonicalPath(Crate c) { none() }
 }
 
-class TypeParamItemNode extends TypeItemNode instanceof TypeParam {
+pragma[nomagic]
+private Path getWherePredPath(WherePred wp) { result = wp.getTypeRepr().(PathTypeRepr).getPath() }
+
+final class TypeParamItemNode extends TypeItemNode instanceof TypeParam {
   /** Gets a where predicate for this type parameter, if any */
-  WherePred getAWherePred() {
+  pragma[nomagic]
+  private WherePred getAWherePred() {
     exists(ItemNode declaringItem |
-      this = resolveTypeParamPathTypeRepr(result.getTypeRepr()) and
+      this = resolvePath(getWherePredPath(result)) and
       result = declaringItem.getADescendant() and
       this = declaringItem.getADescendant()
     )
   }
 
   pragma[nomagic]
-  Path getABoundPath() { result = super.getATypeBound().getTypeRepr().(PathTypeRepr).getPath() }
+  TypeBound getTypeBoundAt(int i, int j) {
+    exists(TypeBoundList tbl | result = tbl.getBound(j) |
+      tbl = super.getTypeBoundList() and i = 0
+      or
+      exists(WherePred wp |
+        wp = this.getAWherePred() and
+        tbl = wp.getTypeBoundList() and
+        wp = any(WhereClause wc).getPredicate(i)
+      )
+    )
+  }
+
+  pragma[nomagic]
+  Path getABoundPath() { result = this.getTypeBoundAt(_, _).getTypeRepr().(PathTypeRepr).getPath() }
 
   pragma[nomagic]
   ItemNode resolveABound() { result = resolvePath(this.getABoundPath()) }
@@ -1016,6 +1050,37 @@ class TypeParamItemNode extends TypeItemNode instanceof TypeParam {
   override string getCanonicalPath(Crate c) { none() }
 }
 
+final private class TypeParamItemNodeImpl extends TypeParamItemNode instanceof TypeParam {
+  /** Gets a where predicate for this type parameter, if any */
+  pragma[nomagic]
+  private WherePred getAWherePred() {
+    exists(ItemNode declaringItem |
+      this = resolvePathCand(getWherePredPath(result)) and
+      result = declaringItem.getADescendant() and
+      this = declaringItem.getADescendant()
+    )
+  }
+
+  pragma[nomagic]
+  TypeBound getTypeBoundAt(int i, int j) {
+    exists(TypeBoundList tbl | result = tbl.getBound(j) |
+      tbl = super.getTypeBoundList() and i = 0
+      or
+      exists(WherePred wp |
+        wp = this.getAWherePred() and
+        tbl = wp.getTypeBoundList() and
+        wp = any(WhereClause wc).getPredicate(i)
+      )
+    )
+  }
+
+  pragma[nomagic]
+  Path getABoundPath() { result = this.getTypeBoundAt(_, _).getTypeRepr().(PathTypeRepr).getPath() }
+
+  pragma[nomagic]
+  ItemNode resolveABound() { result = resolvePathCand(this.getABoundPath()) }
+}
+
 /** Holds if `item` has the name `name` and is a top-level item inside `f`. */
 private predicate sourceFileEdge(SourceFile f, string name, ItemNode item) {
   item = f.(ItemNode).getADescendant() and
@@ -1167,12 +1232,23 @@ private class BuiltinSourceFile extends SourceFileItemNode {
   BuiltinSourceFile() { this.getFile().getParentContainer() instanceof Builtins::BuiltinsFolder }
 }
 
+pragma[nomagic]
+private predicate crateDependency(SourceFileItemNode file, string name, CrateItemNode dep) {
+  exists(CrateItemNode c | dep = c.(Crate).getDependency(name) | file = c.getASourceFile())
+}
+
 /**
  * Holds if `file` depends on crate `dep` named `name`.
  */
 pragma[nomagic]
 private predicate crateDependencyEdge(SourceFileItemNode file, string name, CrateItemNode dep) {
-  exists(CrateItemNode c | dep = c.(Crate).getDependency(name) | file = c.getASourceFile())
+  crateDependency(file, name, dep)
+  or
+  // As a fallback, give all files access to crates that do not conflict with known dependencies
+  // and declarations.
+  name = dep.getName() and
+  not declaresDirectly(file, TTypeNamespace(), name) and
+  not crateDependency(file, name, _)
 }
 
 private predicate useTreeDeclares(UseTree tree, string name) {
@@ -1192,6 +1268,18 @@ private predicate useTreeDeclares(UseTree tree, string name) {
   )
 }
 
+/**
+ * Holds if `item` explicitly declares a sub item named `name` in the
+ * namespace `ns`. This excludes items declared by `use` statements.
+ */
+pragma[nomagic]
+private predicate declaresDirectly(ItemNode item, Namespace ns, string name) {
+  exists(ItemNode child, SuccessorKind kind | child = getAChildSuccessor(item, name, kind) |
+    child.getNamespace() = ns and
+    kind.isInternalOrBoth()
+  )
+}
+
 /**
  * Holds if `item` explicitly declares a sub item named `name` in the
  * namespace `ns`. This includes items declared by `use` statements,
@@ -1199,10 +1287,7 @@ private predicate useTreeDeclares(UseTree tree, string name) {
  */
 pragma[nomagic]
 private predicate declares(ItemNode item, Namespace ns, string name) {
-  exists(ItemNode child, SuccessorKind kind | child = getAChildSuccessor(item, name, kind) |
-    child.getNamespace() = ns and
-    kind.isInternalOrBoth()
-  )
+  declaresDirectly(item, ns, name)
   or
   exists(ItemNode child |
     child.getImmediateParent() = item and
@@ -1218,7 +1303,7 @@ class RelevantPath extends Path {
   pragma[nomagic]
   predicate isUnqualified(string name) {
     not exists(this.getQualifier()) and
-    not this = any(UseTreeList list).getAUseTree().getPath() and
+    not this = any(UseTreeList list).getAUseTree().getPath().getQualifier*() and
     name = this.getText()
   }
 
@@ -1326,25 +1411,39 @@ pragma[nomagic]
 private predicate isUnqualifiedSelfPath(RelevantPath path) { path.isUnqualified("Self") }
 
 pragma[nomagic]
-private ItemNode resolvePath0(RelevantPath path, Namespace ns, SuccessorKind kind) {
+private ItemNode resolvePathCand0(RelevantPath path, Namespace ns) {
   exists(ItemNode res |
-    res = unqualifiedPathLookup(path, ns, kind) and
+    res = unqualifiedPathLookup(path, ns, _) and
     if
       not any(RelevantPath parent).getQualifier() = path and
       isUnqualifiedSelfPath(path) and
       res instanceof ImplItemNode
-    then result = res.(ImplItemNode).resolveSelfTy()
+    then result = res.(ImplItemNodeImpl).resolveSelfTy()
     else result = res
   )
   or
-  exists(ItemNode q, string name |
-    q = resolvePathQualifier(path, name) and
+  result = resolvePathCandQualified(_, _, path, ns)
+  or
+  result = resolveUseTreeListItem(_, _, path, _) and
+  ns = result.getNamespace()
+}
+
+pragma[nomagic]
+private ItemNode resolvePathCandQualifier(RelevantPath qualifier, RelevantPath path, string name) {
+  qualifier = path.getQualifier() and
+  result = resolvePathCand(qualifier) and
+  name = path.getText()
+}
+
+pragma[nomagic]
+private ItemNode resolvePathCandQualified(
+  RelevantPath qualifier, ItemNode q, RelevantPath path, Namespace ns
+) {
+  exists(string name, SuccessorKind kind |
+    q = resolvePathCandQualifier(qualifier, path, name) and
     result = getASuccessor(q, name, ns, kind) and
     kind.isExternalOrBoth()
   )
-  or
-  result = resolveUseTreeListItem(_, _, path, kind) and
-  ns = result.getNamespace()
 }
 
 /** Holds if path `p` must be looked up in namespace `n`. */
@@ -1377,11 +1476,17 @@ private predicate pathUsesNamespace(Path p, Namespace n) {
   )
 }
 
-/** Gets the item that `path` resolves to, if any. */
-cached
-ItemNode resolvePath(RelevantPath path) {
+/**
+ * Gets an item that `path` may resolve to, if any.
+ *
+ * Unlike `resolvePath`, this predicate does not attempt to make resolution
+ * of qualifiers consistent with resolution of their parents, and should
+ * only be used internally within this library.
+ */
+pragma[nomagic]
+private ItemNode resolvePathCand(RelevantPath path) {
   exists(Namespace ns |
-    result = resolvePath0(path, ns, _) and
+    result = resolvePathCand0(path, ns) and
     if path = any(ImplItemNode i).getSelfPath()
     then
       result instanceof TypeItemNode and
@@ -1389,7 +1494,10 @@ ItemNode resolvePath(RelevantPath path) {
     else
       if path = any(ImplItemNode i).getTraitPath()
       then result instanceof TraitItemNode
-      else any()
+      else
+        if path = any(PathTypeRepr p).getPath()
+        then result instanceof TypeItemNode
+        else any()
   |
     pathUsesNamespace(path, ns)
     or
@@ -1398,10 +1506,23 @@ ItemNode resolvePath(RelevantPath path) {
   )
 }
 
+/** Gets the item that `path` resolves to, if any. */
+cached
+ItemNode resolvePath(RelevantPath path) {
+  result = resolvePathCand(path) and
+  not path = any(Path parent | exists(resolvePathCand(parent))).getQualifier()
+  or
+  // if `path` is the qualifier of a resolvable `parent`, then we should
+  // resolve `path` to something consistent with what `parent` resolves to
+  exists(RelevantPath parent |
+    resolvePathCandQualified(path, result, parent, _) = resolvePathParent(path, parent)
+  )
+}
+
 pragma[nomagic]
-private ItemNode resolvePathQualifier(RelevantPath path, string name) {
-  result = resolvePath(path.getQualifier()) and
-  name = path.getText()
+private ItemNode resolvePathParent(RelevantPath path, RelevantPath parent) {
+  result = resolvePath(parent) and
+  path = parent.getQualifier()
 }
 
 private predicate isUseTreeSubPath(UseTree tree, RelevantPath path) {
@@ -1449,7 +1570,7 @@ private ItemNode resolveUseTreeListItemQualifier(
 pragma[nomagic]
 private ItemNode resolveUseTreeListItem(Use use, UseTree tree) {
   tree = use.getUseTree() and
-  result = resolvePath(tree.getPath())
+  result = resolvePathCand(tree.getPath())
   or
   result = resolveUseTreeListItem(use, tree, tree.getPath(), _)
 }
@@ -1502,21 +1623,13 @@ private predicate externCrateEdge(ExternCrateItemNode ec, string name, CrateItem
 
 pragma[nomagic]
 private predicate preludeItem(string name, ItemNode i) {
-  exists(
-    Crate stdOrCore, string stdOrCoreName, ModuleLikeNode mod, ModuleItemNode prelude,
-    ModuleItemNode rust
-  |
-    stdOrCore.getName() = stdOrCoreName and
-    stdOrCoreName = ["std", "core"] and
+  exists(Crate stdOrCore, ModuleLikeNode mod, ModuleItemNode prelude, ModuleItemNode rust |
+    stdOrCore.getName() = ["std", "core"] and
     mod = stdOrCore.getSourceFile() and
     prelude = mod.getASuccessor("prelude") and
-    rust = prelude.getASuccessor(["rust_2015", "rust_2018", "rust_2021", "rust_2024"])
-  |
+    rust = prelude.getASuccessor(["rust_2015", "rust_2018", "rust_2021", "rust_2024"]) and
     i = rust.getASuccessor(name) and
     not name = ["super", "self"]
-    or
-    name = stdOrCoreName and
-    i = stdOrCore
   )
 }
 
@@ -1533,7 +1646,7 @@ private predicate preludeItem(string name, ItemNode i) {
 pragma[nomagic]
 private predicate preludeEdge(SourceFile f, string name, ItemNode i) {
   preludeItem(name, i) and
-  not declares(f, _, name)
+  not declares(f, i.getNamespace(), name)
 }
 
 pragma[nomagic]

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

@@ -19,7 +19,9 @@ query predicate multiplePathResolutions(Path p, ItemNode i) {
     ).getPath() and
   // avoid overlap with `multipleCallTargets` below
   not p = any(CallExpr ce).getFunction().(PathExpr).getPath() and
-  strictcount(resolvePath(p)) > 1
+  // exclude crates when counting: crates can exist in many versions and configurations,
+  // we deliberately want to exhibit them all
+  strictcount(ItemNode i0 | i0 = resolvePath(p) and not i0 instanceof Crate) > 1
 }
 
 /** Holds if `call` has multiple static call targets including `target`. */

rust/ql/test/extractor-tests/macro-expansion/CONSISTENCY/PathResolutionConsistency.expected

@@ -6,5 +6,3 @@ multipleCallTargets
 | proc_macro.rs:41:5:41:10 | ...::new(...) |
 | proc_macro.rs:41:5:41:10 | ...::new(...) |
 | proc_macro.rs:44:27:44:30 | ...::to_tokens(...) |
-multiplePathResolutions
-| macro_expansion.rs:1:5:1:14 | proc_macro |

rust/ql/test/library-tests/path-resolution/CONSISTENCY/PathResolutionConsistency.expected

@@ -1,7 +1,3 @@
 multipleCallTargets
 | main.rs:118:9:118:11 | f(...) |
 | proc_macro.rs:9:5:9:10 | ...::new(...) |
-multiplePathResolutions
-| main.rs:641:3:641:12 | proc_macro |
-| main.rs:647:7:647:16 | proc_macro |
-| main.rs:650:7:650:16 | proc_macro |

rust/ql/test/library-tests/path-resolution/path-resolution.expected

@@ -276,14 +276,11 @@ resolvePath
 | main.rs:635:9:635:36 | GenericStruct::<...> | main.rs:578:5:581:5 | struct GenericStruct |
 | main.rs:635:9:635:47 | ...::call_both | main.rs:601:9:604:9 | fn call_both |
 | main.rs:635:25:635:35 | Implementor | main.rs:607:5:607:23 | struct Implementor |
-| main.rs:641:3:641:12 | proc_macro | {EXTERNAL LOCATION} | Crate(proc_macro@0.0.0) |
 | main.rs:641:3:641:12 | proc_macro | proc_macro.rs:0:0:0:0 | Crate(proc_macro@0.0.1) |
 | main.rs:641:3:641:24 | ...::add_suffix | proc_macro.rs:4:1:12:1 | fn add_suffix |
 | main.rs:645:6:645:12 | AStruct | main.rs:644:1:644:17 | struct AStruct |
-| main.rs:647:7:647:16 | proc_macro | {EXTERNAL LOCATION} | Crate(proc_macro@0.0.0) |
 | main.rs:647:7:647:16 | proc_macro | proc_macro.rs:0:0:0:0 | Crate(proc_macro@0.0.1) |
 | main.rs:647:7:647:28 | ...::add_suffix | proc_macro.rs:4:1:12:1 | fn add_suffix |
-| main.rs:650:7:650:16 | proc_macro | {EXTERNAL LOCATION} | Crate(proc_macro@0.0.0) |
 | main.rs:650:7:650:16 | proc_macro | proc_macro.rs:0:0:0:0 | Crate(proc_macro@0.0.1) |
 | main.rs:650:7:650:28 | ...::add_suffix | proc_macro.rs:4:1:12:1 | fn add_suffix |
 | main.rs:655:9:655:11 | std | {EXTERNAL LOCATION} | Crate(std@0.0.0) |

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

@@ -1,14 +0,0 @@
-multiplePathResolutions
-| test.rs:50:3:50:6 | ctor |
-| test.rs:55:3:55:6 | ctor |
-| test.rs:60:3:60:6 | ctor |
-| test.rs:65:3:65:6 | ctor |
-| test.rs:73:3:73:6 | ctor |
-| test.rs:78:3:78:6 | ctor |
-| test.rs:87:3:87:6 | ctor |
-| test.rs:94:3:94:6 | ctor |
-| test.rs:128:3:128:6 | ctor |
-| test.rs:139:3:139:6 | ctor |
-| test.rs:144:3:144:6 | ctor |
-| test.rs:150:3:150:6 | ctor |
-| test.rs:168:3:168:6 | ctor |