How insecure string copy functions happen in C and how to fix them

How Insecure String Copy Functions Happen in C and How to Fix Them

The File That Handles Your VNC Password

The login/main.c file is responsible for one of the most sensitive operations a program can perform: authenticating a user by reading a stored VNC password from disk. To build the path to that password file, the code needed to combine the user's home directory with the relative path /.vnc/vnc_password. A seemingly simple task — but the way it was implemented introduced a high-severity memory safety vulnerability on line 11.

The Vulnerability Explained

Here is the vulnerable code that Semgrep flagged:

// BEFORE — vulnerable code (login/main.c, lines 11–12)
char pwd_file_name[512] = {0};
strcpy(pwd_file_name, getenv("HOME"));
strcat(pwd_file_name, "/.vnc/vnc_password");

What's Wrong Here?

strcpy copies bytes from its source into the destination buffer and stops only when it hits a null terminator (\0) in the source string. It does not know how large the destination buffer is. If the source string is longer than the destination, strcpy happily writes past the end of the buffer — a classic stack buffer overflow.

In this specific case, the source is getenv("HOME") — the value of the HOME environment variable. On a typical Linux system this might be /home/alice (10 bytes), well within the 512-byte pwd_file_name buffer. But environment variables are user-controlled data. There is no operating-system guarantee that HOME is short.

Consider what happens when an attacker (or a misconfigured environment) sets:

export HOME=$(python3 -c "print('A' * 600)")

Now getenv("HOME") returns a 600-character string. strcpy will write all 600 bytes into pwd_file_name, overflowing the 512-byte stack buffer by 88 bytes, overwriting the stack frame — potentially including the saved return address.

The follow-up strcat call compounds the problem: even if HOME were exactly 512 bytes, strcat would then append /.vnc/vnc_password (19 bytes) further past the end of the buffer.

Why This Matters for a Login Component

This is not an abstract theoretical risk. A login binary often runs with elevated privileges (e.g., setuid root or as a system service). A stack buffer overflow in a privileged login process is a classic privilege escalation primitive. Even if the overflow is not directly exploitable today due to stack canaries or ASLR, it constitutes an exploit primitive — a code pattern that automated exploit-development tooling can chain with other weaknesses to achieve reliable exploitation.

The Fix

The fix replaces the two-step strcpy/strcat pattern with a single, bounds-enforcing snprintf call:

// AFTER — hardened code (login/main.c, line 11)
snprintf(pwd_file_name, sizeof(pwd_file_name), "%s/.vnc/vnc_password", getenv("HOME"));

Before vs. After

Why snprintf Solves This Specific Problem

snprintf(dest, n, fmt, ...) writes at most n - 1 characters into dest and always appends a null terminator, regardless of how long the formatted result would be. By passing sizeof(pwd_file_name) — which the compiler evaluates at compile time as 512 — the call can never write beyond the boundary of the stack buffer.

If HOME is pathologically long, snprintf will truncate the result to 511 characters and null-terminate it. The program will then attempt to open a truncated path, fail gracefully with a file-not-found error, and deny login — a safe failure mode rather than a memory corruption event.

Using sizeof(pwd_file_name) instead of a hardcoded 512 is also a defensive maintenance practice: if the buffer size is ever changed, the snprintf limit updates automatically.

Prevention & Best Practices

1. Ban strcpy and strcat in New Code

Treat strcpy, strcat, gets, and sprintf as deprecated. Most modern C codebases enforce this via compiler warnings (-Wdeprecated-declarations) or static analysis rules.

// Never use these for untrusted or variable-length input:
strcpy(dst, src);          // no bounds check
strcat(dst, src);          // no bounds check
sprintf(dst, fmt, ...);    // no bounds check
gets(buf);                 // removed from C11 entirely

2. Use Bounded Alternatives

Note: strcpy_s is part of Annex K of C11 but is optional and not widely available on Linux. snprintf is the most portable safe choice.

3. Validate Environment Variables Before Use

When your program depends on environment variables like HOME, validate them early:

const char *home = getenv("HOME");
if (home == NULL) {
    fprintf(stderr, "HOME environment variable not set\n");
    exit(EXIT_FAILURE);
}
if (strlen(home) > 480) {  // leave room for the suffix
    fprintf(stderr, "HOME path too long\n");
    exit(EXIT_FAILURE);
}

4. Enable Compiler Hardening Flags

gcc -Wall -Wextra -Wformat-security \
    -fstack-protector-strong \
    -D_FORTIFY_SOURCE=2 \
    -pie -fPIE \
    login/main.c -o login

-D_FORTIFY_SOURCE=2 causes glibc to replace strcpy with a version that checks buffer sizes at runtime when the destination size is known at compile time.

5. Run Static Analysis in CI

The Semgrep rule that caught this issue (c.lang.security.insecure-use-string-copy-fn) is freely available and can be integrated into any CI pipeline:

# .github/workflows/semgrep.yml
- name: Semgrep scan
  run: semgrep --config "p/c" login/main.c

Relevant Standards

• CWE-120: Buffer Copy without Checking Size of Input
• OWASP: Input Validation Cheat Sheet
• SEI CERT C: STR31-C — Guarantee that storage for strings has sufficient space for character data and the null terminator

Key Takeaways

• getenv() returns attacker-controlled data — the HOME environment variable in login/main.c can be set to an arbitrarily long string before the process launches, making it untrusted input that must be bounded before copying.
• Two-step strcpy/strcat is doubly dangerous — even if strcpy didn't overflow, the subsequent strcat(pwd_file_name, "/.vnc/vnc_password") could push the buffer over its limit on its own.
• snprintf with sizeof() is the portable C fix — it enforces the limit, guarantees null termination, and collapses two unsafe calls into one safe one.
• Login code warrants extra scrutiny — a buffer overflow in a privileged authentication binary is a direct path to privilege escalation; the severity of memory safety bugs scales with the privilege level of the process.
• Semgrep's insecure-use-string-copy-fn rule catches this pattern automatically — integrating it into CI would have flagged this before it ever reached production.

How Orbis AppSec Detected This

• Source: The HOME environment variable read via getenv("HOME") at login/main.c:11 — an externally controlled value with no length constraint.
• Sink: strcpy(pwd_file_name, getenv("HOME")) at login/main.c:11, copying into the 512-byte stack buffer pwd_file_name with no size argument.
• Missing control: No length validation of the HOME value before the copy, and no size bound passed to strcpy.
• CWE: CWE-120 — Buffer Copy without Checking Size of Input ('Classic Buffer Overflow').
• Fix: Replaced strcpy(pwd_file_name, getenv("HOME")) + strcat(pwd_file_name, "/.vnc/vnc_password") with snprintf(pwd_file_name, sizeof(pwd_file_name), "%s/.vnc/vnc_password", getenv("HOME")), enforcing a hard 512-byte limit and guaranteeing null termination.

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

A single strcpy call on line 11 of login/main.c turned an environment variable into a potential stack buffer overflow in a security-critical login component. The root cause is one of the oldest mistakes in C programming: trusting that external data will fit in a fixed-size buffer without checking. The fix is equally straightforward — snprintf with an explicit size bound closes the overflow entirely in one line of code.

Memory safety bugs in C don't announce themselves at compile time. They require deliberate use of bounded string functions, compiler hardening flags, and automated static analysis to catch before they reach production. In a login binary — where the stakes of exploitation are highest — there is no acceptable reason to use strcpy on user-controlled input.

References

• CWE-120: Buffer Copy without Checking Size of Input
• OWASP Input Validation Cheat Sheet
• SEI CERT C Coding Standard: STR31-C
• snprintf — C standard library reference (cppreference.com)
• Semgrep rule: insecure-use-string-copy-fn
• harden: use bounded strlcpy/snprintf in main.c...