From 31ca3466653889d34203028f5ed88536ed13c618 Mon Sep 17 00:00:00 2001 From: Tarek Date: Wed, 26 Aug 2026 23:53:37 -0400 Subject: [PATCH 1/4] cpp: Add 'cpp/mmio-unsanitized-memcpy' query Co-authored-by: Cursor --- .../CWE/CWE-120/MmioUnsanitizedMemcpy.qhelp | 35 ++++++++ .../CWE/CWE-120/MmioUnsanitizedMemcpy.ql | 84 +++++++++++++++++++ .../CWE/CWE-120/MmioUnsanitizedMemcpyBad.c | 9 ++ .../CWE/CWE-120/MmioUnsanitizedMemcpyGood.c | 10 +++ .../codeql-suites/cpp-security-extended.qls | 3 + .../MmioUnsanitizedMemcpy.expected | 22 +++++ .../MmioUnsanitizedMemcpy.qlref | 2 + .../CWE/CWE-120/MmioUnsanitizedMemcpy/test.c | 50 +++++++++++ 8 files changed, 215 insertions(+) create mode 100644 cpp/ql/src/Security/CWE/CWE-120/MmioUnsanitizedMemcpy.qhelp create mode 100644 cpp/ql/src/Security/CWE/CWE-120/MmioUnsanitizedMemcpy.ql create mode 100644 cpp/ql/src/Security/CWE/CWE-120/MmioUnsanitizedMemcpyBad.c create mode 100644 cpp/ql/src/Security/CWE/CWE-120/MmioUnsanitizedMemcpyGood.c create mode 100644 cpp/ql/test/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/MmioUnsanitizedMemcpy.expected create mode 100644 cpp/ql/test/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/MmioUnsanitizedMemcpy.qlref create mode 100644 cpp/ql/test/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/test.c diff --git a/cpp/ql/src/Security/CWE/CWE-120/MmioUnsanitizedMemcpy.qhelp b/cpp/ql/src/Security/CWE/CWE-120/MmioUnsanitizedMemcpy.qhelp new file mode 100644 index 000000000000..0b431d77cf53 --- /dev/null +++ b/cpp/ql/src/Security/CWE/CWE-120/MmioUnsanitizedMemcpy.qhelp @@ -0,0 +1,35 @@ + + + +

+Firmware and embedded drivers often copy data into buffers using lengths read from +memory-mapped I/O (MMIO) registers or DMA descriptor fields. 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 and potentially achieve remote +code execution on microcontrollers, WiFi stacks, and cellular basebands. +

+ + +

+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. +

+ + +

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

+ +

Good: defensive bounds check before the copy.

+ + + +
  • +CWE-120: Buffer Copy without Checking Size of Input +
    • +
  • +CWE-787: Out-of-bounds Write +
    • +     +     diff --git a/cpp/ql/src/Security/CWE/CWE-120/MmioUnsanitizedMemcpy.ql b/cpp/ql/src/Security/CWE/CWE-120/MmioUnsanitizedMemcpy.ql new file mode 100644 index 000000000000..8d47e2fc939f --- /dev/null +++ b/cpp/ql/src/Security/CWE/CWE-120/MmioUnsanitizedMemcpy.ql @@ -0,0 +1,84 @@ +/** + * @name MMIO/DMA unsanitized memory copy + * @description Memory copy sizes derived from memory-mapped I/O or DMA + * descriptor fields without bounds validation may overflow + * destination buffers. + * @kind path-problem + * @problem.severity error + * @security-severity 8.6 + * @precision medium + * @id cpp/mmio-unsanitized-memcpy + * @tags security + * external/cwe/cwe-120 + * external/cwe/cwe-787 + */ + +import cpp +import semmle.code.cpp.dataflow.new.TaintTracking +import semmle.code.cpp.controlflow.IRGuards +import MmioFlow::PathGraph + +/** Holds if `e` is an expression that reads MMIO/DMA hardware state. */ +predicate isMmioExpr(Expr e) { + exists(VariableAccess va | va = e and va.getTarget().isVolatile()) + or + exists(FieldAccess fa | fa = e and fa.getTarget().getType().isVolatile()) + or + exists(FunctionCall call | + call = e and + call.getTarget().hasName(["READ_REG", "GET_MMIO", "REG_READ", "DMA_READ"]) + ) + or + exists(PointerDereferenceExpr deref | + deref = e and + deref.getOperand().getUnspecifiedType() instanceof PointerType and + deref.getOperand().getUnspecifiedType().(PointerType).getBaseType().isVolatile() + ) +} + +predicate isMmioSource(DataFlow::Node source) { + isMmioExpr(source.asExpr()) + or + exists(MacroInvocation mi | + mi.getMacro().hasName(["READ_REG", "GET_MMIO", "REG_READ", "DMA_READ"]) and + source.asExpr() = mi.getExpr() + ) +} + +predicate isMemcpySizeSink(DataFlow::Node sink, FunctionCall fc) { + fc.getTarget().hasName(["memcpy", "memmove", "strncpy", "wmemcpy", "wmemmove"]) and + sink.asExpr() = fc.getArgument(2) +} + +/** Recognizes relational comparison bounds checks using public IRGuards API. */ +predicate lessThanOrEqual(IRGuardCondition g, Expr e, boolean branch) { + exists(Operand left | + g.comparesLt(left, _, _, true, branch) or + g.comparesEq(left, _, _, true, branch) + | + left.getDef().getConvertedResultExpression() = e + ) +} + +module MmioConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { isMmioSource(source) } + + predicate isSink(DataFlow::Node sink) { isMemcpySizeSink(sink, _) } + + predicate isBarrier(DataFlow::Node node) { + node = DataFlow::BarrierGuard::getABarrierNode() or + node = DataFlow::BarrierGuard::getAnIndirectBarrierNode() + } + + predicate observeDiffInformedIncrementalMode() { any() } +} + +module MmioFlow = TaintTracking::Global; + +from FunctionCall memcpyCall, MmioFlow::PathNode source, MmioFlow::PathNode sink +where + MmioFlow::flowPath(source, sink) and + isMemcpySizeSink(sink.getNode(), memcpyCall) +select memcpyCall, source, sink, + "Memory copy size argument is derived from $@ without sufficient bounds validation.", + source.getNode(), "an MMIO/DMA hardware register read" diff --git a/cpp/ql/src/Security/CWE/CWE-120/MmioUnsanitizedMemcpyBad.c b/cpp/ql/src/Security/CWE/CWE-120/MmioUnsanitizedMemcpyBad.c new file mode 100644 index 000000000000..13e68fc7561d --- /dev/null +++ b/cpp/ql/src/Security/CWE/CWE-120/MmioUnsanitizedMemcpyBad.c @@ -0,0 +1,9 @@ +#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); +} diff --git a/cpp/ql/src/Security/CWE/CWE-120/MmioUnsanitizedMemcpyGood.c b/cpp/ql/src/Security/CWE/CWE-120/MmioUnsanitizedMemcpyGood.c new file mode 100644 index 000000000000..38c37a3f3c64 --- /dev/null +++ b/cpp/ql/src/Security/CWE/CWE-120/MmioUnsanitizedMemcpyGood.c @@ -0,0 +1,10 @@ +#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); +} diff --git a/cpp/ql/src/codeql-suites/cpp-security-extended.qls b/cpp/ql/src/codeql-suites/cpp-security-extended.qls index 69c014c4c6f8..6c992b3da313 100644 --- a/cpp/ql/src/codeql-suites/cpp-security-extended.qls +++ b/cpp/ql/src/codeql-suites/cpp-security-extended.qls @@ -3,3 +3,6 @@ - apply: security-extended-selectors.yml from: codeql/suite-helpers - apply: codeql-suites/exclude-slow-queries.yml +# CWE-120: MMIO/DMA unsanitized memcpy (also selected by metadata; explicit for review) +- include: + id: cpp/mmio-unsanitized-memcpy diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/MmioUnsanitizedMemcpy.expected b/cpp/ql/test/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/MmioUnsanitizedMemcpy.expected new file mode 100644 index 000000000000..78511215ff39 --- /dev/null +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/MmioUnsanitizedMemcpy.expected @@ -0,0 +1,22 @@ +#select +| test.c:21:3:21:8 | call to memcpy | test.c:20:18:20:37 | * ... | test.c:21:20:21:22 | len | Memory copy size argument is derived from $@ without sufficient bounds validation. | test.c:20:18:20:37 | * ... | an MMIO/DMA hardware register read | +| test.c:26:3:26:9 | call to memmove | test.c:25:18:25:25 | call to GET_MMIO | test.c:26:21:26:23 | len | Memory copy size argument is derived from $@ without sufficient bounds validation. | test.c:25:18:25:25 | call to GET_MMIO | an MMIO/DMA hardware register read | +| test.c:31:3:31:9 | call to strncpy | test.c:30:18:30:29 | mmio_len_reg | test.c:31:21:31:23 | len | Memory copy size argument is derived from $@ without sufficient bounds validation. | test.c:30:18:30:29 | mmio_len_reg | an MMIO/DMA hardware register read | +edges +| test.c:20:18:20:37 | * ... | test.c:20:18:20:37 | * ... | provenance | | +| test.c:20:18:20:37 | * ... | test.c:21:20:21:22 | len | provenance | | +| test.c:25:18:25:25 | call to GET_MMIO | test.c:25:18:25:25 | call to GET_MMIO | provenance | | +| test.c:25:18:25:25 | call to GET_MMIO | test.c:26:21:26:23 | len | provenance | | +| test.c:30:18:30:29 | mmio_len_reg | test.c:30:18:30:29 | mmio_len_reg | provenance | | +| test.c:30:18:30:29 | mmio_len_reg | test.c:31:21:31:23 | len | provenance | | +nodes +| test.c:20:18:20:37 | * ... | semmle.label | * ... | +| test.c:20:18:20:37 | * ... | semmle.label | * ... | +| test.c:21:20:21:22 | len | semmle.label | len | +| test.c:25:18:25:25 | call to GET_MMIO | semmle.label | call to GET_MMIO | +| test.c:25:18:25:25 | call to GET_MMIO | semmle.label | call to GET_MMIO | +| test.c:26:21:26:23 | len | semmle.label | len | +| test.c:30:18:30:29 | mmio_len_reg | semmle.label | mmio_len_reg | +| test.c:30:18:30:29 | mmio_len_reg | semmle.label | mmio_len_reg | +| test.c:31:21:31:23 | len | semmle.label | len | +subpaths diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/MmioUnsanitizedMemcpy.qlref b/cpp/ql/test/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/MmioUnsanitizedMemcpy.qlref new file mode 100644 index 000000000000..e82093d33480 --- /dev/null +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/MmioUnsanitizedMemcpy.qlref @@ -0,0 +1,2 @@ +query: Security/CWE/CWE-120/MmioUnsanitizedMemcpy.ql +postprocess: utils/test/InlineExpectationsTestQuery.ql diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/test.c b/cpp/ql/test/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/test.c new file mode 100644 index 000000000000..01286d2a14b4 --- /dev/null +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/test.c @@ -0,0 +1,50 @@ +/* Semmle test case for MmioUnsanitizedMemcpy.ql + * MMIO/DMA register reads flowing into memcpy/memmove/strncpy size parameters. + */ + +typedef unsigned int uint32_t; + +void *memcpy(void *dest, const void *src, unsigned long n); +void *memmove(void *dest, const void *src, unsigned long n); +char *strncpy(char *dest, const char *src, unsigned long n); + +#define READ_REG(addr) (*(volatile uint32_t *)(addr)) +#define MAX_DMA_LEN 64 + +uint32_t GET_MMIO(unsigned long addr); +uint32_t DMA_READ(unsigned long addr); + +volatile uint32_t mmio_len_reg; + +static void bad_read_reg(char *dst, char *src) { + uint32_t len = READ_REG(0x40001000); // $ Source + memcpy(dst, src, len); // $ Alert +} + +static void bad_get_mmio(char *dst, char *src) { + uint32_t len = GET_MMIO(0x50000000); // $ Source + memmove(dst, src, len); // $ Alert +} + +static void bad_volatile_global(char *dst, char *src) { + uint32_t len = mmio_len_reg; // $ Source + strncpy(dst, src, len); // $ Alert +} + +static void good_bounded(char *dst, char *src) { + uint32_t len = READ_REG(0x40001000); + if (len <= MAX_DMA_LEN) + memcpy(dst, src, len); // GOOD +} + +static void good_early_return(char *dst, char *src) { + uint32_t len = DMA_READ(0x60000000); + if (len > MAX_DMA_LEN) + return; + memcpy(dst, src, len); // GOOD +} + +static void good_constant_size(char *dst, char *src) { + uint32_t len = READ_REG(0x40001000); + memcpy(dst, src, 32); // GOOD — constant size, not tainted sink +} From ac50b215acda00a3af402862ae13dcd8176cbfbc Mon Sep 17 00:00:00 2001 From: Tarek Date: Mon, 31 Aug 2026 11:34:04 -0400 Subject: [PATCH 2/4] cpp: Add change note for mmio-unsanitized-memcpy query Co-authored-by: Cursor --- cpp/ql/src/change-notes/2026-08-31-mmio-unsanitized-memcpy.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 cpp/ql/src/change-notes/2026-08-31-mmio-unsanitized-memcpy.md diff --git a/cpp/ql/src/change-notes/2026-08-31-mmio-unsanitized-memcpy.md b/cpp/ql/src/change-notes/2026-08-31-mmio-unsanitized-memcpy.md new file mode 100644 index 000000000000..e7b396a72345 --- /dev/null +++ b/cpp/ql/src/change-notes/2026-08-31-mmio-unsanitized-memcpy.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* 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. From 2c3434b3321284d08f172c5412928a7c7268697a Mon Sep 17 00:00:00 2001 From: Tarek Date: Wed, 2 Sep 2026 16:00:30 -0400 Subject: [PATCH 3/4] 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 #22438. Co-authored-by: Cursor --- .gitignore | 3 + .../2026-08-31-mmio-unsanitized-memcpy.md | 2 +- .../codeql-suites/cpp-security-extended.qls | 3 - .../CWE/CWE-120/MmioUnsanitizedMemcpy.qhelp | 7 +- .../CWE/CWE-120/MmioUnsanitizedMemcpy.ql | 31 ++----- .../CWE/CWE-120/MmioUnsanitizedMemcpyBad.c | 0 .../CWE/CWE-120/MmioUnsanitizedMemcpyGood.c | 0 .../MmioUnsanitizedMemcpy.expected | 28 +++++++ .../MmioUnsanitizedMemcpy.qlref | 2 + .../CWE/CWE-120/MmioUnsanitizedMemcpy/test.c | 83 +++++++++++++++++++ .../MmioUnsanitizedMemcpy.expected | 22 ----- .../MmioUnsanitizedMemcpy.qlref | 2 - .../CWE/CWE-120/MmioUnsanitizedMemcpy/test.c | 50 ----------- 13 files changed, 126 insertions(+), 107 deletions(-) rename cpp/ql/src/{ => experimental}/Security/CWE/CWE-120/MmioUnsanitizedMemcpy.qhelp (71%) rename cpp/ql/src/{ => experimental}/Security/CWE/CWE-120/MmioUnsanitizedMemcpy.ql (68%) rename cpp/ql/src/{ => experimental}/Security/CWE/CWE-120/MmioUnsanitizedMemcpyBad.c (100%) rename cpp/ql/src/{ => experimental}/Security/CWE/CWE-120/MmioUnsanitizedMemcpyGood.c (100%) create mode 100644 cpp/ql/test/experimental/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/MmioUnsanitizedMemcpy.expected create mode 100644 cpp/ql/test/experimental/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/MmioUnsanitizedMemcpy.qlref create mode 100644 cpp/ql/test/experimental/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/test.c delete mode 100644 cpp/ql/test/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/MmioUnsanitizedMemcpy.expected delete mode 100644 cpp/ql/test/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/MmioUnsanitizedMemcpy.qlref delete mode 100644 cpp/ql/test/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/test.c diff --git a/.gitignore b/.gitignore index 4dbe45b8d28a..d6b0edfe2516 100644 --- a/.gitignore +++ b/.gitignore @@ -79,3 +79,6 @@ node_modules/ # Mergetool files *.orig + +# Local CodeQL harness database cache (veraptos TP/TN validation) +codeql_harness_dbs/ diff --git a/cpp/ql/src/change-notes/2026-08-31-mmio-unsanitized-memcpy.md b/cpp/ql/src/change-notes/2026-08-31-mmio-unsanitized-memcpy.md index e7b396a72345..3a96eb5d6a60 100644 --- a/cpp/ql/src/change-notes/2026-08-31-mmio-unsanitized-memcpy.md +++ b/cpp/ql/src/change-notes/2026-08-31-mmio-unsanitized-memcpy.md @@ -1,4 +1,4 @@ --- category: minorAnalysis --- -* 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. +* 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. diff --git a/cpp/ql/src/codeql-suites/cpp-security-extended.qls b/cpp/ql/src/codeql-suites/cpp-security-extended.qls index 6c992b3da313..69c014c4c6f8 100644 --- a/cpp/ql/src/codeql-suites/cpp-security-extended.qls +++ b/cpp/ql/src/codeql-suites/cpp-security-extended.qls @@ -3,6 +3,3 @@ - apply: security-extended-selectors.yml from: codeql/suite-helpers - apply: codeql-suites/exclude-slow-queries.yml -# CWE-120: MMIO/DMA unsanitized memcpy (also selected by metadata; explicit for review) -- include: - id: cpp/mmio-unsanitized-memcpy diff --git a/cpp/ql/src/Security/CWE/CWE-120/MmioUnsanitizedMemcpy.qhelp b/cpp/ql/src/experimental/Security/CWE/CWE-120/MmioUnsanitizedMemcpy.qhelp similarity index 71% rename from cpp/ql/src/Security/CWE/CWE-120/MmioUnsanitizedMemcpy.qhelp rename to cpp/ql/src/experimental/Security/CWE/CWE-120/MmioUnsanitizedMemcpy.qhelp index 0b431d77cf53..c89496bb1745 100644 --- a/cpp/ql/src/Security/CWE/CWE-120/MmioUnsanitizedMemcpy.qhelp +++ b/cpp/ql/src/experimental/Security/CWE/CWE-120/MmioUnsanitizedMemcpy.qhelp @@ -5,10 +5,9 @@

    Firmware and embedded drivers often copy data into buffers using lengths read from -memory-mapped I/O (MMIO) registers or DMA descriptor fields. 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 and potentially achieve remote -code execution on microcontrollers, WiFi stacks, and cellular basebands. +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.

    diff --git a/cpp/ql/src/Security/CWE/CWE-120/MmioUnsanitizedMemcpy.ql b/cpp/ql/src/experimental/Security/CWE/CWE-120/MmioUnsanitizedMemcpy.ql similarity index 68% rename from cpp/ql/src/Security/CWE/CWE-120/MmioUnsanitizedMemcpy.ql rename to cpp/ql/src/experimental/Security/CWE/CWE-120/MmioUnsanitizedMemcpy.ql index 8d47e2fc939f..17f2a339e553 100644 --- a/cpp/ql/src/Security/CWE/CWE-120/MmioUnsanitizedMemcpy.ql +++ b/cpp/ql/src/experimental/Security/CWE/CWE-120/MmioUnsanitizedMemcpy.ql @@ -1,14 +1,14 @@ /** * @name MMIO/DMA unsanitized memory copy - * @description Memory copy sizes derived from memory-mapped I/O or DMA - * descriptor fields without bounds validation may overflow - * destination buffers. + * @description Memory copy sizes derived from allowlisted MMIO/DMA register-read + * macros without bounds validation may overflow destination buffers. * @kind path-problem * @problem.severity error * @security-severity 8.6 - * @precision medium - * @id cpp/mmio-unsanitized-memcpy + * @precision low + * @id cpp/experimental/mmio-unsanitized-memcpy * @tags security + * experimental * external/cwe/cwe-120 * external/cwe/cwe-787 */ @@ -18,27 +18,8 @@ import semmle.code.cpp.dataflow.new.TaintTracking import semmle.code.cpp.controlflow.IRGuards import MmioFlow::PathGraph -/** Holds if `e` is an expression that reads MMIO/DMA hardware state. */ -predicate isMmioExpr(Expr e) { - exists(VariableAccess va | va = e and va.getTarget().isVolatile()) - or - exists(FieldAccess fa | fa = e and fa.getTarget().getType().isVolatile()) - or - exists(FunctionCall call | - call = e and - call.getTarget().hasName(["READ_REG", "GET_MMIO", "REG_READ", "DMA_READ"]) - ) - or - exists(PointerDereferenceExpr deref | - deref = e and - deref.getOperand().getUnspecifiedType() instanceof PointerType and - deref.getOperand().getUnspecifiedType().(PointerType).getBaseType().isVolatile() - ) -} - +/** Holds if `source` reads MMIO/DMA state through an allowlisted register macro. */ predicate isMmioSource(DataFlow::Node source) { - isMmioExpr(source.asExpr()) - or exists(MacroInvocation mi | mi.getMacro().hasName(["READ_REG", "GET_MMIO", "REG_READ", "DMA_READ"]) and source.asExpr() = mi.getExpr() diff --git a/cpp/ql/src/Security/CWE/CWE-120/MmioUnsanitizedMemcpyBad.c b/cpp/ql/src/experimental/Security/CWE/CWE-120/MmioUnsanitizedMemcpyBad.c similarity index 100% rename from cpp/ql/src/Security/CWE/CWE-120/MmioUnsanitizedMemcpyBad.c rename to cpp/ql/src/experimental/Security/CWE/CWE-120/MmioUnsanitizedMemcpyBad.c diff --git a/cpp/ql/src/Security/CWE/CWE-120/MmioUnsanitizedMemcpyGood.c b/cpp/ql/src/experimental/Security/CWE/CWE-120/MmioUnsanitizedMemcpyGood.c similarity index 100% rename from cpp/ql/src/Security/CWE/CWE-120/MmioUnsanitizedMemcpyGood.c rename to cpp/ql/src/experimental/Security/CWE/CWE-120/MmioUnsanitizedMemcpyGood.c diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/MmioUnsanitizedMemcpy.expected b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/MmioUnsanitizedMemcpy.expected new file mode 100644 index 000000000000..39cd12d18aa4 --- /dev/null +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/MmioUnsanitizedMemcpy.expected @@ -0,0 +1,28 @@ +#select +| 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 | +| 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 | +| 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 | +| 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 | +edges +| test.c:25:18:25:37 | * ... | test.c:25:18:25:37 | * ... | provenance | | +| test.c:25:18:25:37 | * ... | test.c:26:20:26:22 | len | provenance | | +| test.c:30:18:30:37 | * ... | test.c:30:18:30:37 | * ... | provenance | | +| test.c:30:18:30:37 | * ... | test.c:31:21:31:23 | len | provenance | | +| test.c:35:18:35:37 | * ... | test.c:35:18:35:37 | * ... | provenance | | +| test.c:35:18:35:37 | * ... | test.c:36:21:36:23 | len | provenance | | +| test.c:40:18:40:37 | * ... | test.c:40:18:40:37 | * ... | provenance | | +| test.c:40:18:40:37 | * ... | test.c:41:20:41:22 | len | provenance | | +nodes +| test.c:25:18:25:37 | * ... | semmle.label | * ... | +| test.c:25:18:25:37 | * ... | semmle.label | * ... | +| test.c:26:20:26:22 | len | semmle.label | len | +| test.c:30:18:30:37 | * ... | semmle.label | * ... | +| test.c:30:18:30:37 | * ... | semmle.label | * ... | +| test.c:31:21:31:23 | len | semmle.label | len | +| test.c:35:18:35:37 | * ... | semmle.label | * ... | +| test.c:35:18:35:37 | * ... | semmle.label | * ... | +| test.c:36:21:36:23 | len | semmle.label | len | +| test.c:40:18:40:37 | * ... | semmle.label | * ... | +| test.c:40:18:40:37 | * ... | semmle.label | * ... | +| test.c:41:20:41:22 | len | semmle.label | len | +subpaths diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/MmioUnsanitizedMemcpy.qlref b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/MmioUnsanitizedMemcpy.qlref new file mode 100644 index 000000000000..671ab2524272 --- /dev/null +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/MmioUnsanitizedMemcpy.qlref @@ -0,0 +1,2 @@ +query: experimental/Security/CWE/CWE-120/MmioUnsanitizedMemcpy.ql +postprocess: utils/test/InlineExpectationsTestQuery.ql diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/test.c b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/test.c new file mode 100644 index 000000000000..863894a552f7 --- /dev/null +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/test.c @@ -0,0 +1,83 @@ +/* Semmle test case for MmioUnsanitizedMemcpy.ql + * Allowlisted MMIO/DMA register macros flowing into memcpy/memmove/strncpy size parameters. + */ + +typedef unsigned int uint32_t; + +void *memcpy(void *dest, const void *src, unsigned long n); +void *memmove(void *dest, const void *src, unsigned long n); +char *strncpy(char *dest, const char *src, unsigned long n); + +#define READ_REG(addr) (*(volatile uint32_t *)(addr)) +#define GET_MMIO(addr) (*(volatile uint32_t *)(addr)) +#define REG_READ(addr) (*(volatile uint32_t *)(addr)) +#define DMA_READ(addr) (*(volatile uint32_t *)(addr)) +#define MAX_DMA_LEN 64 + +struct VolatileField { + volatile uint32_t len; +}; + +volatile uint32_t mmio_len_reg; +struct VolatileField vf; + +static void bad_read_reg(char *dst, char *src) { + uint32_t len = READ_REG(0x40001000); // $ Source + memcpy(dst, src, len); // $ Alert +} + +static void bad_get_mmio(char *dst, char *src) { + uint32_t len = GET_MMIO(0x50000000); // $ Source + memmove(dst, src, len); // $ Alert +} + +static void bad_reg_read(char *dst, char *src) { + uint32_t len = REG_READ(0x51000000); // $ Source + strncpy(dst, src, len); // $ Alert +} + +static void bad_dma_read(char *dst, char *src) { + uint32_t len = DMA_READ(0x60000000); // $ Source + memcpy(dst, src, len); // $ Alert +} + +static void good_bounded(char *dst, char *src) { + uint32_t len = READ_REG(0x40001000); + if (len <= MAX_DMA_LEN) + memcpy(dst, src, len); // GOOD +} + +static void good_early_return(char *dst, char *src) { + uint32_t len = DMA_READ(0x60000000); + if (len > MAX_DMA_LEN) + return; + memcpy(dst, src, len); // GOOD +} + +static void good_constant_size(char *dst, char *src) { + uint32_t len = READ_REG(0x40001000); + memcpy(dst, src, 32); // GOOD — constant size, not tainted sink +} + +static void negative_volatile_global(char *dst, char *src) { + uint32_t len = mmio_len_reg; + memcpy(dst, src, len); // GOOD +} + +static void negative_volatile_field(char *dst, char *src) { + uint32_t len = vf.len; + memcpy(dst, src, len); // GOOD +} + +static void negative_volatile_deref(char *dst, char *src) { + volatile uint32_t *reg = (volatile uint32_t *)0x40001000; + uint32_t len = *reg; + memcpy(dst, src, len); // GOOD +} + +static uint32_t GET_MMIO_fn(unsigned long addr); + +static void negative_get_mmio_function(char *dst, char *src) { + uint32_t len = GET_MMIO_fn(0x50000000); + memcpy(dst, src, len); // GOOD +} diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/MmioUnsanitizedMemcpy.expected b/cpp/ql/test/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/MmioUnsanitizedMemcpy.expected deleted file mode 100644 index 78511215ff39..000000000000 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/MmioUnsanitizedMemcpy.expected +++ /dev/null @@ -1,22 +0,0 @@ -#select -| test.c:21:3:21:8 | call to memcpy | test.c:20:18:20:37 | * ... | test.c:21:20:21:22 | len | Memory copy size argument is derived from $@ without sufficient bounds validation. | test.c:20:18:20:37 | * ... | an MMIO/DMA hardware register read | -| test.c:26:3:26:9 | call to memmove | test.c:25:18:25:25 | call to GET_MMIO | test.c:26:21:26:23 | len | Memory copy size argument is derived from $@ without sufficient bounds validation. | test.c:25:18:25:25 | call to GET_MMIO | an MMIO/DMA hardware register read | -| test.c:31:3:31:9 | call to strncpy | test.c:30:18:30:29 | mmio_len_reg | test.c:31:21:31:23 | len | Memory copy size argument is derived from $@ without sufficient bounds validation. | test.c:30:18:30:29 | mmio_len_reg | an MMIO/DMA hardware register read | -edges -| test.c:20:18:20:37 | * ... | test.c:20:18:20:37 | * ... | provenance | | -| test.c:20:18:20:37 | * ... | test.c:21:20:21:22 | len | provenance | | -| test.c:25:18:25:25 | call to GET_MMIO | test.c:25:18:25:25 | call to GET_MMIO | provenance | | -| test.c:25:18:25:25 | call to GET_MMIO | test.c:26:21:26:23 | len | provenance | | -| test.c:30:18:30:29 | mmio_len_reg | test.c:30:18:30:29 | mmio_len_reg | provenance | | -| test.c:30:18:30:29 | mmio_len_reg | test.c:31:21:31:23 | len | provenance | | -nodes -| test.c:20:18:20:37 | * ... | semmle.label | * ... | -| test.c:20:18:20:37 | * ... | semmle.label | * ... | -| test.c:21:20:21:22 | len | semmle.label | len | -| test.c:25:18:25:25 | call to GET_MMIO | semmle.label | call to GET_MMIO | -| test.c:25:18:25:25 | call to GET_MMIO | semmle.label | call to GET_MMIO | -| test.c:26:21:26:23 | len | semmle.label | len | -| test.c:30:18:30:29 | mmio_len_reg | semmle.label | mmio_len_reg | -| test.c:30:18:30:29 | mmio_len_reg | semmle.label | mmio_len_reg | -| test.c:31:21:31:23 | len | semmle.label | len | -subpaths diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/MmioUnsanitizedMemcpy.qlref b/cpp/ql/test/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/MmioUnsanitizedMemcpy.qlref deleted file mode 100644 index e82093d33480..000000000000 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/MmioUnsanitizedMemcpy.qlref +++ /dev/null @@ -1,2 +0,0 @@ -query: Security/CWE/CWE-120/MmioUnsanitizedMemcpy.ql -postprocess: utils/test/InlineExpectationsTestQuery.ql diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/test.c b/cpp/ql/test/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/test.c deleted file mode 100644 index 01286d2a14b4..000000000000 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-120/MmioUnsanitizedMemcpy/test.c +++ /dev/null @@ -1,50 +0,0 @@ -/* Semmle test case for MmioUnsanitizedMemcpy.ql - * MMIO/DMA register reads flowing into memcpy/memmove/strncpy size parameters. - */ - -typedef unsigned int uint32_t; - -void *memcpy(void *dest, const void *src, unsigned long n); -void *memmove(void *dest, const void *src, unsigned long n); -char *strncpy(char *dest, const char *src, unsigned long n); - -#define READ_REG(addr) (*(volatile uint32_t *)(addr)) -#define MAX_DMA_LEN 64 - -uint32_t GET_MMIO(unsigned long addr); -uint32_t DMA_READ(unsigned long addr); - -volatile uint32_t mmio_len_reg; - -static void bad_read_reg(char *dst, char *src) { - uint32_t len = READ_REG(0x40001000); // $ Source - memcpy(dst, src, len); // $ Alert -} - -static void bad_get_mmio(char *dst, char *src) { - uint32_t len = GET_MMIO(0x50000000); // $ Source - memmove(dst, src, len); // $ Alert -} - -static void bad_volatile_global(char *dst, char *src) { - uint32_t len = mmio_len_reg; // $ Source - strncpy(dst, src, len); // $ Alert -} - -static void good_bounded(char *dst, char *src) { - uint32_t len = READ_REG(0x40001000); - if (len <= MAX_DMA_LEN) - memcpy(dst, src, len); // GOOD -} - -static void good_early_return(char *dst, char *src) { - uint32_t len = DMA_READ(0x60000000); - if (len > MAX_DMA_LEN) - return; - memcpy(dst, src, len); // GOOD -} - -static void good_constant_size(char *dst, char *src) { - uint32_t len = READ_REG(0x40001000); - memcpy(dst, src, 32); // GOOD — constant size, not tainted sink -} From 5c92f5e86c7917b2ab0b60471bc00c48ff69504e Mon Sep 17 00:00:00 2001 From: Tarek Date: Wed, 2 Sep 2026 20:51:39 -0400 Subject: [PATCH 4/4] cpp: Drop experimental query id prefix and change note --- .gitignore | 3 --- cpp/ql/src/change-notes/2026-08-31-mmio-unsanitized-memcpy.md | 4 ---- .../Security/CWE/CWE-120/MmioUnsanitizedMemcpy.ql | 3 +-- 3 files changed, 1 insertion(+), 9 deletions(-) delete mode 100644 cpp/ql/src/change-notes/2026-08-31-mmio-unsanitized-memcpy.md diff --git a/.gitignore b/.gitignore index d6b0edfe2516..4dbe45b8d28a 100644 --- a/.gitignore +++ b/.gitignore @@ -79,6 +79,3 @@ node_modules/ # Mergetool files *.orig - -# Local CodeQL harness database cache (veraptos TP/TN validation) -codeql_harness_dbs/ diff --git a/cpp/ql/src/change-notes/2026-08-31-mmio-unsanitized-memcpy.md b/cpp/ql/src/change-notes/2026-08-31-mmio-unsanitized-memcpy.md deleted file mode 100644 index 3a96eb5d6a60..000000000000 --- a/cpp/ql/src/change-notes/2026-08-31-mmio-unsanitized-memcpy.md +++ /dev/null @@ -1,4 +0,0 @@ ---- -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. diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-120/MmioUnsanitizedMemcpy.ql b/cpp/ql/src/experimental/Security/CWE/CWE-120/MmioUnsanitizedMemcpy.ql index 17f2a339e553..d48796c62338 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-120/MmioUnsanitizedMemcpy.ql +++ b/cpp/ql/src/experimental/Security/CWE/CWE-120/MmioUnsanitizedMemcpy.ql @@ -4,9 +4,8 @@ * macros without bounds validation may overflow destination buffers. * @kind path-problem * @problem.severity error - * @security-severity 8.6 * @precision low - * @id cpp/experimental/mmio-unsanitized-memcpy + * @id cpp/mmio-unsanitized-memcpy * @tags security * experimental * external/cwe/cwe-120