From 83d53baf82981685af77a3547078100ae4d5ba02 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Wed, 3 Sep 2025 08:14:20 +0200 Subject: [PATCH 1/2] C++: Fix some Ql4Ql violations. --- cpp/ql/lib/Options.qll | 2 +- .../code/cpp/rangeanalysis/RangeAnalysis.qll | 18 +++++----- cpp/ql/lib/semmle/code/cpp/Concept.qll | 4 +-- cpp/ql/lib/semmle/code/cpp/Declaration.qll | 10 +++--- cpp/ql/lib/semmle/code/cpp/commons/Printf.qll | 4 +-- .../semmle/code/cpp/controlflow/Dominance.qll | 4 +-- .../code/cpp/controlflow/internal/CFG.qll | 4 +-- .../cpp/dataflow/internal/DataFlowUtil.qll | 4 +-- .../cpp/ir/dataflow/internal/DataFlowUtil.qll | 4 +-- .../raw/internal/TranslatedFunction.qll | 2 +- .../raw/internal/TranslatedStmt.qll | 2 +- .../code/cpp/ir/internal/IRUtilities.qll | 3 +- .../interfaces/FunctionInputsAndOutputs.qll | 4 +-- .../new/internal/semantic/SemanticExpr.qll | 3 +- .../semmle/code/cpp/security/FileWrite.qll | 4 ++- .../cpp/security/boostorg/asio/protocols.qll | 14 ++++++-- .../code/cpp/valuenumbering/HashCons.qll | 2 +- .../Magic Constants/MagicConstants.qll | 9 +++-- cpp/ql/src/Likely Bugs/Leap Year/LeapYear.qll | 15 ++++++--- .../Protocols/TlsSettingsMisconfiguration.ql | 2 +- .../src/Metrics/Internal/CallableExtents.ql | 4 +-- .../CWE/CWE-190/IntegerOverflowTainted.ql | 8 ++--- .../CWE/CWE-457/UninitializedVariables.qll | 33 ++++++++++--------- .../IncorrectAllocationErrorHandling.ql | 2 +- cpp/ql/src/definitions.ql | 2 +- .../Likely Bugs/RedundantNullCheckParam.ql | 2 +- ...ionOfVariableWithUnnecessarilyWideScope.ql | 13 ++++---- ...erousWorksWithMultibyteOrWideCharacters.ql | 2 +- .../IncorrectChangingWorkingDirectory.ql | 4 +-- .../CWE/CWE-416/UseAfterExpiredLifetime.ql | 2 +- cpp/ql/src/external/DefectFilter.qll | 4 +-- cpp/ql/src/external/MetricFilter.qll | 4 +-- cpp/ql/src/jsf/4.10 Classes/AV Rule 96.ql | 2 +- 33 files changed, 111 insertions(+), 85 deletions(-) diff --git a/cpp/ql/lib/Options.qll b/cpp/ql/lib/Options.qll index a0a13881a941..c4652e3f6cae 100644 --- a/cpp/ql/lib/Options.qll +++ b/cpp/ql/lib/Options.qll @@ -35,7 +35,7 @@ class CustomOptions extends Options { override predicate returnsNull(Call call) { Options.super.returnsNull(call) } /** - * Holds if a call to this function will never return. + * Holds if a call to the function `f` will never return. * * By default, this holds for `exit`, `_exit`, `abort`, `__assert_fail`, * `longjmp`, `error`, `__builtin_unreachable` and any function with a diff --git a/cpp/ql/lib/experimental/semmle/code/cpp/rangeanalysis/RangeAnalysis.qll b/cpp/ql/lib/experimental/semmle/code/cpp/rangeanalysis/RangeAnalysis.qll index e5de44b396d8..e026c4dbe4b9 100644 --- a/cpp/ql/lib/experimental/semmle/code/cpp/rangeanalysis/RangeAnalysis.qll +++ b/cpp/ql/lib/experimental/semmle/code/cpp/rangeanalysis/RangeAnalysis.qll @@ -298,10 +298,11 @@ private predicate boundFlowStep(Instruction i, NonPhiOperand op, int delta, bool else if strictlyNegative(x) then upper = true and delta = -1 - else - if negative(x) - then upper = true and delta = 0 - else none() + else ( + negative(x) and + upper = true and + delta = 0 + ) ) or exists(Operand x | @@ -321,10 +322,11 @@ private predicate boundFlowStep(Instruction i, NonPhiOperand op, int delta, bool else if strictlyNegative(x) then upper = false and delta = 1 - else - if negative(x) - then upper = false and delta = 0 - else none() + else ( + negative(x) and + upper = false and + delta = 0 + ) ) or i.(RemInstruction).getRightOperand() = op and positive(op) and delta = -1 and upper = true diff --git a/cpp/ql/lib/semmle/code/cpp/Concept.qll b/cpp/ql/lib/semmle/code/cpp/Concept.qll index 1770c1965ed2..6e66bca98238 100644 --- a/cpp/ql/lib/semmle/code/cpp/Concept.qll +++ b/cpp/ql/lib/semmle/code/cpp/Concept.qll @@ -198,7 +198,7 @@ class ConceptIdExpr extends Expr, @concept_id { final Locatable getATemplateArgumentKind() { result = this.getTemplateArgumentKind(_) } /** - * Gets the `i`th template argument passed to the concept. + * Gets template argument at index `index` passed to the concept, if any. * * For example, if: * ```cpp @@ -219,7 +219,7 @@ class ConceptIdExpr extends Expr, @concept_id { } /** - * Gets the kind of the `i`th template argument value passed to the concept. + * Gets the kind of the template argument value at index `index` passed to the concept, if any. * * For example, if: * ```cpp diff --git a/cpp/ql/lib/semmle/code/cpp/Declaration.qll b/cpp/ql/lib/semmle/code/cpp/Declaration.qll index cedacca4dfe4..6f791234b638 100644 --- a/cpp/ql/lib/semmle/code/cpp/Declaration.qll +++ b/cpp/ql/lib/semmle/code/cpp/Declaration.qll @@ -223,8 +223,8 @@ class Declaration extends Locatable, @declaration { final Locatable getATemplateArgumentKind() { result = this.getTemplateArgumentKind(_) } /** - * Gets the `i`th template argument used to instantiate this declaration from a - * template. + * Gets the template argument at index `index` used to instantiate this declaration from a + * template, if any. * * For example: * @@ -245,9 +245,9 @@ class Declaration extends Locatable, @declaration { } /** - * Gets the `i`th template argument value used to instantiate this declaration - * from a template. When called on a template, this will return the `i`th template - * parameter value if it exists. + * Gets the template argument value at index `index` used to instantiate this declaration + * from a template. When called on a template, this will return the template + * parameter value at index `index` if it exists. * * For example: * diff --git a/cpp/ql/lib/semmle/code/cpp/commons/Printf.qll b/cpp/ql/lib/semmle/code/cpp/commons/Printf.qll index 51d2e294b36e..023ce09c5c18 100644 --- a/cpp/ql/lib/semmle/code/cpp/commons/Printf.qll +++ b/cpp/ql/lib/semmle/code/cpp/commons/Printf.qll @@ -877,7 +877,7 @@ class FormatLiteral extends Literal instanceof StringLiteral { } /** - * Gets the char type required by the nth conversion specifier. + * Gets the char type required by the `n`th conversion specifier. * - in the base case this is the default for the formatting function * (e.g. `char` for `printf`, `char` or `wchar_t` for `wprintf`). * - the `%C` format character reverses wideness. @@ -922,7 +922,7 @@ class FormatLiteral extends Literal instanceof StringLiteral { } /** - * Gets the string type required by the nth conversion specifier. + * Gets the string type required by the `n`th conversion specifier. * - in the base case this is the default for the formatting function * (e.g. `char *` for `printf`, `char *` or `wchar_t *` for `wprintf`). * - the `%S` format character reverses wideness on some platforms. diff --git a/cpp/ql/lib/semmle/code/cpp/controlflow/Dominance.qll b/cpp/ql/lib/semmle/code/cpp/controlflow/Dominance.qll index 424e615f3ea8..a8a4b867ba67 100644 --- a/cpp/ql/lib/semmle/code/cpp/controlflow/Dominance.qll +++ b/cpp/ql/lib/semmle/code/cpp/controlflow/Dominance.qll @@ -101,7 +101,7 @@ predicate postDominates(ControlFlowNode postDominator, ControlFlowNode node) { */ /** - * Holds if `dominator` is an immediate dominator of `node` in the control-flow + * Holds if `dom` is an immediate dominator of `node` in the control-flow * graph of basic blocks. */ predicate bbIDominates(BasicBlock dom, BasicBlock node) = @@ -117,7 +117,7 @@ private predicate bb_predecessor(BasicBlock succ, BasicBlock pred) { bb_successo private predicate bb_exit(ExitBasicBlock exit) { any() } /** - * Holds if `postDominator` is an immediate post-dominator of `node` in the control-flow + * Holds if `pDom` is an immediate post-dominator of `node` in the control-flow * graph of basic blocks. */ predicate bbIPostDominates(BasicBlock pDom, BasicBlock node) = diff --git a/cpp/ql/lib/semmle/code/cpp/controlflow/internal/CFG.qll b/cpp/ql/lib/semmle/code/cpp/controlflow/internal/CFG.qll index 05eafe28275f..23ef58592216 100644 --- a/cpp/ql/lib/semmle/code/cpp/controlflow/internal/CFG.qll +++ b/cpp/ql/lib/semmle/code/cpp/controlflow/internal/CFG.qll @@ -1042,8 +1042,8 @@ private predicate subEdgeIncludingDestructors(Pos p1, Node n1, Node n2, Pos p2) * - `MicrosoftTryFinallyStmt`: On the edge following the `__finally` block for * the case where an exception was thrown and needs to be propagated. */ -DestructorCall getSynthesisedDestructorCallAfterNode(Node n, int i) { - synthetic_destructor_call(n, i, result) +DestructorCall getSynthesisedDestructorCallAfterNode(Node node, int index) { + synthetic_destructor_call(node, index, result) } /** diff --git a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll index 72e742f13aa0..b388669d0415 100644 --- a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll +++ b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll @@ -829,8 +829,8 @@ class ContentSet instanceof Content { /** * Holds if this element is at the specified location. - * The location spans column `startcolumn` of line `startline` to - * column `endcolumn` of line `endline` in file `filepath`. + * The location spans column `sc` of line `sl` to + * column `ec` of line `el` in file `path`. * For more information, see * [Locations](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/). */ 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 ef4051171afb..51ffbefb027d 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 @@ -2268,8 +2268,8 @@ class ContentSet instanceof Content { /** * Holds if this element is at the specified location. - * The location spans column `startcolumn` of line `startline` to - * column `endcolumn` of line `endline` in file `filepath`. + * The location spans column `sc` of line `sl` to + * column `ec` of line `el` in file `path`. * For more information, see * [Locations](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/). */ diff --git a/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedFunction.qll b/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedFunction.qll index 83736ae98d04..26f5393db103 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedFunction.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedFunction.qll @@ -50,7 +50,7 @@ CppType getEllipsisVariablePRValueType() { CppType getEllipsisVariableGLValueType() { result = getTypeForGLValue(any(UnknownType t)) } /** - * Holds if the function returns a value, as opposed to returning `void`. + * Holds if the function `func` returns a value, as opposed to returning `void`. */ predicate hasReturnValue(Function func) { not func.getUnspecifiedType() instanceof VoidType } diff --git a/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedStmt.qll b/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedStmt.qll index 3914c5e8e597..9dccf7752aa8 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedStmt.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedStmt.qll @@ -601,7 +601,7 @@ class TranslatedReturnVoidStmt extends TranslatedReturnStmt { * The IR translation of an implicit `return` statement generated by the extractor to handle control * flow that reaches the end of a non-`void`-returning function body. Such control flow * produces undefined behavior in C++ but not in C. However even in C using the return value is - * undefined behaviour. We make it return uninitialized memory to get as much flow as possible. + * undefined behavior. We make it return uninitialized memory to get as much flow as possible. */ class TranslatedNoValueReturnStmt extends TranslatedReturnStmt, TranslatedVariableInitialization { TranslatedNoValueReturnStmt() { diff --git a/cpp/ql/lib/semmle/code/cpp/ir/internal/IRUtilities.qll b/cpp/ql/lib/semmle/code/cpp/ir/internal/IRUtilities.qll index bfd850384aca..c42f734a62ac 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/internal/IRUtilities.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/internal/IRUtilities.qll @@ -49,7 +49,8 @@ Type getVariableType(Variable v) { } /** - * Holds if the database contains a `case` label with the specified minimum and maximum value. + * Holds if the database contains a `switchCase` label with the specified minimum `minValue` + * and maximum `maxValue` value. */ predicate hasCaseEdge(SwitchCase switchCase, string minValue, string maxValue) { minValue = switchCase.getExpr().getFullyConverted().getValue() and diff --git a/cpp/ql/lib/semmle/code/cpp/models/interfaces/FunctionInputsAndOutputs.qll b/cpp/ql/lib/semmle/code/cpp/models/interfaces/FunctionInputsAndOutputs.qll index 9c8ee43b52a6..d81bc960988c 100644 --- a/cpp/ql/lib/semmle/code/cpp/models/interfaces/FunctionInputsAndOutputs.qll +++ b/cpp/ql/lib/semmle/code/cpp/models/interfaces/FunctionInputsAndOutputs.qll @@ -371,7 +371,7 @@ class FunctionOutput extends TFunctionOutput { /** * Holds if this is the output value pointed to by a pointer parameter to a function, or the * output value referred to by a reference parameter to a function, where the parameter has - * index `index`. + * index `i`. * * Example: * ``` @@ -389,7 +389,7 @@ class FunctionOutput extends TFunctionOutput { /** * Holds if this is the output value pointed to by a pointer parameter (through `ind` number * of indirections) to a function, or the output value referred to by a reference parameter to - * a function, where the parameter has index `index`. + * a function, where the parameter has index `i`. * * Example: * ``` diff --git a/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/SemanticExpr.qll b/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/SemanticExpr.qll index 668d9b52659e..f269d0dfff2d 100644 --- a/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/SemanticExpr.qll +++ b/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/SemanticExpr.qll @@ -307,13 +307,12 @@ class SemStoreExpr extends SemUnaryExpr { } class SemConditionalExpr extends SemKnownExpr { - SemExpr condition; SemExpr trueResult; SemExpr falseResult; SemConditionalExpr() { opcode instanceof Opcode::Conditional and - Specific::conditionalExpr(this, type, condition, trueResult, falseResult) + Specific::conditionalExpr(this, type, any(SemExpr condition), trueResult, falseResult) } final SemExpr getBranchExpr(boolean branch) { diff --git a/cpp/ql/lib/semmle/code/cpp/security/FileWrite.qll b/cpp/ql/lib/semmle/code/cpp/security/FileWrite.qll index dc421e8f3aee..6c0552a7d188 100644 --- a/cpp/ql/lib/semmle/code/cpp/security/FileWrite.qll +++ b/cpp/ql/lib/semmle/code/cpp/security/FileWrite.qll @@ -21,7 +21,9 @@ class FileWrite extends Expr { Expr getDest() { fileWrite(this, _, result) } /** - * Gets the conversion character for this write, if it exists and is known. For example in the following code the write of `value1` has conversion character `"s"`, whereas the write of `value2` has no conversion specifier. + * Gets the conversion character from `source` for this write, if it exists and is known. + * For example in the following code the write of `value1` has conversion character `"s"`, whereas + * the write of `value2` has no conversion specifier. * ``` * fprintf(file, "%s", value1); * stream << value2; diff --git a/cpp/ql/lib/semmle/code/cpp/security/boostorg/asio/protocols.qll b/cpp/ql/lib/semmle/code/cpp/security/boostorg/asio/protocols.qll index 559ebd444f32..984bd874ae51 100644 --- a/cpp/ql/lib/semmle/code/cpp/security/boostorg/asio/protocols.qll +++ b/cpp/ql/lib/semmle/code/cpp/security/boostorg/asio/protocols.qll @@ -191,11 +191,19 @@ module BoostorgAsio { class SslContextClass extends Class { SslContextClass() { this.getQualifiedName() = "boost::asio::ssl::context" } - ConstructorCall getAContructorCall() { + /** + * Gets a constructor call, if any. + */ + ConstructorCall getAConstructorCall() { this.getAConstructor().getACallToThisFunction() = result and not result.getLocation().getFile().toString().matches("%/boost/asio/%") and result.fromSource() } + + /** + * DEPRECATED: Use `getAConstructorCall` instead. + */ + deprecated ConstructorCall getAContructorCall() { result = this.getAConstructorCall() } } /** @@ -368,7 +376,7 @@ module BoostorgAsio { */ default predicate isSink(DataFlow::Node sink) { exists(ConstructorCall cc, SslContextClass c, Expr e | e = sink.asExpr() | - c.getAContructorCall() = cc and + c.getAConstructorCall() = cc and cc.getArgument(0) = e ) } @@ -468,7 +476,7 @@ module BoostorgAsio { predicate isSource(DataFlow::Node source) { exists(SslContextClass c, ConstructorCall cc | cc = source.asExpr() and - c.getAContructorCall() = cc + c.getAConstructorCall() = cc ) } diff --git a/cpp/ql/lib/semmle/code/cpp/valuenumbering/HashCons.qll b/cpp/ql/lib/semmle/code/cpp/valuenumbering/HashCons.qll index b74e7c3d741b..b7e4fecb4742 100644 --- a/cpp/ql/lib/semmle/code/cpp/valuenumbering/HashCons.qll +++ b/cpp/ql/lib/semmle/code/cpp/valuenumbering/HashCons.qll @@ -648,7 +648,7 @@ private predicate mk_UuidofOperator(Type t, UuidofOperator e) { } private predicate analyzableTypeidType(TypeidOperator e) { - count(e.getAChild()) = 0 and + not exists(e.getAChild()) and strictcount(e.getResultType()) = 1 } diff --git a/cpp/ql/src/Best Practices/Magic Constants/MagicConstants.qll b/cpp/ql/src/Best Practices/Magic Constants/MagicConstants.qll index 53e33ab4fa56..033840764830 100644 --- a/cpp/ql/src/Best Practices/Magic Constants/MagicConstants.qll +++ b/cpp/ql/src/Best Practices/Magic Constants/MagicConstants.qll @@ -164,12 +164,17 @@ predicate valueOccurrenceCount(string value, int n) { n > 20 } -predicate occurenceCount(Literal lit, string value, int n) { +predicate occurrenceCount(Literal lit, string value, int n) { valueOccurrenceCount(value, n) and value = lit.getValue() and nonTrivialValue(_, lit) } +/** + * DEPRECATED: Use `occurrenceCount` instead. + */ +deprecated predicate occurenceCount = occurrenceCount/3; + /* * Literals repeated frequently */ @@ -178,7 +183,7 @@ predicate check(Literal lit, string value, int n, File f) { // Check that the literal is nontrivial not trivial(lit) and // Check that it is repeated a number of times - occurenceCount(lit, value, n) and + occurrenceCount(lit, value, n) and n > 20 and f = lit.getFile() and // Exclude generated files diff --git a/cpp/ql/src/Likely Bugs/Leap Year/LeapYear.qll b/cpp/ql/src/Likely Bugs/Leap Year/LeapYear.qll index 863fd1e61203..2b68730fa58d 100644 --- a/cpp/ql/src/Likely Bugs/Leap Year/LeapYear.qll +++ b/cpp/ql/src/Likely Bugs/Leap Year/LeapYear.qll @@ -128,11 +128,18 @@ abstract class LeapYearFieldAccess extends YearFieldAccess { /** * Holds if the top-level binary operation includes an addition or subtraction operator with an operand specified by `valueToCheck`. */ - predicate additionalAdditionOrSubstractionCheckForLeapYear(int valueToCheck) { + predicate additionalAdditionOrSubtractionCheckForLeapYear(int valueToCheck) { additionalLogicalCheck(this, "+", valueToCheck) or additionalLogicalCheck(this, "-", valueToCheck) } + /** + * DEPRECATED: Use `additionalAdditionOrSubtractionCheckForLeapYear` instead. + */ + deprecated predicate additionalAdditionOrSubstractionCheckForLeapYear(int valueToCheck) { + this.additionalAdditionOrSubtractionCheckForLeapYear(valueToCheck) + } + /** * Holds if this object is used on a modulus 4 operation, which would likely indicate the start of a leap year check. */ @@ -180,13 +187,13 @@ class StructTmLeapYearFieldAccess extends LeapYearFieldAccess { this.additionalModulusCheckForLeapYear(100) and // tm_year represents years since 1900 ( - this.additionalAdditionOrSubstractionCheckForLeapYear(1900) + this.additionalAdditionOrSubtractionCheckForLeapYear(1900) or // some systems may use 2000 for 2-digit year conversions - this.additionalAdditionOrSubstractionCheckForLeapYear(2000) + this.additionalAdditionOrSubtractionCheckForLeapYear(2000) or // converting from/to Unix epoch - this.additionalAdditionOrSubstractionCheckForLeapYear(1970) + this.additionalAdditionOrSubtractionCheckForLeapYear(1970) ) } } diff --git a/cpp/ql/src/Likely Bugs/Protocols/TlsSettingsMisconfiguration.ql b/cpp/ql/src/Likely Bugs/Protocols/TlsSettingsMisconfiguration.ql index f5d1a09d04e9..e50a2d545431 100644 --- a/cpp/ql/src/Likely Bugs/Protocols/TlsSettingsMisconfiguration.ql +++ b/cpp/ql/src/Likely Bugs/Protocols/TlsSettingsMisconfiguration.ql @@ -14,7 +14,7 @@ import cpp import semmle.code.cpp.security.boostorg.asio.protocols predicate isSourceImpl(DataFlow::Node source, ConstructorCall cc) { - exists(BoostorgAsio::SslContextClass c | c.getAContructorCall() = cc and cc = source.asExpr()) + exists(BoostorgAsio::SslContextClass c | c.getAConstructorCall() = cc and cc = source.asExpr()) } predicate isSinkImpl(DataFlow::Node sink, FunctionCall fcSetOptions) { diff --git a/cpp/ql/src/Metrics/Internal/CallableExtents.ql b/cpp/ql/src/Metrics/Internal/CallableExtents.ql index 7a376c6da721..eef36e91bffe 100644 --- a/cpp/ql/src/Metrics/Internal/CallableExtents.ql +++ b/cpp/ql/src/Metrics/Internal/CallableExtents.ql @@ -15,8 +15,8 @@ import cpp class RangeFunction extends Function { /** * Holds if this function is at the specified location. - * The location spans column `startcolumn` of line `startline` to - * column `endcolumn` of line `endline` in file `filepath`. + * The location spans column `sc` of line `sl` to + * column `ec` of line `el` in file `path`. * For more information, see * [Locations](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/). */ diff --git a/cpp/ql/src/Security/CWE/CWE-190/IntegerOverflowTainted.ql b/cpp/ql/src/Security/CWE/CWE-190/IntegerOverflowTainted.ql index 6ff06d355b9b..3978d2ded95d 100644 --- a/cpp/ql/src/Security/CWE/CWE-190/IntegerOverflowTainted.ql +++ b/cpp/ql/src/Security/CWE/CWE-190/IntegerOverflowTainted.ql @@ -25,10 +25,10 @@ import semmle.code.cpp.controlflow.IRGuards as IRGuards predicate outOfBoundsExpr(Expr expr, string kind) { if convertedExprMightOverflowPositively(expr) then kind = "overflow" - else - if convertedExprMightOverflowNegatively(expr) - then kind = "overflow negatively" - else none() + else ( + convertedExprMightOverflowNegatively(expr) and + kind = "overflow negatively" + ) } predicate isSource(FS::FlowSource source, string sourceType) { sourceType = source.getSourceType() } diff --git a/cpp/ql/src/Security/CWE/CWE-457/UninitializedVariables.qll b/cpp/ql/src/Security/CWE/CWE-457/UninitializedVariables.qll index 14eec81ff58f..804d6a48754c 100644 --- a/cpp/ql/src/Security/CWE/CWE-457/UninitializedVariables.qll +++ b/cpp/ql/src/Security/CWE/CWE-457/UninitializedVariables.qll @@ -31,27 +31,28 @@ private predicate hasConditionalInitialization( class ConditionallyInitializedVariable extends LocalVariable { ConditionalInitializationCall call; ConditionalInitializationFunction f; - VariableAccess initAccess; Evidence e; ConditionallyInitializedVariable() { // Find a call that conditionally initializes this variable - hasConditionalInitialization(f, call, this, initAccess, e) and - // Ignore cases where the variable is assigned prior to the call - not reaches(this.getAnAssignedValue(), initAccess) and - // Ignore cases where the variable is assigned field-wise prior to the call. - not exists(FieldAccess fa | - exists(Assignment a | - fa = getAFieldAccess(this) and - a.getLValue() = fa + exists(VariableAccess initAccess | + hasConditionalInitialization(f, call, this, initAccess, e) and + // Ignore cases where the variable is assigned prior to the call + not reaches(this.getAnAssignedValue(), initAccess) and + // Ignore cases where the variable is assigned field-wise prior to the call. + not exists(FieldAccess fa | + exists(Assignment a | + fa = getAFieldAccess(this) and + a.getLValue() = fa + ) + | + reaches(fa, initAccess) + ) and + // Ignore cases where the variable is assigned by a prior call to an initialization function + not exists(Call c | + this.getAnAccess() = getAnInitializedArgument(c).(AddressOfExpr).getOperand() and + reaches(c, initAccess) ) - | - reaches(fa, initAccess) - ) and - // Ignore cases where the variable is assigned by a prior call to an initialization function - not exists(Call c | - this.getAnAccess() = getAnInitializedArgument(c).(AddressOfExpr).getOperand() and - reaches(c, initAccess) ) and /* * Static local variables with constant initializers do not have the initializer expr as part of diff --git a/cpp/ql/src/Security/CWE/CWE-570/IncorrectAllocationErrorHandling.ql b/cpp/ql/src/Security/CWE/CWE-570/IncorrectAllocationErrorHandling.ql index d4d908f8474b..c03ae995090c 100644 --- a/cpp/ql/src/Security/CWE/CWE-570/IncorrectAllocationErrorHandling.ql +++ b/cpp/ql/src/Security/CWE/CWE-570/IncorrectAllocationErrorHandling.ql @@ -41,7 +41,7 @@ predicate deleteMayThrow(DeleteOrDeleteArrayExpr deleteExpr) { } /** - * Holds if the function may throw an exception when called. That is, if the body of the function looks + * Holds if the function `f` may throw an exception when called. That is, if the body of the function looks * like it might throw an exception, and the function does not have a `noexcept` or `throw()` specifier. */ predicate functionMayThrow(Function f) { diff --git a/cpp/ql/src/definitions.ql b/cpp/ql/src/definitions.ql index c12277eaf23a..339b481f5f77 100644 --- a/cpp/ql/src/definitions.ql +++ b/cpp/ql/src/definitions.ql @@ -13,6 +13,6 @@ where def = definitionOf(e, kind) and // We need to exclude definitions for elements inside template instantiations, // as these often lead to multiple links to definitions from the same source location. - // LGTM does not support this behaviour. + // LGTM does not support this behavior. not e.isFromTemplateInstantiation(_) select e, def, kind diff --git a/cpp/ql/src/experimental/Likely Bugs/RedundantNullCheckParam.ql b/cpp/ql/src/experimental/Likely Bugs/RedundantNullCheckParam.ql index 36e42cae92a4..ce3497bb9655 100644 --- a/cpp/ql/src/experimental/Likely Bugs/RedundantNullCheckParam.ql +++ b/cpp/ql/src/experimental/Likely Bugs/RedundantNullCheckParam.ql @@ -47,7 +47,7 @@ where // for a function parameter unchecked.getTarget() = param and // this function parameter is not overwritten - count(param.getAnAssignment()) = 0 and + not exists(param.getAnAssignment()) and check.getTarget() = param and // which is once checked candidateResultChecked(check, eqop) and diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-1126/DeclarationOfVariableWithUnnecessarilyWideScope.ql b/cpp/ql/src/experimental/Security/CWE/CWE-1126/DeclarationOfVariableWithUnnecessarilyWideScope.ql index 136931f00ec6..bbb219a22da7 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-1126/DeclarationOfVariableWithUnnecessarilyWideScope.ql +++ b/cpp/ql/src/experimental/Security/CWE/CWE-1126/DeclarationOfVariableWithUnnecessarilyWideScope.ql @@ -19,16 +19,17 @@ import cpp * Errors when using a variable declaration inside a loop. */ class DangerousWhileLoop extends WhileStmt { - Expr exp; Declaration dl; DangerousWhileLoop() { this = dl.getParentScope().(BlockStmt).getParent*() and - exp = this.getCondition().getAChild*() and - not exp instanceof PointerFieldAccess and - not exp instanceof ValueFieldAccess and - exp.(VariableAccess).getTarget().getName() = dl.getName() and - not exp.getParent*() instanceof FunctionCall + exists(Expr exp | + exp = this.getCondition().getAChild*() and + not exp instanceof PointerFieldAccess and + not exp instanceof ValueFieldAccess and + exp.(VariableAccess).getTarget().getName() = dl.getName() and + not exp.getParent*() instanceof FunctionCall + ) } Declaration getDeclaration() { result = dl } diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-125/DangerousWorksWithMultibyteOrWideCharacters.ql b/cpp/ql/src/experimental/Security/CWE/CWE-125/DangerousWorksWithMultibyteOrWideCharacters.ql index 6529bf6cdf89..74ac8e6da661 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-125/DangerousWorksWithMultibyteOrWideCharacters.ql +++ b/cpp/ql/src/experimental/Security/CWE/CWE-125/DangerousWorksWithMultibyteOrWideCharacters.ql @@ -46,7 +46,7 @@ predicate exprMayBeString(Expr exp) { ) } -/** Holds if expression is constant or operator call `sizeof`. */ +/** Holds if expression `exp` is constant or operator call `sizeof`. */ predicate argConstOrSizeof(Expr exp) { exp.getValue().toInt() > 1 or exp.(SizeofTypeOperator).getTypeOperand().getSize() > 1 diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-243/IncorrectChangingWorkingDirectory.ql b/cpp/ql/src/experimental/Security/CWE/CWE-243/IncorrectChangingWorkingDirectory.ql index ce5f4dd00f87..9d61418fd776 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-243/IncorrectChangingWorkingDirectory.ql +++ b/cpp/ql/src/experimental/Security/CWE/CWE-243/IncorrectChangingWorkingDirectory.ql @@ -15,7 +15,7 @@ import cpp import semmle.code.cpp.commons.Exclusions -/** Holds if a `fc` function call is available before or after a `chdir` function call. */ +/** Holds if a `fcp` function call is available before or after a `chdir` function call. */ predicate inExistsChdir(FunctionCall fcp) { exists(FunctionCall fctmp | ( @@ -29,7 +29,7 @@ predicate inExistsChdir(FunctionCall fcp) { ) } -/** Holds if a `fc` function call is available before or after a function call containing a `chdir` call. */ +/** Holds if a `fcp` function call is available before or after a function call containing a `chdir` call. */ predicate outExistsChdir(FunctionCall fcp) { exists(FunctionCall fctmp | exists(FunctionCall fctmp2 | diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-416/UseAfterExpiredLifetime.ql b/cpp/ql/src/experimental/Security/CWE/CWE-416/UseAfterExpiredLifetime.ql index ffcac802b6dc..fec373ce5216 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-416/UseAfterExpiredLifetime.ql +++ b/cpp/ql/src/experimental/Security/CWE/CWE-416/UseAfterExpiredLifetime.ql @@ -266,7 +266,7 @@ class LifetimePointerType extends LifetimeIndirectionType { class FullExpr extends Expr { FullExpr() { // A full-expression is not a subexpression - not exists(Expr p | this.getParent() = p) + not this.getParent() instanceof Expr or // A sub-expression that is an unevaluated operand this.isUnevaluated() diff --git a/cpp/ql/src/external/DefectFilter.qll b/cpp/ql/src/external/DefectFilter.qll index ad786e9cbc96..a3719140741a 100644 --- a/cpp/ql/src/external/DefectFilter.qll +++ b/cpp/ql/src/external/DefectFilter.qll @@ -5,8 +5,8 @@ import cpp /** * Holds if `id` in the opaque identifier of a result reported by query `queryPath`, * such that `message` is the associated message and the location of the result spans - * column `startcolumn` of line `startline` to column `endcolumn` of line `endline` - * in file `filepath`. + * column `startcol` of line `startline` to column `endcol` of line `endline` + * in file `file`. * * For more information, see [Locations](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/). */ diff --git a/cpp/ql/src/external/MetricFilter.qll b/cpp/ql/src/external/MetricFilter.qll index 0315cd23c8d0..39475451b7a8 100644 --- a/cpp/ql/src/external/MetricFilter.qll +++ b/cpp/ql/src/external/MetricFilter.qll @@ -5,8 +5,8 @@ import cpp /** * Holds if `id` in the opaque identifier of a result reported by query `queryPath`, * such that `value` is the reported metric value and the location of the result spans - * column `startcolumn` of line `startline` to column `endcolumn` of line `endline` - * in file `filepath`. + * column `startcol` of line `startline` to column `endcol` of line `endline` + * in file `file`. * * For more information, see [Locations](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/). */ diff --git a/cpp/ql/src/jsf/4.10 Classes/AV Rule 96.ql b/cpp/ql/src/jsf/4.10 Classes/AV Rule 96.ql index 67e01ffc7c07..4697af80c17c 100644 --- a/cpp/ql/src/jsf/4.10 Classes/AV Rule 96.ql +++ b/cpp/ql/src/jsf/4.10 Classes/AV Rule 96.ql @@ -28,7 +28,7 @@ where exists(FunctionCall c, int i, Function f | c.getArgument(i) = e and c.getTarget() = f and - exists(Parameter p | f.getParameter(i) = p) and // varargs + exists(f.getParameter(i)) and // varargs baseElement(e.getType(), cl) and // only interested in arrays with classes not compatible(f.getParameter(i).getUnspecifiedType(), e.getUnspecifiedType()) ) From 61e8ad264f07a7c2e2570cc01d484d7633b3eb27 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Thu, 4 Sep 2025 12:44:51 +0200 Subject: [PATCH 2/2] C++: Address review comments. --- cpp/ql/lib/change-notes/2025-09-03-rename-api.md | 4 ++++ .../code/cpp/dataflow/internal/DataFlowUtil.qll | 10 ++++++---- .../code/cpp/ir/dataflow/internal/DataFlowUtil.qll | 10 ++++++---- .../semmle/code/cpp/valuenumbering/HashCons.qll | 2 +- cpp/ql/src/Metrics/Internal/CallableExtents.ql | 14 ++++++++------ cpp/ql/src/change-notes/2025-09-03-rename-api.md | 5 +++++ 6 files changed, 30 insertions(+), 15 deletions(-) create mode 100644 cpp/ql/lib/change-notes/2025-09-03-rename-api.md create mode 100644 cpp/ql/src/change-notes/2025-09-03-rename-api.md diff --git a/cpp/ql/lib/change-notes/2025-09-03-rename-api.md b/cpp/ql/lib/change-notes/2025-09-03-rename-api.md new file mode 100644 index 000000000000..23c9fa3c046a --- /dev/null +++ b/cpp/ql/lib/change-notes/2025-09-03-rename-api.md @@ -0,0 +1,4 @@ +--- +category: deprecated +--- +* The predicate `getAContructorCall` in the class `SslContextClass` has been deprecated. Use `getAConstructorCall` instead. diff --git a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll index b388669d0415..e6d53f8bf649 100644 --- a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll +++ b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll @@ -829,13 +829,15 @@ class ContentSet instanceof Content { /** * Holds if this element is at the specified location. - * The location spans column `sc` of line `sl` to - * column `ec` of line `el` in file `path`. + * The location spans column `startcolumn` of line `startline` to + * column `endcolumn` of line `endline` in file `filepath`. * For more information, see * [Locations](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/). */ - predicate hasLocationInfo(string path, int sl, int sc, int el, int ec) { - super.hasLocationInfo(path, sl, sc, el, ec) + predicate hasLocationInfo( + string filepath, int startline, int startcolumn, int endline, int endcolumn + ) { + super.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn) } } 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 51ffbefb027d..82dcd43e1364 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 @@ -2268,13 +2268,15 @@ class ContentSet instanceof Content { /** * Holds if this element is at the specified location. - * The location spans column `sc` of line `sl` to - * column `ec` of line `el` in file `path`. + * The location spans column `startcolumn` of line `startline` to + * column `endcolumn` of line `endline` in file `filepath`. * For more information, see * [Locations](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/). */ - predicate hasLocationInfo(string path, int sl, int sc, int el, int ec) { - super.hasLocationInfo(path, sl, sc, el, ec) + predicate hasLocationInfo( + string filepath, int startline, int startcolumn, int endline, int endcolumn + ) { + super.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn) } } diff --git a/cpp/ql/lib/semmle/code/cpp/valuenumbering/HashCons.qll b/cpp/ql/lib/semmle/code/cpp/valuenumbering/HashCons.qll index b7e4fecb4742..b74e7c3d741b 100644 --- a/cpp/ql/lib/semmle/code/cpp/valuenumbering/HashCons.qll +++ b/cpp/ql/lib/semmle/code/cpp/valuenumbering/HashCons.qll @@ -648,7 +648,7 @@ private predicate mk_UuidofOperator(Type t, UuidofOperator e) { } private predicate analyzableTypeidType(TypeidOperator e) { - not exists(e.getAChild()) and + count(e.getAChild()) = 0 and strictcount(e.getResultType()) = 1 } diff --git a/cpp/ql/src/Metrics/Internal/CallableExtents.ql b/cpp/ql/src/Metrics/Internal/CallableExtents.ql index eef36e91bffe..7ebae0385dab 100644 --- a/cpp/ql/src/Metrics/Internal/CallableExtents.ql +++ b/cpp/ql/src/Metrics/Internal/CallableExtents.ql @@ -15,17 +15,19 @@ import cpp class RangeFunction extends Function { /** * Holds if this function is at the specified location. - * The location spans column `sc` of line `sl` to - * column `ec` of line `el` in file `path`. + * The location spans column `startcolumn` of line `startline` to + * column `endcolumn` of line `endline` in file `filepath`. * For more information, see * [Locations](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/). */ - predicate hasLocationInfo(string path, int sl, int sc, int el, int ec) { - super.getLocation().hasLocationInfo(path, sl, sc, _, _) and + predicate hasLocationInfo( + string filepath, int startline, int startcolumn, int endline, int endcolumn + ) { + super.getLocation().hasLocationInfo(filepath, startline, startcolumn, _, _) and ( - this.getBlock().getLocation().hasLocationInfo(path, _, _, el, ec) + this.getBlock().getLocation().hasLocationInfo(filepath, _, _, endline, endcolumn) or - not exists(this.getBlock()) and el = sl + 1 and ec = 1 + not exists(this.getBlock()) and endline = startline + 1 and endcolumn = 1 ) } } diff --git a/cpp/ql/src/change-notes/2025-09-03-rename-api.md b/cpp/ql/src/change-notes/2025-09-03-rename-api.md new file mode 100644 index 000000000000..5fd788ef76f5 --- /dev/null +++ b/cpp/ql/src/change-notes/2025-09-03-rename-api.md @@ -0,0 +1,5 @@ +--- +category: fix +--- +* The predicate `occurenceCount` in the file module `MagicConstants` has been deprecated. Use `occurrenceCount` instead. +* The predicate `additionalAdditionOrSubstractionCheckForLeapYear` in the file module `LeapYear` has been deprecated. Use `additionalAdditionOrSubtractionCheckForLeapYear` instead.