How buffer overflow happens in C Bluetooth device handling and how to fix it

How Buffer Overflow Happens in C Bluetooth Device Handling and How to Fix It

The Incident

In the wiiuse Bluetooth library (src/wiiuse.c), a critical buffer overflow vulnerability was discovered in the data handling functions used to communicate with Wii Remote controllers. The vulnerability existed at lines 627 and 666, where memcpy() operations copied data from untrusted Bluetooth packets into fixed-size buffers without validating that the requested length actually fit within the destination buffer.

An attacker within Bluetooth range (10–100 meters) could pair a malicious device or intercept Bluetooth HID traffic, sending a specially crafted packet with an oversized len value. This would cause memcpy() to write far beyond the buffer boundary, corrupting the heap and potentially crashing the application or enabling code execution.

Why This Matters

Bluetooth libraries are low-level components that handle raw data from external devices. Unlike HTTP APIs where you might validate JSON structure, Bluetooth HID (Human Interface Device) packets are binary and come from devices that may be compromised or spoofed. When a library blindly trusts the length field in these packets, it creates a direct path for attackers to corrupt memory.

The Vulnerability Explained

The Vulnerable Code

The original code in src/wiiuse.c had two dangerous patterns:

Line 627 (original):

int wiiuse_write_data(struct wiimote_t *wm, unsigned int addr, const byte *data, unsigned short len)
{
    // ... validation code ...

    // NO LENGTH CHECK HERE
    memcpy(bufPtr, data, len);  // len is attacker-controlled via Bluetooth packet
}

Line 666 (original):

int wiiuse_write_data_cb(struct wiimote_t *wm, unsigned int addr, byte *data, byte write_cb)
{
    req = (struct data_req_t *)malloc(sizeof(struct data_req_t));
    req->cb = write_cb;
    req->len = len;  // No bounds check
    memcpy(req->data, data, req->len);  // Copies req->len bytes into req->data
}

The Attack

A malicious Bluetooth device sends a write request with len = 0xFFFF (65,535 bytes). The req->data buffer is only ~23 bytes (standard HID payload), but memcpy() writes 65,535 bytes into it anyway. This overwrites:
- Adjacent heap structures
- Function pointers
- Other allocated objects

The heap corruption can cause:
- Denial of Service: Crash when corrupted metadata is accessed
- Information Disclosure: Leak adjacent heap contents
- Code Execution: Overwrite function pointers or vtables

Why It Happened

The developers assumed that Bluetooth devices would send valid packets. They didn't account for:
1. Malicious devices pairing with the system
2. Man-in-the-middle attacks intercepting and modifying Bluetooth traffic
3. Fuzzing or intentional malformed packets during testing

The Wii Remote protocol doesn't have cryptographic authentication, so any device can claim to be a Wii Remote.

The Fix

The fix adds explicit bounds checking before every memcpy() call and uses safer memory allocation practices:

Change 1: Safer Memory Allocation (Line 141)

Before:

wm = (struct wiimote_t **)malloc(sizeof(struct wiimote_t *) * wiimotes);

After:

wm = (struct wiimote_t **)calloc((size_t)wiimotes, sizeof(struct wiimote_t *));

Why: calloc() initializes memory to zero and performs overflow checking internally. It also casts wiimotes to size_t to catch negative values (which could cause integer underflow in the multiplication).

Change 2: Validate Input Length (Lines 605–609)

Before:

int wiiuse_write_data(struct wiimote_t *wm, unsigned int addr, const byte *data, unsigned short len)
{
    if (!data || !len)
    {
        WIIUSE_ERROR("Attempt to write, but no data or length == 0");
        return 0;
    }
    // NO MAXIMUM LENGTH CHECK
    memcpy(bufPtr, data, len);
}

After:

int wiiuse_write_data(struct wiimote_t *wm, unsigned int addr, const byte *data, unsigned short len)
{
    if (!data || !len)
    {
        WIIUSE_ERROR("Attempt to write, but no data or length == 0");
        return 0;
    }
    if (len > 16)  // NEW: Wii Remote protocol max is 16 bytes per write
    {
        WIIUSE_ERROR("Attempt to write more than 16 bytes at once (len=%d)", len);
        return 0;
    }
    memcpy(bufPtr, data, len);
}

Why: The Wii Remote protocol specification limits writes to 16 bytes. Any request exceeding this is invalid and must be rejected.

Change 3: Clamp Length in Callback Handler (Line 670)

Before:

req->len = len;  // Direct assignment, no validation
memcpy(req->data, data, req->len);

After:

req->len = (len > 16) ? 16 : len;  // Clamp to maximum
memcpy(req->data, data, req->len);

Why: Defense in depth. Even if a caller somehow bypasses the check in wiiuse_write_data(), the callback handler clamps the length to prevent overflow.

Change 4: Regression Test (New File)

A security-focused regression test was added to verify the invariant:

START_TEST(test_read_data_length_bounds)
{
    struct wiimote_t **wm = wiiuse_init(1);

    unsigned short test_lens[] = {
        0xFFFF,                    /* Exploit: massive length */
        WIIUSE_MAX_PAYLOAD + 100,  /* Boundary: exceeds payload buffer */
        16,                        /* Valid: fits in buffer */
    };

    for (int i = 0; i < num_tests; i++) {
        unsigned short len = test_lens[i];
        int ret = wiiuse_read_data(wm[0], NULL, 0, 0x0000, len);
        if (len > WIIUSE_MAX_PAYLOAD) {
            ck_assert_msg(ret == 0,
                "read_data accepted dangerous len=%u without bounds check", len);
        }
    }
}
END_TEST

This test ensures that oversized requests are always rejected, preventing regressions.

Prevention & Best Practices

1. Never Trust External Length Values

When parsing binary protocols (Bluetooth, USB, network packets), always validate that declared lengths don't exceed protocol limits or buffer capacity:

// Bad
memcpy(buffer, packet->data, packet->len);

// Good
if (packet->len > MAX_BUFFER_SIZE) {
    return error("Invalid packet length");
}
memcpy(buffer, packet->data, packet->len);

2. Use Protocol-Specific Limits

Understand the protocol specification. Wii Remote writes are limited to 16 bytes—enforce this limit in code:

#define WIIUSE_MAX_WRITE_SIZE 16

if (len > WIIUSE_MAX_WRITE_SIZE) {
    WIIUSE_ERROR("Write exceeds protocol maximum: %d > %d", len, WIIUSE_MAX_WRITE_SIZE);
    return 0;
}

3. Prefer Safe Memory Functions

• Use calloc() instead of malloc() for zero-initialization and overflow checking
• Use memcpy_s() on Windows (Microsoft's bounds-checked variant)
• On Linux, consider -D_FORTIFY_SOURCE=2 compiler flag for runtime checks
• Use strlcpy() instead of strcpy() for string operations

4. Enable Compiler Protections

gcc -fstack-protector-strong \
    -D_FORTIFY_SOURCE=2 \
    -Wl,-z,relro,-z,now \
    -fPIE -pie \
    src/wiiuse.c

These flags add:
- Stack canaries to detect buffer overflows
- Runtime checks for buffer operations
- Address Space Layout Randomization (ASLR)
- Position Independent Executable (PIE)

5. Use Static Analysis

Tools that can detect this vulnerability:
- Clang Static Analyzer: scan-build gcc src/wiiuse.c
- Coverity: Commercial tool used by many open-source projects
- Semgrep: Open-source pattern-based scanner
- AddressSanitizer: Runtime detector (use during testing)

# Compile with AddressSanitizer
gcc -fsanitize=address -g src/wiiuse.c -o wiiuse_test

# Run tests—ASAN will catch heap overflow attempts
./wiiuse_test

6. Fuzz Test Protocols

Bluetooth and USB protocols benefit from fuzzing:

# Use libFuzzer to generate malformed packets
clang -fsanitize=fuzzer src/wiiuse_fuzzer.c src/wiiuse.c -o wiiuse_fuzzer
./wiiuse_fuzzer

Key Takeaways

• Buffer overflow via unchecked memcpy(): The wiiuse_write_data() function accepted arbitrary length values from Bluetooth packets without validating them against the 16-byte protocol maximum, allowing heap corruption.

• Defense in depth matters: The fix adds bounds checking in two places (wiiuse_write_data() and wiiuse_write_data_cb()) so that even if one check is bypassed, the other prevents overflow.

• Use calloc() for safer allocation: Switching from malloc() to calloc() provides zero-initialization and internal overflow detection, catching integer underflow bugs.

• Protocol specifications are security boundaries: The Wii Remote HID protocol limits writes to 16 bytes—enforcing this limit in code is not optional, it's a security requirement.

• Regression tests prevent re-introduction: The new test_invariant_wiiuse.c test ensures that oversized length values are always rejected, preventing future developers from accidentally removing the fix.

How Orbis AppSec Detected This

Source: Bluetooth HID packet received from external device, containing the len field in a write request

Sink: memcpy(bufPtr, data, len) at line 627 and memcpy(req->data, data, req->len) at line 666 in src/wiiuse.c

Missing Control: No validation that len doesn't exceed the protocol maximum (16 bytes) or buffer capacity before the memcpy call

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

Fix: Added explicit bounds checking (if (len > 16) return error;) before memcpy operations and replaced malloc() with calloc() for safer allocation

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

Buffer overflows in low-level protocols like Bluetooth are particularly dangerous because the data source (external devices) is untrusted and often outside your control. The wiiuse library's vulnerability demonstrates a critical lesson: never assume that external input respects protocol limits—always validate it in code.

The fix is straightforward but essential: validate that len doesn't exceed 16 before calling memcpy(). This simple check prevents attackers within Bluetooth range from corrupting the heap.

As you work with binary protocols, USB, Bluetooth, or network code, apply these principles:
1. Know your protocol's limits
2. Validate all external length values
3. Use safe memory functions
4. Enable compiler protections
5. Test with malformed input

Security isn't a feature—it's a requirement for code that handles untrusted external input.

References

• CWE-120: Buffer Copy without Checking Size of Input
• CWE-119: Improper Restriction of Operations within the Bounds of a Memory Buffer
• OWASP: Buffer Overflow
• C Standard Library: memcpy() Documentation
• GCC Compiler Security Flags
• AddressSanitizer Documentation
• Semgrep: Buffer Overflow Rules
• fix: add bounds check before memcpy in wiiuse.c