How buffer overflow via strcpy() happens in C zlib and how to fix it

How buffer overflow via strcpy() happens in C zlib and how to fix it

Introduction

The general/libzlib/gzlib.c file handles gzip file operations—opening compressed files and reporting errors. But a flaw in the fallback code path (used when NO_snprintf or NO_vsnprintf is defined) created a critical security risk. At line 224, strcpy(state->path, path) copies a file path into a heap-allocated buffer without any bounds verification. Similarly, at lines 586–588, three consecutive strcpy() and strcat() calls construct an error message by concatenating state->path, ": ", and msg into state->msg—again with no overflow protection.

This matters because zlib is one of the most widely deployed compression libraries in existence. Any application linking against this code that allows user-influenced file paths or error strings is potentially vulnerable to heap corruption and arbitrary code execution.

The Vulnerability Explained

The vulnerable code exists in two functions within gzlib.c:

1. Path copy in gz_open() (line 224):

#else
    strcpy(state->path, path);
#endif

Here, state->path is allocated with len + 1 bytes (where len is calculated from the input path). While the allocation should match the path length, strcpy() provides no guarantee that it won't write beyond the buffer. If there's any discrepancy between the calculated len and the actual null-terminated string length of path—for instance, due to concurrent modification or type casting issues with the const void *path parameter—a buffer overflow occurs.

2. Error message construction in gz_error() (lines 586–588):

#else
    strcpy(state->msg, state->path);
    strcat(state->msg, ": ");
    strcat(state->msg, msg);
#endif

This is the more dangerous pattern. Three separate unbounded string operations build an error message by:
1. Copying state->path into state->msg
2. Appending the literal ": "
3. Appending the error message msg

Each strcat() call scans for the null terminator and appends data, with zero verification that the combined result fits in state->msg. An attacker who can influence either the file path or trigger specific error conditions with long messages can overflow this buffer.

Attack scenario: An attacker provides a crafted file path to a program using zlib (e.g., a web server processing user-uploaded gzip files, or a utility accepting file paths from command-line arguments). If the path is long enough and triggers an error condition in gz_error(), the concatenation of path + ": " + error message overflows state->msg, corrupting heap metadata. This enables heap exploitation techniques—potentially redirecting execution flow to attacker-controlled code.

The Fix

The fix replaces all unbounded strcpy()/strcat() calls with memcpy() operations that use pre-calculated lengths:

Before (line 224):

strcpy(state->path, path);

After:

memcpy(state->path, (const char *)path, len + 1);

The len variable is already computed earlier in the function, so memcpy() copies exactly len + 1 bytes (including the null terminator)—no more, no less. This eliminates any possibility of writing beyond the allocated buffer.

Before (lines 586–588):

strcpy(state->msg, state->path);
strcat(state->msg, ": ");
strcat(state->msg, msg);

After:

{
    size_t path_len = strlen(state->path);
    size_t msg_len = strlen(msg);
    memcpy(state->msg, state->path, path_len);
    memcpy(state->msg + path_len, ": ", 2);
    memcpy(state->msg + path_len + 2, msg, msg_len + 1);
}

This replacement:
1. Calculates exact lengths upfront with strlen()
2. Uses pointer arithmetic to position each memcpy() at the correct offset
3. Copies exactly the right number of bytes for each segment
4. Includes the null terminator only in the final memcpy() (msg_len + 1)

The key security improvement is that memcpy() never scans for terminators or appends blindly—it copies a precise number of bytes to a precise location. Combined with the buffer allocation (which uses strlen(state->path) + strlen(msg) + 3), this guarantees writes stay within bounds.

Prevention & Best Practices

1. Ban strcpy() and strcat() in new code. Use memcpy() with explicit lengths, snprintf(), or platform-specific safe alternatives like strlcpy()/strlcat() (BSD) or strcpy_s()/strcat_s() (C11 Annex K).

2. Compile with warnings enabled. GCC's -Wstringop-overflow and -Wstringop-truncation flags catch many unsafe string operations at compile time.

3. Use AddressSanitizer (ASan) in CI. Compile with -fsanitize=address to detect heap/stack buffer overflows during testing. The regression test included in this PR would immediately trigger ASan on the vulnerable code.

4. Audit fallback code paths. The vulnerable code only executed when NO_snprintf was defined—a less common configuration. Security bugs often hide in rarely-executed branches. Ensure all code paths receive equal scrutiny.

5. Prefer bounded APIs even when lengths are "known." Even if you've calculated the correct length, using memcpy() with that length is more robust than strcpy() because it creates a single source of truth for the copy size.

Key Takeaways

• strcpy() in gz_open() line 224 was unsafe despite the buffer being allocated to the correct size—using memcpy(state->path, (const char *)path, len + 1) makes the contract explicit and verifiable.
• Three consecutive strcpy()/strcat() calls in gz_error() are a classic concatenation overflow pattern—replacing them with offset-based memcpy() calls eliminates scanning for null terminators entirely.
• The #else branch (when NO_snprintf is defined) was the vulnerable path—the snprintf() branch was already safe, demonstrating how conditional compilation can create security asymmetries.
• Pre-calculating string lengths once and reusing them is both more secure (bounded copies) and more performant (avoids repeated strlen() scans by strcat()).
• zlib's ubiquity amplifies this vulnerability—any application linking this code on platforms without snprintf support inherits the buffer overflow risk.

How Orbis AppSec Detected This

• Source: File path string passed to gz_open() via the path parameter, and error message string msg passed to gz_error()
• Sink: strcpy(state->path, path) at gzlib.c:224 and strcpy(state->msg, state->path) / strcat(state->msg, msg) at gzlib.c:586-588
• Missing control: No bounds checking or length validation before string copy operations in the NO_snprintf fallback code path
• CWE: CWE-120 (Buffer Copy without Checking Size of Input)
• Fix: Replaced strcpy()/strcat() with memcpy() calls using pre-calculated string lengths to enforce buffer boundaries

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 from strcpy() and strcat() remain one of the most dangerous and exploitable vulnerability classes in C code. This specific instance in zlib's gzlib.c demonstrates how even well-maintained, widely-used libraries can harbor critical flaws in less-tested code paths. The fix is straightforward—replace unbounded string operations with memcpy() using explicit lengths—but the security impact is profound: it closes a heap corruption vector that could enable arbitrary code execution in any application linking this library.

When writing C code that handles strings, always prefer bounded operations. Calculate your lengths once, allocate accordingly, and copy with precision.

References

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