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.

Key Takeaways

• strcpy() with HTTP header names is always dangerous: Header names in ngx_http_51D_module.c came 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].len and ngxHeaderName.len existed and held correct values — the bug was that they were ignored during the strcpy() call.
• strncat() is not a safe drop-in for strcat(): The add_value() function shows a classic strncat misuse where the length parameter doesn't account for existing buffer content. ngx_cpystrn with 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, and ngx_snprintf exist 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 in 51Degrees_module/ngx_http_51D_module.c, and three additional strcpy()/strncat() call sites at lines 1389, 1870, and in add_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 with ngx_memcpy() and ngx_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.