From 96cba8b8c248282c3de20b592f4c2b7c4dba0810 Mon Sep 17 00:00:00 2001 From: Simon Friis Vindum Date: Thu, 22 May 2025 14:04:44 +0200 Subject: [PATCH 1/4] Rust: Add inconsistency check for type mentions without a root type --- .../codeql/typeinference/internal/TypeInference.qll | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/shared/typeinference/codeql/typeinference/internal/TypeInference.qll b/shared/typeinference/codeql/typeinference/internal/TypeInference.qll index 6a1f1c50bb3..b0f5fc67300 100644 --- a/shared/typeinference/codeql/typeinference/internal/TypeInference.qll +++ b/shared/typeinference/codeql/typeinference/internal/TypeInference.qll @@ -1314,6 +1314,10 @@ module Make1 Input1> { getTypeParameterId(tp1) = getTypeParameterId(tp2) and tp1 != tp2 } + + query predicate illFormedTypeMention(TypeMention tm) { + not exists(tm.resolveTypeAt(TypePath::nil())) and exists(tm.getLocation()) + } } } } From 5278064407ea4880713d82f1fda1800eefbd6946 Mon Sep 17 00:00:00 2001 From: Simon Friis Vindum Date: Thu, 22 May 2025 14:08:25 +0200 Subject: [PATCH 2/4] Rust: Only include relevant AST nodes in TypeMention --- .../codeql/rust/internal/TypeInference.qll | 14 +- .../lib/codeql/rust/internal/TypeMention.qll | 154 ++++++++---------- 2 files changed, 77 insertions(+), 91 deletions(-) diff --git a/rust/ql/lib/codeql/rust/internal/TypeInference.qll b/rust/ql/lib/codeql/rust/internal/TypeInference.qll index ca5c65f38ef..ca60efe2864 100644 --- a/rust/ql/lib/codeql/rust/internal/TypeInference.qll +++ b/rust/ql/lib/codeql/rust/internal/TypeInference.qll @@ -314,7 +314,7 @@ private Type getRefAdjustImplicitSelfType(SelfParam self, TypePath suffix, Type pragma[nomagic] private Type resolveImplSelfType(Impl i, TypePath path) { - result = i.getSelfTy().(TypeReprMention).resolveTypeAt(path) + result = i.getSelfTy().(TypeMention).resolveTypeAt(path) } /** Gets the type at `path` of the implicitly typed `self` parameter. */ @@ -377,7 +377,7 @@ private module StructExprMatchingInput implements MatchingInputSig { Type getDeclaredType(DeclarationPosition dpos, TypePath path) { // type of a field - exists(TypeReprMention tp | + exists(TypeMention tp | tp = this.getField(dpos.asFieldPos()).getTypeRepr() and result = tp.resolveTypeAt(path) ) @@ -537,7 +537,7 @@ private module CallExprBaseMatchingInput implements MatchingInputSig { override Type getParameterType(DeclarationPosition dpos, TypePath path) { exists(int pos | - result = this.getTupleField(pos).getTypeRepr().(TypeReprMention).resolveTypeAt(path) and + result = this.getTupleField(pos).getTypeRepr().(TypeMention).resolveTypeAt(path) and dpos = TPositionalDeclarationPosition(pos, false) ) } @@ -560,7 +560,7 @@ private module CallExprBaseMatchingInput implements MatchingInputSig { override Type getParameterType(DeclarationPosition dpos, TypePath path) { exists(int p | - result = this.getTupleField(p).getTypeRepr().(TypeReprMention).resolveTypeAt(path) and + result = this.getTupleField(p).getTypeRepr().(TypeMention).resolveTypeAt(path) and dpos = TPositionalDeclarationPosition(p, false) ) } @@ -608,7 +608,7 @@ private module CallExprBaseMatchingInput implements MatchingInputSig { } override Type getReturnType(TypePath path) { - result = this.getRetType().getTypeRepr().(TypeReprMention).resolveTypeAt(path) + result = this.getRetType().getTypeRepr().(TypeMention).resolveTypeAt(path) } } @@ -646,7 +646,7 @@ private module CallExprBaseMatchingInput implements MatchingInputSig { private import codeql.rust.elements.internal.CallExprImpl::Impl as CallExprImpl class Access extends CallExprBase { - private TypeReprMention getMethodTypeArg(int i) { + private TypeMention getMethodTypeArg(int i) { result = this.(MethodCallExpr).getGenericArgList().getTypeArg(i) } @@ -831,7 +831,7 @@ private module FieldExprMatchingInput implements MatchingInputSig { ) or dpos.isField() and - result = this.getTypeRepr().(TypeReprMention).resolveTypeAt(path) + result = this.getTypeRepr().(TypeMention).resolveTypeAt(path) } } diff --git a/rust/ql/lib/codeql/rust/internal/TypeMention.qll b/rust/ql/lib/codeql/rust/internal/TypeMention.qll index a957a82149f..7e947a35bc4 100644 --- a/rust/ql/lib/codeql/rust/internal/TypeMention.qll +++ b/rust/ql/lib/codeql/rust/internal/TypeMention.qll @@ -31,53 +31,33 @@ abstract class TypeMention extends AstNode { Type resolveTypeAt(TypePath path) { result = this.getMentionAt(path).resolveType() } } -class TypeReprMention extends TypeMention, TypeRepr { - TypeReprMention() { not this instanceof InferTypeRepr } +class ArrayTypeReprMention extends TypeMention instanceof ArrayTypeRepr { + override TypeMention getTypeArgument(int i) { result = super.getElementTypeRepr() and i = 0 } - override TypeReprMention getTypeArgument(int i) { - result = this.(ArrayTypeRepr).getElementTypeRepr() and - i = 0 - or - result = this.(RefTypeRepr).getTypeRepr() and - i = 0 - or - result = this.(PathTypeRepr).getPath().(PathMention).getTypeArgument(i) - } - - override Type resolveType() { - this instanceof ArrayTypeRepr and - result = TArrayType() - or - this instanceof RefTypeRepr and - result = TRefType() - or - result = this.(PathTypeRepr).getPath().(PathMention).resolveType() - } - - override Type resolveTypeAt(TypePath path) { - result = this.(PathTypeRepr).getPath().(PathMention).resolveTypeAt(path) - or - not exists(this.(PathTypeRepr).getPath()) and - result = super.resolveTypeAt(path) - } + override Type resolveType() { result = TArrayType() } } -/** Holds if `path` resolves to the type alias `alias` with the definition `rhs`. */ -private predicate resolvePathAlias(Path path, TypeAlias alias, TypeReprMention rhs) { - alias = resolvePath(path) and rhs = alias.getTypeRepr() +class RefTypeReprMention extends TypeMention instanceof RefTypeRepr { + override TypeMention getTypeArgument(int i) { result = super.getTypeRepr() and i = 0 } + + override Type resolveType() { result = TRefType() } } -abstract class PathMention extends TypeMention, Path { - override TypeMention getTypeArgument(int i) { - result = this.getSegment().getGenericArgList().getTypeArg(i) +class PathTypeReprMention extends TypeMention instanceof PathTypeRepr { + Path path; + ItemNode resolved; + + PathTypeReprMention() { + path = super.getPath() and + // NOTE: This excludes unresolvable paths which is intentional as these + // don't add value to the type inference anyway. + resolved = resolvePath(path) } -} -class NonAliasPathMention extends PathMention { - NonAliasPathMention() { not resolvePathAlias(this, _, _) } + ItemNode getResolved() { result = resolved } override TypeMention getTypeArgument(int i) { - result = super.getTypeArgument(i) + result = path.getSegment().getGenericArgList().getTypeArg(i) or // `Self` paths inside `impl` blocks have implicit type arguments that are // the type parameters of the `impl` block. For example, in @@ -92,17 +72,17 @@ class NonAliasPathMention extends PathMention { // // the `Self` return type is shorthand for `Foo`. exists(ImplItemNode node | - this = node.getASelfPath() and + path = node.getASelfPath() and result = node.(ImplItemNode).getSelfPath().getSegment().getGenericArgList().getTypeArg(i) ) or - // If `this` is the trait of an `impl` block then any associated types + // If `path` is the trait of an `impl` block then any associated types // defined in the `impl` block are type arguments to the trait. // // For instance, for a trait implementation like this // ```rust // impl MyTrait for MyType { - // ^^^^^^^ this + // ^^^^^^^ path // type AssociatedType = i64 // ^^^ result // // ... @@ -110,88 +90,94 @@ class NonAliasPathMention extends PathMention { // ``` // the rhs. of the type alias is a type argument to the trait. exists(ImplItemNode impl, AssociatedTypeTypeParameter param, TypeAlias alias | - this = impl.getTraitPath() and - param.getTrait() = resolvePath(this) and + path = impl.getTraitPath() and + param.getTrait() = resolved and alias = impl.getASuccessor(param.getTypeAlias().getName().getText()) and result = alias.getTypeRepr() and param.getIndex() = i ) } + /** + * Holds if this path resolved to a type alias with a rhs. that has the + * resulting type at `typePath`. + */ + Type aliasResolveTypeAt(TypePath typePath) { + exists(TypeAlias alias, TypeMention rhs | alias = resolved and rhs = alias.getTypeRepr() | + result = rhs.resolveTypeAt(typePath) and + not result = pathGetTypeParameter(alias, _) + or + exists(TypeParameter tp, TypeMention arg, TypePath prefix, TypePath suffix, int i | + tp = rhs.resolveTypeAt(prefix) and + tp = pathGetTypeParameter(alias, i) and + arg = path.getSegment().getGenericArgList().getTypeArg(i) and + result = arg.resolveTypeAt(suffix) and + typePath = prefix.append(suffix) + ) + ) + } + override Type resolveType() { - exists(ItemNode i | i = resolvePath(this) | - result = TStruct(i) + result = this.aliasResolveTypeAt(TypePath::nil()) + or + not exists(resolved.(TypeAlias).getTypeRepr()) and + ( + result = TStruct(resolved) or - result = TEnum(i) + result = TEnum(resolved) or - exists(TraitItemNode trait | trait = i | + exists(TraitItemNode trait | trait = resolved | // If this is a `Self` path, then it resolves to the implicit `Self` // type parameter, otherwise it is a trait bound. - if this = trait.getASelfPath() + if super.getPath() = trait.getASelfPath() then result = TSelfTypeParameter(trait) else result = TTrait(trait) ) or - result = TTypeParamTypeParameter(i) + result = TTypeParamTypeParameter(resolved) or - exists(TypeAlias alias | alias = i | + exists(TypeAlias alias | alias = resolved | result.(AssociatedTypeTypeParameter).getTypeAlias() = alias or - result = alias.getTypeRepr().(TypeReprMention).resolveType() + result = alias.getTypeRepr().(TypeMention).resolveType() ) ) } + + override Type resolveTypeAt(TypePath typePath) { + result = this.aliasResolveTypeAt(typePath) + or + not exists(resolved.(TypeAlias).getTypeRepr()) and + result = super.resolveTypeAt(typePath) + } } -class AliasPathMention extends PathMention { - TypeAlias alias; - TypeReprMention rhs; - - AliasPathMention() { resolvePathAlias(this, alias, rhs) } - - /** Get the `i`th type parameter of the alias itself. */ - private TypeParameter getTypeParameter(int i) { - result = TTypeParamTypeParameter(alias.getGenericParamList().getTypeParam(i)) - } - - override Type resolveType() { result = rhs.resolveType() } - - override Type resolveTypeAt(TypePath path) { - result = rhs.resolveTypeAt(path) and - not result = this.getTypeParameter(_) - or - exists(TypeParameter tp, TypeMention arg, TypePath prefix, TypePath suffix, int i | - tp = rhs.resolveTypeAt(prefix) and - tp = this.getTypeParameter(i) and - arg = this.getTypeArgument(i) and - result = arg.resolveTypeAt(suffix) and - path = prefix.append(suffix) - ) - } +private TypeParameter pathGetTypeParameter(TypeAlias alias, int i) { + result = TTypeParamTypeParameter(alias.getGenericParamList().getTypeParam(i)) } // Used to represent implicit `Self` type arguments in traits and `impl` blocks, // see `PathMention` for details. -class TypeParamMention extends TypeMention, TypeParam { - override TypeReprMention getTypeArgument(int i) { none() } +class TypeParamMention extends TypeMention instanceof TypeParam { + override TypeMention getTypeArgument(int i) { none() } override Type resolveType() { result = TTypeParamTypeParameter(this) } } // Used to represent implicit type arguments for associated types in traits. -class TypeAliasMention extends TypeMention, TypeAlias { +class TypeAliasMention extends TypeMention instanceof TypeAlias { private Type t; TypeAliasMention() { t = TAssociatedTypeTypeParameter(this) } - override TypeReprMention getTypeArgument(int i) { none() } + override TypeMention getTypeArgument(int i) { none() } override Type resolveType() { result = t } } -class TraitMention extends TypeMention, TraitItemNode { +class TraitMention extends TypeMention instanceof TraitItemNode { override TypeMention getTypeArgument(int i) { - result = this.getTypeParam(i) + result = super.getTypeParam(i) or traitAliasIndex(this, i, result) } @@ -203,7 +189,7 @@ class TraitMention extends TypeMention, TraitItemNode { // appears in the AST, we (somewhat arbitrarily) choose the name of a trait as a // type mention. This works because there is a one-to-one correspondence between // a trait and its name. -class SelfTypeParameterMention extends TypeMention, Name { +class SelfTypeParameterMention extends TypeMention instanceof Name { Trait trait; SelfTypeParameterMention() { trait.getName() = this } @@ -212,5 +198,5 @@ class SelfTypeParameterMention extends TypeMention, Name { override Type resolveType() { result = TSelfTypeParameter(trait) } - override TypeReprMention getTypeArgument(int i) { none() } + override TypeMention getTypeArgument(int i) { none() } } From ba4950fb89100d94caf8dfd58627eb98d9418958 Mon Sep 17 00:00:00 2001 From: Simon Friis Vindum Date: Thu, 22 May 2025 14:52:37 +0200 Subject: [PATCH 3/4] Rust: Accept test changes --- .../CONSISTENCY/TypeInferenceConsistency.expected | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 rust/ql/test/query-tests/unusedentities/CONSISTENCY/TypeInferenceConsistency.expected diff --git a/rust/ql/test/query-tests/unusedentities/CONSISTENCY/TypeInferenceConsistency.expected b/rust/ql/test/query-tests/unusedentities/CONSISTENCY/TypeInferenceConsistency.expected new file mode 100644 index 00000000000..56ce1df5c89 --- /dev/null +++ b/rust/ql/test/query-tests/unusedentities/CONSISTENCY/TypeInferenceConsistency.expected @@ -0,0 +1,2 @@ +illFormedTypeMention +| main.rs:403:18:403:24 | FuncPtr | From 52280625eeeee973f1b998cfc062caf93cd53e62 Mon Sep 17 00:00:00 2001 From: Simon Friis Vindum Date: Fri, 23 May 2025 14:20:44 +0200 Subject: [PATCH 4/4] Rust: Add type inference inconsistency counts to the stats summary --- .../internal/TypeInferenceConsistency.qll | 24 +++++++++++++++++++ rust/ql/src/queries/summary/Stats.qll | 15 ++++++++++++ rust/ql/src/queries/summary/SummaryStats.ql | 2 ++ .../TypeInferenceConsistency.expected | 3 +++ 4 files changed, 44 insertions(+) create mode 100644 rust/ql/test/query-tests/security/CWE-089/CONSISTENCY/TypeInferenceConsistency.expected diff --git a/rust/ql/lib/codeql/rust/internal/TypeInferenceConsistency.qll b/rust/ql/lib/codeql/rust/internal/TypeInferenceConsistency.qll index 214ee3e6d49..e9bcf35badc 100644 --- a/rust/ql/lib/codeql/rust/internal/TypeInferenceConsistency.qll +++ b/rust/ql/lib/codeql/rust/internal/TypeInferenceConsistency.qll @@ -2,4 +2,28 @@ * Provides classes for recognizing type inference inconsistencies. */ +private import Type +private import TypeMention +private import TypeInference::Consistency as Consistency import TypeInference::Consistency + +query predicate illFormedTypeMention(TypeMention tm) { + Consistency::illFormedTypeMention(tm) and + // Only include inconsistencies in the source, as we otherwise get + // inconsistencies from library code in every project. + tm.fromSource() +} + +int getTypeInferenceInconsistencyCounts(string type) { + type = "Missing type parameter ID" and + result = count(TypeParameter tp | missingTypeParameterId(tp) | tp) + or + type = "Non-functional type parameter ID" and + result = count(TypeParameter tp | nonFunctionalTypeParameterId(tp) | tp) + or + type = "Non-injective type parameter ID" and + result = count(TypeParameter tp | nonInjectiveTypeParameterId(tp, _) | tp) + or + type = "Ill-formed type mention" and + result = count(TypeMention tm | illFormedTypeMention(tm) | tm) +} diff --git a/rust/ql/src/queries/summary/Stats.qll b/rust/ql/src/queries/summary/Stats.qll index 2199a3ddff0..db0a05629df 100644 --- a/rust/ql/src/queries/summary/Stats.qll +++ b/rust/ql/src/queries/summary/Stats.qll @@ -8,6 +8,7 @@ private import codeql.rust.dataflow.internal.DataFlowImpl private import codeql.rust.dataflow.internal.TaintTrackingImpl private import codeql.rust.internal.AstConsistency as AstConsistency private import codeql.rust.internal.PathResolutionConsistency as PathResolutionConsistency +private import codeql.rust.internal.TypeInferenceConsistency as TypeInferenceConsistency private import codeql.rust.controlflow.internal.CfgConsistency as CfgConsistency private import codeql.rust.dataflow.internal.DataFlowConsistency as DataFlowConsistency private import codeql.rust.dataflow.internal.SsaImpl::Consistency as SsaConsistency @@ -52,6 +53,13 @@ int getTotalPathResolutionInconsistencies() { sum(string type | | PathResolutionConsistency::getPathResolutionInconsistencyCounts(type)) } +/** + * Gets a count of the total number of type inference inconsistencies in the database. + */ +int getTotalTypeInferenceInconsistencies() { + result = sum(string type | | TypeInferenceConsistency::getTypeInferenceInconsistencyCounts(type)) +} + /** * Gets a count of the total number of control flow graph inconsistencies in the database. */ @@ -159,6 +167,13 @@ predicate inconsistencyStats(string key, int value) { key = "Inconsistencies - data flow" and value = getTotalDataFlowInconsistencies() } +/** + * Gets summary statistics about inconsistencies related to type inference. + */ +predicate typeInferenceInconsistencyStats(string key, int value) { + key = "Inconsistencies - Type inference" and value = getTotalTypeInferenceInconsistencies() +} + /** * Gets summary statistics about taint. */ diff --git a/rust/ql/src/queries/summary/SummaryStats.ql b/rust/ql/src/queries/summary/SummaryStats.ql index 57ac5b4004e..515c78c7268 100644 --- a/rust/ql/src/queries/summary/SummaryStats.ql +++ b/rust/ql/src/queries/summary/SummaryStats.ql @@ -17,5 +17,7 @@ where or inconsistencyStats(key, value) or + typeInferenceInconsistencyStats(key, value) + or taintStats(key, value) select key, value order by key diff --git a/rust/ql/test/query-tests/security/CWE-089/CONSISTENCY/TypeInferenceConsistency.expected b/rust/ql/test/query-tests/security/CWE-089/CONSISTENCY/TypeInferenceConsistency.expected new file mode 100644 index 00000000000..9bd56449240 --- /dev/null +++ b/rust/ql/test/query-tests/security/CWE-089/CONSISTENCY/TypeInferenceConsistency.expected @@ -0,0 +1,3 @@ +illFormedTypeMention +| sqlx.rs:158:13:158:81 | ...::BoxDynError | +| sqlx.rs:160:17:160:86 | ...::BoxDynError |