Implement `Memory1` (`RULE-8-7-1`) by jeongsoolee09 · Pull Request #967 · github/codeql-coding-standards

jeongsoolee09 · 2025-11-18T23:09:50Z

Description

Implement Memory1 (RULE-8-7-1) and add rule package description files for the rest of the rules (Memory2-Memory6).

Change request type

Release or process automation (GitHub workflows, internal scripts)
Internal documentation
External documentation
Query files (.ql, .qll, .qls or unit tests)
External scripts (analysis report or other code shipped as part of a release)

Rules with added or modified queries

No rules added
Queries have been added for the following rules:
- RULE-8-7-1
Queries have been modified for the following rules:
- rule number here

Release change checklist

A change note (development_handbook.md#change-notes) is required for any pull request which modifies:

The structure or layout of the release artifacts.
The evaluation performance (memory, execution time) of an existing query.
The results of an existing query in any circumstance.

If you are only adding new rule queries, a change note is not required.

Author: Is a change note required?

Yes
No

🚨🚨🚨
Reviewer: Confirm that format of shared queries (not the .qll file, the
.ql file that imports it) is valid by running them within VS Code.

Confirmed

Reviewer: Confirm that either a change note is not required or the change note is required and has been added.

Confirmed

Query development review checklist

For PRs that add new queries or modify existing queries, the following checklist should be completed by both the author and reviewer:

Author

Have all the relevant rule package description files been checked in?
Have you verified that the metadata properties of each new query is set appropriately?
Do all the unit tests contain both "COMPLIANT" and "NON_COMPLIANT" cases?
Are the alert messages properly formatted and consistent with the style guide?
Have you run the queries on OpenPilot and verified that the performance and results are acceptable?
As a rule of thumb, predicates specific to the query should take no more than 1 minute, and for simple queries be under 10 seconds. If this is not the case, this should be highlighted and agreed in the code review process.
Does the query have an appropriate level of in-query comments/documentation?
Have you considered/identified possible edge cases?
Does the query not reinvent features in the standard library?
Can the query be simplified further (not golfed!)

Reviewer

Have all the relevant rule package description files been checked in?
Have you verified that the metadata properties of each new query is set appropriately?
Do all the unit tests contain both "COMPLIANT" and "NON_COMPLIANT" cases?
Are the alert messages properly formatted and consistent with the style guide?
Have you run the queries on OpenPilot and verified that the performance and results are acceptable?
As a rule of thumb, predicates specific to the query should take no more than 1 minute, and for simple queries be under 10 seconds. If this is not the case, this should be highlighted and agreed in the code review process.
Does the query have an appropriate level of in-query comments/documentation?
Have you considered/identified possible edge cases?
Does the query not reinvent features in the standard library?
Can the query be simplified further (not golfed!)

…or it

…terminator, remove file pointer cases 1. Add headers, Adding missing headers: For obvious reasons. 2. Remove cases without null terminator: Both clang and g++ do not permit strings to be allocated that are declared to be shorter than the actual initializing expression. Since this is a C++ rule, we rule them out. 3. File pointer manipulation functions (e.g. fgets): Not required by the rule.

MichaelRFairhurst

Very nice work, Jeongsoo! The work you've put into this really shows, its elegant and focused. In terms of the taint tracking edge cases, I think we can handle those cleanly on your foundation here if we focus on the array-to-pointer conversion cases, which we can talk more about later! Also, I have to say the tests you've made are awesomely comprehensive, nicely done. That is huge and really shows all your attention to detail!

MichaelRFairhurst · 2026-02-03T01:12:10Z

cpp/common/src/codingstandards/cpp/OutOfBounds.qll

+/**
+ * This module provides classes and predicates for analyzing the size of buffers
+ * or objects from their base or a byte-offset, and identifying the potential for
+ * expressions accessing those buffers to overflow.


Is this a direct copy? We should probably state that, and/or, list modifications that have been made.

It is indeed a copy of c/common/src/codingstandards/c/OutOfBounds.qll and it needed to be copy over because cpp does not depend on c but it's the other way around. The only modification made is to remove import codingstandards.c.Variable because of the same reason: the import would fail.

That means that line is not needed in the original file as well, but that's for another refactor.

cpp/misra/src/rules/RULE-8-7-1/PointerArgumentToCstringFunctionIsInvalid.ql

MichaelRFairhurst · 2026-02-03T01:18:02Z

cpp/misra/src/rules/RULE-8-7-1/PointerArgumentToCstringFunctionIsInvalid.ql

+import codingstandards.cpp.exclusions.c.RuleMetadata
+
+from
+  OOB::BufferAccessLibraryFunctionCall fc, string message, Expr bufferArg, string bufferArgStr,
+  Expr sizeOrOtherBufferArg, string otherStr
+where
+  not isExcluded(fc, OutOfBoundsPackage::libraryFunctionArgumentOutOfBoundsQuery()) and


I believe this should be not isExcluded(Memory1Package::theQueryName())

cpp/misra/src/rules/RULE-8-7-1/PointerArithmeticFormsAnInvalidPointer.ql

MichaelRFairhurst · 2026-02-03T01:24:34Z

cpp/misra/src/rules/RULE-8-7-1/PointerArithmeticFormsAnInvalidPointer.ql

+  CallocFunctionCall() { this.isCallocCall() }
+
+  override int getMinNumBytes() {
+    result = lowerBound(this.getArgument(0)) * lowerBound(this.getArgument(1))


Do we want to use the minimum, or the maximum?

I'd suggest we run this on MRVA and see how many false positives we get. If it is a lot, I'd suggest using upperBound().

Perhaps we should file a bug to come back to this.

In theory, it would be great to have two versions of the query: one where we know with certainty that the resulting pointer is out of bounds if flow analysis is correct -- we assume the maximum allocation size and the smallest pointer offsets. Then another where we suspect a possible invalid pointer, where we assume the minimum allocation size and the largest pointer offsets. These could share most behavior and would have different precisions.

In the meantime, lets ship!

MichaelRFairhurst · 2026-02-03T01:32:14Z

cpp/misra/src/rules/RULE-8-7-1/PointerArithmeticFormsAnInvalidPointer.ql

+   * Gets the offset of this pointer formation as calculated in relation to the base pointer.
+   */
+  int getOffset() {
+    result = this.asArrayExpr().getArrayOffset().getValue().toInt()


instead of getArrayOffset().getValue().toInt(), which only handles constants, we likely want to use range analysis, either upperBound (noisiest) or lowerBound (quietest).

MichaelRFairhurst · 2026-02-03T01:35:25Z

cpp/misra/src/rules/RULE-8-7-1/PointerArithmeticFormsAnInvalidPointer.ql

+      result = pointerAddition.getAnOperand().getValue().toInt() // TODO: only get the number being added
+    )
+    or
+    exists(PointerSubExpr pointerSubtraction | pointerSubtraction = this.asPointerArithmetic() |
+      result = -pointerSubtraction.getAnOperand().getValue().toInt()


Let's handle this todo, but not how copilot is suggesting, so that we can handle both p + n and n + p.

cpp/misra/src/rules/RULE-8-7-1/PointerArithmeticFormsAnInvalidPointer.ql

…/MISRA-C++-2023-Memory-Experimental

…w-nodes-MISRA-C++-2023-Memory-Experimental' into jeongsoolee09/MISRA-C++-2023-Memory-Experimental

These contain false positives due to the limitation of the status quo of the query.

MichaelRFairhurst

So close to ready!

MichaelRFairhurst · 2026-03-25T02:09:44Z

cpp/misra/src/rules/RULE-8-7-1/PointerArithmeticFormsAnInvalidPointer.ql

+ * A declaration of a variable that is of an array type.
+ */
+class ArrayDeclaration extends VariableDeclarationEntry {
+  int length;


We can probably delete this field

cpp/misra/src/rules/RULE-8-7-1/PointerArithmeticFormsAnInvalidPointer.ql

MichaelRFairhurst · 2026-03-25T02:13:57Z

cpp/misra/src/rules/RULE-8-7-1/PointerArithmeticFormsAnInvalidPointer.ql

+  CallocFunctionCall() { this.isCallocCall() }
+
+  override int getMinNumBytes() {
+    result = lowerBound(this.getArgument(0)) * lowerBound(this.getArgument(1))


Perhaps we should file a bug to come back to this.

In theory, it would be great to have two versions of the query: one where we know with certainty that the resulting pointer is out of bounds if flow analysis is correct -- we assume the maximum allocation size and the smallest pointer offsets. Then another where we suspect a possible invalid pointer, where we assume the minimum allocation size and the largest pointer offsets. These could share most behavior and would have different precisions.

In the meantime, lets ship!

MichaelRFairhurst · 2026-03-25T02:20:24Z

cpp/misra/src/rules/RULE-8-7-1/PointerArithmeticFormsAnInvalidPointer.ql

+
+newtype TArrayAllocation =
+  TStackAllocation(ArrayDeclaration arrayDecl) or
+  TDynamicAllocation(NarrowedHeapAllocationFunctionCall narrowedAlloc)


Let's file a bug to come back to the third kind of "allocation," which is just taking the address of a non-array variable or lvalue.

int x = 0; int *p = &x; // p is essentially a buffer of size 1

Partly I say let's come back because we would need to be careful to distinguish:

int x = 0; int arr[5] = {0}; int *p1 = &x; // generally, taking an address to anything should be a buffer of size 1 int *p2 = &arr[0]; // except this // Note that any lvalue expression can create a "buffer" of size 1, not just variables: int &f() { return x; } int *p3 = &f(); // also a "buffer" of size 1 int *p4 = &*p3; // also a "buffer" of size 1

MichaelRFairhurst · 2026-03-25T02:23:13Z

cpp/misra/src/rules/RULE-8-7-1/PointerArithmeticFormsAnInvalidPointer.ql

+   */
+  int getOffset() {
+    if this.asPointerArithmetic() instanceof PointerSubExpr
+    then result = -this.getOffsetExpr().getValue().toInt()


Another thing to file is that this currently only works on constant values, but in the future we could extend this to use range analysis.

MichaelRFairhurst · 2026-03-25T02:26:27Z

cpp/misra/src/rules/RULE-8-7-1/PointerArithmeticFormsAnInvalidPointer.ql

+    sink.getNode() = end.getBasePointerNode()
+  |
+    srcOffset = start.getOffset() and
+    sinkOffset = end.getOffset() and


This overwrites the previous offset, but they should add up.

For example:

int arr[5]; int *p = arr; int p1 = p + 3; // offset: 3, length: 5 int p2 = p1 + 2; // offset: 5, length: 5

Currently, this will produce sinkOffset = 2 for the last line

jeongsoolee09 added 2 commits November 18, 2025 17:43

Number Memory packages

b2231c9

Add rule description files

9b5d8b2

jeongsoolee09 self-assigned this Nov 18, 2025

jeongsoolee09 added 27 commits January 12, 2026 17:29

Add Memory1 package files

a5d4127

Expose malloc, calloc and realloc

1a2cde8

Minor comments

c21e862

Checkpoint

c0b1e55

Split out source and sinks into their cases

9d3bab0

Checkpoint

a8a6db7

Merge branch 'main' into jeongsoolee09/MISRA-C++-2023-Memory

4464702

Checkpoint: Add DynamicAllocation case

e2c5870

First working draft

fe2a3c4

Refine into path-problem

5ea652b

Change TaintTracking to DataFlow

7b860d9

Finalize first working draft for stack / heap arrays

08b8bf7

Document code copy and clean up imports

062c62f

Add multidimensional arrays alloc'ed on stack

e264dfd

Add test.cpp and expected test results

4d2bc8b

Add exclusion for Memory1.qll

21500b8

Adjust precision of existing rule and add a supplementary rule

e9f39a2

Add supplementary query files

a62e2e1

Fix @kind from problem to path-problem

ca62995

Copy OutOfBounds.qll to cpp/common/src/codingstandards/cpp/

8abf097

Remove unused import codingstandards.cpp.Variable in OutOfBounds.qll

f5454de

Add PointerArgumentToCstringFunctionIsInvalid.ql and create testref f…

d82ed6e

…or it

Copy test.c from ARR38-C and add strncpy

356bbf2

Remove testref and add qlref and expected

9ced913

Address case of strncat

1c5dc84

Remove unused predicate and update .expected

2f80208

Update the locations in an expected file

a608d11

MichaelRFairhurst requested changes Feb 3, 2026

View reviewed changes

jeongsoolee09 and others added 19 commits February 3, 2026 15:13

Change the query in that gets excluded to the right one

8435f3c

Add complete docstring to NarrowedHeadAllocationFunctionCall

32be773

Merge branch 'main' into jeongsoolee09/MISRA-C++-2023-Memory

c56946a

Add experimental query

cc0f80a

Centralize TrackArray::flow(Path) to from-where-select

ae922a7

Move flowPath to srcSinkLengthMap and add stack_only.cpp

1286958

Bind the two PathNodes in from to the pointers

1085d38

Add pointer_only.cpp

332a98d

Add more debugging messages

917e2ba

Updates to dataflow nodes & tests

0b3a231

Add cases and relabel compliant / non-compliant

698570a

Merge branch 'jeongsoolee09/MISRA-C++-2023-Memory' into jeongsoolee09…

f1d3a40

…/MISRA-C++-2023-Memory-Experimental

Update pointer_only.cpp according to test.cpp

5d2f62f

Fix redeclaration issue

53c0ef7

Merge remote-tracking branch 'origin/michaelrfairhurst/update-dataflo…

19f8e94

…w-nodes-MISRA-C++-2023-Memory-Experimental' into jeongsoolee09/MISRA-C++-2023-Memory-Experimental

Add multidimensional_only.cpp

2d6c9eb

Add docstrings

9493945

Support the multidimensional array cases

212b266

Finish experimentation with srcSinkLengthMap and merge

e3c80f5

jeongsoolee09 requested a review from MichaelRFairhurst March 23, 2026 21:46

jeongsoolee09 added 6 commits March 23, 2026 17:48

Merge branch 'main' into jeongsoolee09/MISRA-C++-2023-Memory

369c99b

Update RuleMetadata.qll

9272117

Remove unnecessary files and fix formatting

4bcc59f

Fix formatting and mention that OutOfBounds is a copy

14bdcbd

Add test cases

d254ec2

These contain false positives due to the limitation of the status quo of the query.

Format message and finish final draft

a692ba7

MichaelRFairhurst requested changes Mar 25, 2026

View reviewed changes

Merge branch 'main' into jeongsoolee09/MISRA-C++-2023-Memory

49f49e6

Conversation

jeongsoolee09 commented Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Change request type

Rules with added or modified queries

Release change checklist

Query development review checklist

Author

Reviewer

Uh oh!

MichaelRFairhurst left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MichaelRFairhurst left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jeongsoolee09 commented Nov 18, 2025 •

edited

Loading