How buffer overflow via sprintf() happens in C networking code and how to fix it

How buffer overflow via sprintf() happens in C networking code and how to fix it

Introduction

The profile.c file handles server initialization and iteration logic for a networked application, formatting IP address strings like "127.0.0.1:%d" into fixed-size character buffers. At line 99, inside profile_initialize(), and again at line 220, inside profile_iteration(), the code used sprintf() — a function notorious for ignoring buffer boundaries — to write these formatted addresses. This created two high-severity buffer overflow opportunities that a static analysis scanner (Semgrep rule utils.custom.buffer-overflow-strcpy) correctly flagged.

While this is a local CLI tool where exploitation requires controlling command-line arguments or input files, the vulnerability pattern is dangerous and commonly exploited in production systems. The fix demonstrates a clean, minimal change that every C developer should internalize.

The Vulnerability Explained

Here's the vulnerable code at line 99 in profile_initialize():

sprintf( server_address, "127.0.0.1:%d", SERVER_BASE_PORT + i );

And the second instance at line 220 in profile_iteration():

sprintf( server_address[num_server_addresses], "127.0.0.1:%d", SERVER_BASE_PORT + j );

The sprintf() function writes its formatted output into server_address without any knowledge of how large that buffer actually is. It will keep writing bytes until the format string is fully expanded — regardless of whether the destination can hold the result.

Why is this dangerous?

Consider what happens if SERVER_BASE_PORT + i produces an unexpectedly large integer value. The format "127.0.0.1:%d" with a normal port like 40000 produces a 15-character string plus null terminator. But %d can expand to up to 11 characters for a 32-bit signed integer (e.g., -2147483648), making the maximum output "127.0.0.1:-2147483648" — 22 characters plus null.

If server_address is declared with a size that doesn't account for edge cases, or if integer overflow occurs in the port calculation (SERVER_BASE_PORT + i wrapping around), the formatted string could exceed the buffer. This overwrites adjacent stack or heap memory, potentially corrupting:

• Function return addresses (enabling code execution)
• Adjacent variables (altering program logic)
• Heap metadata (enabling heap exploitation)

Attack Scenario Specific to This Code

An attacker who can influence SERVER_BASE_PORT (e.g., through a configuration file or environment variable) could set it to a value near INT_MAX. When the loop variable i is added, integer overflow produces a large negative number. The %d format specifier then writes more characters than expected into server_address, overflowing the buffer and potentially hijacking the subsequent netcode_server_create() call or corrupting the server_config structure on the stack.

The Fix

The fix is surgical and effective — replacing unbounded sprintf() with bounded snprintf() at both locations:

Line 99 — profile_initialize():

Before:

sprintf( server_address, "127.0.0.1:%d", SERVER_BASE_PORT + i );

After:

snprintf( server_address, sizeof( server_address ), "127.0.0.1:%d", SERVER_BASE_PORT + i );

Line 220 — profile_iteration():

Before:

sprintf( server_address[num_server_addresses], "127.0.0.1:%d", SERVER_BASE_PORT + j );

After:

snprintf( server_address[num_server_addresses], 256, "127.0.0.1:%d", SERVER_BASE_PORT + j );

Why This Works

snprintf() takes an explicit size parameter (the second argument) that caps the number of bytes written, including the null terminator. If the formatted output would exceed the buffer:

1. The output is truncated to fit within the specified size
2. The buffer is always null-terminated (unlike strncpy)
3. No memory beyond the buffer boundary is touched

In the first fix, sizeof(server_address) automatically captures the declared array size, making it maintenance-friendly — if the buffer size changes, the bound updates automatically. In the second fix, the explicit 256 matches the size used in the Windows sprintf_s() path on the preceding line, maintaining consistency.

Prevention & Best Practices

1. Ban sprintf() in your codebase: Use compiler warnings (-Wformat-overflow) or linter rules to flag any sprintf() usage. There is almost never a reason to use it over snprintf().

2. Prefer sizeof() over magic numbers: The first fix uses sizeof(server_address) which is self-documenting and resilient to buffer size changes. Use this pattern whenever the buffer is a local array.

3. Check snprintf() return value: While not done in this fix (the format is predictable), best practice is to check whether snprintf() returned a value >= the buffer size, indicating truncation occurred.

4. Use _s suffixed functions on Windows: The code already uses sprintf_s() for MSVC > 1600. The non-Windows path now has equivalent protection with snprintf().

5. Static analysis in CI: Integrate Semgrep or similar tools to catch these patterns before they reach production. The rule utils.custom.buffer-overflow-strcpy caught both instances.

Key Takeaways

• sprintf() in profile_initialize() had no size guard — even though the adjacent Windows codepath (sprintf_s) already used bounded formatting, the non-Windows path was unprotected.
• Two instances in the same file (lines 99 and 220) shared the same vulnerability pattern — when you find one sprintf(), search the entire file for more.
• sizeof(server_address) is superior to hardcoded sizes when the buffer is a stack-allocated array, as demonstrated in the line 99 fix.
• Integer arithmetic in format arguments (SERVER_BASE_PORT + i) can produce unexpectedly long decimal representations, making bounds checking essential even for "simple" format strings.
• The Windows and non-Windows codepaths had different safety levels — cross-platform code must ensure all branches receive equivalent security treatment.

How Orbis AppSec Detected This

• Source: The SERVER_BASE_PORT + i expression and loop variable i/j, which determine the formatted output length
• Sink: sprintf(server_address, ...) at profile.c:99 and sprintf(server_address[num_server_addresses], ...) at profile.c:220
• Missing control: No buffer size limit passed to the formatting function — sprintf() has no size parameter
• CWE: CWE-120 (Buffer Copy without Checking Size of Input)
• Fix: Replaced sprintf() with snprintf() using explicit buffer size bounds (sizeof(server_address) and 256) at both call sites

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 vulnerability is a textbook example of why sprintf() should be considered deprecated in modern C development. The fix — a one-line change from sprintf() to snprintf() with an explicit size — is trivial to implement but prevents an entire class of memory corruption attacks. The fact that the Windows codepath already used sprintf_s() while the non-Windows path remained unprotected highlights how easy it is for security gaps to hide in conditional compilation blocks. Always audit all branches, not just the one you're testing on.

References

• CWE-120: Buffer Copy without Checking Size of Input
• OWASP Buffer Overflow Prevention Cheat Sheet
• snprintf() — POSIX man page
• Semgrep C buffer overflow rules
• fix: use bounded strlcpy/snprintf in profile.c...