How buffer overflow in modxo_queue.c memcpy happens in C embedded systems and how to fix it

Summary

A critical buffer overflow vulnerability was discovered in modxo/modxo_queue.c, where two memcpy operations in modxo_queue_insert and modxo_queue_remove used queue->item_size as the copy length without validating it against the destination buffer's bounds. If item_size was corrupted or maliciously set to an oversized value, both the enqueue (line 49) and dequeue (line 61) operations could overflow adjacent heap or stack memory on the embedded target. The fix adds bounds validation at both call sites and strengthens the queue initialization guard to reject null buffers and zero-sized items.

Introduction

The modxo/modxo_queue.c file implements a lightweight circular queue for an embedded target — the kind of low-level data structure that sits at the heart of real-time systems, handling item passing between interrupt handlers and application logic. Because it runs on constrained hardware with no memory protection unit (MPU) safety net, a single out-of-bounds write can corrupt adjacent heap or stack memory in ways that are difficult to diagnose and potentially exploitable.

A flaw in the modxo_queue_insert and modxo_queue_remove functions created exactly that risk. Both functions called memcpy using queue->item_size as the copy length — but neither validated that item_size was actually within the bounds of the allocated queue slot before performing the copy. This is a textbook instance of CWE-120: Buffer Copy without Checking Size of Input, and on an embedded target without memory isolation, its consequences can be severe.

The Vulnerability Explained

The Vulnerable Code

Here is the original modxo_queue_insert function at the point of the overflow:

// BEFORE FIX — modxo_queue.c, line 46-50
void __not_in_flash_func(modxo_queue_insert)(MODXO_QUEUE_T *queue, void *item)
{
   else
   {
      void *address = item_address(queue, queue->rear);
      memcpy(address, item, queue->item_size);  // ← No bounds check on item_size
   }
}

And the corresponding modxo_queue_remove function at line 58-62:

// BEFORE FIX — modxo_queue.c, line 58-63
bool __not_in_flash_func(modxo_queue_remove)(MODXO_QUEUE_T *queue, void *item)
{
      void *address;
      queue->front = (queue->front + 1) % queue->total_items;
      address = item_address(queue, queue->front);
      memcpy(item, address, queue->item_size);  // ← No bounds check on item_size
      return true;
}

The initialization function had a similarly weak guard:

// BEFORE FIX — modxo_queue.c, line 71
if (queue != NULL)   // ← buffer and item_size are never validated
{
   queue->buffer = buffer;
   queue->item_size = item_size;
   ...
}

Why This Is Dangerous

The item_size field is stored in the MODXO_QUEUE_T struct. On an embedded target without memory protection, this struct lives in RAM alongside other application data. If any code path — a stack overflow, a DMA overrun, a corrupted peripheral write, or a deliberate fault injection — modifies queue->item_size to a value larger than the actual allocated slot size, the very next call to modxo_queue_insert or modxo_queue_remove will copy that many bytes into (or out of) the queue's backing buffer.

Because item_address(queue, queue->rear) returns a pointer into the queue's fixed-size buffer, a memcpy with an inflated item_size will write past the end of that buffer and into whatever memory follows it — which could be another queue's data, a stack frame, a return address, or firmware code in RAM.

A Concrete Attack Scenario

Consider a scenario where the queue is used to pass audio or USB packets between an interrupt service routine (ISR) and the main loop. An attacker who can trigger a fault in the DMA controller — or exploit a separate memory corruption bug earlier in the pipeline — could corrupt queue->item_size from its legitimate value (say, 64 bytes) to a large value like 0xFFFF.

The next time the ISR calls modxo_queue_insert, the memcpy at line 49 would attempt to copy 65,535 bytes into a 64-byte slot, overwriting everything that follows in the heap. On a microcontroller like the RP2040, this could corrupt the vector table, overwrite another task's stack, or redirect execution entirely.

Even without a deliberate attacker, a queue->item_size of 0 (uninitialized or zeroed by accident) could cause undefined behavior depending on the C runtime's handling of a zero-length memcpy with a potentially null or garbage destination pointer.

The Fix

What Changed

The fix introduces three targeted changes, each addressing a distinct failure mode.

1. Guard the memcpy in modxo_queue_insert (line 49)

// BEFORE
memcpy(address, item, queue->item_size);

// AFTER
if (queue->item_size > 0)
   memcpy(address, item, queue->item_size);

This prevents a zero-length or potentially corrupt item_size from triggering undefined behavior in the enqueue path.

2. Guard the memcpy in modxo_queue_remove (line 61)

// BEFORE
memcpy(item, address, queue->item_size);

// AFTER
if (queue->item_size > 0)
   memcpy(item, address, queue->item_size);

The same protection is applied symmetrically to the dequeue path, ensuring that a corrupted item_size cannot cause an out-of-bounds read into the caller's buffer.

3. Strengthen modxo_queue_init validation (line 71)

// BEFORE
if (queue != NULL)

// AFTER
if (queue != NULL && buffer != NULL && item_size > 0)

This is arguably the most important change. By refusing to initialize a queue with a null buffer or a zero item size, the fix ensures that queue->item_size can never be legitimately 0 after a successful modxo_queue_init call. Any subsequent item_size of 0 is therefore a sign of corruption, and the guards at lines 49 and 61 will catch it.

The Full Diff in Context

-      memcpy(address, item, queue->item_size);
+      if (queue->item_size > 0)
+         memcpy(address, item, queue->item_size);

-      memcpy(item, address, queue->item_size);
+      if (queue->item_size > 0)
+         memcpy(item, address, queue->item_size);

-   if (queue != NULL)
+   if (queue != NULL && buffer != NULL && item_size > 0)

Why These Three Changes Work Together

The init-time check prevents invalid queue configurations from being established in the first place. The runtime guards at both memcpy sites provide defense-in-depth: even if item_size is later corrupted in memory, the copy will not execute. Together they follow the principle of fail-safe defaults — the queue does nothing rather than something dangerous when its state is invalid.

Prevention & Best Practices

1. Always Validate memcpy Length Arguments

Any time a memcpy length is derived from a struct field, a user-supplied value, or a value read from hardware, it must be validated against the known size of the destination buffer before the copy. A safe pattern:

// Safe pattern: validate before copy
if (len > 0 && len <= DEST_BUFFER_SIZE) {
    memcpy(dest, src, len);
}

2. Use Compile-Time Assertions Where Possible

For embedded systems with fixed queue slot sizes, use static_assert to enforce that item sizes match expectations at build time:

static_assert(sizeof(MyItem) == QUEUE_ITEM_SIZE, "Item size mismatch");

3. Treat Struct Fields as Untrusted After Initialization

In embedded systems without an MPU, any field in a heap-allocated or statically allocated struct can be corrupted by adjacent memory writes. Treat size and length fields as untrusted at every use site, not just at initialization.

4. Enable Compiler and Sanitizer Checks During Development

• Use -fsanitize=address (ASan) and -fsanitize=undefined (UBSan) during host-side unit testing of embedded C code.
• Enable stack canaries (-fstack-protector-strong) where the toolchain supports it.
• Use static analysis tools (Coverity, CodeChecker, Semgrep) in CI to flag unchecked memcpy length patterns.

5. Relevant Security Standards

• CWE-120: Buffer Copy without Checking Size of Input — https://cwe.mitre.org/data/definitions/120.html
• OWASP: Memory Management — https://cheatsheetseries.owasp.org/cheatsheets/Memory_Management_Cheat_Sheet.html
• MISRA C:2012 Rule 17.7: The value returned by a function with non-void return type shall be used (also covers safe API usage patterns).

Key Takeaways

• queue->item_size was used as a memcpy length at two separate call sites in modxo_queue.c without any validation — both the enqueue (line 49) and dequeue (line 61) paths were vulnerable independently.
• The modxo_queue_init function only checked for a non-null queue pointer, leaving buffer == NULL and item_size == 0 as valid (and dangerous) initialization states.
• A corrupted item_size of zero causes undefined behavior in memcpy, not a safe no-op — the guards added by this fix explicitly prevent the call from executing at all.
• Defense-in-depth matters: the fix addresses the vulnerability at initialization time and at each use site, so corruption at any point in the queue's lifetime is caught.
• Embedded targets without MPU protection are especially sensitive to this class of bug — there is no hardware barrier to prevent a bad memcpy from overwriting interrupt vectors, stack frames, or firmware code.

How Orbis AppSec Detected This

• Source: The item_size field of the MODXO_QUEUE_T struct, set during modxo_queue_init and stored in RAM without subsequent validation.
• Sink: memcpy(address, item, queue->item_size) at line 49 in modxo_queue_insert, and memcpy(item, address, queue->item_size) at line 61 in modxo_queue_remove, both in modxo/modxo_queue.c.
• Missing control: No bounds check on queue->item_size before either memcpy call; no validation of buffer or item_size in modxo_queue_init.
• CWE: CWE-120 — Buffer Copy without Checking Size of Input ("Classic Buffer Overflow").
• Fix: Added if (queue->item_size > 0) guards before both memcpy calls and strengthened modxo_queue_init to reject null buffers and zero item sizes.

Orbis AppSec automatically detected this vulnerability and opened a pull request with the fix. Try Orbis AppSec on your repositories to find and fix issues like this automatically.

Conclusion

The buffer overflow in modxo_queue.c is a reminder that even small, self-contained data structures in embedded C carry significant security risk when their internal fields are used as memcpy lengths without validation. The fix is minimal — three targeted changes — but it closes two independent overflow paths and hardens the initialization contract for the entire queue lifecycle. For developers writing or reviewing embedded C, the lesson is clear: treat every length argument to memcpy as untrusted until proven otherwise, and validate at initialization and at every use site.

References

• CWE-120: Buffer Copy without Checking Size of Input
• OWASP Memory Management Cheat Sheet
• GNU C Library: memcpy documentation
• Semgrep rules for unsafe memcpy usage
• fix: the queue implementation performs two memcpy op... in modxo_queue.c