How buffer overflow happens in C libficus.c sprintf() and how to fix it

How Buffer Overflow Happens in C libficus.c sprintf() and How to Fix It

Introduction

The runtime/ficus/impl/libficus.c file is responsible for exposing runtime introspection utilities in the Ficus language runtime — including fx_cc_version(), a function that reads compiler version macros and formats them into a human-readable string. It sounds mundane. But buried inside that function were two calls to sprintf() writing into fixed-size stack buffers with no length limit, creating a classic stack buffer overflow condition that could corrupt adjacent memory.

This post walks through exactly what went wrong, how the fix works, and what every C developer should take away from this pattern.

The Vulnerability Explained

What the code was doing

Inside fx_cc_version(), the code detects the compiler at build time using preprocessor macros and formats a version string. Here is the vulnerable section for the MSVC path:

// Vulnerable code — runtime/ficus/impl/libficus.c:192
char cver[64];
int major = fullver / 10000000;
int minor = (fullver % 10000000) / 100000;
int build = fullver % 100000;
sprintf(cver, "Microsoft MSVC %d.%02d.%05d.%d", major, minor, build, revision);

And the equivalent for the C standard path at line 196:

// Vulnerable code — runtime/ficus/impl/libficus.c:196
char cver[128];
int stdc_ver = (int)__STDC_VERSION__;
sprintf(cver, "ISO %d.%d-complaint C compiler", (int)stdc_ver/100, (int)stdc_ver%100);

Why this is dangerous

sprintf() is a "write until done" function. It does not know — and does not care — how large the destination buffer is. It will keep writing bytes until the format string is fully consumed, regardless of whether it has overrun the end of cver.

For the MSVC path, cver is declared as char cver[64]. The format string "Microsoft MSVC %d.%02d.%05d.%d" with four integer substitutions can theoretically produce a string far longer than 64 bytes if the integer values are unexpectedly large. In a normal build, _MSC_VER and _MSC_FULL_VER are compile-time constants with well-known ranges — but that assumption is the entire problem.

The attack scenario

An attacker with control over the build environment (e.g., a supply chain attacker who has modified build tooling, a malicious CI/CD pipeline step, or a developer workstation compromise) could manipulate the _MSC_VER or _MSC_FULL_VER macro definitions to produce extreme integer values:

// Attacker-controlled build environment:
#define _MSC_VER 999999
#define _MSC_FULL_VER 99999999999

With values like these, the formatted string "Microsoft MSVC 9999.99.99999.9999999" would blow past the 64-byte cver buffer, overwriting the stack frame. On a stack that includes a return address, this is a textbook path to arbitrary code execution. Even without a full exploit chain, the memory corruption alone can cause unpredictable crashes in production tooling.

The PR's exploitation scenario confirms this: "An attacker with control over the build environment could potentially manipulate compiler version macros (_MSC_VER, _MSC_FULL_VER) to generate large integer values, causing the formatted string to [overflow the buffer]."

The Fix

The fix is surgical and correct: both sprintf() calls are replaced with snprintf(), with sizeof(cver) passed as the maximum number of bytes to write.

Before and After

Before (vulnerable):

// Line 192 — no length limit, can overflow cver[64]
sprintf(cver, "Microsoft MSVC %d.%02d.%05d.%d", major, minor, build, revision);

// Line 196 — no length limit, can overflow cver[128]
sprintf(cver, "ISO %d.%d-complaint C compiler", (int)stdc_ver/100, (int)stdc_ver%100);

After (fixed):

// Line 192 — write is bounded to sizeof(cver) = 64 bytes
snprintf(cver, sizeof(cver), "Microsoft MSVC %d.%02d.%05d.%d", major, minor, build, revision);

// Line 196 — write is bounded to sizeof(cver) = 128 bytes
snprintf(cver, sizeof(cver), "ISO %d.%d-complaint C compiler", (int)stdc_ver/100, (int)stdc_ver%100);

Why this works

snprintf() accepts a second argument — the maximum number of bytes to write, including the null terminator. If the formatted output would exceed that limit, snprintf() truncates the string and always null-terminates the buffer. The stack memory beyond cver is never touched.

Using sizeof(cver) rather than a hardcoded number is the correct idiom because:
1. It stays automatically correct if the buffer size is ever changed.
2. It eliminates the possibility of a typo (e.g., writing 63 instead of 64).
3. It is self-documenting — the intent is explicit.

The fix also addresses both vulnerable call sites in the same function, which is important. The PR note flags that line 196 uses the same pattern as line 192, and both are corrected in the same diff.

Prevention & Best Practices

1. Ban sprintf() from your C codebase

There is almost no legitimate use case for sprintf() in modern C code. Establish a linter rule or compiler warning that flags it:

# With GCC/Clang, enable -Wformat warnings
gcc -Wall -Wformat -Wformat-overflow ...

Clang's -Wformat-overflow will warn when it can statically determine that a sprintf() call may overflow a buffer.

2. Always use snprintf() with sizeof(buf)

// Correct pattern — always do this
char buf[64];
snprintf(buf, sizeof(buf), "format %d", value);

// Also check the return value if truncation matters
int written = snprintf(buf, sizeof(buf), "format %d", value);
if (written >= (int)sizeof(buf)) {
    // String was truncated — handle this case
}

3. Consider safer string libraries

For projects where truncation is unacceptable, consider:
- asprintf() (GNU extension): dynamically allocates the buffer to fit the output.
- strlcpy() / strlcat() (BSD): size-bounded string operations with explicit truncation semantics.
- Modern C++ std::string or std::format if the codebase can adopt C++.

4. Use static analysis in CI

Tools that catch this class of vulnerability automatically:
- Semgrep: semgrep.dev/r?q=sprintf has rules flagging unsafe sprintf() usage.
- Coverity and CodeQL: both model buffer sizes and flag unbounded writes.
- AddressSanitizer (ASan): compile with -fsanitize=address during testing to catch overflows at runtime.

5. Apply the security invariant as a code review checklist item

The PR defines a clear security invariant: "Buffer reads never exceed the declared length." Make this an explicit checklist item in your code review process for any C code that handles string formatting.

Key Takeaways

• sprintf() into a fixed-size stack buffer is always a latent buffer overflow — even when the inputs look safe today, future changes or build environment manipulation can trigger it. Both instances in fx_cc_version() were vulnerable.
• snprintf(buf, sizeof(buf), ...) is the correct replacement — the sizeof() idiom ties the size limit directly to the buffer declaration and stays correct through refactoring.
• Build-time constants are not immune to attacker influence — supply chain attacks and build environment compromises can manipulate compiler macros, making "these values are always small" an unsafe assumption.
• The same vulnerable pattern appeared twice in the same function — when you find one unsafe sprintf() call, always scan the surrounding code for siblings. The PR correctly fixed both lines 192 and 196.
• Static analysis tools can reliably detect this pattern — the Orbis AppSec scanner flagged this automatically, demonstrating that sprintf() misuse is a solved detection problem.

How Orbis AppSec Detected This

• Source: Compiler version integer values derived from _MSC_VER and _MSC_FULL_VER preprocessor macros, which can be influenced by the build environment.
• Sink: sprintf(cver, "Microsoft MSVC %d.%02d.%05d.%d", ...) at runtime/ficus/impl/libficus.c:192 and sprintf(cver, "ISO %d.%d-complaint C compiler", ...) at runtime/ficus/impl/libficus.c:196, writing into fixed-size stack buffers cver[64] and cver[128] respectively.
• Missing control: No maximum write length was specified, so sprintf() could write past the end of either buffer without any bounds check.
• CWE: CWE-120 — Buffer Copy without Checking Size of Input ("Classic Buffer Overflow").
• Fix: Both sprintf() calls were replaced with snprintf(cver, sizeof(cver), ...), bounding all writes to the declared buffer 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

The sprintf() → snprintf() migration in libficus.c is a small diff with meaningful security impact. The vulnerable code trusted that compiler version macros would always produce short strings — a reasonable assumption in a normal build, but an assumption that collapses under adversarial build conditions. By switching to snprintf(cver, sizeof(cver), ...) at both call sites, the fix enforces a hard boundary that makes the buffer overflow impossible regardless of the input values.

For C developers: treat every sprintf() call as a code smell. The function has no safe use case that snprintf() does not cover better. Make the switch, use sizeof() to tie the limit to the buffer declaration, and let static analysis enforce the invariant automatically.

References

• CWE-120: Buffer Copy without Checking Size of Input
• CWE-121: Stack-based Buffer Overflow
• OWASP Buffer Overflow Cheat Sheet
• Linux man page: snprintf(3)
• Semgrep rules for sprintf
• fix: add buffer-length check in libficus.c