Skip to content

Commit 2c3434b

Browse files
Tito0015cursoragent
Tito0015
and
cursoragent
committed
cpp: Move mmio-unsanitized-memcpy to experimental
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>
1 parent 799642e commit 2c3434b

13 files changed

Lines changed: 126 additions & 107 deletions

File tree

‎.gitignore‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,3 +79,6 @@ node_modules/
7979

8080
# Mergetool files
8181
*.orig
82+
83+
# Local CodeQL harness database cache (veraptos TP/TN validation)
84+
codeql_harness_dbs/

‎cpp/ql/src/change-notes/2026-08-31-mmio-unsanitized-memcpy.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
---
22
category: minorAnalysis
33
---
4-
* Added a new query, `cpp/mmio-unsanitized-memcpy`, to detect memory copy operations whose size argument is derived from memory-mapped I/O or DMA hardware state without sufficient bounds validation.
4+
* 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.

‎cpp/ql/src/codeql-suites/cpp-security-extended.qls‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,3 @@
33
- apply: security-extended-selectors.yml
44
from: codeql/suite-helpers
55
- apply: codeql-suites/exclude-slow-queries.yml
6-
# CWE-120: MMIO/DMA unsanitized memcpy (also selected by metadata; explicit for review)
7-
- include:
8-
id: cpp/mmio-unsanitized-memcpy

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,9 @@
55
<overview>
66
<p>
77
Firmware and embedded drivers often copy data into buffers using lengths read from
8-
memory-mapped I/O (MMIO) registers or DMA descriptor fields. When those lengths are not
9-
validated against the destination buffer size, an attacker who can influence hardware
10-
registers or DMA metadata can trigger buffer overflows and potentially achieve remote
11-
code execution on microcontrollers, WiFi stacks, and cellular basebands.
8+
allowlisted MMIO register macros such as <code>READ_REG</code> or <code>GET_MMIO</code>.
9+
When those lengths are not validated against the destination buffer size, an attacker who
10+
can influence hardware registers or DMA metadata can trigger buffer overflows.
1211
</p>
1312
</overview>
1413
<recommendation>

cpp/ql/src/Security/CWE/CWE-120/MmioUnsanitizedMemcpy.ql renamed to cpp/ql/src/experimental/Security/CWE/CWE-120/MmioUnsanitizedMemcpy.ql

Lines changed: 6 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
/**
22
* @name MMIO/DMA unsanitized memory copy
3-
* @description Memory copy sizes derived from memory-mapped I/O or DMA
4-
* descriptor fields without bounds validation may overflow
5-
* destination buffers.
3+
* @description Memory copy sizes derived from allowlisted MMIO/DMA register-read
4+
* macros without bounds validation may overflow destination buffers.
65
* @kind path-problem
76
* @problem.severity error
87
* @security-severity 8.6
9-
* @precision medium
10-
* @id cpp/mmio-unsanitized-memcpy
8+
* @precision low
9+
* @id cpp/experimental/mmio-unsanitized-memcpy
1110
* @tags security
11+
* experimental
1212
* external/cwe/cwe-120
1313
* external/cwe/cwe-787
1414
*/
@@ -18,27 +18,8 @@ import semmle.code.cpp.dataflow.new.TaintTracking
1818
import semmle.code.cpp.controlflow.IRGuards
1919
import MmioFlow::PathGraph
2020

21-
/** Holds if `e` is an expression that reads MMIO/DMA hardware state. */
22-
predicate isMmioExpr(Expr e) {
23-
exists(VariableAccess va | va = e and va.getTarget().isVolatile())
24-
or
25-
exists(FieldAccess fa | fa = e and fa.getTarget().getType().isVolatile())
26-
or
27-
exists(FunctionCall call |
28-
call = e and
29-
call.getTarget().hasName(["READ_REG", "GET_MMIO", "REG_READ", "DMA_READ"])
30-
)
31-
or
32-
exists(PointerDereferenceExpr deref |
33-
deref = e and
34-
deref.getOperand().getUnspecifiedType() instanceof PointerType and
35-
deref.getOperand().getUnspecifiedType().(PointerType).getBaseType().isVolatile()
36-
)
37-
}
38-
21+
/** Holds if `source` reads MMIO/DMA state through an allowlisted register macro. */
3922
predicate isMmioSource(DataFlow::Node source) {
40-
isMmioExpr(source.asExpr())
41-
or
4223
exists(MacroInvocation mi |
4324
mi.getMacro().hasName(["READ_REG", "GET_MMIO", "REG_READ", "DMA_READ"]) and
4425
source.asExpr() = mi.getExpr()

cpp/ql/src/Security/CWE/CWE-120/MmioUnsanitizedMemcpyBad.c renamed to cpp/ql/src/experimental/Security/CWE/CWE-120/MmioUnsanitizedMemcpyBad.c

File renamed without changes.

cpp/ql/src/Security/CWE/CWE-120/MmioUnsanitizedMemcpyGood.c renamed to cpp/ql/src/experimental/Security/CWE/CWE-120/MmioUnsanitizedMemcpyGood.c

File renamed without changes.

‎cpp/ql/test/experimental/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/MmioUnsanitizedMemcpy.expected‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
#select
2+
| 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 |
3+
| 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 |
4+
| 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 |
5+
| 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 |
6+
edges
7+
| test.c:25:18:25:37 | * ... | test.c:25:18:25:37 | * ... | provenance | |
8+
| test.c:25:18:25:37 | * ... | test.c:26:20:26:22 | len | provenance | |
9+
| test.c:30:18:30:37 | * ... | test.c:30:18:30:37 | * ... | provenance | |
10+
| test.c:30:18:30:37 | * ... | test.c:31:21:31:23 | len | provenance | |
11+
| test.c:35:18:35:37 | * ... | test.c:35:18:35:37 | * ... | provenance | |
12+
| test.c:35:18:35:37 | * ... | test.c:36:21:36:23 | len | provenance | |
13+
| test.c:40:18:40:37 | * ... | test.c:40:18:40:37 | * ... | provenance | |
14+
| test.c:40:18:40:37 | * ... | test.c:41:20:41:22 | len | provenance | |
15+
nodes
16+
| test.c:25:18:25:37 | * ... | semmle.label | * ... |
17+
| test.c:25:18:25:37 | * ... | semmle.label | * ... |
18+
| test.c:26:20:26:22 | len | semmle.label | len |
19+
| test.c:30:18:30:37 | * ... | semmle.label | * ... |
20+
| test.c:30:18:30:37 | * ... | semmle.label | * ... |
21+
| test.c:31:21:31:23 | len | semmle.label | len |
22+
| test.c:35:18:35:37 | * ... | semmle.label | * ... |
23+
| test.c:35:18:35:37 | * ... | semmle.label | * ... |
24+
| test.c:36:21:36:23 | len | semmle.label | len |
25+
| test.c:40:18:40:37 | * ... | semmle.label | * ... |
26+
| test.c:40:18:40:37 | * ... | semmle.label | * ... |
27+
| test.c:41:20:41:22 | len | semmle.label | len |
28+
subpaths

‎cpp/ql/test/experimental/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/MmioUnsanitizedMemcpy.qlref‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
query: experimental/Security/CWE/CWE-120/MmioUnsanitizedMemcpy.ql
2+
postprocess: utils/test/InlineExpectationsTestQuery.ql

‎cpp/ql/test/experimental/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/test.c‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
/* Semmle test case for MmioUnsanitizedMemcpy.ql
2+
* Allowlisted MMIO/DMA register macros flowing into memcpy/memmove/strncpy size parameters.
3+
*/
4+
5+
typedef unsigned int uint32_t;
6+
7+
void *memcpy(void *dest, const void *src, unsigned long n);
8+
void *memmove(void *dest, const void *src, unsigned long n);
9+
char *strncpy(char *dest, const char *src, unsigned long n);
10+
11+
#define READ_REG(addr) (*(volatile uint32_t *)(addr))
12+
#define GET_MMIO(addr) (*(volatile uint32_t *)(addr))
13+
#define REG_READ(addr) (*(volatile uint32_t *)(addr))
14+
#define DMA_READ(addr) (*(volatile uint32_t *)(addr))
15+
#define MAX_DMA_LEN 64
16+
17+
struct VolatileField {
18+
volatile uint32_t len;
19+
};
20+
21+
volatile uint32_t mmio_len_reg;
22+
struct VolatileField vf;
23+
24+
static void bad_read_reg(char *dst, char *src) {
25+
uint32_t len = READ_REG(0x40001000); // $ Source
26+
memcpy(dst, src, len); // $ Alert
27+
}
28+
29+
static void bad_get_mmio(char *dst, char *src) {
30+
uint32_t len = GET_MMIO(0x50000000); // $ Source
31+
memmove(dst, src, len); // $ Alert
32+
}
33+
34+
static void bad_reg_read(char *dst, char *src) {
35+
uint32_t len = REG_READ(0x51000000); // $ Source
36+
strncpy(dst, src, len); // $ Alert
37+
}
38+
39+
static void bad_dma_read(char *dst, char *src) {
40+
uint32_t len = DMA_READ(0x60000000); // $ Source
41+
memcpy(dst, src, len); // $ Alert
42+
}
43+
44+
static void good_bounded(char *dst, char *src) {
45+
uint32_t len = READ_REG(0x40001000);
46+
if (len <= MAX_DMA_LEN)
47+
memcpy(dst, src, len); // GOOD
48+
}
49+
50+
static void good_early_return(char *dst, char *src) {
51+
uint32_t len = DMA_READ(0x60000000);
52+
if (len > MAX_DMA_LEN)
53+
return;
54+
memcpy(dst, src, len); // GOOD
55+
}
56+
57+
static void good_constant_size(char *dst, char *src) {
58+
uint32_t len = READ_REG(0x40001000);
59+
memcpy(dst, src, 32); // GOOD — constant size, not tainted sink
60+
}
61+
62+
static void negative_volatile_global(char *dst, char *src) {
63+
uint32_t len = mmio_len_reg;
64+
memcpy(dst, src, len); // GOOD
65+
}
66+
67+
static void negative_volatile_field(char *dst, char *src) {
68+
uint32_t len = vf.len;
69+
memcpy(dst, src, len); // GOOD
70+
}
71+
72+
static void negative_volatile_deref(char *dst, char *src) {
73+
volatile uint32_t *reg = (volatile uint32_t *)0x40001000;
74+
uint32_t len = *reg;
75+
memcpy(dst, src, len); // GOOD
76+
}
77+
78+
static uint32_t GET_MMIO_fn(unsigned long addr);
79+
80+
static void negative_get_mmio_function(char *dst, char *src) {
81+
uint32_t len = GET_MMIO_fn(0x50000000);
82+
memcpy(dst, src, len); // GOOD
83+
}

0 commit comments

Comments
 (0)