Spotting unsafe strcpy in code

Overview

Security objective. The asset you are protecting is the integrity of program memory — specifically fixed-size buffers on the stack (return addresses, saved registers, adjacent locals). The threat is a buffer overflow: an attacker sends a string longer than the destination buffer, and an unbounded copy walks past the end, corrupting memory an attacker can steer toward a crash or code execution. In this lesson you learn to detect the vulnerable pattern during code review and prevent it before it ships.

This is a code-review skill, not an exploitation skill. You will look at real C, find the smell, and remove it. It builds directly on your prerequisite, Safe string functions: there you learned which functions are safe and how they behave; here you learn how to hunt down the unsafe ones already sitting in a codebase and replace them systematically.

Where does this fit in real work? Nearly every legacy C buffer-overflow CVE traces back to one of four calls. A security reviewer, a maintainer accepting a pull request, or an auditor doing a first pass on an unfamiliar C project all start the same way: grep for the dangerous functions, then reason about each hit. It is the cheapest, highest-yield audit you can run on C code.

Everything here runs on your own machine — a local file, a local compile. There is no remote target, no network. The 'attacker input' is just a long string you type into your test program.

Why it matters

In authorized professional work, this single audit pays for itself constantly:

• Code review / PR gating. When you review C, the first thing a competent reviewer does is search for unbounded copies. Catching one strcpy in review is far cheaper than shipping it and issuing a patch later.
• Security assessments of your own products. Engagement rules of engagement often include a source-code review component. Finding these patterns and writing them up (with severity based on whether the source is attacker-reachable) is bread-and-butter work.
• Maintaining legacy C. Codebases written before size-checked functions were common are full of these calls. A methodical replacement pass measurably reduces the attack surface.
• Compliance and secure-SDLC. Standards like CERT C and MISRA explicitly ban or restrict gets, strcpy, and friends. Being able to find and justify each finding maps directly to those requirements.

The skill is durable because the pattern is durable: the functions are decades old, still legal C, and still compile without warning by default. The tool that finds them — a careful reviewer with grep — never goes out of date.

Core concepts

1. The unbounded copy — the root cause

Definition. An unbounded copy writes bytes into a destination buffer using the length of the source to decide when to stop, ignoring the size of the destination.

Plain explanation. strcpy(dst, src) copies characters from src into dst one at a time until it reaches src's terminating null byte (\0). It never asks how big dst is. If src is longer than dst, the extra bytes are written past the end of dst into whatever memory sits next to it.

How it works. A local array like char dst[16]; lives on the stack, next to other locals, saved registers, and the function's return address. Writing 40 bytes into a 16-byte buffer overwrites those neighbours. Corrupting the return address is what classically lets an attacker redirect execution.

When / when-not. strcpy is only ever 'safe' when you can prove the source can never exceed the destination — e.g. copying a compile-time constant string. As soon as any part of the source is influenced by input (a file, an argument, a network read, an environment variable), it is a candidate finding.

Pitfall. People assume 'the input is usually short.' Attackers do not send usual input. Size safety must be structural, not statistical.

2. The four classic offenders

gets is special: it has no safe drop-in and should simply be banned. It cannot be used correctly because the caller has no way to tell it how big the buffer is.

3. Static analysis by grep — and its limits

Definition. Static analysis means inspecting source code without running it. Grepping for function names is the simplest form.

How it works. You search the tree for strcpy(, strcat(, sprintf(, gets( and treat every hit as a candidate. Then you reason about each: where does the source come from, and can the destination overflow?

When / when-not. Grep is a starting filter, not a verdict. It produces false positives (a strcpy of a constant into a large-enough buffer is not exploitable) and false negatives (a hand-rolled copy loop, or the call hidden behind a macro or a function pointer, will not match). Never claim code is safe because grep found nothing.

Pitfall. Grepping strcpy without the ( also matches strncpy, strcpy_s, comments, and strings. Anchor on strcpy( and still read the surrounding lines.

4. Making misuse impossible (defence in depth)

Remembering to avoid a function is fragile. Structural defences make it fail loudly:

• Compiler warnings / linters. -Wall plus tools like clang-tidy (with clang-analyzer / bugprone checks) or cppcheck flag these calls automatically.
• A poison macro in a shared header turns any use into a compile error, so a new contributor cannot reintroduce one by accident.
• _FORTIFY_SOURCE (glibc, with optimisation) adds runtime bounds checks to many of these calls, turning some overflows into a clean abort instead of silent corruption.

Threat model

            +-----------------------------+
            |   Local test program (lab)  |
            |                             |
  attacker  |   char name[16];            |
  input --->|   strcpy(name, argv[1]); <--+-- ENTRY POINT (unbounded copy)
 (argv,     |        |                    |
  stdin,    |        v                    |
  file)     |   [name][saved regs][ret]  |  <-- ASSET: stack memory
            |     16b  overwritten...     |      + return address
            +-----------------------------+
              ^
              |  TRUST BOUNDARY: everything crossing into the process
                 from argv / stdin / files / env is UNTRUSTED and
                 must be length-checked before it reaches a fixed buffer.

Knowledge check.

1. What asset is being protected here, and which specific part of it does a stack overflow corrupt first?
2. Where is the trust boundary, and which insecure assumption (about the input) lets the overflow happen?
3. Why must an overflow demonstration like this run only in a local, isolated, authorized lab rather than against any real program?

Syntax notes

The audit itself is a shell one-liner; the fix is standard C.

The search (lab-safe, read-only):

# -r recurse, -n show line numbers, -E extended regex.
# Anchor on the opening paren so strncpy / strcpy_s do NOT match.
grep -rnE 'strcpy\(|strcat\(|sprintf\(|gets\(' src/

The dangerous call (what a hit looks like):

char dst[16];
strcpy(dst, src);   /* no destination size anywhere -> candidate finding */

The size-checked replacement pattern:

char dst[16];
/* snprintf always writes at most sizeof dst bytes INCLUDING the '\0'. */
int n = snprintf(dst, sizeof dst, "%s", src);
/* n is the length it WOULD have written; n >= sizeof dst means it was truncated. */
if (n < 0 || (size_t)n >= sizeof dst) {
    /* handle truncation / error instead of silently losing data */
}

Key points: pass sizeof dst (works because dst is an array in this scope, not a pointer), check the return value, and never assume the input fits.

Lesson

The shape of a classic bug

Most legacy buffer-overflow CVEs share the same pattern:

strcpy(dst, src);

Here src is data the attacker controls, and dst is a fixed-size array. Because strcpy keeps copying until it hits a terminating null byte, a long src overruns dst and corrupts memory.

A CVE (Common Vulnerabilities and Exposures) is a publicly catalogued security flaw.

What to look for

When reviewing a codebase, search (grep) for these functions:

• strcpy
• strcat
• sprintf
• gets

None of them check the size of the destination. Treat each occurrence as a candidate for a safe-replacement pass.

In your own code

Don't rely on remembering to avoid these functions. Make them impossible to use by accident:

• Block them with a wrapper macro, or
• Flag them with a linter,

so each use shows up as a compile error or a warning.

Code examples

Below: the vulnerable pattern, the secure fix, and a self-test that proves the fix rejects oversized input and accepts good input. Compile and run each locally.

(1) WARNING: intentionally vulnerable — use only in a local, isolated, authorized lab. Do not deploy.

/* vuln.c  -- demonstrates the smell. Build WITHOUT hardening to observe it:
 *   cc -std=c11 -O0 -fno-stack-protector -o vuln vuln.c
 * Run only on your own machine:  ./vuln AAAAAAAAAAAAAAAAAAAAAAAA
 */
#include <stdio.h>
#include <string.h>

static void greet(const char *user_input) {
    char name[16];
    strcpy(name, user_input);   /* <-- unbounded copy: overflows if input > 15 chars */
    printf("Hello, %s\n", name);
}

int main(int argc, char **argv) {
    if (argc < 2) {
        fprintf(stderr, "usage: %s <name>\n", argv[0]);
        return 2;
    }
    greet(argv[1]);
    return 0;
}

With a short name it prints normally. With a long argument it corrupts the stack: expect a crash such as *** stack smashing detected *** (if the compiler's stack protector is on) or a segmentation fault. That crash is the memory-safety failure — you are observing the bug, not exploiting anything.

(2) The SECURE fix

/* safe.c  -- size-checked copy with explicit truncation handling.
 *   cc -std=c11 -Wall -Wextra -o safe safe.c
 */
#include <stdio.h>
#include <string.h>

/* Returns 0 on success, -1 if the input did not fit (rejected). */
static int greet(const char *user_input) {
    char name[16];
    int n = snprintf(name, sizeof name, "%s", user_input);
    if (n < 0 || (size_t)n >= sizeof name) {
        fprintf(stderr, "input rejected: too long for buffer\n");
        return -1;
    }
    printf("Hello, %s\n", name);
    return 0;
}

int main(int argc, char **argv) {
    if (argc < 2) {
        fprintf(stderr, "usage: %s <name>\n", argv[0]);
        return 2;
    }
    return greet(argv[1]) == 0 ? 0 : 1;
}

snprintf never writes past sizeof name, always null-terminates, and its return value tells us whether truncation happened so we can reject rather than silently mangle the input.

(3) VERIFY — prove the fix rejects bad input and accepts good input

/* test_safe.c  -- self-contained checks. cc -std=c11 -Wall -o test_safe test_safe.c && ./test_safe */
#include <stdio.h>
#include <string.h>
#include <assert.h>

/* copy_checked: 0 on success, -1 if it would not fit. dstsz includes room for '\0'. */
static int copy_checked(char *dst, size_t dstsz, const char *src) {
    int n = snprintf(dst, dstsz, "%s", src);
    if (n < 0 || (size_t)n >= dstsz) return -1;
    return 0;
}

int main(void) {
    char buf[16];

    /* ACCEPT: valid input that fits (<= 15 chars) */
    assert(copy_checked(buf, sizeof buf, "Ada") == 0);
    assert(strcmp(buf, "Ada") == 0);

    /* ACCEPT: exactly fills the buffer (15 chars + '\0') */
    assert(copy_checked(buf, sizeof buf, "123456789012345") == 0);
    assert(strlen(buf) == 15);

    /* REJECT: one byte too long -- must be refused, never truncated silently */
    assert(copy_checked(buf, sizeof buf, "1234567890123456") == -1);

    /* REJECT: hostile long input */
    char big[100];
    memset(big, 'A', sizeof big - 1);
    big[sizeof big - 1] = '\0';
    assert(copy_checked(buf, sizeof buf, big) == -1);

    printf("all checks passed: fix accepts valid input and rejects oversized input\n");
    return 0;
}

Expected output: all checks passed: fix accepts valid input and rejects oversized input. If any assert fires, the program aborts and names the failing line — that is your signal the fix is wrong.

Line by line

Walkthrough of the vulnerable greet and how the secure version changes the outcome.

Tracing the overflow with input "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" (30 A's):

1. strcpy reads user_input[0]='A', writes name[0]='A'.
2. It keeps going: name[1], name[2], ... up to name[15] — the buffer is now full and legal.
3. It does not stop, because the source has more characters. It writes name[16], name[17], ... which are no longer inside name. These bytes land on adjacent stack memory.
4. Eventually it overwrites the saved return address region.
5. When greet returns, the corrupted address is used → crash (or, with a stack protector, a detected abort).

In the secure version, step 2 is where snprintf stops: it has been told the limit is sizeof name == 16, so it writes 15 data bytes plus a \0 and returns the would-be length 30. The check 30 >= 16 is true, so we never trust the truncated result — we reject.

Common mistakes

Mistake 1 — Grepping the bare name.

• WRONG: grep -r strcpy src/ — matches strncpy, strcpy_s, comments, and the word in strings, drowning you in noise and false positives.
• WHY wrong: you cannot tell real calls from safe ones or from mentions.
• CORRECTED: anchor on the paren: grep -rnE 'strcpy\(' src/, then read each hit's surrounding lines.
• RECOGNISE/PREVENT: if your result count seems huge, you probably forgot the (.

Mistake 2 — Treating strncpy as automatically safe.

• WRONG: replacing strcpy(dst, src) with strncpy(dst, src, sizeof dst) and moving on.
• WHY wrong: strncpy does not null-terminate when the source is as long as or longer than the size — you get a buffer with no terminator, and the next string operation overruns.
• CORRECTED: prefer snprintf(dst, sizeof dst, "%s", src), or if you use strncpy you must force dst[size-1] = '\0'; and still handle truncation.
• RECOGNISE/PREVENT: any strncpy with no following explicit terminator is a fresh finding.

Mistake 3 — Passing sizeof on a pointer.

• WRONG: inside a function taking char *dst, writing snprintf(dst, sizeof dst, ...).
• WHY wrong: sizeof dst is the size of the pointer (8 bytes), not the buffer. You have re-introduced an overflow.
• CORRECTED: pass the real size as a parameter: int copy_checked(char *dst, size_t dstsz, const char *src).
• RECOGNISE/PREVENT: sizeof only gives the buffer size where the array is declared, not after it decays to a pointer.

Mistake 4 — Ignoring snprintf's return value.

• WRONG: calling snprintf and assuming success because it 'can't overflow.'
• WHY wrong: it can silently truncate, producing wrong-but-not-crashing behaviour (a security bug of its own, e.g. a truncated path or hostname).
• CORRECTED: compare the return value against the buffer size and treat >= as rejected.
• RECOGNISE/PREVENT: every snprintf in security-relevant code should have a truncation check.

Mistake 5 — 'grep found nothing, so it's safe.'

• WRONG: concluding the code is memory-safe after a clean grep.
• WHY wrong: hand-rolled copy loops, macros, function pointers, and other libc calls (memcpy with a bad length, read into a fixed buffer) all overflow without matching your pattern.
• CORRECTED: use grep as a first pass, then add compiler warnings and a static analyzer.
• RECOGNISE/PREVENT: never state 'no vulnerabilities' — state 'no instances of these four patterns.'

Debugging tips

When the vulnerable program crashes:

• *** stack smashing detected *** followed by abort → the stack canary caught the overflow. This confirms the smell is real. Rebuild the safe version.
• Segmentation fault with no message → likely the corrupted return address. Reproduce under a debugger: gdb --args ./vuln $(python3 -c 'print("A"*40)'), run, and bt to see the corrupted frame.
• Use the sanitizer for a precise report: cc -std=c11 -fsanitize=address -g -o vuln vuln.c then run. AddressSanitizer prints the exact overflowed buffer, sizes, and the writing line.

When the audit misses things:

• Empty grep result but you suspect a bug → widen the search to memcpy(, strncat(, raw read(/recv( into fixed buffers, and any char buf[ followed by a copy loop.
• Too many hits → confirm you anchored on ( and are scoping to source dirs (exclude build/, vendor/, tests/ if appropriate).

When the fix still misbehaves:

• Output silently cut off → you are ignoring the snprintf return value; add the truncation check.
• Overflow still detected after the fix → you likely passed sizeof on a pointer; print sizeof dst and confirm it equals the array size (16), not 8.

Questions to ask when a copy fails:

1. Where does the source come from, and can any caller make it arbitrarily long?
2. What is the exact declared size of the destination, at the point of the copy?
3. Does the copy null-terminate in every path, including the truncation path?
4. Does the code check for and react to truncation, or assume success?

Memory safety

Security & safety — detection and logging (lab). When you build a test harness that rejects oversized input, log the decision, never the payload's secrets.

What to log on a rejected (too-long) input:

• Timestamp (ISO-8601, UTC).
• Source of the input (e.g. argv[1], stdin, filename, or a lab client id) — the channel, not necessarily the full content.
• The resource / function that rejected it (greet, buffer size 16).
• The result / security decision: REJECTED: input length 30 exceeds capacity 15.
• A correlation id so one request can be traced across log lines.

What to NEVER log:

• The full raw input if it may contain passwords, tokens, session cookies, private keys, full card numbers (PANs), or unneeded PII. Log the length and maybe a short, sanitized prefix — not the whole hostile string.
• Anything that would let the log itself become an injection vector (sanitize control characters and newlines before logging).

Events that signal abuse:

• A burst of over-length inputs from one source → someone probing buffer limits.
• Inputs whose length hovers exactly around your buffer sizes → boundary-testing.
• Repeated crashes / aborts in the same function → active overflow attempts.

How false positives arise:

• A legitimate but long value (a long real name, a long file path) can trip a too-tight limit. That means your buffer is too small, not that the user is an attacker — widen the buffer or handle long values, and tune the alert threshold so normal-but-long input does not page anyone.

C memory-safety reminders specific to this topic:

• strcpy/strcat/sprintf/gets write out of bounds → undefined behaviour; the 'crash' is the lucky outcome, silent corruption is the dangerous one.
• Always ensure a \0 terminator on every path; an unterminated buffer turns the next string op into an overread/overflow.
• Build lab code with -fsanitize=address,undefined to catch overflows the compiler cannot see statically.

Real-world uses

Authorized real-world use case. You are reviewing an internal C service before release (you own the code / are on the team). Your first pass is grep -rnE 'strcpy\(|strcat\(|sprintf\(|gets\(' src/. You get twelve hits. You classify each: three copy compile-time constants into large buffers (not exploitable — note and move on), eight take request-derived data into fixed buffers (real findings — replace with snprintf + truncation handling), and one is a gets reading a config line (ban outright, switch to fgets). You open a PR that removes the risky calls and adds a poison macro so they cannot return.

Professional best-practice habits:

Correcting a common misconception: passing a static scanner or a clean grep does not prove a program is secure — it proves those specific patterns were not found. Nothing here makes code 'completely secure'; it removes one well-understood class of bug.

Practice tasks

All tasks are lab-only: your own machine, your own files, inputs you generate yourself. Each ends by remediating and verifying.

Beginner 1 — Run the audit.

• Objective: find every unbounded-copy call in a small source tree.
• Requirements: create a folder with 3–4 .c files, some containing strcpy(, strcat(, sprintf(, gets(, and some safe calls (snprintf, strncpy with a terminator). Run the anchored grep.
• Output: a list of file:line hits.
• Constraints: read-only; do not modify yet.
• Hints: use -rnE and anchor on (.
• Concepts: static analysis, the four offenders.
• Conclude by: noting which hits are real (input-derived) versus safe (constant), i.e. triage before you fix.

Beginner 2 — Classify each hit.

• Objective: decide, for each hit from task 1, whether it is a real finding.
• Requirements: for every hit, write one line: source of the copied data, destination size, verdict (real / not-exploitable) with a reason.
• Constraints: justify each verdict; no fixing yet.
• Hints: constant source into a big buffer = not exploitable; any input-derived source = real.
• Concepts: trust boundary, false positives.
• Conclude by: producing a prioritized fix list.

Intermediate 1 — Replace and verify.

• Objective: fix one real finding and prove the fix.
• Requirements: replace a strcpy/sprintf with snprintf + truncation check; add asserts that a valid short input succeeds and an over-length input is rejected.
• Input/output: short name → accepted; 100-char string → rejected, no crash.
• Constraints: must reject, not silently truncate.
• Hints: check n >= sizeof dst.
• Concepts: size-checked copy, mitigation verification.
• Conclude by: running the test and confirming rejects/accepts as designed.

Intermediate 2 — Prove the smell with a sanitizer.

• Objective: observe the overflow safely, then confirm the fix removes it.
• Requirements: build the vulnerable version with -fsanitize=address -g, feed an over-length argument, capture the ASan report. Rebuild the fixed version and show ASan is now clean.
• Constraints: local only; never point the binary at anything but your own input.
• Hints: ASan names the exact buffer and the writing line.
• Concepts: dynamic detection, verification.
• Conclude by: a two-line before/after: 'ASan: heap/stack-buffer-overflow at greet' → 'ASan: clean'.

Challenge — Make it un-reintroducible.

• Objective: prevent the whole class at build time.
• Requirements: add a shared header that poisons the four functions (so any use is a compile error), then intentionally add a strcpy( call and show the build fails with a clear message. Also add a logging shim to your safe copy that records rejects (timestamp, source channel, length, decision) without logging the raw input.
• Constraints: the log must never contain the full hostile payload or any secret; log length, not content.
• Hints: a poison macro like #define strcpy(...) STRCPY_IS_BANNED_USE_snprintf makes misuse fail to compile; guard it so it does not break third-party headers.
• Concepts: secure defaults, defence in depth, safe logging.
• Authorization checklist (before any lab overflow demo): (1) Is this my own machine / an isolated VM or container? (2) Is the code mine or explicitly authorized? (3) Is there no network target involved? (4) Do I have a reset/cleanup step? Cleanup: delete the built binaries (rm -f vuln safe test_safe), remove any log files you created, and discard scratch source. Only proceed if all four are yes.

Summary

Main concepts. Unbounded copies (strcpy, strcat, sprintf, gets) decide when to stop from the source length and ignore the destination size. When any source data crosses a trust boundary (argv, stdin, files, network) into a fixed-size buffer, that is a candidate buffer-overflow finding — the protected asset is stack memory, including the return address.

Key syntax / commands. Audit with grep -rnE 'strcpy\(|strcat\(|sprintf\(|gets\(' src/ (anchor on the paren). Fix with snprintf(dst, sizeof dst, "%s", src) and check the return value: n >= sizeof dst means truncated → reject. gets has no safe form — ban it, use fgets.

Common mistakes. Grepping the bare name; trusting strncpy (it may not terminate); using sizeof on a pointer; ignoring snprintf's return value; and claiming 'safe' because grep found nothing.

What to remember. Grep is a first pass, not a verdict — pair it with -Wall -Wextra, clang-tidy/cppcheck, and ASan/UBSan. Every fix needs a verification step that proves it rejects oversized input and accepts valid input. Log the security decision (timestamp, source, resource, result, correlation id) but never the raw payload or any secret. Do overflow demos only on your own, isolated, authorized machine, and clean up afterward. Nothing here makes code 'completely secure' — it removes one well-understood, high-impact bug class.

Practice with these exercises

• Does the code call strcpy?
• Count unsafe calls