From 97532525d85c838d58625247905ca2f881cac144 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Thu, 24 Apr 2025 11:03:59 +0200 Subject: [PATCH 1/2] Rust: Crate graph extraction workarounds --- .../codeql/rust/internal/PathResolution.qll | 35 +++++++++++++++-- rust/ql/lib/codeql/rust/internal/Type.qll | 39 ++++++++++++++----- 2 files changed, 61 insertions(+), 13 deletions(-) diff --git a/rust/ql/lib/codeql/rust/internal/PathResolution.qll b/rust/ql/lib/codeql/rust/internal/PathResolution.qll index a6cc51a21c54..37a895c0dcd4 100644 --- a/rust/ql/lib/codeql/rust/internal/PathResolution.qll +++ b/rust/ql/lib/codeql/rust/internal/PathResolution.qll @@ -323,7 +323,16 @@ abstract private class AssocItemNode extends ItemNode, AssocItem { private class ConstItemNode extends AssocItemNode instanceof Const { override string getName() { result = Const.super.getName().getText() } - override predicate hasImplementation() { super.hasBody() } + override predicate hasImplementation() { + super.hasBody() + or + // for trait items from library code, we do not currently know if they + // have default implementations or not, so we assume they do + exists(TraitItemNode t | + this = t.getAnAssocItem() and + not this.fromSource() + ) + } override Namespace getNamespace() { result.isValue() } @@ -359,7 +368,16 @@ private class VariantItemNode extends ItemNode instanceof Variant { class FunctionItemNode extends AssocItemNode instanceof Function { override string getName() { result = Function.super.getName().getText() } - override predicate hasImplementation() { super.hasBody() } + override predicate hasImplementation() { + super.hasBody() + or + // for trait items from library code, we do not currently know if they + // have default implementations or not, so we assume they do + exists(TraitItemNode t | + this = t.getAnAssocItem() and + not this.fromSource() + ) + } override Namespace getNamespace() { result.isValue() } @@ -862,6 +880,12 @@ class RelevantPath extends Path { this.getQualifier().(RelevantPath).isCratePath("$crate", _) and this.getText() = name } + + // TODO: Remove once the crate graph extractor generates publicly visible paths + predicate requiresExtractorWorkaround() { + not this.fromSource() and + this = any(RelevantPath p).getQualifier() + } } private predicate isModule(ItemNode m) { m instanceof Module } @@ -1029,6 +1053,7 @@ pragma[nomagic] private ItemNode resolvePathPrivate( RelevantPath path, ModuleLikeNode itemParent, ModuleLikeNode pathParent ) { + not path.requiresExtractorWorkaround() and result = resolvePath1(path) and itemParent = result.getImmediateParentModule() and not result.isPublic() and @@ -1062,7 +1087,11 @@ private ModuleLikeNode getAPrivateVisibleModule(ModuleLikeNode itemParent) { cached ItemNode resolvePath(RelevantPath path) { result = resolvePath1(path) and - result.isPublic() + ( + result.isPublic() + or + path.requiresExtractorWorkaround() + ) or exists(ModuleLikeNode itemParent, ModuleLikeNode pathParent | result = resolvePathPrivate(path, itemParent, pathParent) and diff --git a/rust/ql/lib/codeql/rust/internal/Type.qll b/rust/ql/lib/codeql/rust/internal/Type.qll index 9e063d215161..5abada5726d3 100644 --- a/rust/ql/lib/codeql/rust/internal/Type.qll +++ b/rust/ql/lib/codeql/rust/internal/Type.qll @@ -29,7 +29,26 @@ newtype TType = abstract class Type extends TType { /** Gets the method `name` belonging to this type, if any. */ pragma[nomagic] - abstract Function getMethod(string name); + final Function getMethod(string name) { + result = this.getAMethod(name) and + ( + // when a method exists in both source code and in library code, it is because + // we also extracted the source code as library code, and hence we only want + // the method from source code + result.fromSource() + or + not this.getAMethod(name).fromSource() + ) + } + + /** + * Gets a method `name` belonging to this type, if any. + * + * Multiple methods may exist with the same name when it exists in both + * source code and in library code. + */ + pragma[nomagic] + abstract Function getAMethod(string name); /** Gets the struct field `name` belonging to this type, if any. */ pragma[nomagic] @@ -74,7 +93,7 @@ abstract class Type extends TType { abstract private class StructOrEnumType extends Type { abstract ItemNode asItemNode(); - final override Function getMethod(string name) { + final override Function getAMethod(string name) { result = this.asItemNode().getASuccessor(name) and exists(ImplOrTraitItemNode impl | result = impl.getAnAssocItem() | impl instanceof Trait @@ -138,7 +157,7 @@ class TraitType extends Type, TTrait { TraitType() { this = TTrait(trait) } - override Function getMethod(string name) { result = trait.(ItemNode).getASuccessor(name) } + override Function getAMethod(string name) { result = trait.(ItemNode).getASuccessor(name) } override StructField getStructField(string name) { none() } @@ -220,7 +239,7 @@ class ImplType extends Type, TImpl { ImplType() { this = TImpl(impl) } - override Function getMethod(string name) { result = impl.(ItemNode).getASuccessor(name) } + override Function getAMethod(string name) { result = impl.(ItemNode).getASuccessor(name) } override StructField getStructField(string name) { none() } @@ -247,7 +266,7 @@ class ImplType extends Type, TImpl { class ArrayType extends Type, TArrayType { ArrayType() { this = TArrayType() } - override Function getMethod(string name) { none() } + override Function getAMethod(string name) { none() } override StructField getStructField(string name) { none() } @@ -273,7 +292,7 @@ class ArrayType extends Type, TArrayType { class RefType extends Type, TRefType { RefType() { this = TRefType() } - override Function getMethod(string name) { none() } + override Function getAMethod(string name) { none() } override StructField getStructField(string name) { none() } @@ -318,7 +337,7 @@ class TypeParamTypeParameter extends TypeParameter, TTypeParamTypeParameter { TypeParam getTypeParam() { result = typeParam } - override Function getMethod(string name) { + override Function getAMethod(string name) { // NOTE: If the type parameter has trait bounds, then this finds methods // on the bounding traits. result = typeParam.(ItemNode).getASuccessor(name) @@ -377,7 +396,7 @@ class AssociatedTypeTypeParameter extends TypeParameter, TAssociatedTypeTypePara int getIndex() { traitAliasIndex(_, result, typeAlias) } - override Function getMethod(string name) { none() } + override Function getAMethod(string name) { none() } override string toString() { result = typeAlias.getName().getText() } @@ -388,7 +407,7 @@ class AssociatedTypeTypeParameter extends TypeParameter, TAssociatedTypeTypePara /** An implicit reference type parameter. */ class RefTypeParameter extends TypeParameter, TRefTypeParameter { - override Function getMethod(string name) { none() } + override Function getAMethod(string name) { none() } override string toString() { result = "&T" } @@ -411,7 +430,7 @@ class SelfTypeParameter extends TypeParameter, TSelfTypeParameter { override TypeMention getABaseTypeMention() { result = trait } - override Function getMethod(string name) { + override Function getAMethod(string name) { // The `Self` type parameter is an implementation of the trait, so it has // all the trait's methods. result = trait.(ItemNode).getASuccessor(name) From 52bd99b852280eae52e31d13e8861eb44bb0f230 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Wed, 30 Apr 2025 11:01:29 +0200 Subject: [PATCH 2/2] Address review comments --- .../lib/codeql/rust/internal/CachedStages.qll | 2 +- .../codeql/rust/internal/PathResolution.qll | 55 ++++++++++++------- rust/ql/lib/codeql/rust/internal/Type.qll | 39 ++++--------- 3 files changed, 47 insertions(+), 49 deletions(-) diff --git a/rust/ql/lib/codeql/rust/internal/CachedStages.qll b/rust/ql/lib/codeql/rust/internal/CachedStages.qll index 4041b2731f97..2a7447ed7a3f 100644 --- a/rust/ql/lib/codeql/rust/internal/CachedStages.qll +++ b/rust/ql/lib/codeql/rust/internal/CachedStages.qll @@ -120,7 +120,7 @@ module Stages { or exists(resolvePath(_)) or - exists(any(ItemNode i).getASuccessor(_)) + exists(any(ItemNode i).getASuccessorFull(_)) or exists(any(ItemNode i).getASuccessorRec(_)) or diff --git a/rust/ql/lib/codeql/rust/internal/PathResolution.qll b/rust/ql/lib/codeql/rust/internal/PathResolution.qll index 37a895c0dcd4..5bc45afecf17 100644 --- a/rust/ql/lib/codeql/rust/internal/PathResolution.qll +++ b/rust/ql/lib/codeql/rust/internal/PathResolution.qll @@ -172,9 +172,14 @@ abstract class ItemNode extends Locatable { result = this.(TypeParamItemNode).resolveABound().getASuccessorRec(name).(AssocItemNode) } - /** Gets a successor named `name` of this item, if any. */ + /** + * Gets a successor named `name` of this item, if any. + * + * Whenever a function exists in both source code and in library code, + * both are included + */ cached - ItemNode getASuccessor(string name) { + ItemNode getASuccessorFull(string name) { Stages::PathResolutionStage::ref() and result = this.getASuccessorRec(name) or @@ -202,6 +207,22 @@ abstract class ItemNode extends Locatable { result.(CrateItemNode).isPotentialDollarCrateTarget() } + /** Gets a successor named `name` of this item, if any. */ + pragma[nomagic] + ItemNode getASuccessor(string name) { + result = this.getASuccessorFull(name) and + ( + // when a function exists in both source code and in library code, it is because + // we also extracted the source code as library code, and hence we only want + // the function from source code + result.fromSource() + or + not result instanceof Function + or + not this.getASuccessorFull(name).(Function).fromSource() + ) + } + /** Gets the location of this item. */ Location getLocation() { result = super.getLocation() } } @@ -234,7 +255,7 @@ abstract private class ModuleLikeNode extends ItemNode { private class SourceFileItemNode extends ModuleLikeNode, SourceFile { pragma[nomagic] ModuleLikeNode getSuper() { - result = any(ModuleItemNode mod | fileImport(mod, this)).getASuccessor("super") + result = any(ModuleItemNode mod | fileImport(mod, this)).getASuccessorFull("super") } override string getName() { result = "(source file)" } @@ -297,7 +318,7 @@ class CrateItemNode extends ItemNode instanceof Crate { predicate isPotentialDollarCrateTarget() { exists(string name, RelevantPath p | p.isDollarCrateQualifiedPath(name) and - exists(this.getASuccessor(name)) + exists(this.getASuccessorFull(name)) ) } @@ -328,10 +349,8 @@ private class ConstItemNode extends AssocItemNode instanceof Const { or // for trait items from library code, we do not currently know if they // have default implementations or not, so we assume they do - exists(TraitItemNode t | - this = t.getAnAssocItem() and - not this.fromSource() - ) + not this.fromSource() and + this = any(TraitItemNode t).getAnAssocItem() } override Namespace getNamespace() { result.isValue() } @@ -373,10 +392,8 @@ class FunctionItemNode extends AssocItemNode instanceof Function { or // for trait items from library code, we do not currently know if they // have default implementations or not, so we assume they do - exists(TraitItemNode t | - this = t.getAnAssocItem() and - not this.fromSource() - ) + not this.fromSource() and + this = any(TraitItemNode t).getAnAssocItem() } override Namespace getNamespace() { result.isValue() } @@ -940,8 +957,8 @@ private predicate unqualifiedPathLookup(ItemNode encl, string name, Namespace ns } pragma[nomagic] -private ItemNode getASuccessor(ItemNode pred, string name, Namespace ns) { - result = pred.getASuccessor(name) and +private ItemNode getASuccessorFull(ItemNode pred, string name, Namespace ns) { + result = pred.getASuccessorFull(name) and ns = result.getNamespace() } @@ -978,7 +995,7 @@ private predicate keywordLookup(ItemNode encl, string name, Namespace ns, Releva pragma[nomagic] private ItemNode unqualifiedPathLookup(RelevantPath p, Namespace ns) { - exists(ItemNode encl, string name | result = getASuccessor(encl, name, ns) | + exists(ItemNode encl, string name | result = getASuccessorFull(encl, name, ns) | unqualifiedPathLookup(encl, name, ns, p) or keywordLookup(encl, name, ns, p) @@ -1002,7 +1019,7 @@ private ItemNode resolvePath0(RelevantPath path, Namespace ns) { or exists(ItemNode q, string name | q = resolvePathQualifier(path, name) and - result = getASuccessor(q, name, ns) + result = getASuccessorFull(q, name, ns) ) or result = resolveUseTreeListItem(_, _, path) and @@ -1127,12 +1144,12 @@ private ItemNode resolveUseTreeListItem(Use use, UseTree tree, RelevantPath path mid = resolveUseTreeListItem(use, midTree) and tree = midTree.getUseTreeList().getAUseTree() and isUseTreeSubPathUnqualified(tree, path, pragma[only_bind_into](name)) and - result = mid.getASuccessor(pragma[only_bind_into](name)) + result = mid.getASuccessorFull(pragma[only_bind_into](name)) ) or exists(ItemNode q, string name | q = resolveUseTreeListItemQualifier(use, tree, path, name) and - result = q.getASuccessor(name) + result = q.getASuccessorFull(name) ) } @@ -1162,7 +1179,7 @@ private predicate useImportEdge(Use use, string name, ItemNode item) { then exists(ItemNode encl, Namespace ns | encl.getADescendant() = use and - item = getASuccessor(used, name, ns) and + item = getASuccessorFull(used, name, ns) and // glob imports can be shadowed not declares(encl, ns, name) and not name = ["super", "self", "Self", "$crate", "crate"] diff --git a/rust/ql/lib/codeql/rust/internal/Type.qll b/rust/ql/lib/codeql/rust/internal/Type.qll index 5abada5726d3..9e063d215161 100644 --- a/rust/ql/lib/codeql/rust/internal/Type.qll +++ b/rust/ql/lib/codeql/rust/internal/Type.qll @@ -29,26 +29,7 @@ newtype TType = abstract class Type extends TType { /** Gets the method `name` belonging to this type, if any. */ pragma[nomagic] - final Function getMethod(string name) { - result = this.getAMethod(name) and - ( - // when a method exists in both source code and in library code, it is because - // we also extracted the source code as library code, and hence we only want - // the method from source code - result.fromSource() - or - not this.getAMethod(name).fromSource() - ) - } - - /** - * Gets a method `name` belonging to this type, if any. - * - * Multiple methods may exist with the same name when it exists in both - * source code and in library code. - */ - pragma[nomagic] - abstract Function getAMethod(string name); + abstract Function getMethod(string name); /** Gets the struct field `name` belonging to this type, if any. */ pragma[nomagic] @@ -93,7 +74,7 @@ abstract class Type extends TType { abstract private class StructOrEnumType extends Type { abstract ItemNode asItemNode(); - final override Function getAMethod(string name) { + final override Function getMethod(string name) { result = this.asItemNode().getASuccessor(name) and exists(ImplOrTraitItemNode impl | result = impl.getAnAssocItem() | impl instanceof Trait @@ -157,7 +138,7 @@ class TraitType extends Type, TTrait { TraitType() { this = TTrait(trait) } - override Function getAMethod(string name) { result = trait.(ItemNode).getASuccessor(name) } + override Function getMethod(string name) { result = trait.(ItemNode).getASuccessor(name) } override StructField getStructField(string name) { none() } @@ -239,7 +220,7 @@ class ImplType extends Type, TImpl { ImplType() { this = TImpl(impl) } - override Function getAMethod(string name) { result = impl.(ItemNode).getASuccessor(name) } + override Function getMethod(string name) { result = impl.(ItemNode).getASuccessor(name) } override StructField getStructField(string name) { none() } @@ -266,7 +247,7 @@ class ImplType extends Type, TImpl { class ArrayType extends Type, TArrayType { ArrayType() { this = TArrayType() } - override Function getAMethod(string name) { none() } + override Function getMethod(string name) { none() } override StructField getStructField(string name) { none() } @@ -292,7 +273,7 @@ class ArrayType extends Type, TArrayType { class RefType extends Type, TRefType { RefType() { this = TRefType() } - override Function getAMethod(string name) { none() } + override Function getMethod(string name) { none() } override StructField getStructField(string name) { none() } @@ -337,7 +318,7 @@ class TypeParamTypeParameter extends TypeParameter, TTypeParamTypeParameter { TypeParam getTypeParam() { result = typeParam } - override Function getAMethod(string name) { + override Function getMethod(string name) { // NOTE: If the type parameter has trait bounds, then this finds methods // on the bounding traits. result = typeParam.(ItemNode).getASuccessor(name) @@ -396,7 +377,7 @@ class AssociatedTypeTypeParameter extends TypeParameter, TAssociatedTypeTypePara int getIndex() { traitAliasIndex(_, result, typeAlias) } - override Function getAMethod(string name) { none() } + override Function getMethod(string name) { none() } override string toString() { result = typeAlias.getName().getText() } @@ -407,7 +388,7 @@ class AssociatedTypeTypeParameter extends TypeParameter, TAssociatedTypeTypePara /** An implicit reference type parameter. */ class RefTypeParameter extends TypeParameter, TRefTypeParameter { - override Function getAMethod(string name) { none() } + override Function getMethod(string name) { none() } override string toString() { result = "&T" } @@ -430,7 +411,7 @@ class SelfTypeParameter extends TypeParameter, TSelfTypeParameter { override TypeMention getABaseTypeMention() { result = trait } - override Function getAMethod(string name) { + override Function getMethod(string name) { // The `Self` type parameter is an implementation of the trait, so it has // all the trait's methods. result = trait.(ItemNode).getASuccessor(name)