From c29ca80cee5a590ec0b6d65c9e3df93fe1134c5d Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Tue, 3 Feb 2026 16:38:36 -0500 Subject: [PATCH 01/11] Add lifetime rule RULE-11-6-2 also adjust formatting on prev changenote which was incorrect --- .vscode/tasks.json | 1 + change_notes/2026-01-26-improve-queries.md | 2 +- .../2026-02-03-uninitialized-mem-improve.md | 2 + .../ReadOfUninitializedMemory.qll | 15 +++- .../ReadOfUninitializedMemory.expected | 3 + .../rules/readofuninitializedmemory/test.cpp | 71 +++++++++++++++++++ ...AnObjectMustNotBeReadBeforeItHasBeenSet.ql | 24 +++++++ ...ectMustNotBeReadBeforeItHasBeenSet.testref | 1 + rule_packages/cpp/Lifetime.json | 47 ++++++++++++ rules.csv | 2 +- 10 files changed, 164 insertions(+), 4 deletions(-) create mode 100644 change_notes/2026-02-03-uninitialized-mem-improve.md create mode 100644 cpp/misra/src/rules/RULE-11-6-2/ValueOfAnObjectMustNotBeReadBeforeItHasBeenSet.ql create mode 100644 cpp/misra/test/rules/RULE-11-6-2/ValueOfAnObjectMustNotBeReadBeforeItHasBeenSet.testref create mode 100644 rule_packages/cpp/Lifetime.json diff --git a/.vscode/tasks.json b/.vscode/tasks.json index c5b2bbd1ba..9dbf661255 100644 --- a/.vscode/tasks.json +++ b/.vscode/tasks.json @@ -259,6 +259,7 @@ "Language1", "Language2", "Language3", + "Lifetime", "Linkage1", "Linkage2", "Literals", diff --git a/change_notes/2026-01-26-improve-queries.md b/change_notes/2026-01-26-improve-queries.md index 856f4949ee..ca45a56ab9 100644 --- a/change_notes/2026-01-26-improve-queries.md +++ b/change_notes/2026-01-26-improve-queries.md @@ -1,4 +1,4 @@ - `A3-1-1` - `ViolationsOfOneDefinitionRule.ql`: - The query previously would incorrectly allow cases where something was defined with `extern` and did not use the defined external linkage library to find external linkage. This change may result in the query finding more results. Additionally a typo has been fixed in the alert message which will cause the old alerts for this query to now show up as new ones. - - `RULE-6-0-2`, `A3-1-4` - `ExternalLinkageArrayWithoutExplicitSizeMisra.ql`, `ExternalLinkageArrayWithoutExplicitSizeAutosar.ql`: +- `RULE-6-0-2`, `A3-1-4` - `ExternalLinkageArrayWithoutExplicitSizeMisra.ql`, `ExternalLinkageArrayWithoutExplicitSizeAutosar.ql`: - The queries listed now find flexible member arrays in structs, as those do not have an explicit size. \ No newline at end of file diff --git a/change_notes/2026-02-03-uninitialized-mem-improve.md b/change_notes/2026-02-03-uninitialized-mem-improve.md new file mode 100644 index 0000000000..2fb2fd6539 --- /dev/null +++ b/change_notes/2026-02-03-uninitialized-mem-improve.md @@ -0,0 +1,2 @@ +- `A8-5-0`, `EXP53-CPP`, `EXP33-C`, `RULE-9-1` - `MemoryNotInitializedBeforeItIsRead.ql`, `DoNotReadUninitializedMemory.ql`, `DoNotReadUninitializedMemory.ql`, `ObjectWithAutoStorageDurationReadBeforeInit.ql`: + - The queries listed now find uses of the operator 'new' where there is no value initialization provided. \ No newline at end of file diff --git a/cpp/common/src/codingstandards/cpp/rules/readofuninitializedmemory/ReadOfUninitializedMemory.qll b/cpp/common/src/codingstandards/cpp/rules/readofuninitializedmemory/ReadOfUninitializedMemory.qll index 8d701cb26c..1560050d49 100644 --- a/cpp/common/src/codingstandards/cpp/rules/readofuninitializedmemory/ReadOfUninitializedMemory.qll +++ b/cpp/common/src/codingstandards/cpp/rules/readofuninitializedmemory/ReadOfUninitializedMemory.qll @@ -7,6 +7,7 @@ import codingstandards.cpp.Customizations import codingstandards.cpp.Exclusions import semmle.code.cpp.controlflow.Guards import semmle.code.cpp.controlflow.SubBasicBlocks +import codingstandards.cpp.enhancements.AggregateLiteralEnhancements abstract class ReadOfUninitializedMemorySharedQuery extends Query { } @@ -126,8 +127,18 @@ class InitializationContext extends TInitializationContext { */ class UninitializedVariable extends LocalVariable { UninitializedVariable() { - // Not initialized at declaration - not exists(getInitializer()) and + ( + // Not initialized at declaration + not exists(getInitializer()) + or + //or is a builtin type used with new operator but there is no value initialization as far as we can see + exists(Initializer i, NewExpr n | + i = getInitializer() and + n = i.getExpr() and + this.getUnspecifiedType().stripType() instanceof BuiltInType and + not i.isBraced() + ) + ) and // Not static or thread local, because they are not initialized with indeterminate values not isStatic() and not isThreadLocal() and diff --git a/cpp/common/test/rules/readofuninitializedmemory/ReadOfUninitializedMemory.expected b/cpp/common/test/rules/readofuninitializedmemory/ReadOfUninitializedMemory.expected index 2fb2b79a0f..00a81400c4 100644 --- a/cpp/common/test/rules/readofuninitializedmemory/ReadOfUninitializedMemory.expected +++ b/cpp/common/test/rules/readofuninitializedmemory/ReadOfUninitializedMemory.expected @@ -3,3 +3,6 @@ | test.cpp:39:7:39:8 | l3 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:38:6:38:7 | l3 | l3 | | test.cpp:86:9:86:16 | arrayPtr | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:79:8:79:15 | arrayPtr | arrayPtr | | test.cpp:93:9:93:10 | l5 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:89:7:89:8 | l5 | l5 | +| test.cpp:134:11:134:11 | i | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:133:7:133:7 | i | i | +| test.cpp:137:13:137:14 | i1 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:136:8:136:9 | i1 | i1 | +| test.cpp:141:12:141:13 | i1 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:136:8:136:9 | i1 | i1 | diff --git a/cpp/common/test/rules/readofuninitializedmemory/test.cpp b/cpp/common/test/rules/readofuninitializedmemory/test.cpp index 6ed07d795f..ec5d12fb38 100644 --- a/cpp/common/test/rules/readofuninitializedmemory/test.cpp +++ b/cpp/common/test/rules/readofuninitializedmemory/test.cpp @@ -123,4 +123,75 @@ void test_non_default_init() { use(tlp); // COMPLIANT - thread local variables are zero initialized _Atomic int ai; use(ai); // COMPLIANT - atomics are special and not covered by this rule +} + +namespace { +int i; // COMPLIANT +} + +void extra_test() { + int i; + int j = i + 1; // NON_COMPLIANT + + int *i1 = new int; + int i2 = *i1; // NON_COMPLIANT + + int *i3; + + if (i3 = i1) { // NON_COMPLIANT + } +} + +void extra_conditionals(bool b) { + if (b) { + goto L; + } + int i; + i = 1; +L: + i = i + 1; // NON_COMPLIANT[FALSE_NEGATIVE] +} + +struct S { + int m1; + int m2; +}; + +void struct_test() { + S s1; + S s2 = {1}; + + auto i1 = s1.m1; // NON_COMPLIANT[FALSE_NEGATIVE] - rule currently is not + // field sensitive + auto i2 = s2.m2; // COMPLIANT + + int a1[10] = {1, 1, 1}; + int a2[10]; + + auto a3 = a1[5]; // COMPLIANT + auto a4 = a2[5]; // NON_COMPLIANT[FALSE_NEGATIVE] +} + +class C { +private: + int m1; + int m2; + +public: + C() : m1(1), m2(1) {} + + C(int a) : m1(a) {} + + int getm2() { return m2; } +}; + +void test_class() { + C c1; + if (c1.getm2() > 0) { // COMPLIANT + } + + C c2(5); + if (c2.getm2() > 0) { // NON_COMPLIANT[FALSE_NEGATIVE] - rule currently is not + // field sensitive + } } \ No newline at end of file diff --git a/cpp/misra/src/rules/RULE-11-6-2/ValueOfAnObjectMustNotBeReadBeforeItHasBeenSet.ql b/cpp/misra/src/rules/RULE-11-6-2/ValueOfAnObjectMustNotBeReadBeforeItHasBeenSet.ql new file mode 100644 index 0000000000..77b45a5a19 --- /dev/null +++ b/cpp/misra/src/rules/RULE-11-6-2/ValueOfAnObjectMustNotBeReadBeforeItHasBeenSet.ql @@ -0,0 +1,24 @@ +/** + * @id cpp/misra/value-of-an-object-must-not-be-read-before-it-has-been-set + * @name RULE-11-6-2: The value of an object must not be read before it has been set + * @description Reading from uninitialized indeterminate values may produce undefined behavior. + * @kind problem + * @precision medium + * @problem.severity error + * @tags external/misra/id/rule-11-6-2 + * correctness + * security + * scope/system + * external/misra/enforcement/undecidable + * external/misra/obligation/mandatory + */ + +import cpp +import codingstandards.cpp.misra +import codingstandards.cpp.rules.readofuninitializedmemory.ReadOfUninitializedMemory + +class ValueOfAnObjectMustNotBeReadBeforeItHasBeenSetQuery extends ReadOfUninitializedMemorySharedQuery { + ValueOfAnObjectMustNotBeReadBeforeItHasBeenSetQuery() { + this = LifetimePackage::valueOfAnObjectMustNotBeReadBeforeItHasBeenSetQuery() + } +} diff --git a/cpp/misra/test/rules/RULE-11-6-2/ValueOfAnObjectMustNotBeReadBeforeItHasBeenSet.testref b/cpp/misra/test/rules/RULE-11-6-2/ValueOfAnObjectMustNotBeReadBeforeItHasBeenSet.testref new file mode 100644 index 0000000000..e2ec5838e7 --- /dev/null +++ b/cpp/misra/test/rules/RULE-11-6-2/ValueOfAnObjectMustNotBeReadBeforeItHasBeenSet.testref @@ -0,0 +1 @@ +cpp/common/test/rules/readofuninitializedmemory/ReadOfUninitializedMemory.ql \ No newline at end of file diff --git a/rule_packages/cpp/Lifetime.json b/rule_packages/cpp/Lifetime.json new file mode 100644 index 0000000000..30a22a8d2d --- /dev/null +++ b/rule_packages/cpp/Lifetime.json @@ -0,0 +1,47 @@ +{ + "MISRA-C++-2023": { + "RULE-11-6-2": { + "properties": { + "enforcement": "undecidable", + "obligation": "mandatory" + }, + "queries": [ + { + "description": "Reading from uninitialized indeterminate values may produce undefined behavior.", + "kind": "problem", + "name": "The value of an object must not be read before it has been set", + "precision": "medium", + "severity": "error", + "short_name": "ValueOfAnObjectMustNotBeReadBeforeItHasBeenSet", + "shared_implementation_short_name": "ReadOfUninitializedMemory", + "tags": [ + "correctness", + "security", + "scope/system" + ] + } + ], + "title": "The value of an object must not be read before it has been set" + }, + "RULE-6-8-3": { + "properties": { + "enforcement": "decidable", + "obligation": "required" + }, + "queries": [ + { + "description": "An assignment operator shall not assign the address of an object with automatic storage duration to an object with a greater lifetime", + "kind": "problem", + "name": "An assignment operator shall not assign the address of an object with automatic storage duration to", + "precision": "very-high", + "severity": "error", + "short_name": "AssignmentOperatorAssignTheAddressOfAnObjectWithAutomaticStorageDurationToAnObjectWithAGreaterLifetime", + "tags": [ + "scope/single-translation-unit" + ] + } + ], + "title": "An assignment operator shall not assign the address of an object with automatic storage duration to an object with a greater lifetime" + } + } +} \ No newline at end of file diff --git a/rules.csv b/rules.csv index cca308583d..1a023e85ef 100644 --- a/rules.csv +++ b/rules.csv @@ -925,7 +925,7 @@ cpp,MISRA-C++-2023,RULE-10-4-1,Yes,Required,Decidable,Single Translation Unit,Th cpp,MISRA-C++-2023,RULE-11-3-1,Yes,Advisory,Decidable,Single Translation Unit,Variables of array type should not be declared,,Declarations2,Easy, cpp,MISRA-C++-2023,RULE-11-3-2,Yes,Advisory,Decidable,Single Translation Unit,The declaration of an object should contain no more than two levels of pointer indirection,A5-0-3,ImportMisra23,Import, cpp,MISRA-C++-2023,RULE-11-6-1,Yes,Advisory,Decidable,Single Translation Unit,All variables should be initialized,,Declarations2,Easy, -cpp,MISRA-C++-2023,RULE-11-6-2,Yes,Mandatory,Undecidable,System,The value of an object must not be read before it has been set,A8-5-0,Lifetime,Very Hard, +cpp,MISRA-C++-2023,RULE-11-6-2,Yes,Mandatory,Undecidable,System,The value of an object must not be read before it has been set,A8-5-0,Lifetime,Import cpp,MISRA-C++-2023,RULE-11-6-3,Yes,Required,Decidable,Single Translation Unit,"Within an enumerator list, the value of an implicitly-specified enumeration constant shall be unique",RULE-8-12,ImportMisra23,Import, cpp,MISRA-C++-2023,RULE-12-2-1,Yes,Advisory,Decidable,Single Translation Unit,Bit-fields should not be declared,A9-6-2,Banned,Easy, cpp,MISRA-C++-2023,RULE-12-2-2,Yes,Required,Decidable,Single Translation Unit,A bit-field shall have an appropriate type,RULE-6-1,ImportMisra23,Import, From 9651122852575afbafebbb784a929925ff4a093a Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Tue, 10 Mar 2026 10:20:53 -0400 Subject: [PATCH 02/11] Add improvements to readofuninitializedmemory and testcase --- .../cpp/lifetimes/CppObjects.qll | 2 + .../InitializationFunctions.qll | 687 ++++++++++++++++++ .../ReadOfUninitializedMemory.qll | 50 +- .../ReadOfUninitializedMemory.expected | 7 +- .../rules/readofuninitializedmemory/test.cpp | 28 +- 5 files changed, 764 insertions(+), 10 deletions(-) create mode 100644 cpp/common/src/codingstandards/cpp/rules/readofuninitializedmemory/InitializationFunctions.qll diff --git a/cpp/common/src/codingstandards/cpp/lifetimes/CppObjects.qll b/cpp/common/src/codingstandards/cpp/lifetimes/CppObjects.qll index 4ef3122805..f1d26047d2 100644 --- a/cpp/common/src/codingstandards/cpp/lifetimes/CppObjects.qll +++ b/cpp/common/src/codingstandards/cpp/lifetimes/CppObjects.qll @@ -262,6 +262,8 @@ class AggregateLiteralObjectIdentity extends AggregateLiteral, ObjectIdentityBas class AllocatedObjectIdentity extends AllocationExpr, ObjectIdentityBase { AllocatedObjectIdentity() { this.(FunctionCall).getTarget().(AllocationFunction).requiresDealloc() + or + this = any(NewOrNewArrayExpr new | not exists(new.getPlacementPointer())) } override StorageDuration getStorageDuration() { result.isAllocated() } diff --git a/cpp/common/src/codingstandards/cpp/rules/readofuninitializedmemory/InitializationFunctions.qll b/cpp/common/src/codingstandards/cpp/rules/readofuninitializedmemory/InitializationFunctions.qll new file mode 100644 index 0000000000..ca68fd526d --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/rules/readofuninitializedmemory/InitializationFunctions.qll @@ -0,0 +1,687 @@ +/** + * Provides classes and predicates for identifying functions that initialize their arguments. + * This is a copy of the out of the box library that is in a query specific library + * https://github.com/github/codeql/blob/main/cpp/ql/src/Security/CWE/CWE-457/InitializationFunctions.qll + * at cc7a9ef. + */ + +import cpp +import external.ExternalArtifact +private import semmle.code.cpp.dispatch.VirtualDispatchPrototype +import semmle.code.cpp.NestedFields +import semmle.code.cpp.controlflow.Guards + +/** A context under which a function may be called. */ +private newtype TContext = + /** No specific call context. */ + NoContext() or + /** + * The call context is that the given other parameter is null. + * + * This context is created for all parameters that are null checked in the body of the function. + */ + ParamNull(Parameter p) { p = any(ParameterNullCheck pnc).getParameter() } or + /** + * The call context is that the given other parameter is not null. + * + * This context is created for all parameters that are null checked in the body of the function. + */ + ParamNotNull(Parameter p) { p = any(ParameterNullCheck pnc).getParameter() } + +/** + * A context under which a function may be called. + * + * Some functions may conditionally initialize a parameter depending on the value of another + * parameter. Consider: + * ``` + * int MyInitFunction(int* paramToBeInitialized, int* paramToCheck) { + * if (!paramToCheck) { + * // fail! + * return -1; + * } + * paramToBeInitialized = 0; + * } + * ``` + * In this case, whether `paramToBeInitialized` is initialized when this function call completes + * depends on whether `paramToCheck` is or is not null. A call-context insensitive analysis will + * determine that any call to this function may leave the parameter uninitialized, even if the + * argument to paramToCheck is guaranteed to be non-null (`&foo`, for example). + * + * This class models call contexts that can be considered when calculating whether a given parameter + * initializes or not. The supported contexts are: + * - `ParamNull(otherParam)` - the given `otherParam` is considered to be null. Applies when + * exactly one parameter other than this one is null checked. + * - `ParamNotNull(otherParam)` - the given `otherParam` is considered to be not null. Applies when + * exactly one parameter other than this one is null checked. + * - `NoContext()` - applies in all other circumstances. + */ +class Context extends TContext { + string toString() { + this = NoContext() and result = "NoContext" + or + this = ParamNull(any(Parameter p | result = "ParamNull(" + p.getName() + ")")) + or + this = ParamNotNull(any(Parameter p | result = "ParamNotNull(" + p.getName() + ")")) + } +} + +/** + * A check against a parameter. + */ +abstract class ParameterCheck extends Expr { + /** + * Gets a successor of this check that should be ignored for the given context. + */ + abstract ControlFlowNode getIgnoredSuccessorForContext(Context c); +} + +/** A null-check expression on a parameter. */ +class ParameterNullCheck extends ParameterCheck { + Parameter p; + ControlFlowNode nullSuccessor; + ControlFlowNode notNullSuccessor; + + ParameterNullCheck() { + this.isCondition() and + p.getFunction() instanceof InitializationFunction and + p.getType().getUnspecifiedType() instanceof PointerType and + exists(VariableAccess va | va = p.getAnAccess() | + nullSuccessor = this.getATrueSuccessor() and + notNullSuccessor = this.getAFalseSuccessor() and + ( + va = this.(NotExpr).getOperand() or + va = any(EQExpr eq | eq = this and eq.getAnOperand().getValue() = "0").getAnOperand() or + va = getCheckedFalseCondition(this) or + va = + any(NEExpr eq | eq = getCheckedFalseCondition(this) and eq.getAnOperand().getValue() = "0") + .getAnOperand() + ) + or + nullSuccessor = this.getAFalseSuccessor() and + notNullSuccessor = this.getATrueSuccessor() and + ( + va = this or + va = any(NEExpr eq | eq = this and eq.getAnOperand().getValue() = "0").getAnOperand() or + va = + any(EQExpr eq | eq = getCheckedFalseCondition(this) and eq.getAnOperand().getValue() = "0") + .getAnOperand() + ) + ) + } + + /** The parameter being null-checked. */ + Parameter getParameter() { result = p } + + override ControlFlowNode getIgnoredSuccessorForContext(Context c) { + c = ParamNull(p) and result = notNullSuccessor + or + c = ParamNotNull(p) and result = nullSuccessor + } + + /** The successor at which the parameter is confirmed to be null. */ + ControlFlowNode getNullSuccessor() { result = nullSuccessor } + + /** The successor at which the parameter is confirmed to be not-null. */ + ControlFlowNode getNotNullSuccessor() { result = notNullSuccessor } +} + +/** + * An entry in a CSV file in cond-init that contains externally defined functions that are + * conditional initializers. These files are typically produced by running the + * ConditionallyInitializedFunction companion query. + */ +class ValidatedExternalCondInitFunction extends ExternalData { + ValidatedExternalCondInitFunction() { this.getDataPath().matches("%cond-init%.csv") } + + predicate isExternallyVerified(Function f, int param) { + functionSignature(f, this.getField(1), this.getField(2)) and param = this.getFieldAsInt(3) + } +} + +/** + * The type of evidence used to determine whether a function initializes a parameter. + */ +newtype Evidence = + /** + * The function is defined in the snapshot, and the CFG has been analyzed to determine that the + * parameter is not initialized on at least one path to the exit. + */ + DefinitionInSnapshot() or + /** + * The function is externally defined, but the parameter has an `_out` SAL annotation which + * suggests that it is initialized in the function. + */ + SuggestiveSalAnnotation() or + /** + * We have been given a CSV file which indicates this parameter is conditionally initialized. + */ + ExternalEvidence() + +/** + * A call to an function which initializes one or more of its parameters. + */ +class InitializationFunctionCall extends FunctionCall { + Expr initializedArgument; + + InitializationFunctionCall() { initializedArgument = getAnInitializedArgument(this) } + + /** Gets a parameter that is initialized by this call. */ + Parameter getAnInitParameter() { result.getAnAccess() = initializedArgument } +} + +/** + * A variable access which is dereferenced then assigned to. + */ +private predicate isPointerDereferenceAssignmentTarget(VariableAccess target) { + target.getParent().(PointerDereferenceExpr) = any(Assignment e).getLValue() +} + +/** + * A function which initializes one or more of its parameters. + */ +class InitializationFunction extends Function { + int i; + Evidence evidence; + + InitializationFunction() { + evidence = DefinitionInSnapshot() and + ( + // Assignment by pointer dereferencing the parameter + isPointerDereferenceAssignmentTarget(this.getParameter(i).getAnAccess()) or + // Field wise assignment to the parameter + any(Assignment e).getLValue() = getAFieldAccess(this.getParameter(i)) or + i = + this.(MemberFunction) + .getAnOverridingFunction+() + .(InitializationFunction) + .initializedParameter() or + this.getParameter(i) = any(InitializationFunctionCall c).getAnInitParameter() + ) + or + // We have some external information that this function conditionally initializes + not this.hasDefinition() and + any(ValidatedExternalCondInitFunction vc).isExternallyVerified(this, i) and + evidence = ExternalEvidence() + } + + /** Gets a parameter index which is initialized by this function. */ + int initializedParameter() { result = i } + + /** Gets a `ControlFlowNode` which assigns a new value to the parameter with the given index. */ + ControlFlowNode paramReassignment(int index) { + index = i and + ( + result = this.getParameter(i).getAnAccess() and + ( + result = any(Assignment a).getLValue().(PointerDereferenceExpr).getOperand() + or + // Field wise assignment to the parameter + result = any(Assignment a).getLValue().(FieldAccess).getQualifier() + or + // Assignment to a nested field of the parameter + result = any(Assignment a).getLValue().(NestedFieldAccess).getUltimateQualifier() + or + result = getAnInitializedArgument(any(Call c)) + or + exists(IfStmt check | result = check.getCondition().getAChild*() | + this.paramReassignmentCondition(check) + ) + ) + or + result = + any(AssumeExpr e | + e.getEnclosingFunction() = this and e.getAChild().(Literal).getValue() = "0" + ) + ) + } + + /** + * Helper predicate: holds if the `if` statement `check` contains a + * reassignment to the `i`th parameter within its `then` statement. + */ + pragma[noinline] + private predicate paramReassignmentCondition(IfStmt check) { + this.paramReassignment(i).getEnclosingStmt().getParentStmt*() = check.getThen() + } + + /** Holds if `n` can be reached without the parameter at `index` being reassigned. */ + predicate paramNotReassignedAt(ControlFlowNode n, int index, Context c) { + c = this.getAContext(index) and + ( + not exists(this.getEntryPoint()) and index = i and n = this + or + n = this.getEntryPoint() and index = i + or + exists(ControlFlowNode mid | this.paramNotReassignedAt(mid, index, c) | + n = mid.getASuccessor() and + not n = this.paramReassignment(index) and + /* + * Ignore successor edges where the parameter is null, because it is then confirmed to be + * initialized. + */ + + not exists(ParameterNullCheck nullCheck | + nullCheck = mid and + nullCheck = this.getANullCheck(index) and + n = nullCheck.getNullSuccessor() + ) and + /* + * Ignore successor edges which are excluded by the given context + */ + + not exists(ParameterCheck paramCheck | paramCheck = mid | + n = paramCheck.getIgnoredSuccessorForContext(c) + ) + ) + ) + } + + /** Gets a null-check on the parameter at `index`. */ + private ParameterNullCheck getANullCheck(int index) { + this.getParameter(index) = result.getParameter() + } + + /** Gets a parameter which is not at the given index. */ + private Parameter getOtherParameter(int index) { + index = i and + result = this.getAParameter() and + not result.getIndex() = index + } + + /** + * Gets a call `Context` that is applicable when considering whether parameter at the `index` can + * be conditionally initialized. + */ + Context getAContext(int index) { + index = i and + /* + * If there is one and only one other parameter which is null checked in the body of the method, + * then we have two contexts to consider - that the other param is null, or that the other param + * is not null. + */ + + if + strictcount(Parameter p | + exists(Context c | c = ParamNull(p) or c = ParamNotNull(p)) and + p = this.getOtherParameter(index) + ) = 1 + then + exists(Parameter p | p = this.getOtherParameter(index) | + result = ParamNull(p) or result = ParamNotNull(p) + ) + else + // Otherwise, only consider NoContext. + result = NoContext() + } + + /** + * Holds if this function should be whitelisted - that is, not considered as conditionally + * initializing its parameters. + */ + predicate whitelisted() { + exists(string name | this.hasName(name) | + // Return value is not a success code but the output functions never fail. + name.matches("_Interlocked%") + or + name = + [ + // Functions that never fail, according to MSDN. + "QueryPerformanceCounter", "QueryPerformanceFrequency", + // Functions that never fail post-Vista, according to MSDN. + "InitializeCriticalSectionAndSpinCount", + // `rand_s` writes 0 to a non-null argument if it fails, according to MSDN. + "rand_s", + // IntersectRect initializes the argument regardless of whether the input intersects + "IntersectRect", "SetRect", "UnionRect", + // These functions appears to have an incorrect CFG, which leads to false positives + "PhysicalToLogicalDPIPoint", "LogicalToPhysicalDPIPoint", + // Sets NtProductType to default on error + "RtlGetNtProductType", + // Our CFG is not sophisticated enough to detect that the argument is always initialized + "StringCchLengthA", + // All paths init the argument, and always returns SUCCESS. + "RtlUnicodeToMultiByteSize", + // All paths init the argument, and always returns SUCCESS. + "RtlMultiByteToUnicodeSize", + // All paths init the argument, and always returns SUCCESS. + "RtlUnicodeToMultiByteN", + // Always initializes argument + "RtlGetFirstRange", + // Destination range is zeroed out on failure, assuming first two parameters are valid + "memcpy_s", + // This zeroes the memory unconditionally + "SeCreateAccessState", + // Argument initialization is optional, but always succeeds + "KeGetCurrentProcessorNumberEx" + ] + ) + } +} + +/** + * A function which initializes one or more of its parameters, but not on all paths. + */ +class ConditionalInitializationFunction extends InitializationFunction { + Context c; + + ConditionalInitializationFunction() { + c = this.getAContext(i) and + not this.whitelisted() and + exists(Type status | status = this.getType().getUnspecifiedType() | + status instanceof IntegralType or + status instanceof Enum + ) and + not this.getType().getName().toLowerCase() = "size_t" and + ( + /* + * If there is no definition, consider this to be conditionally initializing (based on either + * SAL or external data). + */ + + not evidence = DefinitionInSnapshot() + or + /* + * If this function is defined in this snapshot, then it conditionally initializes if there + * is at least one path through the function which doesn't initialize the parameter. + * + * Explicitly ignore pure virtual functions. + */ + + this.hasDefinition() and + this.paramNotReassignedAt(this, i, c) and + not this instanceof PureVirtualFunction + ) + } + + /** Gets the evidence associated with the given parameter. */ + Evidence getEvidence(int param) { + /* + * Note: due to the way the predicate dispatch interacts with fields, this needs to be + * implemented on this class, not `InitializationFunction`. If implemented on the latter it + * can return evidence that does not result in conditional initialization. + */ + + param = i and evidence = result + } + + /** Gets the index of a parameter which is conditionally initialized. */ + int conditionallyInitializedParameter(Context context) { result = i and context = c } +} + +/** + * More elaborate tracking, flagging cases where the status is checked after + * the potentially uninitialized variable has been used, and ignoring cases + * where the status is not checked but there is no use of the potentially + * uninitialized variable, may be obtained via `getARiskyAccess`. + */ +class ConditionalInitializationCall extends FunctionCall { + ConditionalInitializationFunction target; + + ConditionalInitializationCall() { target = getTarget(this) } + + /** Gets the argument passed for the given parameter to this call. */ + Expr getArgumentForParameter(Parameter p) { + p = this.getTarget().getAParameter() and + result = this.getArgument(p.getIndex()) + } + + /** + * Gets an argument conditionally initialized by this call. + */ + Expr getAConditionallyInitializedArgument(ConditionalInitializationFunction condTarget, Evidence e) { + condTarget = target and + exists(Context context | + result = getAConditionallyInitializedArgument(this, condTarget, context, e) + | + context = NoContext() + or + exists(Parameter otherP, Expr otherArg | + context = ParamNotNull(otherP) or + context = ParamNull(otherP) + | + otherArg = this.getArgumentForParameter(otherP) and + (otherArg instanceof AddressOfExpr implies context = ParamNotNull(otherP)) and + (otherArg.getType() instanceof ArrayType implies context = ParamNotNull(otherP)) and + (otherArg.getValue() = "0" implies context = ParamNull(otherP)) + ) + ) + } + + VariableAccess getAConditionallyInitializedVariable() { + not result.getTarget().getAnAssignedValue().getASuccessor+() = result and + // Should not be assigned field-wise prior to the call. + not exists(Assignment a, FieldAccess fa | + fa.getQualifier() = result.getTarget().getAnAccess() and + a.getLValue() = fa and + fa.getASuccessor+() = result + ) and + result = + this.getArgument(getTarget(this) + .(ConditionalInitializationFunction) + .conditionallyInitializedParameter(_)).(AddressOfExpr).getOperand() + } + + Variable getStatusVariable() { + exists(AssignExpr a | a.getLValue() = result.getAnAccess() | a.getRValue() = this) + or + result.getInitializer().getExpr() = this + } + + Expr getSuccessCheck() { + exists(this.getAFalseSuccessor()) and result = this + or + result = this.getParent() and + ( + result instanceof NotExpr or + result.(EQExpr).getAnOperand().getValue() = "0" or + result.(GEExpr).getLesserOperand().getValue() = "0" + ) + } + + Expr getFailureCheck() { + result = this.getParent() and + ( + result instanceof NotExpr or + result.(NEExpr).getAnOperand().getValue() = "0" or + result.(LTExpr).getLesserOperand().getValue() = "0" + ) + } + + private predicate inCheckedContext() { + exists(Call parent | this = parent.getAnArgument() | + parent.getTarget() instanceof Operator or + parent.getTarget().hasName("VerifyOkCatastrophic") + ) + } + + ControlFlowNode uncheckedReaches(LocalVariable var) { + ( + not exists(var.getInitializer()) and + var = this.getAConditionallyInitializedVariable().getTarget() and + if exists(this.getFailureCheck()) + then result = this.getFailureCheck().getATrueSuccessor() + else + if exists(this.getSuccessCheck()) + then result = this.getSuccessCheck().getAFalseSuccessor() + else ( + result = this.getASuccessor() and not this.inCheckedContext() + ) + ) + or + exists(ControlFlowNode mid | mid = this.uncheckedReaches(var) | + not mid = this.getStatusVariable().getAnAccess() and + not mid = var.getAnAccess() and + not exists(VariableAccess write | result = write and write = var.getAnAccess() | + write = any(AssignExpr a).getLValue() or + write = any(AddressOfExpr a).getOperand() + ) and + result = mid.getASuccessor() + ) + } + + VariableAccess getARiskyRead(Function f) { + f = this.getTarget() and + exists(this.getFile().getRelativePath()) and + result = this.uncheckedReaches(result.getTarget()) and + not this.(GuardCondition).controls(result.getBasicBlock(), _) + } +} + +/** + * Gets the position of an argument to the call which is initialized by the call. + */ +pragma[nomagic] +int initializedArgument(Call call) { + exists(InitializationFunction target | + target = getTarget(call) and + result = target.initializedParameter() + ) +} + +/** + * Gets an argument which is initialized by the call. + */ +Expr getAnInitializedArgument(Call call) { result = call.getArgument(initializedArgument(call)) } + +/** + * Gets the position of an argument to the call to the target which is conditionally initialized by + * the call, under the given context and evidence. + */ +pragma[nomagic] +private int conditionallyInitializedArgument( + Call call, ConditionalInitializationFunction target, Context c, Evidence e +) { + target = getTarget(call) and + c = target.getAContext(result) and + e = target.getEvidence(result) and + result = target.conditionallyInitializedParameter(c) +} + +/** + * Gets an argument which is conditionally initialized by the call to the given target under the given context and evidence. + */ +Expr getAConditionallyInitializedArgument( + Call call, ConditionalInitializationFunction target, Context c, Evidence e +) { + result = call.getArgument(conditionallyInitializedArgument(call, target, c, e)) +} + +/** + * Gets the type signature for the functions parameters. + */ +private string typeSig(Function f) { + result = + concat(int i, Type pt | + pt = f.getParameter(i).getType() + | + pt.getUnspecifiedType().toString(), "," order by i + ) +} + +/** + * Holds where qualifiedName and typeSig make up the signature for the function. + */ +private predicate functionSignature(Function f, string qualifiedName, string typeSig) { + qualifiedName = f.getQualifiedName() and + typeSig = typeSig(f) +} + +/** + * Gets a possible definition for the undefined function by matching the undefined function name + * and parameter arity with a defined function. + * + * This is useful for identifying call to target dependencies across libraries, where the libraries + * are never statically linked together. + */ +private Function getAPossibleDefinition(Function undefinedFunction) { + not undefinedFunction.hasDefinition() and + exists(string qn, string typeSig | + functionSignature(undefinedFunction, qn, typeSig) and functionSignature(result, qn, typeSig) + ) and + result.hasDefinition() +} + +/** + * Helper predicate for `getTarget`, that computes possible targets of a `Call`. + * + * If there is at least one defined target after performing some simple virtual dispatch + * resolution, then the result is all the defined targets. + */ +private Function getTarget1(Call c) { + result = VirtualDispatch::getAViableTarget(c) and + result.hasDefinition() +} + +/** + * Helper predicate for `getTarget`, that computes possible targets of a `Call`. + * + * If we can use the heuristic matching of functions to find definitions for some of the viable + * targets, return those. + */ +private Function getTarget2(Call c) { + not exists(getTarget1(c)) and + result = getAPossibleDefinition(VirtualDispatch::getAViableTarget(c)) +} + +/** + * Helper predicate for `getTarget`, that computes possible targets of a `Call`. + * + * Otherwise, the result is the undefined `Function` instances. + */ +private Function getTarget3(Call c) { + not exists(getTarget1(c)) and + not exists(getTarget2(c)) and + result = VirtualDispatch::getAViableTarget(c) +} + +/** + * Gets a possible target for the `Call`, using the name and parameter matching if we did not associate + * this call with a specific definition at link or compile time, and performing simple virtual + * dispatch resolution. + */ +Function getTarget(Call c) { + result = getTarget1(c) or + result = getTarget2(c) or + result = getTarget3(c) +} + +/** + * Get an access of a field on `Variable` v. + */ +FieldAccess getAFieldAccess(Variable v) { + exists(VariableAccess va, Expr qualifierExpr | + // Find an access of the variable, or an AddressOfExpr that has the access + va = v.getAnAccess() and + ( + qualifierExpr = va or + qualifierExpr.(AddressOfExpr).getOperand() = va + ) + | + // Direct field access + qualifierExpr = result.getQualifier() + or + // Nested field access + qualifierExpr = result.(NestedFieldAccess).getUltimateQualifier() + ) +} + +/** + * Gets a condition which is checked to be false by the given `ne` expression, according to this pattern: + * ``` + * int a = !!result; + * if (!a) { // <- ne + * .... + * } + * ``` + */ +private Expr getCheckedFalseCondition(NotExpr ne) { + exists(LocalVariable v | + result = v.getInitializer().getExpr().(NotExpr).getOperand().(NotExpr).getOperand() and + ne.getOperand() = v.getAnAccess() and + nonAssignedVariable(v) + // and not passed by val? + ) +} + +pragma[noinline] +private predicate nonAssignedVariable(Variable v) { not exists(v.getAnAssignment()) } diff --git a/cpp/common/src/codingstandards/cpp/rules/readofuninitializedmemory/ReadOfUninitializedMemory.qll b/cpp/common/src/codingstandards/cpp/rules/readofuninitializedmemory/ReadOfUninitializedMemory.qll index 1560050d49..025e3b728a 100644 --- a/cpp/common/src/codingstandards/cpp/rules/readofuninitializedmemory/ReadOfUninitializedMemory.qll +++ b/cpp/common/src/codingstandards/cpp/rules/readofuninitializedmemory/ReadOfUninitializedMemory.qll @@ -8,6 +8,7 @@ import codingstandards.cpp.Exclusions import semmle.code.cpp.controlflow.Guards import semmle.code.cpp.controlflow.SubBasicBlocks import codingstandards.cpp.enhancements.AggregateLiteralEnhancements +import InitializationFunctions abstract class ReadOfUninitializedMemorySharedQuery extends Query { } @@ -122,21 +123,37 @@ class InitializationContext extends TInitializationContext { } } +class NewNotInit extends NewExpr { + NewNotInit() { + this.getAllocatedType() instanceof BuiltInType and + not exists(this.getAChild()) + } +} + +class NonInitAssignment extends Assignment { + NonInitAssignment() { this.getRValue() instanceof NewNotInit } +} + +class VariableAccessOnLHSOfNonInitAssignment extends VariableAccess { + VariableAccessOnLHSOfNonInitAssignment() { exists(NonInitAssignment a | this = a.getLValue()) } +} + /** * A local variable without an initializer which is amenable to initialization analysis. */ class UninitializedVariable extends LocalVariable { UninitializedVariable() { + // Not initialized at declaration ( - // Not initialized at declaration - not exists(getInitializer()) + not exists(getInitializer().getExpr()) or //or is a builtin type used with new operator but there is no value initialization as far as we can see exists(Initializer i, NewExpr n | i = getInitializer() and n = i.getExpr() and this.getUnspecifiedType().stripType() instanceof BuiltInType and - not i.isBraced() + //ignore value init + not exists(n.getAChild()) ) ) and // Not static or thread local, because they are not initialized with indeterminate values @@ -187,16 +204,33 @@ class UninitializedVariable extends LocalVariable { ) } - /** Get a access of the variable that is used to initialize the variable. */ + /** + * Get a access of the variable that is assumed to initialize the variable. + * This approximates that any access in the lvalue category may be a definition. + */ VariableAccess getADefinitionAccess() { result = getAnAccess() and - result.isLValueCategory() + result.isLValueCategory() and + // Not a pointless read + not result = any(ExprStmt es).getExpr() and + // not involved in a new expr assignment since that does not define + not result instanceof VariableAccessOnLHSOfNonInitAssignment } - /** Get a read of the variable. */ + /** + * Gets an access of the variable `v` which is not used as an lvalue, and not used as an argument + * to an initialization function. + */ VariableAccess getAUse() { - result = getAnAccess() and - result.isRValueCategory() and + result = this.getAnAccess() and + // Not used as an lvalue + not result = any(AssignExpr a).getLValue() and + // Not passed to another initialization function + not exists(Call c, int j | j = c.getTarget().(InitializationFunction).initializedParameter() | + result = c.getArgument(j).(AddressOfExpr).getOperand() + ) and + // Not a pointless read + not result = any(ExprStmt es).getExpr() and // sizeof operators are not real uses not result.getParent+() instanceof SizeofOperator } diff --git a/cpp/common/test/rules/readofuninitializedmemory/ReadOfUninitializedMemory.expected b/cpp/common/test/rules/readofuninitializedmemory/ReadOfUninitializedMemory.expected index 00a81400c4..2393dd2b37 100644 --- a/cpp/common/test/rules/readofuninitializedmemory/ReadOfUninitializedMemory.expected +++ b/cpp/common/test/rules/readofuninitializedmemory/ReadOfUninitializedMemory.expected @@ -5,4 +5,9 @@ | test.cpp:93:9:93:10 | l5 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:89:7:89:8 | l5 | l5 | | test.cpp:134:11:134:11 | i | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:133:7:133:7 | i | i | | test.cpp:137:13:137:14 | i1 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:136:8:136:9 | i1 | i1 | -| test.cpp:141:12:141:13 | i1 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:136:8:136:9 | i1 | i1 | +| test.cpp:141:7:141:8 | i3 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:139:8:139:9 | i3 | i3 | +| test.cpp:141:13:141:14 | i1 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:136:8:136:9 | i1 | i1 | +| test.cpp:205:4:205:5 | p1 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:204:8:204:9 | p1 | p1 | +| test.cpp:207:4:207:5 | p1 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:204:8:204:9 | p1 | p1 | +| test.cpp:215:10:215:11 | p2 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:209:8:209:9 | p2 | p2 | +| test.cpp:221:4:221:5 | p4 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:219:8:219:9 | p4 | p4 | diff --git a/cpp/common/test/rules/readofuninitializedmemory/test.cpp b/cpp/common/test/rules/readofuninitializedmemory/test.cpp index ec5d12fb38..e6cc987dd1 100644 --- a/cpp/common/test/rules/readofuninitializedmemory/test.cpp +++ b/cpp/common/test/rules/readofuninitializedmemory/test.cpp @@ -138,7 +138,7 @@ void extra_test() { int *i3; - if (i3 = i1) { // NON_COMPLIANT + if (i3 == i1) { // NON_COMPLIANT } } @@ -194,4 +194,30 @@ void test_class() { if (c2.getm2() > 0) { // NON_COMPLIANT[FALSE_NEGATIVE] - rule currently is not // field sensitive } +} + +void extra_extra_test() { + int *p0 = new int; + p0; // COMPLIANT -- this is uninitialized but its access not a meaningful read + // to this rule + + int *p1 = new int; + *p1 = 0; // COMPLIANT[FALSE_POSITIVE] -- this is not found bc this is not an + // lvalue access + *p1; // COMPLIANT[FALSE_POSITIVE] -- the pointee of p1 has been initialized + + int *p2 = new int; + p2 = new int; + p2; // COMPLIANT -- this is uninitialized but its access not a meaningful read + // to this rule + + int *p3 = new int(1); + *p3 = *p2; // NON_COMPLIANT -- the pointee of p2 has not been + // initialized + *p3; // NON_COMPLIANT[FALSE_NEGATIVE] -- the pointee of p3 has be overridden + + int *p4; + p4 = new int; + *p4; // NON_COMPLIANT -- the pointee of p4 has not been + // initialized } \ No newline at end of file From d0f2edeb36816943ad38c67f1e387a44f8461b06 Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Tue, 10 Mar 2026 10:44:36 -0400 Subject: [PATCH 03/11] Remove unnecessary import in readofuninitializedmemory --- .../readofuninitializedmemory/ReadOfUninitializedMemory.qll | 1 - 1 file changed, 1 deletion(-) diff --git a/cpp/common/src/codingstandards/cpp/rules/readofuninitializedmemory/ReadOfUninitializedMemory.qll b/cpp/common/src/codingstandards/cpp/rules/readofuninitializedmemory/ReadOfUninitializedMemory.qll index 025e3b728a..8a74cc2e2e 100644 --- a/cpp/common/src/codingstandards/cpp/rules/readofuninitializedmemory/ReadOfUninitializedMemory.qll +++ b/cpp/common/src/codingstandards/cpp/rules/readofuninitializedmemory/ReadOfUninitializedMemory.qll @@ -7,7 +7,6 @@ import codingstandards.cpp.Customizations import codingstandards.cpp.Exclusions import semmle.code.cpp.controlflow.Guards import semmle.code.cpp.controlflow.SubBasicBlocks -import codingstandards.cpp.enhancements.AggregateLiteralEnhancements import InitializationFunctions abstract class ReadOfUninitializedMemorySharedQuery extends Query { } From 1c610ee7153aae49cef287988ae4d3708e26ef4a Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Tue, 10 Mar 2026 12:33:39 -0400 Subject: [PATCH 04/11] Add shared query for RULE-6-8-3 --- .../cpp/exclusions/cpp/Lifetime.qll | 44 +++++++++++++++++++ .../cpp/exclusions/cpp/RuleMetadata.qll | 3 ++ ...sOfAutoStorageObjectToOtherObject.expected | 2 + .../test.cpp | 19 ++++++++ ...cStorageAssignedToObjectGreaterLifetime.ql | 25 +++++++++++ ...ageAssignedToObjectGreaterLifetime.testref | 1 + rule_packages/cpp/Lifetime.json | 14 ++++-- 7 files changed, 104 insertions(+), 4 deletions(-) create mode 100644 cpp/common/src/codingstandards/cpp/exclusions/cpp/Lifetime.qll create mode 100644 cpp/misra/src/rules/RULE-6-8-3/AutomaticStorageAssignedToObjectGreaterLifetime.ql create mode 100644 cpp/misra/test/rules/RULE-6-8-3/AutomaticStorageAssignedToObjectGreaterLifetime.testref diff --git a/cpp/common/src/codingstandards/cpp/exclusions/cpp/Lifetime.qll b/cpp/common/src/codingstandards/cpp/exclusions/cpp/Lifetime.qll new file mode 100644 index 0000000000..bb06eaaf8f --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/exclusions/cpp/Lifetime.qll @@ -0,0 +1,44 @@ +//** THIS FILE IS AUTOGENERATED, DO NOT MODIFY DIRECTLY. **/ +import cpp +import RuleMetadata +import codingstandards.cpp.exclusions.RuleMetadata + +newtype LifetimeQuery = + TValueOfAnObjectMustNotBeReadBeforeItHasBeenSetQuery() or + TAutomaticStorageAssignedToObjectGreaterLifetimeQuery() + +predicate isLifetimeQueryMetadata(Query query, string queryId, string ruleId, string category) { + query = + // `Query` instance for the `valueOfAnObjectMustNotBeReadBeforeItHasBeenSet` query + LifetimePackage::valueOfAnObjectMustNotBeReadBeforeItHasBeenSetQuery() and + queryId = + // `@id` for the `valueOfAnObjectMustNotBeReadBeforeItHasBeenSet` query + "cpp/misra/value-of-an-object-must-not-be-read-before-it-has-been-set" and + ruleId = "RULE-11-6-2" and + category = "mandatory" + or + query = + // `Query` instance for the `automaticStorageAssignedToObjectGreaterLifetime` query + LifetimePackage::automaticStorageAssignedToObjectGreaterLifetimeQuery() and + queryId = + // `@id` for the `automaticStorageAssignedToObjectGreaterLifetime` query + "cpp/misra/automatic-storage-assigned-to-object-greater-lifetime" and + ruleId = "RULE-6-8-3" and + category = "required" +} + +module LifetimePackage { + Query valueOfAnObjectMustNotBeReadBeforeItHasBeenSetQuery() { + //autogenerate `Query` type + result = + // `Query` type for `valueOfAnObjectMustNotBeReadBeforeItHasBeenSet` query + TQueryCPP(TLifetimePackageQuery(TValueOfAnObjectMustNotBeReadBeforeItHasBeenSetQuery())) + } + + Query automaticStorageAssignedToObjectGreaterLifetimeQuery() { + //autogenerate `Query` type + result = + // `Query` type for `automaticStorageAssignedToObjectGreaterLifetime` query + TQueryCPP(TLifetimePackageQuery(TAutomaticStorageAssignedToObjectGreaterLifetimeQuery())) + } +} diff --git a/cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll b/cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll index f21e63bdc6..e3c8191fa6 100644 --- a/cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll +++ b/cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll @@ -45,6 +45,7 @@ import IntegerConversion import Invariants import Iterators import Lambdas +import Lifetime import Linkage1 import Linkage2 import Literals @@ -132,6 +133,7 @@ newtype TCPPQuery = TInvariantsPackageQuery(InvariantsQuery q) or TIteratorsPackageQuery(IteratorsQuery q) or TLambdasPackageQuery(LambdasQuery q) or + TLifetimePackageQuery(LifetimeQuery q) or TLinkage1PackageQuery(Linkage1Query q) or TLinkage2PackageQuery(Linkage2Query q) or TLiteralsPackageQuery(LiteralsQuery q) or @@ -219,6 +221,7 @@ predicate isQueryMetadata(Query query, string queryId, string ruleId, string cat isInvariantsQueryMetadata(query, queryId, ruleId, category) or isIteratorsQueryMetadata(query, queryId, ruleId, category) or isLambdasQueryMetadata(query, queryId, ruleId, category) or + isLifetimeQueryMetadata(query, queryId, ruleId, category) or isLinkage1QueryMetadata(query, queryId, ruleId, category) or isLinkage2QueryMetadata(query, queryId, ruleId, category) or isLiteralsQueryMetadata(query, queryId, ruleId, category) or diff --git a/cpp/common/test/rules/donotcopyaddressofautostorageobjecttootherobject/DoNotCopyAddressOfAutoStorageObjectToOtherObject.expected b/cpp/common/test/rules/donotcopyaddressofautostorageobjecttootherobject/DoNotCopyAddressOfAutoStorageObjectToOtherObject.expected index 8699ae2d8a..35d2fd438a 100644 --- a/cpp/common/test/rules/donotcopyaddressofautostorageobjecttootherobject/DoNotCopyAddressOfAutoStorageObjectToOtherObject.expected +++ b/cpp/common/test/rules/donotcopyaddressofautostorageobjecttootherobject/DoNotCopyAddressOfAutoStorageObjectToOtherObject.expected @@ -62,3 +62,5 @@ | stack_escapes_test.cpp:367:3:367:17 | ... = ... | A stack address ($@) may be assigned to a non-local variable. | stack_escapes_test.cpp:367:17:367:17 | x | source | | stack_escapes_test.cpp:368:3:368:20 | ... = ... | A stack address ($@) may be assigned to a non-local variable. | stack_escapes_test.cpp:368:20:368:20 | x | source | | test.cpp:7:5:7:10 | ... = ... | A stack address ($@) may be assigned to a non-local variable. | test.cpp:7:10:7:10 | c | source | +| test.cpp:15:5:15:11 | ... = ... | A stack address ($@) may be assigned to a non-local variable. | test.cpp:15:9:15:11 | l_a | source | +| test.cpp:26:5:26:10 | ... = ... | A stack address ($@) may be assigned to a non-local variable. | test.cpp:25:11:25:11 | l | source | diff --git a/cpp/common/test/rules/donotcopyaddressofautostorageobjecttootherobject/test.cpp b/cpp/common/test/rules/donotcopyaddressofautostorageobjecttootherobject/test.cpp index 0cd3970c7e..5014ecf9cb 100644 --- a/cpp/common/test/rules/donotcopyaddressofautostorageobjecttootherobject/test.cpp +++ b/cpp/common/test/rules/donotcopyaddressofautostorageobjecttootherobject/test.cpp @@ -6,4 +6,23 @@ void test_simple_aliasing() { int c; a = &c; // NON_COMPLIANT - different scope } +} + +void extra_test_simple_aliasing() { + int *p; + { + int l_a[1]; + p = l_a; // NON_COMPLIANT + } +} + +void extra_test2_simple_aliasing() { + int *p; + { + int *p2 = nullptr; + int l; + + p2 = &l; // COMPLIANT + p = p2; // NON_COMPLIANT + } } \ No newline at end of file diff --git a/cpp/misra/src/rules/RULE-6-8-3/AutomaticStorageAssignedToObjectGreaterLifetime.ql b/cpp/misra/src/rules/RULE-6-8-3/AutomaticStorageAssignedToObjectGreaterLifetime.ql new file mode 100644 index 0000000000..285bb4c8ea --- /dev/null +++ b/cpp/misra/src/rules/RULE-6-8-3/AutomaticStorageAssignedToObjectGreaterLifetime.ql @@ -0,0 +1,25 @@ +/** + * @id cpp/misra/automatic-storage-assigned-to-object-greater-lifetime + * @name RULE-6-8-3: Declare objects with appropriate storage durations + * @description When storage durations are not compatible between assigned pointers it can lead to + * referring to objects outside of their lifetime, which is undefined behaviour. + * @kind problem + * @precision very-high + * @problem.severity error + * @tags external/misra/id/rule-6-8-3 + * correctness + * security + * scope/single-translation-unit + * external/misra/enforcement/decidable + * external/misra/obligation/required + */ + +import cpp +import codingstandards.cpp.misra +import codingstandards.cpp.rules.donotcopyaddressofautostorageobjecttootherobject.DoNotCopyAddressOfAutoStorageObjectToOtherObject + +module AutomaticStorageAssignedToObjectGreaterLifetimeConfig implements DoNotCopyAddressOfAutoStorageObjectToOtherObjectConfigSig { + Query getQuery() { result = LifetimePackage::automaticStorageAssignedToObjectGreaterLifetimeQuery() } +} + +import DoNotCopyAddressOfAutoStorageObjectToOtherObject diff --git a/cpp/misra/test/rules/RULE-6-8-3/AutomaticStorageAssignedToObjectGreaterLifetime.testref b/cpp/misra/test/rules/RULE-6-8-3/AutomaticStorageAssignedToObjectGreaterLifetime.testref new file mode 100644 index 0000000000..fc0808753a --- /dev/null +++ b/cpp/misra/test/rules/RULE-6-8-3/AutomaticStorageAssignedToObjectGreaterLifetime.testref @@ -0,0 +1 @@ +cpp/common/test/rules/donotcopyaddressofautostorageobjecttootherobject/DoNotCopyAddressOfAutoStorageObjectToOtherObject.ql \ No newline at end of file diff --git a/rule_packages/cpp/Lifetime.json b/rule_packages/cpp/Lifetime.json index 30a22a8d2d..ea5d9e73c3 100644 --- a/rule_packages/cpp/Lifetime.json +++ b/rule_packages/cpp/Lifetime.json @@ -30,15 +30,21 @@ }, "queries": [ { - "description": "An assignment operator shall not assign the address of an object with automatic storage duration to an object with a greater lifetime", + "description": "When storage durations are not compatible between assigned pointers it can lead to referring to objects outside of their lifetime, which is undefined behaviour.", "kind": "problem", - "name": "An assignment operator shall not assign the address of an object with automatic storage duration to", + "name": "Declare objects with appropriate storage durations", "precision": "very-high", "severity": "error", - "short_name": "AssignmentOperatorAssignTheAddressOfAnObjectWithAutomaticStorageDurationToAnObjectWithAGreaterLifetime", + "short_name": "AutomaticStorageAssignedToObjectGreaterLifetime", + "shared_implementation_short_name": "DoNotCopyAddressOfAutoStorageObjectToOtherObject", "tags": [ + "correctness", + "security", "scope/single-translation-unit" - ] + ], + "implementation_scope": { + "description": "The rule checks specifically for pointers to objects with automatic storage duration that are assigned to static storage duration variables." + } } ], "title": "An assignment operator shall not assign the address of an object with automatic storage duration to an object with a greater lifetime" From 1704bb89ee49b7075e4568c09f67b69251b4b21e Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Tue, 10 Mar 2026 12:38:52 -0400 Subject: [PATCH 05/11] Format query --- .../AutomaticStorageAssignedToObjectGreaterLifetime.ql | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/cpp/misra/src/rules/RULE-6-8-3/AutomaticStorageAssignedToObjectGreaterLifetime.ql b/cpp/misra/src/rules/RULE-6-8-3/AutomaticStorageAssignedToObjectGreaterLifetime.ql index 285bb4c8ea..f3d32730c6 100644 --- a/cpp/misra/src/rules/RULE-6-8-3/AutomaticStorageAssignedToObjectGreaterLifetime.ql +++ b/cpp/misra/src/rules/RULE-6-8-3/AutomaticStorageAssignedToObjectGreaterLifetime.ql @@ -18,8 +18,12 @@ import cpp import codingstandards.cpp.misra import codingstandards.cpp.rules.donotcopyaddressofautostorageobjecttootherobject.DoNotCopyAddressOfAutoStorageObjectToOtherObject -module AutomaticStorageAssignedToObjectGreaterLifetimeConfig implements DoNotCopyAddressOfAutoStorageObjectToOtherObjectConfigSig { - Query getQuery() { result = LifetimePackage::automaticStorageAssignedToObjectGreaterLifetimeQuery() } +module AutomaticStorageAssignedToObjectGreaterLifetimeConfig implements + DoNotCopyAddressOfAutoStorageObjectToOtherObjectConfigSig +{ + Query getQuery() { + result = LifetimePackage::automaticStorageAssignedToObjectGreaterLifetimeQuery() + } } import DoNotCopyAddressOfAutoStorageObjectToOtherObject From ac214819411e09baf4780a71cbff445159671737 Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Tue, 10 Mar 2026 12:41:34 -0400 Subject: [PATCH 06/11] Add another missing query format --- .../ValueOfAnObjectMustNotBeReadBeforeItHasBeenSet.ql | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cpp/misra/src/rules/RULE-11-6-2/ValueOfAnObjectMustNotBeReadBeforeItHasBeenSet.ql b/cpp/misra/src/rules/RULE-11-6-2/ValueOfAnObjectMustNotBeReadBeforeItHasBeenSet.ql index 77b45a5a19..3b1ab81c2e 100644 --- a/cpp/misra/src/rules/RULE-11-6-2/ValueOfAnObjectMustNotBeReadBeforeItHasBeenSet.ql +++ b/cpp/misra/src/rules/RULE-11-6-2/ValueOfAnObjectMustNotBeReadBeforeItHasBeenSet.ql @@ -17,7 +17,8 @@ import cpp import codingstandards.cpp.misra import codingstandards.cpp.rules.readofuninitializedmemory.ReadOfUninitializedMemory -class ValueOfAnObjectMustNotBeReadBeforeItHasBeenSetQuery extends ReadOfUninitializedMemorySharedQuery { +class ValueOfAnObjectMustNotBeReadBeforeItHasBeenSetQuery extends ReadOfUninitializedMemorySharedQuery +{ ValueOfAnObjectMustNotBeReadBeforeItHasBeenSetQuery() { this = LifetimePackage::valueOfAnObjectMustNotBeReadBeforeItHasBeenSetQuery() } From b2e8f3a08abd878aa4d2840c3a7246053d664049 Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Tue, 10 Mar 2026 13:05:14 -0400 Subject: [PATCH 07/11] Revert shared rule to class not module since it shares with old style we will need to change these cases in batches --- .../AutomaticStorageAssignedToObjectGreaterLifetime.ql | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/cpp/misra/src/rules/RULE-6-8-3/AutomaticStorageAssignedToObjectGreaterLifetime.ql b/cpp/misra/src/rules/RULE-6-8-3/AutomaticStorageAssignedToObjectGreaterLifetime.ql index f3d32730c6..fe6911e35b 100644 --- a/cpp/misra/src/rules/RULE-6-8-3/AutomaticStorageAssignedToObjectGreaterLifetime.ql +++ b/cpp/misra/src/rules/RULE-6-8-3/AutomaticStorageAssignedToObjectGreaterLifetime.ql @@ -18,12 +18,9 @@ import cpp import codingstandards.cpp.misra import codingstandards.cpp.rules.donotcopyaddressofautostorageobjecttootherobject.DoNotCopyAddressOfAutoStorageObjectToOtherObject -module AutomaticStorageAssignedToObjectGreaterLifetimeConfig implements - DoNotCopyAddressOfAutoStorageObjectToOtherObjectConfigSig +class AutomaticStorageAssignedToObjectGreaterLifetimeConfig extends DoNotCopyAddressOfAutoStorageObjectToOtherObjectSharedQuery { - Query getQuery() { - result = LifetimePackage::automaticStorageAssignedToObjectGreaterLifetimeQuery() + AutomaticStorageAssignedToObjectGreaterLifetimeConfig() { + this = LifetimePackage::automaticStorageAssignedToObjectGreaterLifetimeQuery() } } - -import DoNotCopyAddressOfAutoStorageObjectToOtherObject From 1649c213b34de55a40740c7fe05400874e18c969 Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Wed, 11 Mar 2026 15:00:47 -0400 Subject: [PATCH 08/11] Improve tests to be more clear on whether certain cases are supported or not --- .../ReadOfUninitializedMemory.expected | 10 ++++++---- .../rules/readofuninitializedmemory/test.cpp | 16 ++++++++-------- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/cpp/common/test/rules/readofuninitializedmemory/ReadOfUninitializedMemory.expected b/cpp/common/test/rules/readofuninitializedmemory/ReadOfUninitializedMemory.expected index 2393dd2b37..5cfa1d51f6 100644 --- a/cpp/common/test/rules/readofuninitializedmemory/ReadOfUninitializedMemory.expected +++ b/cpp/common/test/rules/readofuninitializedmemory/ReadOfUninitializedMemory.expected @@ -7,7 +7,9 @@ | test.cpp:137:13:137:14 | i1 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:136:8:136:9 | i1 | i1 | | test.cpp:141:7:141:8 | i3 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:139:8:139:9 | i3 | i3 | | test.cpp:141:13:141:14 | i1 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:136:8:136:9 | i1 | i1 | -| test.cpp:205:4:205:5 | p1 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:204:8:204:9 | p1 | p1 | -| test.cpp:207:4:207:5 | p1 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:204:8:204:9 | p1 | p1 | -| test.cpp:215:10:215:11 | p2 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:209:8:209:9 | p2 | p2 | -| test.cpp:221:4:221:5 | p4 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:219:8:219:9 | p4 | p4 | +| test.cpp:201:7:201:8 | p0 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:200:8:200:9 | p0 | p0 | +| test.cpp:204:4:204:5 | p1 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:203:8:203:9 | p1 | p1 | +| test.cpp:206:7:206:8 | p1 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:203:8:203:9 | p1 | p1 | +| test.cpp:211:7:211:8 | p2 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:209:8:209:9 | p2 | p2 | +| test.cpp:214:10:214:11 | p2 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:209:8:209:9 | p2 | p2 | +| test.cpp:221:7:221:8 | p4 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:219:8:219:9 | p4 | p4 | diff --git a/cpp/common/test/rules/readofuninitializedmemory/test.cpp b/cpp/common/test/rules/readofuninitializedmemory/test.cpp index e6cc987dd1..7d5600a19a 100644 --- a/cpp/common/test/rules/readofuninitializedmemory/test.cpp +++ b/cpp/common/test/rules/readofuninitializedmemory/test.cpp @@ -198,26 +198,26 @@ void test_class() { void extra_extra_test() { int *p0 = new int; - p0; // COMPLIANT -- this is uninitialized but its access not a meaningful read - // to this rule + use(p0); // NON_COMPLIANT int *p1 = new int; *p1 = 0; // COMPLIANT[FALSE_POSITIVE] -- this is not found bc this is not an // lvalue access - *p1; // COMPLIANT[FALSE_POSITIVE] -- the pointee of p1 has been initialized + use(p1); // COMPLIANT[FALSE_POSITIVE] -- the pointee of p1 has been + // initialized int *p2 = new int; p2 = new int; - p2; // COMPLIANT -- this is uninitialized but its access not a meaningful read - // to this rule + use(p2); // NON_COMPLIANT int *p3 = new int(1); *p3 = *p2; // NON_COMPLIANT -- the pointee of p2 has not been // initialized - *p3; // NON_COMPLIANT[FALSE_NEGATIVE] -- the pointee of p3 has be overridden + use(p3); // NON_COMPLIANT[FALSE_NEGATIVE] -- the pointee of p3 has be + // overridden int *p4; p4 = new int; - *p4; // NON_COMPLIANT -- the pointee of p4 has not been - // initialized + use(p4); // NON_COMPLIANT -- the pointee of p4 has not been + // initialized } \ No newline at end of file From db1a631d5cc5d647436450093d04c3349003b0f7 Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Tue, 17 Mar 2026 17:13:02 -0400 Subject: [PATCH 09/11] Address review comments --- .../2026-02-03-uninitialized-mem-improve.md | 2 +- .../cpp/lifetimes/CppObjects.qll | 2 +- .../ReadOfUninitializedMemory.qll | 60 ++++++++++++++----- .../ReadOfUninitializedMemory.expected | 15 ++--- .../rules/readofuninitializedmemory/test.cpp | 36 ++++++++--- ...cStorageAssignedToObjectGreaterLifetime.ql | 2 +- rule_packages/cpp/Lifetime.json | 7 ++- 7 files changed, 87 insertions(+), 37 deletions(-) diff --git a/change_notes/2026-02-03-uninitialized-mem-improve.md b/change_notes/2026-02-03-uninitialized-mem-improve.md index 2fb2fd6539..c3db99391b 100644 --- a/change_notes/2026-02-03-uninitialized-mem-improve.md +++ b/change_notes/2026-02-03-uninitialized-mem-improve.md @@ -1,2 +1,2 @@ - `A8-5-0`, `EXP53-CPP`, `EXP33-C`, `RULE-9-1` - `MemoryNotInitializedBeforeItIsRead.ql`, `DoNotReadUninitializedMemory.ql`, `DoNotReadUninitializedMemory.ql`, `ObjectWithAutoStorageDurationReadBeforeInit.ql`: - - The queries listed now find uses of the operator 'new' where there is no value initialization provided. \ No newline at end of file + - The queries listed now find uses of the operator 'new' where there is no value initialization provided. The queries listed now also uses an out of the box library to consider initialization within another function as valid initialization (`InitializationFunctions.qll`). We do not yet track finely track the initialization/use of `p` vs `*p`. \ No newline at end of file diff --git a/cpp/common/src/codingstandards/cpp/lifetimes/CppObjects.qll b/cpp/common/src/codingstandards/cpp/lifetimes/CppObjects.qll index f1d26047d2..1c38859cb8 100644 --- a/cpp/common/src/codingstandards/cpp/lifetimes/CppObjects.qll +++ b/cpp/common/src/codingstandards/cpp/lifetimes/CppObjects.qll @@ -246,7 +246,7 @@ class AggregateLiteralObjectIdentity extends AggregateLiteral, ObjectIdentityBas } /** - * An object identified by a call to `malloc`. + * An object identified by a call to `malloc` or allcoated with a `new` or `new[]` expression. * * Note: the malloc expression returns an address to this object, not the object itself. Therefore, * `getAnAccess()` returns cases where this malloc result is dereferenced, and not the malloc call diff --git a/cpp/common/src/codingstandards/cpp/rules/readofuninitializedmemory/ReadOfUninitializedMemory.qll b/cpp/common/src/codingstandards/cpp/rules/readofuninitializedmemory/ReadOfUninitializedMemory.qll index 8a74cc2e2e..0ef95aa3c2 100644 --- a/cpp/common/src/codingstandards/cpp/rules/readofuninitializedmemory/ReadOfUninitializedMemory.qll +++ b/cpp/common/src/codingstandards/cpp/rules/readofuninitializedmemory/ReadOfUninitializedMemory.qll @@ -122,6 +122,10 @@ class InitializationContext extends TInitializationContext { } } +/** + * Catches `new int;` as an expression that doesn't initialize its value. Note that the pointer returned has been initialized (ie it is a valid pointer), + * but the pointee/value has not. In our analysis, we simply count `x` as uninitialized in `x = new int` for now, though a more thorough analysis might track the initialization of `x` and `*x` separately. + */ class NewNotInit extends NewExpr { NewNotInit() { this.getAllocatedType() instanceof BuiltInType and @@ -133,10 +137,6 @@ class NonInitAssignment extends Assignment { NonInitAssignment() { this.getRValue() instanceof NewNotInit } } -class VariableAccessOnLHSOfNonInitAssignment extends VariableAccess { - VariableAccessOnLHSOfNonInitAssignment() { exists(NonInitAssignment a | this = a.getLValue()) } -} - /** * A local variable without an initializer which is amenable to initialization analysis. */ @@ -146,14 +146,7 @@ class UninitializedVariable extends LocalVariable { ( not exists(getInitializer().getExpr()) or - //or is a builtin type used with new operator but there is no value initialization as far as we can see - exists(Initializer i, NewExpr n | - i = getInitializer() and - n = i.getExpr() and - this.getUnspecifiedType().stripType() instanceof BuiltInType and - //ignore value init - not exists(n.getAChild()) - ) + getInitializer().getExpr() instanceof NewNotInit ) and // Not static or thread local, because they are not initialized with indeterminate values not isStatic() and @@ -213,17 +206,32 @@ class UninitializedVariable extends LocalVariable { // Not a pointless read not result = any(ExprStmt es).getExpr() and // not involved in a new expr assignment since that does not define - not result instanceof VariableAccessOnLHSOfNonInitAssignment + not result = any(NonInitAssignment a).getLValue() } /** - * Gets an access of the variable `v` which is not used as an lvalue, and not used as an argument + * Gets an access of the this variable which is not used as an lvalue, and not used as an argument * to an initialization function. */ VariableAccess getAUse() { result = this.getAnAccess() and - // Not used as an lvalue - not result = any(AssignExpr a).getLValue() and + ( + //count rvalue x as a use if not new int + result.isRValue() and + not exists(PointerDereferenceExpr e | result = e.getAChild()) and + not this.getInitializer().getExpr() instanceof NewNotInit + or + //count lvalue x as a use if used in *x and not new int + result.isLValue() and + exists(PointerDereferenceExpr e | result = e.getAChild()) and + exists(this.getInitializer()) and + not this.getInitializer().getExpr() instanceof NewNotInit + or + //count rvalue *x as a use if has new int + result.isRValue() and + this.getInitializer().getExpr() instanceof NewNotInit and + exists(PointerDereferenceExpr e | result = e.getAChild()) + ) and // Not passed to another initialization function not exists(Call c, int j | j = c.getTarget().(InitializationFunction).initializedParameter() | result = c.getArgument(j).(AddressOfExpr).getOperand() @@ -234,6 +242,26 @@ class UninitializedVariable extends LocalVariable { not result.getParent+() instanceof SizeofOperator } + // /** + // * Gets an access of the this variable which is not used as an lvalue, and not used as an argument + // * to an initialization function. + // */ + // VariableAccess getAUse() { + // result = this.getAnAccess() and + // // Not used as an lvalue + // not result = any(AssignExpr a).getLValue() and + // //(result.isRValue() and not result.getType() instanceof PointerType and not this.getInitializer().getExpr() instanceof NewNotInit) and + // // Not passed to another initialization function + // not exists(Call c, int j | j = c.getTarget().(InitializationFunction).initializedParameter() | + // result = c.getArgument(j).(AddressOfExpr).getOperand() + // or + // result.isRValue() and result = c.getArgument(j) + // ) and + // // Not a pointless read + // not result = any(ExprStmt es).getExpr() and + // // sizeof operators are not real uses + // not result.getParent+() instanceof SizeofOperator + // } /** Get a read of the variable that may occur while the variable is uninitialized. */ VariableAccess getAnUnitializedUse() { exists(SubBasicBlock useSbb | diff --git a/cpp/common/test/rules/readofuninitializedmemory/ReadOfUninitializedMemory.expected b/cpp/common/test/rules/readofuninitializedmemory/ReadOfUninitializedMemory.expected index 5cfa1d51f6..3e7614e18e 100644 --- a/cpp/common/test/rules/readofuninitializedmemory/ReadOfUninitializedMemory.expected +++ b/cpp/common/test/rules/readofuninitializedmemory/ReadOfUninitializedMemory.expected @@ -6,10 +6,11 @@ | test.cpp:134:11:134:11 | i | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:133:7:133:7 | i | i | | test.cpp:137:13:137:14 | i1 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:136:8:136:9 | i1 | i1 | | test.cpp:141:7:141:8 | i3 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:139:8:139:9 | i3 | i3 | -| test.cpp:141:13:141:14 | i1 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:136:8:136:9 | i1 | i1 | -| test.cpp:201:7:201:8 | p0 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:200:8:200:9 | p0 | p0 | -| test.cpp:204:4:204:5 | p1 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:203:8:203:9 | p1 | p1 | -| test.cpp:206:7:206:8 | p1 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:203:8:203:9 | p1 | p1 | -| test.cpp:211:7:211:8 | p2 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:209:8:209:9 | p2 | p2 | -| test.cpp:214:10:214:11 | p2 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:209:8:209:9 | p2 | p2 | -| test.cpp:221:7:221:8 | p4 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:219:8:219:9 | p4 | p4 | +| test.cpp:204:8:204:9 | p0 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:202:8:202:9 | p0 | p0 | +| test.cpp:207:4:207:5 | p1 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:206:8:206:9 | p1 | p1 | +| test.cpp:211:8:211:9 | p1 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:206:8:206:9 | p1 | p1 | +| test.cpp:217:8:217:9 | p2 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:214:8:214:9 | p2 | p2 | +| test.cpp:220:10:220:11 | p2 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:214:8:214:9 | p2 | p2 | +| test.cpp:229:7:229:8 | p4 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:227:8:227:9 | p4 | p4 | +| test.cpp:233:14:233:15 | p5 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:232:8:232:9 | p5 | p5 | +| test.cpp:237:8:237:9 | p6 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:235:8:235:9 | p6 | p6 | diff --git a/cpp/common/test/rules/readofuninitializedmemory/test.cpp b/cpp/common/test/rules/readofuninitializedmemory/test.cpp index 7d5600a19a..239d2a0b21 100644 --- a/cpp/common/test/rules/readofuninitializedmemory/test.cpp +++ b/cpp/common/test/rules/readofuninitializedmemory/test.cpp @@ -196,28 +196,46 @@ void test_class() { } } +void initialize(int *p) { *p = 0; } + void extra_extra_test() { int *p0 = new int; - use(p0); // NON_COMPLIANT + use(p0); // COMPLIANT -- the pointer is valid + use(*p0); // COMPLIANT[FALSE_POSITIVE] -- the pointer is valid int *p1 = new int; - *p1 = 0; // COMPLIANT[FALSE_POSITIVE] -- this is not found bc this is not an - // lvalue access - use(p1); // COMPLIANT[FALSE_POSITIVE] -- the pointee of p1 has been - // initialized + *p1 = 0; // COMPLIANT[FALSE_POSITIVE] -- this is not found bc this is not an + // lvalue access + use(p1); // COMPLIANT -- the pointee of p1 has been + // initialized + use(*p1); // COMPLIANT[FALSE_POSITIVE] -- the pointee of p1 has been + // initialized int *p2 = new int; p2 = new int; - use(p2); // NON_COMPLIANT + use(p2); // COMPLIANT -- the pointer is valid + use(*p2); // NON_COMPLIANT -- the value may be read int *p3 = new int(1); *p3 = *p2; // NON_COMPLIANT -- the pointee of p2 has not been // initialized - use(p3); // NON_COMPLIANT[FALSE_NEGATIVE] -- the pointee of p3 has be + use(p3); // NON_COMPLIANT[FALSE_NEGATIVE] -- the pointee of p3 has been + // overridden + use(*p3); // NON_COMPLIANT[FALSE_NEGATIVE] -- the pointee of p3 has been // overridden int *p4; p4 = new int; - use(p4); // NON_COMPLIANT -- the pointee of p4 has not been - // initialized + use(p4); // COMPLIANT[FALSE_POSITIVE] -- the pointer is valid but new int isnt seen + use(*p4); // COMPLIANT -- the value is not read and the pointer is valid + + int *p5; + initialize(p5); // NON_COMPLIANT + + int *p6 = new int; + initialize(p6); // COMPLIANT + use(*p6); // COMPLIANT[FALSE_POSITIVE] + + int p7; + initialize(&p7); // COMPLIANT } \ No newline at end of file diff --git a/cpp/misra/src/rules/RULE-6-8-3/AutomaticStorageAssignedToObjectGreaterLifetime.ql b/cpp/misra/src/rules/RULE-6-8-3/AutomaticStorageAssignedToObjectGreaterLifetime.ql index fe6911e35b..d1d155e5ab 100644 --- a/cpp/misra/src/rules/RULE-6-8-3/AutomaticStorageAssignedToObjectGreaterLifetime.ql +++ b/cpp/misra/src/rules/RULE-6-8-3/AutomaticStorageAssignedToObjectGreaterLifetime.ql @@ -1,6 +1,6 @@ /** * @id cpp/misra/automatic-storage-assigned-to-object-greater-lifetime - * @name RULE-6-8-3: Declare objects with appropriate storage durations + * @name RULE-6-8-3: Do not assign the address of an object with automatic storage to an object that may persist after it's lifetime * @description When storage durations are not compatible between assigned pointers it can lead to * referring to objects outside of their lifetime, which is undefined behaviour. * @kind problem diff --git a/rule_packages/cpp/Lifetime.json b/rule_packages/cpp/Lifetime.json index ea5d9e73c3..a69a6d8c07 100644 --- a/rule_packages/cpp/Lifetime.json +++ b/rule_packages/cpp/Lifetime.json @@ -18,7 +18,10 @@ "correctness", "security", "scope/system" - ] + ], + "implementation_scope": { + "description": "The rule currently does not track member initialization or arrays at all (that have been declared with array types when they have not been assigned via pointers)." + } } ], "title": "The value of an object must not be read before it has been set" @@ -32,7 +35,7 @@ { "description": "When storage durations are not compatible between assigned pointers it can lead to referring to objects outside of their lifetime, which is undefined behaviour.", "kind": "problem", - "name": "Declare objects with appropriate storage durations", + "name": "Do not assign the address of an object with automatic storage to an object that may persist after it's lifetime", "precision": "very-high", "severity": "error", "short_name": "AutomaticStorageAssignedToObjectGreaterLifetime", From 3baadfb79fc52ded55ec2379209207e1260d1811 Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Tue, 17 Mar 2026 17:16:18 -0400 Subject: [PATCH 10/11] Fix testcase formatting --- .../ReadOfUninitializedMemory.expected | 4 ++-- cpp/common/test/rules/readofuninitializedmemory/test.cpp | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/cpp/common/test/rules/readofuninitializedmemory/ReadOfUninitializedMemory.expected b/cpp/common/test/rules/readofuninitializedmemory/ReadOfUninitializedMemory.expected index 3e7614e18e..fdf0f716d9 100644 --- a/cpp/common/test/rules/readofuninitializedmemory/ReadOfUninitializedMemory.expected +++ b/cpp/common/test/rules/readofuninitializedmemory/ReadOfUninitializedMemory.expected @@ -12,5 +12,5 @@ | test.cpp:217:8:217:9 | p2 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:214:8:214:9 | p2 | p2 | | test.cpp:220:10:220:11 | p2 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:214:8:214:9 | p2 | p2 | | test.cpp:229:7:229:8 | p4 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:227:8:227:9 | p4 | p4 | -| test.cpp:233:14:233:15 | p5 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:232:8:232:9 | p5 | p5 | -| test.cpp:237:8:237:9 | p6 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:235:8:235:9 | p6 | p6 | +| test.cpp:234:14:234:15 | p5 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:233:8:233:9 | p5 | p5 | +| test.cpp:238:8:238:9 | p6 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:236:8:236:9 | p6 | p6 | diff --git a/cpp/common/test/rules/readofuninitializedmemory/test.cpp b/cpp/common/test/rules/readofuninitializedmemory/test.cpp index 239d2a0b21..2755c03a84 100644 --- a/cpp/common/test/rules/readofuninitializedmemory/test.cpp +++ b/cpp/common/test/rules/readofuninitializedmemory/test.cpp @@ -226,7 +226,8 @@ void extra_extra_test() { int *p4; p4 = new int; - use(p4); // COMPLIANT[FALSE_POSITIVE] -- the pointer is valid but new int isnt seen + use(p4); // COMPLIANT[FALSE_POSITIVE] -- the pointer is valid but new int isnt + // seen use(*p4); // COMPLIANT -- the value is not read and the pointer is valid int *p5; @@ -234,7 +235,7 @@ void extra_extra_test() { int *p6 = new int; initialize(p6); // COMPLIANT - use(*p6); // COMPLIANT[FALSE_POSITIVE] + use(*p6); // COMPLIANT[FALSE_POSITIVE] int p7; initialize(&p7); // COMPLIANT From 242422b71194df447c4039724c69902a3ee2fb57 Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Wed, 18 Mar 2026 13:59:11 -0400 Subject: [PATCH 11/11] Address review comments --- .../ReadOfUninitializedMemory.qll | 23 +------------------ .../ReadOfUninitializedMemory.expected | 7 +++--- .../rules/readofuninitializedmemory/test.cpp | 7 +++--- 3 files changed, 8 insertions(+), 29 deletions(-) diff --git a/cpp/common/src/codingstandards/cpp/rules/readofuninitializedmemory/ReadOfUninitializedMemory.qll b/cpp/common/src/codingstandards/cpp/rules/readofuninitializedmemory/ReadOfUninitializedMemory.qll index 0ef95aa3c2..ad1453b927 100644 --- a/cpp/common/src/codingstandards/cpp/rules/readofuninitializedmemory/ReadOfUninitializedMemory.qll +++ b/cpp/common/src/codingstandards/cpp/rules/readofuninitializedmemory/ReadOfUninitializedMemory.qll @@ -216,9 +216,8 @@ class UninitializedVariable extends LocalVariable { VariableAccess getAUse() { result = this.getAnAccess() and ( - //count rvalue x as a use if not new int + //count rvalue x (or *x) as a use if not new int result.isRValue() and - not exists(PointerDereferenceExpr e | result = e.getAChild()) and not this.getInitializer().getExpr() instanceof NewNotInit or //count lvalue x as a use if used in *x and not new int @@ -242,26 +241,6 @@ class UninitializedVariable extends LocalVariable { not result.getParent+() instanceof SizeofOperator } - // /** - // * Gets an access of the this variable which is not used as an lvalue, and not used as an argument - // * to an initialization function. - // */ - // VariableAccess getAUse() { - // result = this.getAnAccess() and - // // Not used as an lvalue - // not result = any(AssignExpr a).getLValue() and - // //(result.isRValue() and not result.getType() instanceof PointerType and not this.getInitializer().getExpr() instanceof NewNotInit) and - // // Not passed to another initialization function - // not exists(Call c, int j | j = c.getTarget().(InitializationFunction).initializedParameter() | - // result = c.getArgument(j).(AddressOfExpr).getOperand() - // or - // result.isRValue() and result = c.getArgument(j) - // ) and - // // Not a pointless read - // not result = any(ExprStmt es).getExpr() and - // // sizeof operators are not real uses - // not result.getParent+() instanceof SizeofOperator - // } /** Get a read of the variable that may occur while the variable is uninitialized. */ VariableAccess getAnUnitializedUse() { exists(SubBasicBlock useSbb | diff --git a/cpp/common/test/rules/readofuninitializedmemory/ReadOfUninitializedMemory.expected b/cpp/common/test/rules/readofuninitializedmemory/ReadOfUninitializedMemory.expected index fdf0f716d9..d350f38cf7 100644 --- a/cpp/common/test/rules/readofuninitializedmemory/ReadOfUninitializedMemory.expected +++ b/cpp/common/test/rules/readofuninitializedmemory/ReadOfUninitializedMemory.expected @@ -11,6 +11,7 @@ | test.cpp:211:8:211:9 | p1 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:206:8:206:9 | p1 | p1 | | test.cpp:217:8:217:9 | p2 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:214:8:214:9 | p2 | p2 | | test.cpp:220:10:220:11 | p2 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:214:8:214:9 | p2 | p2 | -| test.cpp:229:7:229:8 | p4 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:227:8:227:9 | p4 | p4 | -| test.cpp:234:14:234:15 | p5 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:233:8:233:9 | p5 | p5 | -| test.cpp:238:8:238:9 | p6 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:236:8:236:9 | p6 | p6 | +| test.cpp:228:7:228:8 | p4 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:226:8:226:9 | p4 | p4 | +| test.cpp:230:8:230:9 | p4 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:226:8:226:9 | p4 | p4 | +| test.cpp:233:14:233:15 | p5 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:232:8:232:9 | p5 | p5 | +| test.cpp:237:8:237:9 | p6 | Local variable $@ is read here and may not be initialized on all paths. | test.cpp:235:8:235:9 | p6 | p6 | diff --git a/cpp/common/test/rules/readofuninitializedmemory/test.cpp b/cpp/common/test/rules/readofuninitializedmemory/test.cpp index 2755c03a84..347903296d 100644 --- a/cpp/common/test/rules/readofuninitializedmemory/test.cpp +++ b/cpp/common/test/rules/readofuninitializedmemory/test.cpp @@ -201,7 +201,7 @@ void initialize(int *p) { *p = 0; } void extra_extra_test() { int *p0 = new int; use(p0); // COMPLIANT -- the pointer is valid - use(*p0); // COMPLIANT[FALSE_POSITIVE] -- the pointer is valid + use(*p0); // NON_COMPLIANT -- the pointer is valid but there is no value yet int *p1 = new int; *p1 = 0; // COMPLIANT[FALSE_POSITIVE] -- this is not found bc this is not an @@ -219,8 +219,7 @@ void extra_extra_test() { int *p3 = new int(1); *p3 = *p2; // NON_COMPLIANT -- the pointee of p2 has not been // initialized - use(p3); // NON_COMPLIANT[FALSE_NEGATIVE] -- the pointee of p3 has been - // overridden + use(p3); // COMPLIANT -- the pointer is valid use(*p3); // NON_COMPLIANT[FALSE_NEGATIVE] -- the pointee of p3 has been // overridden @@ -228,7 +227,7 @@ void extra_extra_test() { p4 = new int; use(p4); // COMPLIANT[FALSE_POSITIVE] -- the pointer is valid but new int isnt // seen - use(*p4); // COMPLIANT -- the value is not read and the pointer is valid + use(*p4); // NON_COMPLIANT -- the value may be read int *p5; initialize(p5); // NON_COMPLIANT