How heap buffer overflow happens in C dupstr() and how to fix it

How heap buffer overflow happens in C dupstr() and how to fix it

Introduction

The tools/strliteral.c file is a utility used during the build process to convert raw string literals into C identifiers and escaped representations. It's the kind of file that rarely gets a second look — small, unglamorous, and seemingly low-risk. But buried inside its dupstr() function at line 19 was a critical heap buffer overflow: strcpy() was called on the return value of malloc() without ever checking whether the allocation had actually succeeded.

This is a deceptively simple mistake that has caused real-world exploits for decades. The C standard is explicit: if malloc() fails, it returns NULL. Writing through a NULL pointer is undefined behavior — and in practice, on modern systems, it can corrupt heap metadata, crash the process, or in adversarial conditions, enable an attacker to redirect control flow.

The Vulnerability Explained

Here is the vulnerable version of dupstr():

static char *dupstr(char *str) {
  size_t len = strlen(str);
  char *dup = malloc(len + 1);
  strcpy(dup, str);   // ← line 22: no NULL check before this write
  dup[len] = '\0';
  return dup;
}

The logic looks reasonable at first glance: calculate the string length, allocate exactly enough space, copy the string in, null-terminate it, return the pointer. The problem is in the ordering and the absence of a guard.

What happens when malloc() fails?

malloc() returns NULL when the system cannot satisfy the allocation request — under memory pressure, in resource-constrained environments, or when a fuzzer or attacker deliberately exhausts the heap. The code does not check for this. It passes NULL directly to strcpy():

strcpy(NULL, str);  // undefined behavior — writes to address 0x0

On most modern operating systems, address 0x0 is not mapped, so this immediately segfaults. But heap allocators are complex. Depending on the allocator implementation (glibc, jemalloc, tcmalloc), a NULL or near-NULL write can silently corrupt allocator bookkeeping structures — free-list pointers, chunk size fields, or canary values — setting up a classic heap exploitation primitive.

The redundant null-termination compounds the issue

Notice that after strcpy(dup, str), there is a second line:

dup[len] = '\0';

strcpy() already writes the null terminator. This line is redundant — and it also dereferences dup a second time without a NULL check, doubling the exposure.

Exploitation scenario

An attacker who controls the input to strliteral.c (via command-line arguments or a crafted input file processed during the build) could provide a string designed to trigger a malloc() failure — for example, by supplying an extremely long string that exhausts available heap memory before dupstr() is called. The resulting strcpy(NULL, ...) corrupts heap metadata. In a more sophisticated scenario targeting a persistent process, this could be chained into arbitrary code execution by overwriting a function pointer stored in a heap-adjacent allocation.

The Fix

The fix makes two targeted changes to dupstr():

Before:

static char *dupstr(char *str) {
  size_t len = strlen(str);
  char *dup = malloc(len + 1);
  strcpy(dup, str);
  dup[len] = '\0';
  return dup;
}

After:

static char *dupstr(char *str) {
  size_t len = strlen(str);
  char *dup = malloc(len + 1);
  if (dup == NULL) {
    return NULL;
  }
  memcpy(dup, str, len + 1);
  return dup;
}

Change 1: NULL guard after malloc()

if (dup == NULL) {
  return NULL;
}

This is the most critical change. Before touching dup for any write operation, the code now verifies that malloc() succeeded. If it did not, the function returns NULL immediately, propagating the allocation failure to the caller rather than writing through a null pointer. This is the standard CERT C pattern (MEM32-C: Detect and handle memory allocation errors).

Change 2: strcpy() replaced with memcpy()

memcpy(dup, str, len + 1);

strcpy() is replaced with memcpy(dup, str, len + 1). Since len was already computed as strlen(str), copying len + 1 bytes copies the string content plus its null terminator in a single, length-bounded operation. memcpy() does not scan for a null terminator and does not write beyond the specified count — it is strictly bounded by the third argument. The redundant dup[len] = '\0' line is also removed, since memcpy() already copies the terminator as part of the len + 1 byte range.

This change eliminates both the unbounded copy risk and the redundant write, making the function's memory behavior fully explicit and auditable.

Key Takeaways

• strcpy() after malloc() without a NULL check is always a bug — even in small utility functions like dupstr() that appear low-risk.
• The redundant dup[len] = '\0' after strcpy() was a code smell that signaled the author was uncertain about the copy behavior, which should prompt a closer review of the surrounding logic.
• memcpy() with a pre-computed length is the correct replacement for strcpy() when the string length is already known — it is bounded, explicit, and does not re-scan for the null terminator.
• Build-time tools are not exempt from security review — tools/strliteral.c is a production file processed during the build pipeline, and a compromised build tool can be as dangerous as a compromised runtime binary.
• A single missing NULL check can turn a correct-looking allocation into a heap corruption primitive — the fix is two lines, but the impact of the original bug was critical severity.

How Orbis AppSec Detected This

• Source: User-controlled string input passed to dupstr() via command-line arguments or input files processed by tools/strliteral.c
• Sink: strcpy(dup, str) at tools/strliteral.c:22, called on the unchecked return value of malloc(len + 1)
• Missing control: No NULL check on the malloc() return value before the strcpy() write; no bounds argument passed to the copy function
• CWE: CWE-122 — Heap-based Buffer Overflow
• Fix: Added a NULL guard immediately after malloc() and replaced strcpy() with memcpy(dup, str, len + 1) for a bounded, explicit copy

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 dupstr() vulnerability in tools/strliteral.c is a textbook example of how a two-line oversight — missing a NULL check and using an unbounded copy function — can elevate a utility function to a critical security risk. The fix is equally concise: guard the allocation, use memcpy() with an explicit length, and remove the redundant null-termination. These are not exotic techniques; they are foundational C memory-safety practices that every C developer should have in their muscle memory.

The broader lesson is that no file in a production codebase is too small or too peripheral to warrant security review. Build tools, code generators, and utility scripts all represent attack surface. Treat them accordingly.