How heap buffer overflow happens in C UART response handling and how to fix it

How Heap Buffer Overflow Happens in C UART Response Handling and How to Fix It

Introduction

The lib/sm_at_client/sm_at_client.c file handles AT command responses received over UART from a cellular modem, but a flaw in the response_handler function at line 313 created a critical security risk. The function uses memcpy to copy incoming data into the at_cmd_resp buffer at offset resp_len, but never verifies that resp_len + copy_length stays within the buffer's allocated size (CONFIG_SM_AT_CLIENT_AT_CMD_RESP_MAX_SIZE).

This is a textbook CWE-120 vulnerability — a buffer copy without checking the size of input — but what makes it particularly dangerous is the attack surface: any entity that can inject data onto the UART line (a compromised modem, a hardware implant, or a man-in-the-middle on the serial bus) can trigger heap corruption on the host MCU.

The Vulnerability Explained

The AT client processes modem responses in a streaming fashion. When data arrives over UART, the response_handler function accumulates bytes in the static at_cmd_resp buffer. The vulnerable code path occurs when a response contains a terminator (like \r\n) partway through the received data — the function processes the first portion, then copies the remaining bytes into the buffer for the next response:

/* Copy the possibly remaining data to the buffer. */
if (copy_len < len) {
    assert((sizeof(at_cmd_resp) - resp_len) >= (len - copy_len));

    memcpy(at_cmd_resp + resp_len, data + copy_len, len - copy_len);
    resp_len += len - copy_len;
}

The critical problems:

1. assert() is not a security control. In production builds compiled with NDEBUG, assertions are stripped entirely. The assert on line 310 provides zero runtime protection in release firmware.

2. No bounds clamping. Even if the assertion fires in debug builds, it aborts the program rather than gracefully handling the overflow. There's no code path that limits len - copy_len to the available buffer space.

3. Attacker-controlled input. The data and len parameters come directly from UART receive callbacks. A compromised cellular modem — or an attacker with physical access to the UART lines — controls both the content and length of this data.

Attack scenario:

An attacker sends a crafted AT response where the total payload exceeds CONFIG_SM_AT_CLIENT_AT_CMD_RESP_MAX_SIZE (256 bytes). For example:

1. The attacker sends a legitimate-looking response prefix that fills most of at_cmd_resp (e.g., 250 bytes).
2. In the same UART transfer, after a \r\n terminator, the attacker includes 200+ bytes of "remaining" data.
3. The code calculates len - copy_len = 200, but only 256 - 0 = 256 bytes (or less) are available.
4. memcpy writes past the heap buffer boundary, corrupting adjacent heap metadata or application data.

This 2-step exploitation chain (fill buffer + overflow on remainder) can achieve arbitrary write primitives on embedded systems with predictable heap layouts.

The Fix

The fix replaces the dangerous assert-then-copy pattern with proper bounds clamping:

Before (vulnerable):

/* Copy the possibly remaining data to the buffer. */
if (copy_len < len) {
    assert((sizeof(at_cmd_resp) - resp_len) >= (len - copy_len));

    memcpy(at_cmd_resp + resp_len, data + copy_len, len - copy_len);
    resp_len += len - copy_len;
}

After (fixed):

/* Copy the possibly remaining data to the buffer. */
if (copy_len < len) {
    size_t remaining = sizeof(at_cmd_resp) - resp_len;
    size_t copy2_len = MIN(len - copy_len, remaining);

    memcpy(at_cmd_resp + resp_len, data + copy_len, copy2_len);
    resp_len += copy2_len;
}

What changed and why:

1. size_t remaining = sizeof(at_cmd_resp) - resp_len; — Explicitly calculates how many bytes the buffer can still accept. This makes the available capacity a first-class value in the logic.

2. size_t copy2_len = MIN(len - copy_len, remaining); — Clamps the actual copy length to the lesser of "data available" and "space available." This is the core security invariant: buffer writes never exceed the declared length.

3. Removed assert() — The assertion provided no protection in production. The MIN() macro provides deterministic, always-active protection regardless of build configuration.

4. Uses copy2_len in both memcpy and resp_len update — Ensures the buffer position tracker stays consistent with what was actually written.

This is a minimal, surgical fix: 4 lines changed, no behavioral change for valid inputs, and complete protection against oversized inputs.

Prevention & Best Practices

For embedded C developers working with UART/serial data:

1. Never trust peripheral input lengths. Data arriving from UART, SPI, I2C, or any external interface must be treated as attacker-controlled. Always validate lengths before copying.

2. Don't use assert() for security checks. Assertions are debugging aids, not security controls. They're compiled out in release builds. Use explicit if checks with proper error handling.

3. Adopt the MIN() clamping pattern. When copying variable-length data into fixed buffers:
   c size_t safe_len = MIN(input_len, sizeof(dest_buf) - current_offset); memcpy(dest_buf + current_offset, src, safe_len);

4. Audit all memcpy calls with external data. Search your codebase for memcpy where the length parameter derives from external input without a preceding bounds check.

5. Use compiler sanitizers in CI. AddressSanitizer (-fsanitize=address) catches heap overflows at runtime during testing, even when assertions are disabled.

6. Consider safe buffer abstractions. Libraries like Zephyr's net_buf or custom ring buffers with built-in capacity tracking can eliminate entire classes of overflow bugs.

Key Takeaways

• assert() is not a security boundary — the original code at line 310 of sm_at_client.c relied on an assertion that vanishes in production builds, leaving the memcpy completely unguarded.
• UART response handlers are a real attack surface — a compromised modem can send arbitrarily long responses, making the AT client's response_handler a 2-step exploitation target.
• The MIN(data_to_copy, space_available) pattern before memcpy is the canonical fix for CWE-120 in streaming data handlers.
• The same vulnerable pattern existed at lines 285, 305, and 313 — when one unchecked memcpy is found, always audit nearby code for the same pattern.
• Truncation is safer than corruption — dropping excess bytes (as the fix does) may cause a protocol error, but heap corruption can cause arbitrary code execution.

How Orbis AppSec Detected This

• Source: UART receive callback providing data and len parameters from external modem hardware
• Sink: memcpy(at_cmd_resp + resp_len, data + copy_len, len - copy_len) in lib/sm_at_client/sm_at_client.c:313
• Missing control: No validation that len - copy_len does not exceed sizeof(at_cmd_resp) - resp_len; the existing assert() is stripped in production builds
• CWE: CWE-120 (Buffer Copy without Checking Size of Input)
• Fix: Replace assertion with MIN() clamping to ensure copy length never exceeds remaining buffer capacity

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 vulnerability demonstrates a pattern that's alarmingly common in embedded C code: using assert() as the sole guard against buffer overflow, then shipping production firmware with assertions disabled. The fix is elegant in its simplicity — four lines that enforce the invariant "buffer writes never exceed declared length" — but the security impact is profound. For any developer working with streaming data over hardware interfaces, the lesson is clear: validate every copy length against available capacity, every time, with code that survives all build configurations.

References

• CWE-120: Buffer Copy without Checking Size of Input
• OWASP Buffer Overflow Prevention
• SEI CERT C Coding Standard - ARR38-C: Guarantee that library functions do not form invalid pointers
• Semgrep rules for buffer overflow
• fix: add buffer-length check in sm_at_client.c