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

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

Introduction

The tpl.c file implements a template processing CLI tool with a daemonize() function that reconstructs command-line arguments into a single string for re-execution as a background process. At line 70, this function uses a fixed-size char buf[8192] and iterates over argv[], calling strcat(buf, argv[i]) for each argument—without ever checking whether the accumulated string still fits within the 8192-byte boundary.

This is a textbook CWE-120 buffer overflow: a local attacker who controls command-line arguments (or an upstream process that passes crafted arguments) can supply inputs totaling more than 8KB, overwriting stack memory beyond buf and potentially hijacking control flow.

The vulnerability was flagged as critical because the overflow is trivially exploitable and the tool runs in production environments where argument sources may not be fully trusted.

The Vulnerability Explained

The vulnerable pattern in daemonize() looks like this:

void daemonize(int argc, char *argv[]) {
    char buf[8192];
    buf[0] = '\0';

    for (int i = 0; i < argc; i++) {
        strcat(buf, argv[i]);
        strcat(buf, " ");
    }
    // ... fork and exec with buf ...
}

Why this is dangerous:

1. buf is allocated on the stack with a fixed size of 8192 bytes.
2. strcat() appends data to the end of the existing string and writes a null terminator—it has no concept of the destination buffer's capacity.
3. Each iteration blindly appends argv[i] plus a space character without checking how much room remains.
4. If the sum of all argument lengths (plus spaces and null terminator) exceeds 8192, strcat() writes past the end of buf.

Concrete exploitation scenario:

An attacker runs:

tpl -d $(python -c "print('A'*9000)")

This passes a single 9000-byte argument. Combined with "tpl", "-d", and the space separators, the total exceeds 8192 bytes. The overflow corrupts the saved return address on the stack. A sophisticated attacker can craft the overflow payload to redirect execution to shellcode or a ROP chain, achieving arbitrary code execution with the privileges of the tpl process.

Threat model context: While tpl is a local CLI tool (meaning the attacker needs local access or control over how the tool is invoked), many deployment scenarios involve wrapper scripts, cron jobs, or orchestration systems that pass arguments from external sources—making this a realistic attack surface.

The Fix

The fix adds a cumulative length check before each strcat() call, ensuring the buffer is never written beyond its declared size. Here's the before/after comparison:

Before (vulnerable):

void daemonize(int argc, char *argv[]) {
    char buf[8192];
    buf[0] = '\0';

    for (int i = 0; i < argc; i++) {
        strcat(buf, argv[i]);
        strcat(buf, " ");
    }
    // ...
}

After (fixed):

void daemonize(int argc, char *argv[]) {
    char buf[8192];
    size_t remaining = sizeof(buf);
    buf[0] = '\0';

    for (int i = 0; i < argc; i++) {
        size_t arg_len = strlen(argv[i]) + 1; /* +1 for space */
        if (arg_len >= remaining) {
            break; /* or handle error: truncate rather than overflow */
        }
        strcat(buf, argv[i]);
        strcat(buf, " ");
        remaining -= arg_len;
    }
    // ...
}

Key aspects of the fix:

1. Tracks remaining capacity: A remaining variable is initialized to sizeof(buf) and decremented after each successful append.
2. Pre-checks before write: Before calling strcat(), the code verifies that argv[i] plus the space separator fits within the remaining capacity.
3. Fails safely: If arguments would overflow the buffer, the loop breaks—truncating the command rather than corrupting memory.
4. Enforces the security invariant: "Buffer reads never exceed the declared length."

The accompanying regression test (tests/test_invariant_tpl.c) exercises three scenarios:
- A 9000-byte argument (exceeds buffer) — must not crash
- An 8192-byte argument (boundary value) — must not overflow
- A small normal argument — must work correctly

This ensures the fix handles edge cases and prevents future regressions.

Prevention & Best Practices

1. Never use unbounded string functions with external input:
- Replace strcat() with strncat() or snprintf() which accept a maximum length parameter.
- Better yet, use snprintf() which returns the number of characters that would have been written, making truncation detection trivial.

2. Prefer dynamic allocation for variable-length data:

// Safer approach: calculate needed size first
size_t total = 0;
for (int i = 0; i < argc; i++)
    total += strlen(argv[i]) + 1;
char *buf = malloc(total + 1);

3. Enable compiler protections:
- Compile with -fstack-protector-strong to detect stack buffer overflows at runtime.
- Use -D_FORTIFY_SOURCE=2 which replaces strcat with a bounds-checked version when the buffer size is known at compile time.

4. Static analysis:
- Run tools like cppcheck, Coverity, or Semgrep with rules targeting strcat() into fixed-size buffers.
- Enable -Wall -Wextra to catch related warnings.

5. Use AddressSanitizer during testing:

gcc -fsanitize=address -g tpl.c -o tpl_test

This catches overflows immediately during test execution.

Key Takeaways

• Never use strcat() in a loop without tracking cumulative buffer usage — the daemonize() function's pattern of iterating over argv[] and appending to a fixed buffer is a classic overflow recipe.
• An 8192-byte buffer is not "large enough" — attackers craft inputs specifically to exceed whatever size you chose; only explicit bounds checking is safe.
• CLI tools are not immune to exploitation — even though tpl requires local access, automated pipelines and orchestration systems can pass attacker-controlled arguments.
• Regression tests for buffer boundaries catch future mistakes — the test at tests/test_invariant_tpl.c exercises the exact overflow scenario and boundary condition.
• The security invariant "buffer reads never exceed the declared length" should be enforced programmatically, not assumed by convention.

How Orbis AppSec Detected This

• Source: Command-line arguments (argv[]) passed to the tpl binary
• Sink: strcat(buf, argv[i]) in daemonize() at tpl.c:70, writing into a stack-allocated char buf[8192]
• Missing control: No bounds checking or remaining-capacity tracking before each strcat() call
• CWE: CWE-120 — Buffer Copy without Checking Size of Input
• Fix: Added a cumulative length check that validates strlen(argv[i]) + 1 < remaining before each concatenation, breaking the loop if the buffer would overflow

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

This buffer overflow in tpl.c's daemonize() function is a stark reminder that C's string functions provide zero safety guarantees—the programmer must enforce bounds manually. The fix is conceptually simple (check length before writing), but the consequences of missing it are severe: memory corruption, crashes, and potential arbitrary code execution.

If you're writing C code that handles variable-length input—whether from command-line arguments, files, or network data—always track your buffer's remaining capacity and validate before every write. Use bounded alternatives like snprintf(), enable compiler hardening flags, and write regression tests that specifically exercise boundary conditions.

References

• CWE-120: Buffer Copy without Checking Size of Input
• OWASP Buffer Overflow Prevention
• Linux man page: strncat(3)
• Semgrep rules for buffer overflow
• fix: add buffer-length check in tpl.c (PR link)