How buffer overflow in memcpy() happens in C/C++ embedded firmware and how to fix it

How buffer overflow in memcpy() happens in C/C++ embedded firmware and how to fix it

Introduction

In the micro-journal ESP32 firmware repository, we discovered a critical buffer overflow in micro-journal-rev-4-esp32/lib/FatFSUSB/FatFSUSB.cpp at line 176. The tud_msc_inquiry_cb() callback function—responsible for responding to USB Mass Storage Class INQUIRY commands—copies vendor, product, and revision strings into fixed-size buffers using memcpy() with strlen() as the size parameter, without any validation that the source string actually fits within the destination.

While the current hardcoded strings ("ESP32S3", "Mass Storage", "1.0") happen to fit their respective buffers, this code pattern is inherently unsafe. If these strings are ever changed, derived from configuration, or if the callback receives externally-influenced data, the result is immediate memory corruption. More critically, in a USB host scenario, a malicious device presenting oversized descriptor strings could exploit this exact pattern to overflow the buffers and corrupt adjacent memory on the ESP32.

The Vulnerability Explained

The vulnerability exists in the tud_msc_inquiry_cb function, which is a TinyUSB callback invoked when a USB host (or device in host mode) sends an INQUIRY command. The function signature provides three fixed-size output buffers:

extern "C" void tud_msc_inquiry_cb(uint8_t lun, uint8_t vendor_id[8], uint8_t product_id[16], uint8_t product_rev[4])

The buffer sizes are defined by the SCSI specification:
- vendor_id: 8 bytes
- product_id: 16 bytes
- product_rev: 4 bytes

The vulnerable code at lines 176-178 was:

const char vid[] = "ESP32S3";
const char pid[] = "Mass Storage";
const char rev[] = "1.0";

memcpy(vendor_id, vid, strlen(vid));
memcpy(product_id, pid, strlen(pid));
memcpy(product_rev, rev, strlen(rev));

Why is this dangerous?

The strlen() function returns the length of the source string with zero awareness of the destination buffer's capacity. This creates a classic CWE-120 condition where the copy operation is bounded only by the source, not the destination.

Concrete attack scenario: Consider a firmware modification or configuration change where vid becomes a longer string—say "Espressif Systems" (17 characters). The memcpy(vendor_id, vid, strlen(vid)) call would write 17 bytes into an 8-byte buffer, overwriting 9 bytes of whatever sits adjacent in memory. On an ESP32, this could corrupt:

• The stack frame's return address (enabling code execution)
• Adjacent local variables (altering program logic)
• The product_id or product_rev buffers themselves (since they're likely contiguous in the stack frame)

Physical attack vector: An attacker with physical access could connect a malicious USB device that triggers this callback path. If the descriptor strings are ever sourced from the connected device's data (a common pattern in USB host implementations), oversized strings would directly overflow these buffers.

The PR notes also identify similar vulnerable patterns at lines 133, 249, and 259 in the same file, suggesting this is a systemic coding pattern rather than an isolated mistake.

The Fix

The fix is elegant and minimal—replacing unbounded strlen() with strnlen() that enforces the destination buffer sizes:

Before (vulnerable):

memcpy(vendor_id, vid, strlen(vid));
memcpy(product_id, pid, strlen(pid));
memcpy(product_rev, rev, strlen(rev));

After (fixed):

memcpy(vendor_id, vid, strnlen(vid, 8));
memcpy(product_id, pid, strnlen(pid, 16));
memcpy(product_rev, rev, strnlen(rev, 4));

How strnlen() solves the problem

The strnlen(s, maxlen) function returns the length of string s, but never more than maxlen. This means:

• strnlen(vid, 8) → returns min(strlen(vid), 8) → copy is capped at 8 bytes
• strnlen(pid, 16) → returns min(strlen(pid), 16) → copy is capped at 16 bytes
• strnlen(rev, 4) → returns min(strlen(rev), 4) → copy is capped at 4 bytes

No matter how long the source strings become—whether through code changes, configuration, or external input—the copy operation can never write beyond the destination buffer's boundaries. The SCSI INQUIRY response buffers are space-padded by convention, so truncation is acceptable and expected behavior.

Regression test

The PR also includes a comprehensive Google Test suite (tests/test_invariant_FatFSUSB.cpp) that validates the security invariant with adversarial inputs:

INSTANTIATE_TEST_SUITE_P(
    AdversarialInputs,
    UsbDescriptorSecurityTest,
    ::testing::Values(
        std::string(256, 'A'),   // Far exceeds all buffers
        std::string(17, 'B'),    // One byte over product_id (16)
        std::string(8, 'C'),     // Exactly at vendor_id limit
        std::string("VID")       // Valid: well within limits
    )
);

This test uses canary values around the buffers to detect any overflow, ensuring the fix holds under boundary conditions.

Prevention & Best Practices

1. Never use strlen() as the sole size parameter for memcpy() when copying into fixed-size buffers. Always use strnlen() or compare against the destination size first.

2. Prefer strncpy() or snprintf() for string copies where null-termination is needed. For SCSI descriptor buffers (which are space-padded, not null-terminated), memcpy with strnlen is appropriate.

3. Use compiler warnings: GCC's -Wstringop-overflow and Clang's -Wbuffer-overflow can catch some of these patterns at compile time.

4. Static analysis in CI: Tools like Coverity, PVS-Studio, or even Semgrep with custom rules can flag memcpy(dst, src, strlen(src)) patterns where dst has a known fixed size.

5. Embedded-specific considerations: On ESP32 and similar microcontrollers, ASLR and stack canaries may not be available, making buffer overflows even more reliably exploitable.

6. Code review checklist: Any memcpy call should have a comment or assertion documenting that the size parameter is bounded by the destination capacity.

Key Takeaways

• memcpy(buf, str, strlen(str)) into a fixed-size buffer is always a latent vulnerability—even when current string values fit, future changes or external input can trigger overflow.
• The tud_msc_inquiry_cb callback's buffer sizes (8, 16, 4) are defined by the SCSI spec, not by the firmware—so the firmware must enforce these limits regardless of source string content.
• strnlen(s, max) is the minimal-change fix that preserves the existing code structure while adding bounds enforcement.
• Physical attack vectors on embedded devices are real—a malicious USB device with oversized descriptors could exploit this pattern if the firmware ever processes device-supplied strings through this path.
• Similar patterns at lines 133, 249, and 259 in the same file indicate systematic review is needed when one instance of an unsafe pattern is found.

How Orbis AppSec Detected This

• Source: USB descriptor string data (the vid, pid, and rev character arrays) that could be influenced by configuration changes or external USB device data
• Sink: memcpy(vendor_id, vid, strlen(vid)) at FatFSUSB.cpp:176 where the copy size is unbounded relative to the 8-byte destination buffer
• Missing control: No validation that strlen() of the source string is less than or equal to the destination buffer's fixed capacity (8, 16, or 4 bytes)
• CWE: CWE-120 (Buffer Copy without Checking Size of Input)
• Fix: Replaced strlen() with strnlen() bounded to each destination buffer's size (8, 16, and 4 bytes respectively)

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

This buffer overflow in FatFSUSB.cpp demonstrates how a seemingly harmless code pattern—memcpy with strlen—creates a critical vulnerability in embedded firmware. The fix is straightforward: use strnlen() to enforce destination buffer limits. For embedded developers working with USB stacks, SCSI callbacks, or any fixed-size protocol buffers, always ensure your copy operations are bounded by the destination size, not the source size. Memory safety in embedded systems requires extra vigilance because the hardware-level protections available on desktop systems (ASLR, stack canaries, guard pages) are often absent.

References

• CWE-120: Buffer Copy without Checking Size of Input
• OWASP Buffer Overflow
• strnlen() man page - Linux Programmer's Manual
• Semgrep rules for buffer overflow
• fix: add bounds check before memcpy in FatFSUSB.cpp