From 4f8f78b60043d617d9779acfbc2a0c4269666637 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Wed, 4 Nov 2026 20:14:27 +0100 Subject: [PATCH 1/4] C++: Move predicates around. This just makes the subsequent diffs smaller. --- .../semmle/code/cpp/dataflow/ExternalFlow.qll | 92 +++++++++---------- 1 file changed, 46 insertions(+), 46 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll b/cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll index ee894f95648f..604ce1699722 100644 --- a/cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll +++ b/cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll @@ -1285,13 +1285,6 @@ module ConstructorForwarding { private predicate isUnderlyingType(Type type) { type = type.getUnderlyingType() } - private newtype TTypeState = - MkTypeState(Type type, ValueCategory category, Boolean conversionUsed, Phase phase) { - not type instanceof Cpp::ReferenceType and - not type instanceof FunctionReferenceType and - isUnderlyingType(type) - } - private ValueCategory getCategoryForRef(Cpp::ReferenceType reference) { reference instanceof Cpp::LValueReferenceType and result = LValue() or @@ -1318,45 +1311,6 @@ module ConstructorForwarding { preservesQualifiers(arg, param) } - private class TypeState extends TTypeState { - Type getType() { this = MkTypeState(result, _, _, _) } - - ValueCategory getCategory() { this = MkTypeState(_, result, _, _) } - - Phase getPhase() { this = MkTypeState(_, _, _, result) } - - /** Holds if this state's phase permits a value or base-class conversion. */ - predicate canConvertValue() { - this.getPhase() = Initial() or this.getPhase() = AfterTransformation() - } - - predicate hasNotUsedConversion() { this = MkTypeState(_, _, false, _) } - - string toString() { result = this.getType().toString() } - - predicate isSource(Type argType) { - argType = getForwardedArgumentType(_, _, _) and - exists(ValueCategory category | - this = MkTypeState(getValueType(argType, category), category, false, Initial()) - ) - } - - predicate matchesParameter(Type paramType) { - exists(Type base | base = paramType.(Cpp::ReferenceType).getBaseType() | - qualificationCompatible(this.getType(), base) and - referenceAcceptsCategory(paramType, this.getCategory()) - ) - or - not paramType instanceof Cpp::ReferenceType and - this.getType().stripTopLevelSpecifiers() = paramType.stripTopLevelSpecifiers() - } - - predicate isSink(Type paramType, Cpp::Constructor constructor) { - paramType = getConstructorParameterType(constructor, _) and - this.matchesParameter(paramType) - } - } - private Cpp::PointerType pointerType(Type base) { result.getBaseType() = base.getUnderlyingType() } @@ -1488,6 +1442,52 @@ module ConstructorForwarding { ) } + private newtype TTypeState = + MkTypeState(Type type, ValueCategory category, Boolean conversionUsed, Phase phase) { + not type instanceof Cpp::ReferenceType and + not type instanceof FunctionReferenceType and + isUnderlyingType(type) + } + + private class TypeState extends TTypeState { + Type getType() { this = MkTypeState(result, _, _, _) } + + ValueCategory getCategory() { this = MkTypeState(_, result, _, _) } + + Phase getPhase() { this = MkTypeState(_, _, _, result) } + + /** Holds if this state's phase permits a value or base-class conversion. */ + predicate canConvertValue() { + this.getPhase() = Initial() or this.getPhase() = AfterTransformation() + } + + predicate hasNotUsedConversion() { this = MkTypeState(_, _, false, _) } + + string toString() { result = this.getType().toString() } + + predicate isSource(Type argType) { + argType = getForwardedArgumentType(_, _, _) and + exists(ValueCategory category | + this = MkTypeState(getValueType(argType, category), category, false, Initial()) + ) + } + + predicate matchesParameter(Type paramType) { + exists(Type base | base = paramType.(Cpp::ReferenceType).getBaseType() | + qualificationCompatible(this.getType(), base) and + referenceAcceptsCategory(paramType, this.getCategory()) + ) + or + not paramType instanceof Cpp::ReferenceType and + this.getType().stripTopLevelSpecifiers() = paramType.stripTopLevelSpecifiers() + } + + predicate isSink(Type paramType, Cpp::Constructor constructor) { + paramType = getConstructorParameterType(constructor, _) and + this.matchesParameter(paramType) + } + } + private predicate typeRev(TypeState state, Cpp::Constructor constructor) { typeFwd(state) and ( From a4ca10118eb68a941e81a7a725957242d40e4bbb Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Wed, 4 Nov 2026 20:15:47 +0100 Subject: [PATCH 2/4] C++: Add some bindingset annotations. --- .../semmle/code/cpp/dataflow/ExternalFlow.qll | 33 +++++++++++-------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll b/cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll index 604ce1699722..07b48d3766fc 100644 --- a/cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll +++ b/cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll @@ -1220,6 +1220,7 @@ module ConstructorForwarding { (t1.isVolatile() implies t2.isVolatile()) } + bindingset[arg] private predicate baseTypeCompatible(Type arg, Type param) { param.stripTopLevelSpecifiers() = arg.stripTopLevelSpecifiers().(Class).getABaseClass+() and preservesQualifiers(arg, param) @@ -1229,6 +1230,7 @@ module ConstructorForwarding { private predicate routineType(Type t) { t.stripTopLevelSpecifiers() instanceof RoutineType } + bindingset[arg] private predicate pointerConversion(Cpp::PointerType arg, Cpp::PointerType param) { exists(Type argBase, Type paramBase | argBase = arg.getBaseType() and @@ -1256,6 +1258,7 @@ module ConstructorForwarding { t instanceof RoutineType } + bindingset[t, category] private predicate referenceAcceptsCategory(Cpp::ReferenceType t, ValueCategory category) { if t instanceof Cpp::LValueReferenceType then @@ -1291,6 +1294,7 @@ module ConstructorForwarding { reference instanceof Cpp::RValueReferenceType and result = XValue() } + bindingset[t] private Type getValueType(Type t, ValueCategory category) { result = t.(FunctionReferenceType).getBaseType().getUnderlyingType() and category = LValue() @@ -1304,13 +1308,23 @@ module ConstructorForwarding { category = PRValue() } - bindingset[arg, param] - pragma[inline_late] + bindingset[arg] private predicate qualificationCompatible(Type arg, Type param) { arg.stripTopLevelSpecifiers() = param.stripTopLevelSpecifiers() and preservesQualifiers(arg, param) } + bindingset[argType, category] + private predicate matchesParameter(Type argType, ValueCategory category, Type paramType) { + exists(Type base | base = paramType.(Cpp::ReferenceType).getBaseType() | + qualificationCompatible(argType, base) and + referenceAcceptsCategory(paramType, category) + ) + or + not paramType instanceof Cpp::ReferenceType and + argType.stripTopLevelSpecifiers() = paramType.stripTopLevelSpecifiers() + } + private Cpp::PointerType pointerType(Type base) { result.getBaseType() = base.getUnderlyingType() } @@ -1398,7 +1412,8 @@ module ConstructorForwarding { private predicate convertingConstructorStep(TypeState argState, TypeState paramState) { exists(ConvertingConstructor constructor | argState.hasNotUsedConversion() and - argState.matchesParameter(constructor.getUnderlyingFromType()) and + matchesParameter(argState.getType(), argState.getCategory(), + constructor.getUnderlyingFromType()) and paramState = MkTypeState(constructor.getDeclaringType(), PRValue(), true, Initial()) ) } @@ -1472,19 +1487,9 @@ module ConstructorForwarding { ) } - predicate matchesParameter(Type paramType) { - exists(Type base | base = paramType.(Cpp::ReferenceType).getBaseType() | - qualificationCompatible(this.getType(), base) and - referenceAcceptsCategory(paramType, this.getCategory()) - ) - or - not paramType instanceof Cpp::ReferenceType and - this.getType().stripTopLevelSpecifiers() = paramType.stripTopLevelSpecifiers() - } - predicate isSink(Type paramType, Cpp::Constructor constructor) { paramType = getConstructorParameterType(constructor, _) and - this.matchesParameter(paramType) + matchesParameter(this.getType(), this.getCategory(), paramType) } } From 33b834623235a3e0aa5db540e2cca7b37ec7b6eb Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Wed, 4 Nov 2026 20:17:00 +0100 Subject: [PATCH 3/4] C++: Do not define `typeStep` using `TypeState` columns. Instead we: 1. first do a forward pass to first compute all the reachable type states, and then 2. define the `TypeState` type. This commit does (1). The next commit does (2). --- .../semmle/code/cpp/dataflow/ExternalFlow.qll | 145 ++++++++++-------- 1 file changed, 78 insertions(+), 67 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll b/cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll index 07b48d3766fc..a8530e0c73ca 100644 --- a/cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll +++ b/cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll @@ -1329,21 +1329,12 @@ module ConstructorForwarding { result.getBaseType() = base.getUnderlyingType() } - private predicate arrayToPointerStep(TypeState argState, TypeState paramState) { - exists(Cpp::ArrayType array, boolean conversionUsed | - argState = MkTypeState(array, _, conversionUsed, Initial()) and - paramState = - MkTypeState(pointerType(array.getBaseType()), PRValue(), conversionUsed, - AfterTransformation()) - ) + private predicate arrayToPointerStep(Cpp::ArrayType array, Cpp::PointerType pointer) { + pointer = pointerType(array.getBaseType()) } - private predicate functionToPointerStep(TypeState argState, TypeState paramState) { - exists(RoutineType routine, FunctionPointerType pointer, boolean conversionUsed | - argState = MkTypeState(routine, _, conversionUsed, Initial()) and - routine = pointer.getBaseType().getUnderlyingType() and - paramState = MkTypeState(pointer, PRValue(), conversionUsed, AfterTransformation()) - ) + private predicate functionToPointerStep(RoutineType routine, FunctionPointerType pointer) { + routine = pointer.getBaseType().getUnderlyingType() } private predicate arithmeticConversion(Type t) { @@ -1354,14 +1345,12 @@ module ConstructorForwarding { private predicate hasNoTopLevelSpecifiers(Type type) { type = type.stripTopLevelSpecifiers() } - private predicate valueConversionStep(TypeState argState, TypeState paramState) { - exists(Type argType, Type paramType, boolean conversionUsed | - argState = MkTypeState(_, _, conversionUsed, _) and - argState.canConvertValue() and - argType = argState.getType().stripTopLevelSpecifiers() and + bindingset[type] + private predicate valueConversionStep(Type type, Type paramType) { + exists(Type argType | + argType = type.stripTopLevelSpecifiers() and hasNoTopLevelSpecifiers(paramType) and - argType != paramType and - paramState = MkTypeState(paramType, PRValue(), conversionUsed, AfterValueConversion()) + argType != paramType | paramType instanceof ArithmeticType and arithmeticConversion(argType) @@ -1388,64 +1377,82 @@ module ConstructorForwarding { k = FunPtrKind() } - private predicate pointerQualificationStep(TypeState argState, TypeState paramState) { - exists(Type argType, Type paramType, boolean conversionUsed, PtrKind k | - argState = MkTypeState(_, _, conversionUsed, _) and - argState.getPhase() != AfterQualification() and - argType = argState.getType().stripTopLevelSpecifiers() and + bindingset[type] + private predicate pointerQualificationStep(Type type, Type paramType) { + exists(Type argType, PtrKind k | + argType = type.stripTopLevelSpecifiers() and hasNoTopLevelSpecifiers(paramType) and argType != paramType and - qualificationCompatible(pointerBase(argType, k), pointerBase(paramType, k)) and - paramState = MkTypeState(paramType, PRValue(), conversionUsed, AfterQualification()) + qualificationCompatible(pointerBase(argType, pragma[only_bind_into](k)), + pointerBase(paramType, pragma[only_bind_into](k))) ) } - private predicate baseClassStep(TypeState argState, TypeState paramState) { - exists(Type arg, Type param, ValueCategory category, boolean conversionUsed | - argState = MkTypeState(arg, category, conversionUsed, _) and - argState.canConvertValue() and - baseTypeCompatible(arg, param) and - paramState = MkTypeState(param, category, conversionUsed, AfterValueConversion()) - ) + bindingset[argType] + private predicate baseClassStep(Type argType, Type paramType) { + baseTypeCompatible(argType, paramType) } - private predicate convertingConstructorStep(TypeState argState, TypeState paramState) { + bindingset[argType, argCategory] + private predicate convertingConstructorStep( + Type argType, ValueCategory argCategory, Class paramType + ) { exists(ConvertingConstructor constructor | - argState.hasNotUsedConversion() and - matchesParameter(argState.getType(), argState.getCategory(), - constructor.getUnderlyingFromType()) and - paramState = MkTypeState(constructor.getDeclaringType(), PRValue(), true, Initial()) + matchesParameter(argType, argCategory, constructor.getUnderlyingFromType()) and + paramType = constructor.getDeclaringType() ) } - private predicate conversionOperatorStep(TypeState argState, TypeState paramState) { - exists(ConversionOperator conversion, ValueCategory category | + bindingset[argType] + private predicate conversionOperatorStep(Type argType, Type paramType, ValueCategory paramCategory) { + exists(ConversionOperator conversion | not conversion.isFromUninstantiatedTemplate(_) and not conversion.isExplicit() and not conversion.isDeleted() and - argState.hasNotUsedConversion() and - conversion.getSourceType() = - argState.getType().stripTopLevelSpecifiers().(Class).getABaseClass*() and - paramState = - MkTypeState(getValueType(conversion.getDestType().getUnderlyingType(), category), category, - true, Initial()) + conversion.getSourceType() = argType.stripTopLevelSpecifiers().(Class).getABaseClass*() and + paramType = getValueType(conversion.getDestType().getUnderlyingType(), paramCategory) ) } - private predicate step(TypeState argState, TypeState paramState) { - arrayToPointerStep(argState, paramState) - or - functionToPointerStep(argState, paramState) - or - valueConversionStep(argState, paramState) - or - pointerQualificationStep(argState, paramState) - or - baseClassStep(argState, paramState) - or - convertingConstructorStep(argState, paramState) + bindingset[argType, argCategory, argConversionUsed, argPhase] + pragma[inline_late] + private predicate typeStep( + Type argType, ValueCategory argCategory, boolean argConversionUsed, Phase argPhase, + Type paramType, ValueCategory paramCategory, boolean paramConversionUsed, Phase paramPhase + ) { + paramConversionUsed = argConversionUsed and + ( + argPhase = Initial() and + paramCategory = PRValue() and + paramPhase = AfterTransformation() and + ( + arrayToPointerStep(argType, paramType) + or + functionToPointerStep(argType, paramType) + ) + or + (argPhase = Initial() or argPhase = AfterTransformation()) and + paramPhase = AfterValueConversion() and + ( + valueConversionStep(argType, paramType) and paramCategory = PRValue() + or + baseClassStep(argType, paramType) and paramCategory = argCategory + ) + or + argPhase != AfterQualification() and + pointerQualificationStep(argType, paramType) and + paramCategory = PRValue() and + paramPhase = AfterQualification() + ) or - conversionOperatorStep(argState, paramState) + argConversionUsed = false and + paramConversionUsed = true and + paramPhase = Initial() and + ( + convertingConstructorStep(argType, argCategory, paramType) and paramCategory = PRValue() + or + conversionOperatorStep(argType, paramType, paramCategory) + ) } private predicate typeFwd(TypeState state) { @@ -1471,13 +1478,6 @@ module ConstructorForwarding { Phase getPhase() { this = MkTypeState(_, _, _, result) } - /** Holds if this state's phase permits a value or base-class conversion. */ - predicate canConvertValue() { - this.getPhase() = Initial() or this.getPhase() = AfterTransformation() - } - - predicate hasNotUsedConversion() { this = MkTypeState(_, _, false, _) } - string toString() { result = this.getType().toString() } predicate isSource(Type argType) { @@ -1493,6 +1493,17 @@ module ConstructorForwarding { } } + private predicate step(TypeState argState, TypeState paramState) { + exists( + Type t1, Type t2, ValueCategory vc1, ValueCategory vc2, boolean b1, boolean b2, Phase p1, + Phase p2 + | + argState = MkTypeState(t1, vc1, b1, p1) and + paramState = MkTypeState(t2, vc2, b2, p2) and + typeStep(t1, vc1, b1, p1, t2, vc2, b2, p2) + ) + } + private predicate typeRev(TypeState state, Cpp::Constructor constructor) { typeFwd(state) and ( From dacd42aa26269deca6b16061940f0891f7599104 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Wed, 4 Nov 2026 20:17:31 +0100 Subject: [PATCH 4/4] C++: Restrict `TypeState` to reachable tuples. --- .../semmle/code/cpp/dataflow/ExternalFlow.qll | 40 +++++++++++-------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll b/cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll index a8530e0c73ca..008a7c51c8e7 100644 --- a/cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll +++ b/cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll @@ -1455,20 +1455,29 @@ module ConstructorForwarding { ) } - private predicate typeFwd(TypeState state) { - state.isSource(_) - or - exists(TypeState previous | - typeFwd(previous) and - step(previous, state) + private predicate typeFwd(Type type, ValueCategory category, boolean conversionUsed, Phase phase) { + not type instanceof Cpp::ReferenceType and + not type instanceof FunctionReferenceType and + isUnderlyingType(type) and + ( + type = getValueType(getForwardedArgumentType(_, _, _), category) and + conversionUsed = false and + phase = Initial() + or + exists( + Type previousType, ValueCategory previousCategory, boolean previousConversionUsed, + Phase previousPhase + | + typeFwd(previousType, previousCategory, previousConversionUsed, previousPhase) and + typeStep(previousType, previousCategory, previousConversionUsed, previousPhase, type, + category, conversionUsed, phase) + ) ) } private newtype TTypeState = MkTypeState(Type type, ValueCategory category, Boolean conversionUsed, Phase phase) { - not type instanceof Cpp::ReferenceType and - not type instanceof FunctionReferenceType and - isUnderlyingType(type) + typeFwd(type, category, conversionUsed, phase) } private class TypeState extends TTypeState { @@ -1505,14 +1514,11 @@ module ConstructorForwarding { } private predicate typeRev(TypeState state, Cpp::Constructor constructor) { - typeFwd(state) and - ( - state.isSink(_, constructor) - or - exists(TypeState next | - typeRev(next, constructor) and - step(state, next) - ) + state.isSink(_, constructor) + or + exists(TypeState next | + typeRev(next, constructor) and + step(state, next) ) }