From 94e08e318de8d73c35ab126ee4fc437ae9dbe863 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Thu, 10 Apr 2025 14:18:47 +0100 Subject: [PATCH 1/6] C++: Expose a few predicates from 'ExternalFlow'. --- .../semmle/code/cpp/dataflow/ExternalFlow.qll | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll b/cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll index 9e335dbcbfed..456768081a11 100644 --- a/cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll +++ b/cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll @@ -465,7 +465,7 @@ private predicate isFunctionConstructedFrom(Function f, Function templateFunc) { } /** Gets the fully templated version of `f`. */ -private Function getFullyTemplatedFunction(Function f) { +Function getFullyTemplatedFunction(Function f) { not f.isFromUninstantiatedTemplate(_) and ( exists(Class c, Class templateClass, int i | @@ -559,12 +559,15 @@ private string getTypeName(Type t, boolean needsSpace) { /** * Gets a type name for the `n`'th parameter of `f` without any template - * arguments. The result may be a string representing a type for which the - * typedefs have been resolved. + * arguments. + * + * If `canonical = false` then the result may be a string representing a type + * for which the typedefs have been resolved. If `canonical = true` then the + * result will be a string representing a type without resolving `typedefs`. */ bindingset[f] pragma[inline_late] -string getParameterTypeWithoutTemplateArguments(Function f, int n) { +string getParameterTypeWithoutTemplateArguments(Function f, int n, boolean canonical) { exists(string s, string base, string specifiers, Type t | t = f.getParameter(n).getType() and // The name of the string can either be the possibly typedefed name @@ -572,14 +575,19 @@ string getParameterTypeWithoutTemplateArguments(Function f, int n) { // `getTypeName(t, _)` is almost equal to `t.resolveTypedefs().getName()`, // except that `t.resolveTypedefs()` doesn't have a result when the // resulting type doesn't appear in the database. - s = [t.getName(), getTypeName(t, _)] and + ( + s = t.getName() and canonical = true + or + s = getTypeName(t, _) and canonical = false + ) and parseAngles(s, base, _, specifiers) and result = base + specifiers ) or f.isVarargs() and n = f.getNumberOfParameters() and - result = "..." + result = "..." and + canonical = true } /** @@ -590,7 +598,7 @@ private string getTypeNameWithoutFunctionTemplates(Function f, int n, int remain exists(Function templateFunction | templateFunction = getFullyTemplatedFunction(f) and remaining = templateFunction.getNumberOfTemplateArguments() and - result = getParameterTypeWithoutTemplateArguments(templateFunction, n) + result = getParameterTypeWithoutTemplateArguments(templateFunction, n, _) ) or exists(string mid, TypeTemplateParameter tp, Function templateFunction | @@ -627,7 +635,7 @@ private string getTypeNameWithoutClassTemplates(Function f, int n, int remaining } /** Gets the string representation of the `i`'th parameter of `c`. */ -private string getParameterTypeName(Function c, int i) { +string getParameterTypeName(Function c, int i) { result = getTypeNameWithoutClassTemplates(c, i, 0) } From 960e9900af2bacc094d64e162adfbac931e7e3b7 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Thu, 10 Apr 2025 14:20:56 +0100 Subject: [PATCH 2/6] C++: Move the 'getArgumentIndex' into the abstract 'Position' class. It is implemented in all subclasses anyway. --- .../cpp/ir/dataflow/internal/DataFlowPrivate.qll | 14 +++++++++++--- .../code/cpp/ir/dataflow/internal/DataFlowUtil.qll | 6 +++--- .../cpp/ir/dataflow/internal/TaintTrackingUtil.qll | 4 ++-- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll index 8a5155239304..fdfbb07f2a7c 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll @@ -371,7 +371,7 @@ private class PrimaryArgumentNode extends ArgumentNode, OperandNode { PrimaryArgumentNode() { exists(CallInstruction call | op = call.getAnArgumentOperand()) } override predicate argumentOf(DataFlowCall call, ArgumentPosition pos) { - op = call.getArgumentOperand(pos.(DirectPosition).getIndex()) + op = call.getArgumentOperand(pos.(DirectPosition).getArgumentIndex()) } } @@ -410,8 +410,16 @@ class ParameterPosition = Position; class ArgumentPosition = Position; abstract class Position extends TPosition { + /** Gets a textual representation of this position. */ abstract string toString(); + /** + * Gets the argument index of this position. The qualifier of a call has + * argument index `-1`. + */ + abstract int getArgumentIndex(); + + /** Gets the indirection index of this position. */ abstract int getIndirectionIndex(); } @@ -428,7 +436,7 @@ class DirectPosition extends Position, TDirectPosition { result = index.toString() } - int getIndex() { result = index } + override int getArgumentIndex() { result = index } final override int getIndirectionIndex() { result = 0 } } @@ -445,7 +453,7 @@ class IndirectionPosition extends Position, TIndirectionPosition { else result = repeatStars(indirectionIndex) + argumentIndex.toString() } - int getArgumentIndex() { result = argumentIndex } + override int getArgumentIndex() { result = argumentIndex } final override int getIndirectionIndex() { result = indirectionIndex } } diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll index 3b6e190cc981..62ad9f02fe29 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll @@ -1445,7 +1445,7 @@ private class ExplicitParameterInstructionNode extends AbstractExplicitParameter ExplicitParameterInstructionNode() { exists(instr.getParameter()) } override predicate isSourceParameterOf(Function f, ParameterPosition pos) { - f.getParameter(pos.(DirectPosition).getIndex()) = instr.getParameter() + f.getParameter(pos.(DirectPosition).getArgumentIndex()) = instr.getParameter() } override string toStringImpl() { result = instr.getParameter().toString() } @@ -1460,7 +1460,7 @@ class ThisParameterInstructionNode extends AbstractExplicitParameterNode, ThisParameterInstructionNode() { instr.getIRVariable() instanceof IRThisVariable } override predicate isSourceParameterOf(Function f, ParameterPosition pos) { - pos.(DirectPosition).getIndex() = -1 and + pos.(DirectPosition).getArgumentIndex() = -1 and instr.getEnclosingFunction() = f } @@ -1494,7 +1494,7 @@ private class DirectBodyLessParameterNode extends AbstractExplicitParameterNode, override predicate isSourceParameterOf(Function f, ParameterPosition pos) { this.getFunction() = f and - f.getParameter(pos.(DirectPosition).getIndex()) = p + f.getParameter(pos.(DirectPosition).getArgumentIndex()) = p } override Parameter getParameter() { result = p } diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/TaintTrackingUtil.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/TaintTrackingUtil.qll index b6d332e3d4c2..83fac3ebb49a 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/TaintTrackingUtil.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/TaintTrackingUtil.qll @@ -229,11 +229,11 @@ private module SpeculativeTaintFlow { not exists(DataFlowDispatch::viableCallable(call)) and src.(DataFlowPrivate::ArgumentNode).argumentOf(call, argpos) | - not argpos.(DirectPosition).getIndex() = -1 and + not argpos.(DirectPosition).getArgumentIndex() = -1 and sink.(PostUpdateNode) .getPreUpdateNode() .(DataFlowPrivate::ArgumentNode) - .argumentOf(call, any(DirectPosition qualpos | qualpos.getIndex() = -1)) + .argumentOf(call, any(DirectPosition qualpos | qualpos.getArgumentIndex() = -1)) or sink.(DataFlowPrivate::OutNode).getCall() = call ) From b678112f4d1d82facf6de22654e22b403994888f Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Thu, 10 Apr 2025 14:21:50 +0100 Subject: [PATCH 3/6] C++: Add a few predicates to 'ReturnKind'. --- .../ir/dataflow/internal/DataFlowPrivate.qll | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll index fdfbb07f2a7c..44d7a6d725a0 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll @@ -509,6 +509,15 @@ class ReturnKind extends TReturnKind { /** Gets a textual representation of this return kind. */ abstract string toString(); + + /** Holds if this `ReturnKind` is generated from a `return` statement. */ + abstract predicate isNormalReturn(); + + /** + * Holds if this `ReturnKind` is generated from a write to the parameter with + * index `argumentIndex` + */ + abstract predicate isIndirectReturn(int argumentIndex); } /** @@ -522,6 +531,10 @@ class NormalReturnKind extends ReturnKind, TNormalReturnKind { override int getIndirectionIndex() { result = indirectionIndex } override string toString() { result = "indirect return" } + + override predicate isNormalReturn() { any() } + + override predicate isIndirectReturn(int argumentIndex) { none() } } /** @@ -536,6 +549,10 @@ private class IndirectReturnKind extends ReturnKind, TIndirectReturnKind { override int getIndirectionIndex() { result = indirectionIndex } override string toString() { result = "indirect outparam[" + argumentIndex.toString() + "]" } + + override predicate isNormalReturn() { none() } + + override predicate isIndirectReturn(int argumentIndex_) { argumentIndex_ = argumentIndex } } /** A data flow node that occurs as the result of a `ReturnStmt`. */ From 3bb249f580eb28c84f6e523d9428fb0abca32ac6 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Thu, 10 Apr 2025 14:23:38 +0100 Subject: [PATCH 4/6] C++: Ensure we always have 'Position's even if there are no calls in the DB. --- .../cpp/ir/dataflow/internal/DataFlowPrivate.qll | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll index 44d7a6d725a0..1cf67dba1ee5 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll @@ -459,10 +459,23 @@ class IndirectionPosition extends Position, TIndirectionPosition { } newtype TPosition = - TDirectPosition(int argumentIndex) { exists(any(CallInstruction c).getArgument(argumentIndex)) } or + TDirectPosition(int argumentIndex) { + exists(any(CallInstruction c).getArgument(argumentIndex)) + or + // Handle the rare case where the is a function definition but no call to + // the function. + exists(any(Cpp::Function f).getParameter(argumentIndex)) + } or TIndirectionPosition(int argumentIndex, int indirectionIndex) { Ssa::hasIndirectOperand(any(CallInstruction call).getArgumentOperand(argumentIndex), indirectionIndex) + or + // Handle the rare case where the is a function definition but no call to + // the function. + exists(Cpp::Function f, Cpp::Parameter p | + p = f.getParameter(argumentIndex) and + indirectionIndex = [1 .. Ssa::getMaxIndirectionsForType(p.getUnspecifiedType()) - 1] + ) } private newtype TReturnKind = From bfc494c0e1d08da309bf0c6872b677531ab409b1 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Fri, 11 Apr 2025 12:43:51 +0100 Subject: [PATCH 5/6] Update cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll Co-authored-by: Taus --- .../semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll index 1cf67dba1ee5..6c096d098cd0 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll @@ -462,7 +462,7 @@ newtype TPosition = TDirectPosition(int argumentIndex) { exists(any(CallInstruction c).getArgument(argumentIndex)) or - // Handle the rare case where the is a function definition but no call to + // Handle the rare case where there is a function definition but no call to // the function. exists(any(Cpp::Function f).getParameter(argumentIndex)) } or From deef95d3847c899e20deb9cb80794966860f06b6 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Fri, 11 Apr 2025 12:43:59 +0100 Subject: [PATCH 6/6] Update cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll Co-authored-by: Taus --- .../semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll index 6c096d098cd0..c5024d07dcb4 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll @@ -470,7 +470,7 @@ newtype TPosition = Ssa::hasIndirectOperand(any(CallInstruction call).getArgumentOperand(argumentIndex), indirectionIndex) or - // Handle the rare case where the is a function definition but no call to + // Handle the rare case where there is a function definition but no call to // the function. exists(Cpp::Function f, Cpp::Parameter p | p = f.getParameter(argumentIndex) and