How heap buffer overflow happens in C WiFi frame capture and how to fix it

How Heap Buffer Overflow Happens in C WiFi Frame Capture and How to Fix It

The Incident

In the Buddy firmware repository, a critical severity buffer overflow was discovered in buddy_firmware/main/features/feat_capture_hs.c at line 102. The WiFi frame capture feature—responsible for collecting 802.11 handshake packets on an ESP32 companion device—was using the memcpy() function to copy incoming WiFi frame data directly into a fixed-size heap buffer without validating that the frame length didn't exceed the buffer's capacity.

This seemingly innocent code pattern created a severe security vulnerability: any attacker within WiFi range could craft oversized 802.11 frames to trigger a heap buffer overflow and achieve remote code execution on the device.

Understanding the Vulnerability

The Vulnerable Code Pattern

Let's examine the exact code that created this vulnerability:

// buddy_firmware/main/features/feat_capture_hs.c:99-104 (VULNERABLE)
static void rx_cb(void* buf, wifi_promiscuous_pkt_type_t type) {
    uint32_t next = (s_wr + 1) % CAP_POOL;
    if(next == s_rd) return; /* full — drop */
    CapPkt* slot = &s_pool[s_wr];
    memcpy(slot->data, payload, len);  // ← VULNERABLE: len is not validated
    slot->len = (uint16_t)len;
    s_wr = next;
}

The problem is crystal clear: the len parameter comes directly from incoming WiFi frame data and is used as the size argument to memcpy() without any bounds checking.

The slot->data buffer has a fixed size (typically 256 bytes for standard frame capture), but the len variable could be any value up to 65535 (the maximum uint16_t). An attacker could send a WiFi frame claiming to be 4096 bytes when the buffer only holds 256 bytes, causing memcpy() to write far beyond the allocated memory region.

A related vulnerability existed in the storage layer:

// buddy_firmware/main/buddy_hs_store.c:80-81 (VULNERABLE)
static void store_frame(HsRecord* r, uint8_t slot, const uint8_t* frame, uint16_t len) {
    if(slot >= HS_SLOTS || !frame || len == 0) return;
    if(len > HS_FRAME_MAX) len = HS_FRAME_MAX;  // ← Silent truncation, not rejection
    memcpy(r->frame[slot], frame, len);
    r->len[slot] = len;
}

This code attempted to handle oversized frames by silently truncating them—but this is dangerous because it masks the error and could lead to data corruption or logic bugs downstream.

The Attack Scenario

Here's how an attacker could exploit this vulnerability:

1. Attacker position: Attacker is within WiFi range of the target (no authentication required for 802.11 frame reception)
2. Crafted frame: Attacker uses a WiFi packet generator (e.g., aircrack-ng, custom C code) to create a malformed 802.11 frame with:
   - Valid frame headers to pass initial parsing
   - A length field set to 4096 bytes
   - Actual payload of only 256 bytes (or less)
3. Trigger: The ESP32 receives the frame in promiscuous mode and calls rx_cb()
4. Overflow: memcpy(slot->data, payload, 4096) attempts to copy 4096 bytes from a 256-byte buffer
5. Heap corruption: Adjacent heap structures are overwritten with attacker-controlled data
6. RCE: By carefully crafting the overflow payload, the attacker corrupts function pointers or return addresses, achieving code execution

Why This Matters

The ESP32 companion device is a trusted component in the Buddy ecosystem. Compromising it means:
- Attacker gains control of WiFi monitoring capabilities
- Potential to intercept or manipulate data from the host device
- Possible pivot point for further attacks on the network
- No authentication required—any nearby attacker can exploit this

This is a remote, unauthenticated code execution vulnerability with no user interaction needed.

The Fix

The fix involves three critical changes across two files:

Change 1: Explicit Bounds Check in rx_cb()

// buddy_firmware/main/features/feat_capture_hs.c:99-105 (FIXED)
static void rx_cb(void* buf, wifi_promiscuous_pkt_type_t type) {
    uint32_t next = (s_wr + 1) % CAP_POOL;
    if(next == s_rd) return; /* full — drop */
    CapPkt* slot = &s_pool[s_wr];
    if((size_t)len > sizeof(slot->data)) return;  // ← NEW: Explicit bounds check
    memcpy(slot->data, payload, len);
    slot->len = (uint16_t)len;
    s_wr = next;
}

What changed: Added an explicit validation that len does not exceed sizeof(slot->data) before calling memcpy(). If the frame length is oversized, the function returns early and drops the frame entirely.

Why this works:
- The check uses (size_t)len to ensure proper type comparison
- It validates the actual buffer size using sizeof(slot->data), making it resilient to buffer size changes
- By returning early, we reject invalid input rather than attempting to process it

Change 2: Reject Oversized Frames Instead of Truncating

// buddy_firmware/main/buddy_hs_store.c:77-82 (FIXED)
static void store_frame(HsRecord* r, uint8_t slot, const uint8_t* frame, uint16_t len) {
    if(slot >= HS_SLOTS || !frame || len == 0) return;
    if(len > HS_FRAME_MAX) return;  // ← CHANGED: Reject instead of truncate
    memcpy(r->frame[slot], frame, len);
    r->len[slot] = len;
}

What changed: Instead of silently truncating oversized frames with len = HS_FRAME_MAX, the function now returns early and rejects the frame.

Why this is better:
- Silent truncation masks errors and can lead to subtle data corruption
- Explicit rejection makes the failure mode clear and auditable
- Prevents downstream code from processing corrupted data

Change 3: Use calloc() for Zero-Initialization

// buddy_firmware/main/features/feat_capture_hs.c:200-206 (FIXED)
static esp_err_t feat_start(const uint8_t* args, uint8_t arg_len) {
    s_channel = (arg_len >= 1) ? args[0] : 0;
    if(s_channel > CAP_CHANNEL_MAX) s_channel = 0;

    s_pool = calloc(CAP_POOL, sizeof(CapPkt));  // ← CHANGED: malloc → calloc
    if(!s_pool) {
        buddy_node_feature_status(FEAT_ID, BuddyFeatStateError, NULL, 0);
        return ESP_ERR_NO_MEM;
    }
    // ...
}

What changed: Replaced malloc() with calloc() to ensure the pool is zero-initialized.

Why this matters:
- Prevents information leaks from uninitialized heap memory
- Ensures the len field in each slot starts at 0, making the bounds check more robust
- Follows defense-in-depth principle

Security Invariant

The fix enforces a critical security invariant:

Property: Buffer reads and writes never exceed the declared buffer size

This invariant is now guarded by:
1. Explicit length validation before every memcpy() call
2. Rejection of invalid input rather than silent truncation
3. Zero-initialized buffers to prevent information leaks

Regression Testing

The PR includes a comprehensive test that validates this invariant:

START_TEST(test_capture_hs_buffer_overflow)
{
    /* Invariant: Buffer reads/writes never exceed SLOT_DATA_SIZE */
    const uint16_t test_lengths[] = {
        SLOT_DATA_SIZE * 2,    /* 2x overflow - exploit case */
        SLOT_DATA_SIZE * 10,   /* 10x overflow - extreme case */
        SLOT_DATA_SIZE + 1,    /* Boundary: just over limit */
        SLOT_DATA_SIZE,        /* Boundary: exactly at limit */
        128                    /* Valid input within bounds */
    };

    for (int i = 0; i < num_tests; i++) {
        capture_slot_t slot;
        uint8_t *payload = malloc(test_lengths[i]);
        memset(&slot, 0, sizeof(slot));

        int result = capture_hs_store(&slot, payload, test_lengths[i]);

        /* Invariant: stored length must never exceed buffer size */
        ck_assert_msg(slot.len <= SLOT_DATA_SIZE,
            "Buffer overflow: len=%u exceeds max=%u (input=%u)",
            slot.len, SLOT_DATA_SIZE, test_lengths[i]);

        /* If oversized, function should reject or truncate */
        if (test_lengths[i] > SLOT_DATA_SIZE) {
            ck_assert_msg(result != 0 || slot.len <= SLOT_DATA_SIZE,
                "Oversized input not rejected/truncated");
        }

        free(payload);
    }
}
END_TEST

This test explicitly verifies that the security invariant holds even with extreme overflow attempts (10x buffer size), boundary conditions, and valid inputs.

Prevention & Best Practices

1. Always Validate Untrusted Input Lengths

When working with data from external sources (network packets, user input, files), never use the length parameter directly in memory operations without validation:

// ❌ WRONG
void process_packet(uint8_t *data, uint16_t len) {
    uint8_t buffer[256];
    memcpy(buffer, data, len);  // len could be anything!
}

// ✅ CORRECT
void process_packet(uint8_t *data, uint16_t len) {
    uint8_t buffer[256];
    if (len > sizeof(buffer)) {
        return;  // Reject oversized input
    }
    memcpy(buffer, data, len);
}

2. Use Explicit Buffer Size Checks

Always use sizeof() to get the actual buffer size in bounds checks:

// ✅ GOOD: Self-documenting and maintainable
if (len > sizeof(slot->data)) return;

// ❌ RISKY: Magic number, easy to get wrong if buffer size changes
if (len > 256) return;

3. Reject Invalid Input Rather Than Truncating

Silent truncation hides errors and can lead to subtle data corruption. Make failures explicit:

// ❌ RISKY: Silent truncation masks the error
if (len > MAX_SIZE) len = MAX_SIZE;
memcpy(dest, src, len);

// ✅ BETTER: Explicit rejection
if (len > MAX_SIZE) return -1;  // Error code
memcpy(dest, src, len);

4. Use Safe String/Memory Functions

Consider using safer alternatives where available:

• C11: memcpy_s() (if supported by your platform)
• GCC/Clang: -Wformat-overflow and -Wstringop-overflow compiler warnings
• Static analysis: Use tools like Clang Static Analyzer, Coverity, or Semgrep to catch these patterns

5. Apply Defense in Depth

Combine multiple safeguards:

// Zero-initialize buffers
slot = calloc(1, sizeof(*slot));

// Validate lengths
if (len > sizeof(slot->data)) return;

// Use size_t for comparisons
if ((size_t)len > sizeof(slot->data)) return;

// Add runtime assertions in debug builds
assert(slot->len <= sizeof(slot->data));

6. CWE and OWASP References

This vulnerability is classified as:
- CWE-120: Buffer Copy without Checking Size of Input ('Classic Buffer Overflow')
- CWE-680: Integer Overflow to Buffer Overflow
- OWASP A06:2021: Vulnerable and Outdated Components

Key Takeaways

• Never trust external input lengths: WiFi frame lengths, network packet sizes, and user-supplied data are all attacker-controlled and must be validated before use in memory operations.

• Explicit bounds checking is non-negotiable: The pattern if(len > sizeof(buffer)) return; should become second nature in any code handling untrusted input.

• Silent truncation is dangerous: The original if(len > HS_FRAME_MAX) len = HS_FRAME_MAX; pattern masked the error and could cause downstream data corruption. Always reject invalid input explicitly.

• Use defensive APIs: calloc() instead of malloc() for zero-initialization, and prefer safe string functions where available to reduce the attack surface.

• Test with boundary conditions: The regression test specifically includes 2x and 10x overflow attempts—always test with extreme values to verify your bounds checks actually prevent exploitation.

How Orbis AppSec Detected This

Source: Attacker-controlled WiFi frame data received via rx_cb() callback parameter len

Sink: memcpy(slot->data, payload, len) at line 102 in buddy_firmware/main/features/feat_capture_hs.c and memcpy(r->frame[slot], frame, len) at line 81 in buddy_firmware/main/buddy_hs_store.c

Missing control: No validation that len parameter doesn't exceed the destination buffer size before memcpy operations

CWE: CWE-120 (Buffer Copy without Checking Size of Input)

Fix: Added explicit length validation if((size_t)len > sizeof(slot->data)) return; before memcpy calls and changed silent truncation to explicit rejection of oversized frames

Orbis AppSec automatically detected this vulnerability using multi-agent AI analysis that tracks taint flow from external inputs (WiFi frame data) to dangerous sinks (memcpy operations). The scanner identified the missing bounds check and generated a pull request with the fix, including regression tests to prevent future regressions. Try Orbis AppSec on your repositories to find and fix issues like this automatically.

Conclusion

Buffer overflow vulnerabilities remain one of the most critical and exploitable vulnerability classes in C/C++ code, particularly in embedded systems and firmware where memory is constrained and attacker access is often easier than in traditional server environments.

This vulnerability demonstrates that even in well-maintained projects, the simple act of forgetting a bounds check can create a remote code execution vulnerability. The fix is straightforward—validate input lengths before use—but it requires discipline and awareness across your entire codebase.

Key lessons for developers:

1. Treat all external input as untrusted, including network packets, file data, and user input
2. Make bounds checking automatic by using safe APIs, compiler warnings, and static analysis tools
3. Test with malicious input that violates your assumptions about data sizes
4. Use defense in depth with multiple layers of protection rather than relying on a single check

By adopting these practices and using automated security tools like Orbis AppSec to catch these patterns before they reach production, you can significantly reduce the attack surface of your applications and firmware.

References

• CWE-120: Buffer Copy without Checking Size of Input
• OWASP: Buffer Overflow
• ESP-IDF Documentation: WiFi Promiscuous Mode
• Semgrep Rule: Potential Buffer Overflow
• GitHub PR: fix: add buffer-length check in feat_capture_hs.c