cpp: Add 'cpp/mmio-unsanitized-memcpy' query by Tito0015 · Pull Request #22438 · github/codeql

Tito0015 · 2026-08-27T03:57:23Z

Summary

Adds a new security query cpp/mmio-unsanitized-memcpy targeting unsanitized memory copy operations (memcpy, memmove, strncpy) where size parameters derive directly from hardware registers (MMIO/DMA) without relational bounds checks.

Motivation & Domain Context

Standard buffer overflow queries (UnboundedWrite.ql, OverrunWrite.ql) model user-space strings and generic memory ops, but do not model volatile register macro reads (READ_REG, GET_MMIO) commonly found in microcontroller drivers, RTOS kernels, and embedded hardware stacks. This query fills a gap for embedded C/C++ static analysis.

Query Design & Architecture

Taint Engine: Modern DataFlow::ConfigSig with TaintTracking::Global.
Sources: Volatile variables, volatile struct fields, volatile pointer dereferences, and MMIO macros (READ_REG, GET_MMIO, REG_READ, DMA_READ).
Sinks: Parameter index 2 (size/count) of memcpy, memmove, strncpy, wmemcpy, wmemmove.
Barriers: Public IRGuards via DataFlow::BarrierGuard<lessThanOrEqual/3> to recognize if (len <= MAX) conditions and prevent false positives. lessThanOrEqual uses the public Operand + getConvertedResultExpression() pattern.
Public API Compliance: Uses only public APIs (cpp, TaintTracking, IRGuards). Zero internal. / DataFlowImplCommon dependencies.

Verification & Test Results

Test Command: codeql test run cpp/ql/test/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/
Result: All 1 tests passed (3 positive alerts, 3 false-positive barrier test cases clean).
Documentation: codeql generate query-help passed DTD verification and rendered clean markdown.
Suite Integration: Explicit include of cpp/mmio-unsanitized-memcpy in cpp-security-extended.qls.

Checklist

Query metadata follows upstream style guidelines (@kind path-problem, @precision medium, @security-severity 8.6).
.qhelp file provided with valid DTD structure and Bad/Good examples.
Test directory contains .qlref, test.c, and verified .expected output.
Query added to appropriate .qls suite.
No internal. module imports used.

Co-authored-by: Cursor <cursoragent@cursor.com>

Tito0015 · 2026-09-02T05:44:05Z

Hi @github/codeql-cpp just checking in on this query submission! The query adds MMIO/DMA-to-memcpy bounds modeling for embedded C/C++ drivers, complete with unit tests and QL documentation. Let me know whenever the team has a moment to review or trigger CI.

jketema

Hi, I'll try to find a reviewer for you. However, this query has very low quality (see below), not what we would consider "medium". Hence, at the very least it should be moved into the directory for experimental queries.

I ran the query on about 1000 databases, and most of the results seem unrelated to memory mapped I/O and look more cases where volatile is used for other (incorrect) reasons.

jketema · 2026-09-02T06:23:14Z

+# CWE-120: MMIO/DMA unsanitized memcpy (also selected by metadata; explicit for review)
+- include:
+    id: cpp/mmio-unsanitized-memcpy


This will need to go.

jketema · 2026-09-02T06:23:35Z

+  or
+  exists(FunctionCall call |
+    call = e and
+    call.getTarget().hasName(["READ_REG", "GET_MMIO", "REG_READ", "DMA_READ"])


Going by your test some or all of these are macros, which means this will not work, as macros are not picked up as functions. You correct for this below, but if some of these are always macros or always functions, it would be better to avoid this duplication.

Relocate the query under experimental/, narrow sources to allowlisted MMIO register macros only, drop the security-extended suite include, and update tests and change notes for maintainer feedback on PR github#22438. Co-authored-by: Cursor <cursoragent@cursor.com>

Tito0015 · 2026-09-02T20:08:10Z

Hi @jketema — thank you again for the feedback and for running this across the DB corpus! You were spot on regarding the generic volatile False Positive trap.

I have updated the PR with the following changes:

Relocated: Moved the query out of cpp-security-extended.qls and relocated it to cpp/ql/src/experimental/Security/CWE/CWE-120/MmioUnsanitizedMemcpy.ql with @precision low and query ID cpp/experimental/mmio-unsanitized-memcpy.
AST Deduplication: Removed redundant FunctionCall checks and strictly anchored sources to MacroInvocation expansions (READ_REG, GET_MMIO, REG_READ, DMA_READ).
Eliminated FP Trap: Completely removed generic volatile variable/field/pointer checks to eliminate noise on user-space applications.
Local SARIF Verification: Built a local verification harness and tested the query against a built CodeQL database of curl. The False Positive rate on standard C desktop code is now zero (0 alerts), while catching target MMIO/DMA register flows into memcpy without bounds checks in unit tests.

Let me know if this updated AST modeling looks ready for the experimental queue!

jketema · 2026-09-02T21:52:35Z

+
+# Local CodeQL harness database cache (veraptos TP/TN validation)
+codeql_harness_dbs/


Please remove this junk.

jketema · 2026-09-02T21:57:17Z

+---
+category: minorAnalysis
+---
+* Added a new experimental query, `cpp/experimental/mmio-unsanitized-memcpy`, to detect memory copy operations whose size argument is derived from allowlisted MMIO/DMA register-read macros without sufficient bounds validation.


We don't publish change notes for experimental queries.

jketema · 2026-09-02T21:58:40Z

+ *              macros without bounds validation may overflow destination buffers.
+ * @kind path-problem
+ * @problem.severity error
+ * @security-severity 8.6


Likely wrong and not needed for experimental queries.

Suggested change

* @security-severity 8.6

jketema · 2026-09-02T22:00:22Z

+ * @problem.severity error
+ * @security-severity 8.6
+ * @precision low
+ * @id cpp/experimental/mmio-unsanitized-memcpy


I'm pretty sure we can just leave this as:

Suggested change

* @id cpp/experimental/mmio-unsanitized-memcpy

* @id cpp/mmio-unsanitized-memcpy

github-actions · 2026-09-03T09:29:04Z

QHelp previews:

cpp/ql/src/experimental/Security/CWE/CWE-120/MmioUnsanitizedMemcpy.qhelp

MMIO/DMA unsanitized memory copy

Firmware and embedded drivers often copy data into buffers using lengths read from allowlisted MMIO register macros such as READ_REG or GET_MMIO. When those lengths are not validated against the destination buffer size, an attacker who can influence hardware registers or DMA metadata can trigger buffer overflows.

Recommendation

Always validate MMIO/DMA-derived lengths before passing them to memcpy, memmove, or strncpy. Compare against a compile-time maximum and reject or clamp out-of-range values before copying.

Example

Bad: length from an MMIO register used directly as the copy size.

#define READ_REG(addr) (*(volatile unsigned int *)(addr))
#define MAX_DMA_LEN 64

void *memcpy(void *dest, const void *src, unsigned long n);

void bad_mmio_memcpy(char *dst, char *src) {
  unsigned int len = READ_REG(0x40001000);
  memcpy(dst, src, len);
}

Good: defensive bounds check before the copy.

#define READ_REG(addr) (*(volatile unsigned int *)(addr))
#define MAX_DMA_LEN 64

void *memcpy(void *dest, const void *src, unsigned long n);

void good_mmio_memcpy(char *dst, char *src) {
  unsigned int len = READ_REG(0x40001000);
  if (len <= MAX_DMA_LEN)
    memcpy(dst, src, len);
}

References

CWE-120: Buffer Copy without Checking Size of Input
CWE-787: Out-of-bounds Write
Common Weakness Enumeration: CWE-120.
Common Weakness Enumeration: CWE-787.

jketema · 2026-09-03T10:43:15Z

ql/cpp/ql/integration-tests/query-suite/test.py::test_not_included_queries fails because the new query has not been added to the test results.

jketema · 2026-09-03T11:59:45Z

+where
+  MmioFlow::flowPath(source, sink) and
+  isMemcpySizeSink(sink.getNode(), memcpyCall)
+select memcpyCall, source, sink,


Because you're not using the sink as alert location (you're using memcpyCall instead. CI is giving the errors below. To solve this you should either use the sink as alert location, or implement getASelectedSinkLocation (see here)

Cannot find (26,3)-(26,8).expected file. --- expected +++ actual @@ -1,1 +1,3 @@ - +Filtering alerts to these ranges: + test.c:(26,3)-(26,8) +Wrongly omitted: | test.c:26:3:26:8 | call to memcpy | test.c:25:18:25:37 | * ... | test.c:26:20:26:22 | len | Memory copy size argument is derived from $@ without sufficient bounds validation. | test.c:25:18:25:37 | * ... | an MMIO/DMA hardware register read | Error: [2/3] [eval 307ms] FAILED(RESULT) /home/runner/work/semmle-code/semmle-code/ql/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/DIFF-INFORMED/MmioUnsanitizedMemcpy/test.c/(26,3)-(26,8) Cannot find (31,3)-(31,9).expected file. --- expected +++ actual @@ -1,1 +1,3 @@ - +Filtering alerts to these ranges: + test.c:(31,3)-(31,9) +Wrongly omitted: | test.c:31:3:31:9 | call to memmove | test.c:30:18:30:37 | * ... | test.c:31:21:31:23 | len | Memory copy size argument is derived from $@ without sufficient bounds validation. | test.c:30:18:30:37 | * ... | an MMIO/DMA hardware register read | Error: [2/3] [eval 292ms] FAILED(RESULT) /home/runner/work/semmle-code/semmle-code/ql/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/DIFF-INFORMED/MmioUnsanitizedMemcpy/test.c/(31,3)-(31,9) Cannot find (36,3)-(36,9).expected file. --- expected +++ actual @@ -1,1 +1,3 @@ - +Filtering alerts to these ranges: + test.c:(36,3)-(36,9) +Wrongly omitted: | test.c:36:3:36:9 | call to strncpy | test.c:35:18:35:37 | * ... | test.c:36:21:36:23 | len | Memory copy size argument is derived from $@ without sufficient bounds validation. | test.c:35:18:35:37 | * ... | an MMIO/DMA hardware register read | Error: [2/3] [eval 254ms] FAILED(RESULT) /home/runner/work/semmle-code/semmle-code/ql/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/DIFF-INFORMED/MmioUnsanitizedMemcpy/test.c/(36,3)-(36,9) Cannot find (41,3)-(41,8).expected file. --- expected +++ actual @@ -1,1 +1,3 @@ - +Filtering alerts to these ranges: + test.c:(41,3)-(41,8) +Wrongly omitted: | test.c:41:3:41:8 | call to memcpy | test.c:40:18:40:37 | * ... | test.c:41:20:41:22 | len | Memory copy size argument is derived from $@ without sufficient bounds validation. | test.c:40:18:40:37 | * ... | an MMIO/DMA hardware register read | Error: [2/3] [eval 234ms] FAILED(RESULT) /home/runner/work/semmle-code/semmle-code/ql/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/DIFF-INFORMED/MmioUnsanitizedMemcpy/test.c/(41,3)-(41,8)

cpp: Add 'cpp/mmio-unsanitized-memcpy' query

31ca346

Co-authored-by: Cursor <cursoragent@cursor.com>

Tito0015 requested a review from a team as a code owner August 27, 2026 03:57

github-actions Bot added C++ documentation labels Aug 27, 2026

Tito0015 and others added 3 commits August 31, 2026 11:32

Merge upstream/main into feature/cpp-mmio-unsanitized-memcpy

b200dd7

cpp: Add change note for mmio-unsanitized-memcpy query

ac50b21

Co-authored-by: Cursor <cursoragent@cursor.com>

Merge branch 'main' into feature/cpp-mmio-unsanitized-memcpy

597608a

jketema reviewed Sep 2, 2026

View reviewed changes

Tito0015 and others added 2 commits September 2, 2026 14:00

Merge upstream/main into feature/cpp-mmio-unsanitized-memcpy

799642e

Tito0015 requested a review from a team as a code owner September 2, 2026 20:00

jketema reviewed Sep 2, 2026

View reviewed changes

cpp: Drop experimental query id prefix and change note

5c92f5e

Tito0015 force-pushed the feature/cpp-mmio-unsanitized-memcpy branch from 971e182 to 5c92f5e Compare September 3, 2026 00:51

jketema reviewed Sep 3, 2026

View reviewed changes


		# Local CodeQL harness database cache (veraptos TP/TN validation)
		codeql_harness_dbs/

	* @id cpp/experimental/mmio-unsanitized-memcpy
	* @id cpp/mmio-unsanitized-memcpy

Uh oh!

Conversation

Tito0015 commented Aug 27, 2026

Summary

Motivation & Domain Context

Query Design & Architecture

Verification & Test Results

Checklist

Uh oh!

Tito0015 commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jketema left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jketema Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

jketema Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Tito0015 commented Sep 2, 2026

Uh oh!

jketema Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

jketema Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

jketema Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

jketema Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Sep 3, 2026

MMIO/DMA unsanitized memory copy

Recommendation

Example

References

Uh oh!

jketema commented Sep 3, 2026

Uh oh!

jketema Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Tito0015 commented Sep 2, 2026 •

edited

Loading

jketema left a comment •

edited

Loading

jketema Sep 2, 2026 •

edited

Loading