How buffer overflow in fgets() happens in C and how to fix it

Introduction

The mdbx_load.c file is a CLI utility for loading data into MDBX databases from stdin or redirected input files. Within its readline() function—a performance-critical hot path marked with __hot—a subtle off-by-one error in an fgets() call at line 426 created a critical buffer overflow vulnerability. By passing buf->iov_len + 1 instead of buf->iov_len as the size parameter, the code allowed fgets() to write one byte beyond the allocated buffer, opening the door to heap corruption and potential arbitrary code execution.

This is the kind of bug that haunts C developers: a single + 1 that transforms a safe read into an exploitable overflow. If you work with buffer management in C, this case study demonstrates exactly why every byte counts.

The Vulnerability Explained

The Vulnerable Code

Here's the problematic code in the readline() function at line 426 of mdbx_load.c:

__hot static int readline(MDBX_val *out, MDBX_val *buf) {
    // ... buffer setup ...
    c1 = buf->iov_base;
    c1 += l2;
    errno = 0;
    if (fgets((char *)c1, (int)buf->iov_len + 1, stdin) == nullptr)
      return errno ? errno : EOF;
    buf->iov_len *= 2;
    len = strlen((char *)c1);
    // ...
}

The critical issue is (int)buf->iov_len + 1. The fgets() function reads at most size - 1 characters (reserving one byte for the null terminator). By passing buf->iov_len + 1, the code tells fgets() it can read up to buf->iov_len characters of actual data—but this exceeds the remaining space in the buffer by one byte.

Why This Is Dangerous

The MDBX_val structure uses iov_len to track the buffer's available length. When fgets() is told the buffer is one byte larger than it actually is, it can write a character (plus null terminator) beyond the allocated region. This is a classic heap buffer overflow because:

1. The buffer is dynamically allocated — buf->iov_base points to heap memory
2. The overflow is controllable — an attacker controls the input data via stdin
3. The function is in a hot loop — marked __hot, it's called repeatedly during data loading

Attack Scenario

An attacker exploiting this vulnerability would:

1. Craft an input file with lines precisely sized to fill the buffer to its iov_len boundary
2. Redirect this file as stdin to mdbx_load: ./mdbx_load < malicious_input.dat
3. The fgets() call writes one byte past the heap allocation
4. Depending on the heap allocator's metadata layout, this corrupts heap management structures or adjacent data
5. Through careful heap grooming, the attacker achieves arbitrary write primitives, leading to code execution

While mdbx_load is a local CLI tool (requiring the attacker to control input), it's commonly used in automated pipelines where input may come from untrusted sources—backup restoration, data migration scripts, or CI/CD environments.

The Subtlety of Off-By-One

What makes this bug particularly insidious is that fgets() already subtracts one from its size parameter for the null terminator. Developers sometimes add + 1 thinking they need to account for this, but in reality:

• fgets(buf, n, stream) reads at most n - 1 chars and appends \0
• If you have a buffer of size N, you pass N to fgets()—not N + 1

The + 1 here essentially tells fgets() "you have one more byte than you actually do," which is never safe.

The Fix

The fix is elegant in its simplicity—a single character change that eliminates the vulnerability:

Before (Vulnerable)

if (fgets((char *)c1, (int)buf->iov_len + 1, stdin) == nullptr)

After (Fixed)

if (fgets((char *)c1, (int)buf->iov_len, stdin) == nullptr)

By removing the + 1, the code now correctly tells fgets() the actual available buffer size. Since fgets() internally reserves one byte for the null terminator, it will read at most buf->iov_len - 1 characters of data—guaranteeing the write stays within bounds.

Why This Works

The security invariant is now maintained: buffer reads never exceed the declared length. After this fix:

• fgets() receives buf->iov_len as the size
• fgets() writes at most buf->iov_len - 1 data bytes + 1 null byte = buf->iov_len total bytes
• This exactly fills the available space without overflow
• The subsequent buf->iov_len *= 2 doubling for the next iteration remains correct

Key Takeaways

• A single + 1 in an fgets() size parameter created a critical heap overflow in mdbx_load.c's hot-path readline() function — off-by-one errors in C are never "just one byte"
• The fgets() contract already accounts for the null terminator — passing buf->iov_len + 1 double-compensates and overflows; pass the exact buffer size
• Dynamic buffer tracking with iov_len requires extreme precision — when buf->iov_len *= 2 doubles the size for reallocation, the size passed to fgets() must always reflect current available space, not future space
• Local CLI tools are not exempt from security review — mdbx_load processes untrusted input in automated pipelines, making this overflow exploitable in real-world deployments
• AddressSanitizer would have caught this immediately — enabling ASan in CI for C projects with buffer manipulation is a minimal-cost, high-value defense

How Orbis AppSec Detected This

• Source: User-controlled input from stdin (redirected file or piped data) entering the readline() function in mdbx_load.c
• Sink: fgets((char *)c1, (int)buf->iov_len + 1, stdin) at mdbx_load.c:426 — the dangerous call site where the oversized length parameter enables heap buffer overflow
• Missing control: No validation that the size parameter passed to fgets() does not exceed the actual allocated buffer size; the + 1 arithmetic error bypasses the natural safety of fgets()
• CWE: CWE-120 (Buffer Copy without Checking Size of Input)
• Fix: Removed the erroneous + 1 from the fgets() size parameter, changing (int)buf->iov_len + 1 to (int)buf->iov_len to ensure reads respect buffer boundaries

Orbis AppSec detects issues like this automatically. Try Orbis AppSec on your repositories to find and fix issues like this automatically.

Conclusion

This vulnerability is a textbook example of how a single arithmetic error in C can create a critical security flaw. The + 1 in the fgets() call within mdbx_load.c's readline() function seemed innocuous—perhaps even intentional by a developer thinking about null terminator accounting—but it violated the fundamental contract of fgets() and created an exploitable heap overflow.

The fix was one character: removing the +. But the lesson is broader. In C, every buffer operation demands precise size tracking. When you use fgets(), pass the exact buffer size—the function handles the null terminator internally. When you track sizes in structures like MDBX_val, document and enforce the invariant that the size reflects available writable space. And always run your input-processing code under AddressSanitizer.

Buffer overflows remain one of the most dangerous vulnerability classes in C code. Stay vigilant with every + 1.