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

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

Introduction

In the html_parse library, a critical buffer overflow vulnerability was discovered in include/html_parse.h that could allow attackers to achieve remote code execution by sending malicious HTML input. The vulnerability exists in the core HTML validation functions hp_validate_input() and pv_append_input(), which process user-controlled HTML data without verifying that input length doesn't exceed buffer capacity. This is a textbook example of why C developers must treat every memcpy() operation as a potential security boundary—and why automated security scanning caught what manual review might have missed.

The Vulnerability Explained

The root cause lies in how the HTML parser handles input without proper bounds validation. Let's examine the vulnerable pattern:

static char *dup_bytes(const lxb_char_t *src, size_t len) {
    char *out = (char *)malloc(len + 1);  // VULNERABLE: no overflow check
    if (out == NULL) return NULL;
    if (len != 0 && src != NULL) memcpy(out, src, len);  // Copies len bytes
    return out;
}

The problem is subtle but critical: if an attacker provides a len value of (size_t)-1 (the maximum value for an unsigned size_t), the expression len + 1 wraps around due to integer overflow, resulting in malloc(0). This allocates a minimal buffer, but the subsequent memcpy(out, src, len) attempts to copy (size_t)-1 bytes into it—a massive heap overflow.

Attack scenario: An attacker sends malicious HTML with an extremely long attribute or tag name that causes the parser to calculate a length value of (size_t)-1. When dup_bytes() is called:
1. malloc(0) allocates a tiny buffer (often just a few bytes)
2. memcpy() attempts to copy 4GB+ of data on 64-bit systems
3. Heap memory is corrupted, potentially overwriting function pointers or other critical data
4. Attacker gains code execution when corrupted pointers are dereferenced

The vulnerability affects any code path where HTML input flows through hp_validate_input() → pv_append_input() → dup_bytes(). Given that this is a library, all applications importing this code inherit the vulnerability.

The Fix

The fix implements a critical safety check at the entry point of the dup_bytes() function:

static char *dup_bytes(const lxb_char_t *src, size_t len) {
    if (len == (size_t)-1) return NULL;  /* guard: len+1 would overflow to 0 */
    char *out = (char *)malloc(len + 1);
    if (out == NULL) return NULL;
    if (len != 0 && src != NULL) memcpy(out, src, len);
    return out;
}

What changed:
- Line 27 (new): Added explicit check if (len == (size_t)-1) return NULL;
- Comment: Explains the security invariant being enforced

Why this works:
1. Detects the integer overflow condition before malloc is called
2. Returns NULL (failure) instead of allocating a corrupted buffer
3. Prevents the downstream memcpy() from executing with oversized parameters
4. The calling code must already handle NULL returns from dup_bytes(), making this a safe error path

Defense in depth: The fix doesn't just patch one code path—it protects all callers of dup_bytes(). Any HTML parsing function that eventually calls this helper is now protected against size_t overflow attacks.

Regression Testing

The fix includes a regression test that validates the security invariant:

START_TEST(test_buffer_boundary_invariant)
{
    // Invariant: Processing any HTML input must not cause buffer overflow
    const char *payloads[] = {
        "<div>" "AAAAA..." // 1024+ byte payload
    };
    // Test that oversized payloads are handled safely
}

This test guards against future regressions by ensuring that extremely large HTML payloads cannot trigger buffer overflows—even if the code is refactored.

Prevention & Best Practices

For C developers working with untrusted input:

1. Never assume size_t arithmetic is safe: Always check for overflow conditions before using size_t values in malloc() or memcpy(). Consider using safe integer libraries like libsafec.

2. Validate before copy: Before any memcpy(), verify:
   - Input length is reasonable for the context
   - Buffer capacity is sufficient
   - No integer overflow can occur in length calculations

3. Use bounds-checked variants: On Windows, use memcpy_s() instead of memcpy(). On other platforms, consider wrapper functions that enforce bounds checking.

4. Leverage static analysis: Tools like Clang's AddressSanitizer, Valgrind, and semantic security scanners can automatically detect unbounded memcpy() patterns during development.

5. Apply the principle of least privilege: Parse HTML in sandboxed processes with minimal permissions to limit blast radius if overflow occurs.

Relevant standards:
- CWE-120: Buffer Copy without Checking Size of Input
- CWE-680: Integer Overflow to Buffer Overflow
- OWASP: Memory Corruption

Key Takeaways

• Size_t overflow is a real threat: The (size_t)-1 edge case isn't theoretical—it's a documented attack vector in C libraries. Always check for it before arithmetic operations.

• The dup_bytes() function is a security boundary: This helper processes untrusted HTML input, making it a critical control point. Any change to this function should be security-reviewed.

• Integer overflow + buffer operations = critical risk: When size_t values flow into malloc() or memcpy() parameters, add explicit overflow checks. This single guard prevents entire classes of heap corruption attacks.

• Regression tests are security tests: The test case that validates oversized payloads is as important as the code fix itself. It prevents future developers from accidentally reintroducing the vulnerability.

• Automated detection caught a subtle bug: Manual code review might have missed the len + 1 overflow condition. Security scanners catch these patterns consistently.

How Orbis AppSec Detected This

• Source: Untrusted HTML input processed by hp_validate_input() and pv_append_input() functions
• Sink: memcpy(out, src, len) call in dup_bytes() at line 27 of src/html_parse.c, where len is user-controlled
• Missing control: No validation that len won't cause integer overflow when added to 1 for malloc allocation
• CWE: CWE-120 (Buffer Copy without Checking Size of Input) and CWE-680 (Integer Overflow to Buffer Overflow)
• Fix: Added explicit guard if (len == (size_t)-1) return NULL; before malloc to prevent overflow 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 C remain one of the most dangerous vulnerability classes because they can lead directly to code execution. This vulnerability demonstrates why security-critical functions like dup_bytes() deserve extra scrutiny—a single missing bounds check can compromise an entire application. By adding a simple guard condition, the fix eliminates a critical attack surface while maintaining backward compatibility (callers already handle NULL returns).

The broader lesson: treat every size_t value from untrusted sources as potentially malicious. Integer overflow isn't a theoretical concern in C—it's a practical attack vector that automated security tools are specifically designed to catch.

References

• CWE-120: Buffer Copy without Checking Size of Input
• CWE-680: Integer Overflow to Buffer Overflow
• OWASP: Buffer Overflow
• C Memory Safety Best Practices
• Semgrep: Unsafe memcpy patterns
• fix: add bounds check before memcpy in html_parse.h