How unbounded strcpy() causes heap buffer overflow in C NGINX modules and how to fix it
Introduction
The 51Degrees_module/ngx_http_51D_module.c file is the heart of the 51Degrees device detection NGINX module — it reads incoming HTTP request headers, extracts evidence for device fingerprinting, and populates response headers with detection results. But buried across four separate locations in this file, the same dangerous pattern repeated itself: HTTP header names provided by the outside world were being copied into fixed-size heap buffers using strcpy(), with zero validation of the input length.
Any unauthenticated attacker who can send an HTTP request to a server running this module could exploit this to corrupt heap memory — and potentially achieve remote code execution.
The Vulnerability Explained
What goes wrong with strcpy()
strcpy(dst, src) copies bytes from src to dst until it hits a null terminator. It does not know — and does not care — how large dst is. If src is longer than the allocated destination buffer, the copy continues past the end of that buffer, overwriting whatever memory comes next on the heap.
In ngx_http_51D_module.c, the vulnerable pattern appeared in the initRespHeaders() function at line 1303:
// VULNERABLE — before the fix
strcpy((char *)headerNames[respHeaderCount].data, headerName);
Here, headerNames[respHeaderCount].data is a buffer allocated based on an expected header name length. The variable headerName is derived from HTTP request data. If an attacker sends a request with a header name longer than what was allocated, strcpy() blows straight past the end of the buffer.
The same pattern appeared three more times:
Line 1389 — copying a property name into a response header struct:
// VULNERABLE
strcpy((char *)respHeader->property[respHeader->propertyCount]->data,
propertyName);
Line 1870 — inside get_evidence(), copying a header name for query string evidence lookup:
// VULNERABLE
strcpy((char *)ngxHeaderName.data, headerName);
The add_value() function — using strncat() incorrectly:
// VULNERABLE — strncat length is not adjusted for existing content
strncat(dst, delimiter, length);
length -= strlen(delimiter);
strncat(dst, val, length);
The strncat() call passes length as the maximum number of characters to append, but length represents the total buffer size, not the remaining space after existing content. If dst already contains data, this calculation is wrong and a write past the buffer end is possible.
How an attacker exploits this
The attack surface is straightforward: HTTP header names. An attacker sends a crafted request like:
GET / HTTP/1.1
Host: target.example.com
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA...[thousands of A's]: value
When NGINX routes this request through the 51Degrees module, initRespHeaders() processes the header name, allocates a buffer for it, and then calls strcpy() to copy the full (oversized) name into that undersized buffer. The excess bytes overwrite adjacent heap chunks — potentially corrupting allocator metadata, function pointers, or other live data structures.
On modern systems with heap hardening (ASLR, heap canaries), exploitation requires additional steps, but the memory corruption itself is reliable and deterministic. In worst-case scenarios on older or embedded deployments, this is a direct path to remote code execution from an unauthenticated HTTP request.
The Fix
The fix replaces every unsafe string operation with NGINX's own bounded memory functions, which require an explicit length argument and cannot write beyond the specified limit.
Fix 1 — initRespHeaders() at line 1303
// BEFORE
strcpy((char *)headerNames[respHeaderCount].data, headerName);
// AFTER
ngx_memcpy(headerNames[respHeaderCount].data, headerName, headerNames[respHeaderCount].len + 1);
ngx_memcpy is a thin wrapper around memcpy that copies exactly n bytes. The length used is headerNames[respHeaderCount].len + 1 — the pre-computed length of the header name (already stored in the struct) plus one for the null terminator. The copy cannot exceed what was allocated.
Fix 2 — initRespHeaders() at line 1389
// BEFORE
strcpy((char *)respHeader->property[respHeader->propertyCount]->data,
propertyName);
// AFTER
ngx_memcpy(respHeader->property[respHeader->propertyCount]->data,
propertyName, len + 1);
Same pattern: the already-computed len variable (used earlier to allocate the buffer) is now passed explicitly to bound the copy.
Fix 3 — get_evidence() at line 1870
// BEFORE
strcpy((char *)ngxHeaderName.data, headerName);
// AFTER
ngx_memcpy(ngxHeaderName.data, headerName, ngxHeaderName.len + 1);
Again, ngxHeaderName.len was already set before this line — the fix simply uses it to constrain the copy.
Fix 4 — add_value() function
// BEFORE
strncat(dst, delimiter, length);
length -= strlen(delimiter);
strncat(dst, val, length);
// AFTER
ngx_cpystrn((u_char *)(dst + ngx_strlen(dst)), (u_char *)delimiter, length + 1);
length -= ngx_strlen(delimiter);
ngx_cpystrn((u_char *)(dst + ngx_strlen(dst)), (u_char *)val, length + 1);
ngx_cpystrn(dst, src, n) copies at most n-1 characters and always null-terminates — it behaves like a correct strncpy. By computing the current end of dst with ngx_strlen(dst) and passing length + 1 as the limit, the function now correctly appends into only the remaining space in the buffer, regardless of how much content is already there.
Bonus fix — strtok_r safety
// BEFORE
char *tok, *tokPos = NULL;
// AFTER
char *tok, *tokPos = NULL, *saveptr = NULL;
A saveptr variable was added for thread-safe tokenization, a small but important correctness improvement alongside the primary fix.
Prevention & Best Practices
1. Never use strcpy() with externally-controlled input
In C, strcpy() should be treated as deprecated for any data that crosses a trust boundary. Replace it with:
| Unsafe | Safe alternative |
|---|---|
strcpy(dst, src) |
ngx_memcpy(dst, src, known_len + 1) or strncpy with correct size |
strcat(dst, src) |
ngx_cpystrn(dst + ngx_strlen(dst), src, remaining + 1) |
strncat(dst, src, n) |
Compute remaining space explicitly before calling |
sprintf(dst, ...) |
ngx_snprintf(dst, size, ...) |
2. Use NGINX's string API throughout NGINX modules
NGINX provides a full set of safe string primitives: ngx_memcpy, ngx_cpystrn, ngx_snprintf, ngx_strncasecmp. These functions are designed for use in NGINX's memory model and always require explicit size parameters. Prefer them over raw C library functions in any NGINX module code.
3. Store computed lengths and use them
A key pattern in this fix is that the buffer lengths were already computed and stored in struct fields (headerNames[i].len, ngxHeaderName.len) — they just weren't being used to bound the copy. Make it a habit: if you allocate a buffer of size n, pass n to every function that writes into it.
4. Enable compiler and linker hardening
- Compile with
-D_FORTIFY_SOURCE=2to enable glibc buffer overflow detection for common functions - Use
-fstack-protector-strongfor stack canaries - Link with
-Wl,-z,relro,-z,nowfor read-only relocations - Enable AddressSanitizer (
-fsanitize=address) in CI/test builds to catch overflows at runtime
5. Use static analysis in your CI pipeline
Tools that catch this class of vulnerability:
- Semgrep: Rules for
strcpywith tainted input — https://semgrep.dev/r?q=strcpy - CodeQL:
cpp/unbounded-writequery - Coverity: BUFFER_SIZE_WARNING, STRING_OVERFLOW detectors
- Clang Static Analyzer:
alpha.security.taint.TaintPropagation
Security standards
- CWE-122: Heap-based Buffer Overflow
- CWE-119: Improper Restriction of Operations within the Bounds of a Memory Buffer
- OWASP: Buffer Overflow
Key Takeaways
strcpy()with HTTP header names is always dangerous: Header names inngx_http_51D_module.ccame directly from attacker-controlled HTTP requests. Any unbounded copy of this data is a critical vulnerability.- Pre-computed lengths must be used at copy time: The struct fields
headerNames[i].lenandngxHeaderName.lenexisted and held correct values — the bug was that they were ignored during thestrcpy()call. strncat()is not a safe drop-in forstrcat(): Theadd_value()function shows a classicstrncatmisuse where the length parameter doesn't account for existing buffer content.ngx_cpystrnwith a pointer to the current end of the string is the correct pattern.- NGINX modules should use NGINX's string API: Mixing raw C library string functions with NGINX's memory pool model creates mismatches that are hard to audit.
ngx_memcpy,ngx_cpystrn, andngx_snprintfexist precisely for this. - Four instances of the same bug in one file signals a copy-paste pattern: When you fix one
strcpy()in a codebase, search for all others in the same file — they likely share the same root cause and the same fix.
How Orbis AppSec Detected This
- Source: HTTP request header names processed by the 51Degrees NGINX module — fully attacker-controlled, unauthenticated input
- Sink:
strcpy((char *)headerNames[respHeaderCount].data, headerName)at line 1303 in51Degrees_module/ngx_http_51D_module.c, and three additionalstrcpy()/strncat()call sites at lines 1389, 1870, and inadd_value() - Missing control: No length validation or bounds check before copying the user-supplied header name string into the fixed-size allocated buffer
- CWE: CWE-122 — Heap-based Buffer Overflow
- Fix: All four unsafe
strcpy()/strncat()calls were replaced withngx_memcpy()andngx_cpystrn()using the pre-computed buffer lengths already available in the surrounding code
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
Four lines of strcpy() in a single C file were enough to expose every server running the 51Degrees NGINX module to unauthenticated heap corruption. The root cause wasn't complex — it was the classic C mistake of trusting that a source string will fit in its destination. The fix was equally straightforward: use the lengths that were already computed and pass them to bounded copy functions.
For developers writing or reviewing C code in NGINX modules, the lesson is concrete: treat strcpy() as a red flag in code reviews, use NGINX's own string API consistently, and run static analysis tools that specifically target unbounded writes with tainted input. Memory safety bugs at the HTTP parsing layer are among the most dangerous a web server can have — they're reachable before any authentication, from any network client, with a single malformed request.
References
- CWE-122: Heap-based Buffer Overflow
- CWE-119: Improper Restriction of Operations within the Bounds of a Memory Buffer
- OWASP Buffer Overflow Attack
- OWASP Input Validation Cheat Sheet
- NGINX Development Guide — String Operations
- Semgrep rules for strcpy
- fix: the module uses unbounded strcpy() at four loca... in...