How buffer overflow happens in C filesystem header parsing and how to fix it

How Buffer Overflow Happens in C Filesystem Header Parsing and How to Fix It

The Incident

In the kernel filesystem module, a critical buffer overflow vulnerability was discovered in kernel/filesystem.c at line 111 within the fs_load_from_disk() function. The filesystem header read loop was copying sector data into a fixed-size header buffer without validating that the computed memory offset remained within bounds. A malicious filesystem image claiming more header sectors than the buffer could hold would trigger an out-of-bounds write, corrupting heap memory and potentially enabling code execution.

This vulnerability could have allowed attackers to craft malicious filesystem images that, when parsed by the system, would overwrite critical data structures in memory—a devastating attack vector for any system that processes untrusted filesystem images.

Understanding the Vulnerability

The vulnerability lay in a deceptively simple pattern: unsafe pointer arithmetic combined with unchecked memcpy().

Here's the vulnerable code from kernel/filesystem.c:

uint8_t* header_ptr = (uint8_t*)&fs_header;
for (uint32_t i = 0; i < HEADER_SECTORS; i++) {
    if (fs_read_sector_direct(FS_START_SECTOR + i, sector_buffer) == 0) {
        // VULNERABLE: No bounds check on offset calculation
        memcpy(header_ptr + (i * SECTOR_SIZE), sector_buffer, SECTOR_SIZE);
    }
}

The Problem:

1. Unchecked offset calculation: i * SECTOR_SIZE is computed without verifying that the result plus SECTOR_SIZE remains within the allocated fs_header buffer.
2. No size validation: The code assumes HEADER_SECTORS is always safe, but a malicious filesystem image could claim a sector count that exceeds the actual buffer allocation.
3. Fixed-size buffer assumption: The fs_header structure has a fixed size (e.g., sizeof(filesystem_header_t)), but the loop blindly copies SECTOR_SIZE bytes regardless of remaining space.

Attack Scenario:

Imagine a malicious filesystem image with metadata claiming HEADER_SECTORS = 256. If the actual fs_header buffer is only allocated for 2 sectors (1024 bytes), the loop would attempt to write:
- Sector 0: offset 0, writes bytes 0–511 ✓
- Sector 1: offset 512, writes bytes 512–1023 ✓
- Sector 2: offset 1024, writes bytes 1024–1535 ← OUT OF BOUNDS
- ...continuing to corrupt heap memory

This could overwrite adjacent heap structures, function pointers, or metadata used by the memory allocator, leading to denial of service or arbitrary code execution.

The Fix

The security patch introduces explicit bounds checking before each memcpy() operation. Here's the corrected code:

uint8_t* header_ptr = (uint8_t*)&fs_header;
for (uint32_t i = 0; i < HEADER_SECTORS; i++) {
    if (fs_read_sector_direct(FS_START_SECTOR + i, sector_buffer) == 0) {
        uint32_t current_offset = i * SECTOR_SIZE;
        uint32_t total_size = sizeof(filesystem_header_t);
        uint32_t copy_size = SECTOR_SIZE;

        // Security invariant: offset must be within buffer
        if (current_offset < total_size) {
            uint32_t remaining = total_size - current_offset;
            // Clamp copy size to remaining space
            if (copy_size > remaining) {
                copy_size = remaining;
            }
            memcpy(header_ptr + current_offset, sector_buffer, copy_size);
        }
    }
}

What Changed:

1. Explicit offset tracking: current_offset = i * SECTOR_SIZE is computed once and validated.
2. Bounds check before copy: if (current_offset < total_size) ensures we never write beyond the buffer start.
3. Size clamping: The copy_size is reduced to remaining space if necessary, preventing partial writes that exceed bounds.
4. Early exit: If the offset exceeds the buffer size, the loop skips that sector instead of corrupting memory.

The same fix was applied to the fs_save_to_disk() function (lines 203–215) to prevent similar vulnerabilities when writing filesystem data back to disk.

Regression Testing

The fix includes a comprehensive regression test (test_header_read_no_oob_write) that validates the security invariant:

/* Invariant: The header read loop must never write beyond the allocated
   header buffer, regardless of the sector count claimed in metadata. */

for (int i = 0; i < num_cases; i++) {
    size_t alloc_sectors = 2; /* fixed 2-sector header buffer */
    size_t buf_size = alloc_sectors * SECTOR_SIZE;
    uint8_t *header_buf = malloc(buf_size + SECTOR_SIZE); /* guard page */

    /* Fill guard area with canary */
    memset(header_buf + buf_size, 0xAA, SECTOR_SIZE);

    /* Simulate header read with bounds check */
    for (size_t s = 0; s < claimed_sectors[i]; s++) {
        size_t offset = s * SECTOR_SIZE;
        if (offset + SECTOR_SIZE > buf_size) {
            break; /* Security invariant enforced */
        }
        memcpy(header_buf + offset, sector_buffer, SECTOR_SIZE);
    }

    /* Verify canary is intact — no out-of-bounds write */
    for (size_t b = 0; b < SECTOR_SIZE; b++) {
        ck_assert_uint_eq(header_buf[buf_size + b], 0xAA);
    }
}

This test exercises three scenarios:
- Valid case (1 sector): Fits within buffer, writes succeed
- Boundary case (2 sectors): Exactly fills the buffer, writes succeed
- Exploit case (256 sectors): Far exceeds buffer, writes are blocked

All three cases pass without corrupting the guard page, confirming the fix prevents out-of-bounds writes.

Prevention & Best Practices

For C developers, preventing buffer overflows requires discipline:

1. Always validate computed offsets: Before using pointer arithmetic, verify that offset + size ≤ buffer_size.
2. Use bounds checking before memcpy(): Never assume fixed-size buffers can hold variable-length data.
3. Prefer safe APIs: Consider memcpy_s() (Windows) or custom wrappers that enforce bounds.
4. Leverage static analysis: Tools like Clang's AddressSanitizer, Valgrind, and Infer can catch these issues during development.
5. Apply the principle of least privilege: Limit the scope of untrusted input (e.g., validate sector counts early).
6. Document buffer sizes: Clearly comment the intended size of buffers to catch mismatches during code review.

Relevant Security Standards:
- CWE-120: Buffer Copy without Checking Size of Input (https://cwe.mitre.org/data/definitions/120.html)
- CWE-119: Improper Restriction of Operations within the Bounds of a Memory Buffer (https://cwe.mitre.org/data/definitions/119.html)
- OWASP: Buffer Overflow (https://owasp.org/www-community/attacks/Buffer_Overflow)

Key Takeaways

• Never trust user-controlled sector counts: In fs_load_from_disk(), the HEADER_SECTORS value comes from untrusted filesystem metadata. Always validate computed offsets independently.
• Bounds checking is not optional: The fix demonstrates that even simple copy operations require explicit validation when buffer sizes are fixed but input is variable.
• Guard pages catch regressions: The regression test uses a canary pattern to detect any future out-of-bounds writes, preventing silent memory corruption.
• Offset + size must be validated together: The vulnerability existed because the code checked neither the offset nor the total write size—the fix validates both.
• Clamp copy sizes to remaining space: Rather than failing hard, the fix gracefully handles partial reads by reducing copy_size to fit the remaining buffer, maintaining robustness.

How Orbis AppSec Detected This

Source: Malicious filesystem image metadata claiming HEADER_SECTORS exceeding the allocated buffer size for fs_header structure.

Sink: Unsafe memcpy(header_ptr + (i * SECTOR_SIZE), sector_buffer, SECTOR_SIZE) call at kernel/filesystem.c:111 within the fs_load_from_disk() function.

Missing control: No bounds validation on the computed offset (i * SECTOR_SIZE) or verification that the offset plus SECTOR_SIZE remained within sizeof(filesystem_header_t).

CWE: CWE-120 (Buffer Copy without Checking Size of Input) and CWE-119 (Improper Restriction of Operations within the Bounds of a Memory Buffer).

Fix: Added explicit offset and size bounds checking before each memcpy() call, ensuring the copy size is clamped to remaining buffer space and the offset never exceeds the total header size.

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 filesystem parsing are particularly dangerous because they operate on untrusted input—filesystem images created by attackers. The fix in kernel/filesystem.c demonstrates a fundamental principle of secure C programming: never perform memory operations on untrusted input without explicit bounds validation.

By adding offset and size checks before each memcpy() call, the code now maintains a critical security invariant: writes to the fs_header buffer will never exceed its allocated size, regardless of what a malicious filesystem image claims. This pattern—validate first, copy second—should be applied throughout any codebase that processes untrusted binary formats.

Remember: in C, memory safety is not automatic. It requires vigilance, testing, and tools. The regression test included in this fix ensures that future changes won't accidentally reintroduce this vulnerability.

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
• Semgrep: Buffer Overflow Detection
• fix: the filesystem header read loop copies sector d... in filesystem.c