How buffer overflow via sprintf() happens in C string formatting and how to fix it

How buffer overflow via sprintf() happens in C string formatting and how to fix it

Introduction

In the bench/strbuild/strbuild.c file of a Node.js native library, we discovered a high-severity buffer overflow at line 42. The run_kernel() function formats benchmark data — an index, a name string, and a computed integer — into a 64-byte stack-allocated buffer called line[64] using sprintf(). Because sprintf() has no concept of a maximum write length, any combination of inputs that produces a formatted string longer than 63 characters (plus null terminator) would overflow the buffer, corrupting the stack frame.

This matters because the library is consumed by downstream Node.js packages. If any code path allows external influence over the NAMES[] array contents or the integer values passed to run_kernel(), an attacker could trigger a stack smash in the native addon — crashing the Node.js process or, worse, achieving arbitrary code execution.

The Vulnerability Explained

Here's the vulnerable code at line 42 of bench/strbuild/strbuild.c:

static uint32_t run_kernel(const int32_t* c, const int32_t* v) {
  char line[64];
  for (int it = 0; it < N_ITERS; it++) {
    for (int i = 0; i < N; i++) {
      int len = sprintf(line, "%d,%s,%d\n", i, NAMES[c[i]], (int)(v[i] + it));
      for (int j = 0; j < len; j++) h = mix_u32(h, (uint32_t)(uint8_t)line[j]);
    }
  }
}

The format string "%d,%s,%d\n" concatenates:
- An integer i (up to 10 digits for a 32-bit int)
- A comma
- A string NAMES[c[i]] (unbounded length)
- A comma
- An integer v[i] + it (up to 11 characters including sign)
- A newline

If NAMES[c[i]] is, say, 50 characters long and i is a multi-digit number, the total formatted output easily exceeds 64 bytes. sprintf() will happily write past the end of line[], overwriting:

1. The saved frame pointer
2. The return address
3. Other local variables on the stack

Attack Scenario

Consider this concrete exploitation path:

1. An attacker influences the NAMES[] array (perhaps through a configuration file, environment variable, or upstream data source that populates the benchmark names).
2. They supply a name string of 60+ characters.
3. When run_kernel() iterates and formats "%d,%s,%d\n" with this long name, sprintf() writes ~75 bytes into the 64-byte line buffer.
4. The overflow overwrites the return address on the stack.
5. When run_kernel() returns, execution jumps to an attacker-controlled address.

Even if exploitation for code execution is difficult due to stack canaries or ASLR, the overflow reliably causes a crash — a denial-of-service condition for any Node.js application using this native module.

The Fix

The fix is a single-character-level change with enormous security impact — replacing sprintf with snprintf and passing sizeof(line) as the size bound:

Before (vulnerable):

int len = sprintf(line, "%d,%s,%d\n", i, NAMES[c[i]], (int)(v[i] + it));

After (fixed):

int len = snprintf(line, sizeof(line), "%d,%s,%d\n", i, NAMES[c[i]], (int)(v[i] + it));

Why this works:

1. snprintf() guarantees bounded writes: It will write at most sizeof(line) - 1 characters (63 bytes) plus a null terminator, regardless of how long the formatted output would be.

2. Truncation over corruption: If the formatted string exceeds 63 characters, snprintf() truncates the output rather than overflowing the buffer. The return value len still reports what the full length would have been, but the buffer is never overwritten past its boundary.

3. No behavior change for normal inputs: For benchmark strings that fit within 64 bytes (the expected case), the output is identical. The fix only activates as a safety net when inputs would have caused an overflow.

4. sizeof(line) is compile-time constant: Since line is a stack array, sizeof(line) evaluates to 64 at compile time — zero runtime overhead for the bounds check.

Prevention & Best Practices

Immediate Rules

• Never use sprintf() with fixed-size buffers. Always use snprintf() with an explicit size parameter.
• Treat all format string targets as potentially too small. Even if you "know" the data fits today, maintenance changes or upstream data changes can break that assumption.
• Check snprintf() return values. If len >= sizeof(line), truncation occurred — log it or handle the error.

Detection Tools

• Semgrep: The rule utils.custom.buffer-overflow-strcpy caught this exact pattern. Configure it in CI to block PRs that introduce sprintf(), strcpy(), or strcat() on fixed buffers.
• GCC/Clang warnings: Compile with -Wformat-overflow=2 to get compile-time warnings about potential sprintf() overflows.
• AddressSanitizer (ASan): Build with -fsanitize=address during testing to catch overflows at runtime.

Coding Standards

• Follow CERT C Coding Standard STR31-C: "Guarantee that storage for strings has sufficient space for character data and the null terminator."
• Use snprintf() as the default for all string formatting in C. There is no valid reason to use sprintf() in modern code.

Key Takeaways

• sprintf() into char line[64] is a ticking time bomb — the format "%d,%s,%d\n" with an unbounded NAMES[] string can trivially exceed 64 bytes.
• snprintf(line, sizeof(line), ...) is a zero-cost fix that prevents stack corruption while preserving correct behavior for normal-length inputs.
• Benchmark code is still production code — bench/strbuild/strbuild.c ships with the package and runs in the same process as application code.
• The len variable used in the subsequent loop (for (int j = 0; j < len; j++)) now safely reflects the truncated length, preventing out-of-bounds reads as well.
• Static analysis in CI catches these patterns before they reach production — this was detected by Semgrep's utils.custom.buffer-overflow-strcpy rule automatically.

How Orbis AppSec Detected This

• Source: Data entering through the NAMES[c[i]] lookup and integer values v[i] + it, which are derived from function parameters c and v passed into run_kernel().
• Sink: sprintf(line, "%d,%s,%d\n", i, NAMES[c[i]], (int)(v[i] + it)) at bench/strbuild/strbuild.c:42 — an unbounded write into a 64-byte stack buffer.
• Missing control: No size limit on the sprintf() write operation; the destination buffer size is never passed to the formatting function.
• CWE: CWE-120 (Buffer Copy without Checking Size of Input)
• Fix: Replaced sprintf(line, ...) with snprintf(line, sizeof(line), ...) to enforce a 64-byte maximum write.

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 via sprintf() remain one of the most common and dangerous vulnerabilities in C code — even in 2024. This specific instance in bench/strbuild/strbuild.c demonstrates how a seemingly harmless benchmark utility can harbor a high-severity stack corruption bug. The fix — a one-line change from sprintf to snprintf with sizeof(line) — is trivial to implement, has zero performance cost, and completely eliminates the overflow risk. If you write C code, make snprintf() your default. There is no excuse for unbounded string formatting in production codebases.

References

• CWE-120: Buffer Copy without Checking Size of Input
• OWASP Buffer Overflow
• snprintf() — Linux man page
• CERT C STR31-C
• Semgrep C buffer overflow rules
• fix: use bounded strlcpy/snprintf in strbuild.c...