Fixing command injection in toy C code

Overview

Security objective. The asset we protect here is the shell / operating system of a machine running a small C helper program. The threat is command injection (CWE-78): an attacker supplies text that a program splices into a shell command, causing the shell to run their commands instead of only the intended one. In this lesson you will learn to detect this pattern in a code review and remediate it so the untrusted input can never reach a shell interpreter.

This builds directly on your prerequisite, Command injection prevention. There you saw the vulnerability and the general principle. Here you do the hands-on engineering: take a concrete toy program that calls system() on a built-up string, and convert it into a version that launches the same program with fork + execvp, passing user input as a separate, literal argument the shell never parses.

The running example is an archive helper: it takes a name from the user and creates /tmp/<name>.tgz. Written with system() it is exploitable; written with execvp plus a validation allow-list it is not. Everything in this lesson runs on your own machine or a container — you are attacking code you own, in an isolated lab, to learn how to defend it.

Why it matters

Command injection is consistently one of the most damaging web and system vulnerabilities: it can hand an attacker full command execution as whatever user your program runs as. In authorized professional work this shows up constantly:

• Secure code review / SAST triage. Reviewers and static-analysis tools flag every system(), popen(), and sh -c call. You must be able to decide quickly whether the argument is attacker-influenced and, if so, propose the correct fix — not a hand-wavy "escape it."
• Penetration testing & reporting. When you find an injection point in an authorized engagement, the remediation section of your report has to be concrete and correct. "Use an argument vector (execvp/parameterized API) instead of a shell string" is the fix clients act on.
• Building tools safely. Security engineers write lots of small wrapper programs that call other tools. Getting the launch pattern right the first time means the tool cannot become an injection foothold.

Knowing the remediation cold — and how to verify it — is what separates "I ran a scanner" from "I fixed the class of bug and proved it."

Core concepts

1. The shell as an interpreter (the root cause)

Definition. system(cmd) hands cmd to /bin/sh -c and lets the shell parse it. The shell is a full programming language: ;, |, &, $(...), backticks, >, <, *, and newlines are all operators.

How it works. If you build "tar -czf /tmp/" + name + ".tgz /data" and name is x.tgz /data; rm -rf ~ #, the shell sees a command separator (;) and runs a second command. The program never intended a second command — the shell created it from data.

When / when-not. system() is fine only for a fully constant command with no untrusted parts. The moment any part comes from a user, a file, an environment variable, or the network, it is unsafe.

Pitfall. People assume the input is "just a filename." Nothing enforces that assumption — the input is whatever bytes arrive.

2. Argument vectors: removing the shell entirely

Definition. The exec family (execvp, execv, execve, …) launches a program by passing an explicit argv[] array. There is no shell in the picture, so no string is ever parsed for metacharacters.

How it works. execvp("tar", (char*[]){"tar","-czf",path,"/data",NULL}) runs tar with exactly four arguments. If path contains ; or $(), those are ordinary characters in a filename — tar receives them literally. There is no interpreter to hijack.

When / when-not. Use an argument vector whenever you launch a subprocess with any untrusted data. Do not reach back for system() or popen() for convenience.

Pitfall. execvp searches $PATH for the program name. In hardened code prefer execv/execve with an absolute path (/usr/bin/tar) so a poisoned PATH cannot substitute a different binary.

3. Why escaping is the wrong fix

Definition. "Escaping" means trying to neutralise every dangerous character before putting the input into a shell string.

Plain explanation. Shells have many metacharacters and quoting modes (single quotes, double quotes, $'...', backslash, IFS word-splitting, glob expansion, locale quirks). A hand-rolled escaper only has to miss one edge case to be bypassed. This is exactly what the lesson quiz points at.

When / when-not. Almost never hand-write shell escaping. If you truly must build a shell string, use a vetted library that quotes an entire argument — but the better answer is to avoid the shell (concept 2).

Pitfall. An escaper that looks correct on ASCII test cases can still be defeated by newlines, NUL bytes, or unusual locales. Robust defences remove the interpreter; they don't try to out-clever it.

4. Input validation as defence in depth

Definition. An allow-list accepts only known-good input (e.g. name matches ^[A-Za-z0-9_-]{1,64}$) and rejects everything else.

How it works. Even after switching to execvp, validating the name means a value like ../../etc/passwd or an empty string is refused before you build a path — this stops path traversal and other logic abuse, not just injection.

When / when-not. Always validate untrusted input at the trust boundary. Validation is a second layer — it does not replace the argument vector, and the argument vector does not replace validation. Use both.

Pitfall. A deny-list ("block ; and |") is the mirror image of escaping: you will forget a character. Prefer allow-lists.

Threat model

            UNTRUSTED                    | TRUSTED (your process / host)
                                         |
  [ user / caller ] --name--> [ argv or ]==> validate(name)? --no--> REJECT + log
     (attacker-             [  stdin   ] |        | yes
      controlled)                        |        v
                                         |   build path "/tmp/<name>.tgz"
                                         |        |
   ===== TRUST BOUNDARY (the '|') =======|        v
                                         |   fork() + execvp("tar", argv[])
  ASSET PROTECTED: the shell / OS -------|        |  (NO /bin/sh involved)
  command interpreter and host           |        v
  ENTRY POINT: the 'name' value          |   tar runs with literal args

Entry point: the name value. Trust boundary: the point where that value enters your process. Asset: the OS command interpreter (and thereby the whole host). The insecure design lets untrusted data cross the boundary into a shell; the fix keeps it as inert data in an argv slot.

Knowledge check.

1. What asset is protected here, and what is the single entry point for attacker data?
2. Where is the trust boundary, and which insecure assumption ("it's just a filename") lets the attack work?
3. After the fix, which log line would let a defender detect someone probing with name="x; rm -rf /" — and why must this whole exercise stay on a machine you own or are authorized to test?

Syntax notes

The key structural change is string-to-shell becoming array-to-exec.

#include <unistd.h>   /* fork, execvp, _exit          */
#include <sys/wait.h> /* waitpid                       */
#include <sys/types.h>

/* Build an argv: each element is ONE literal argument.
 * The list MUST be NULL-terminated. argv[0] is the program name. */
char *argv[] = { "tar", "-czf", out_path, "/data", NULL };

pid_t pid = fork();          /* create a child process           */
if (pid == 0) {              /* child: replace image with tar     */
    execvp(argv[0], argv);   /* on success, never returns         */
    _exit(127);              /* only reached if execvp failed     */
}
/* parent: reap the child so it is not left a zombie */
int status;
waitpid(pid, &status, 0);

Key points: execvp takes the program name plus a NULL-terminated char *[]; on success it does not return, so the line after it runs only on failure. Use _exit (not exit) in the child after a failed execvp to avoid flushing the parent's buffers twice.

Lesson

The vulnerable program

You are given a small program that builds a shell command from user input:

char cmd[256];
snprintf(cmd, sizeof cmd, "tar -czf /tmp/%s.tgz /data", user_supplied);
system(cmd);

system() runs its argument through the shell. That means any shell metacharacters in user_supplied (such as ;, |, or $()) are interpreted as commands, not as plain text. This is command injection.

The fix

The fix is not to escape the input. Escaping is fragile and easy to get wrong.

The real fix is to avoid the shell entirely:

• Use fork to create a child process.
• Use execvp to run tar directly in that child.
• Pass tar as argv[0] and user_supplied as a separate, literal argument.

Because the arguments are passed as a list, the shell never sees the input. Metacharacters lose their special meaning and become ordinary characters.

Your task

Practice this rewrite on a few similar patterns.

Code examples

Below: the vulnerable version, the secure rewrite, and a verification harness.

(1) Insecure version

/* WARNING: intentionally vulnerable — use only in a local, isolated,
   authorized lab. Do not deploy. */
#include <stdio.h>
#include <stdlib.h>

int archive_insecure(const char *name) {
    char cmd[256];
    /* User input is spliced straight into a shell command. */
    snprintf(cmd, sizeof cmd, "tar -czf /tmp/%s.tgz /data", name);
    return system(cmd);   /* /bin/sh -c "..."  -> injection point */
}
/* If name = "x /data; id #" the shell also runs `id`.
   The program never intended a second command. */

(2) Secure version — argument vector + allow-list validation

#include <stdio.h>
#include <string.h>
#include <ctype.h>
#include <unistd.h>
#include <sys/wait.h>
#include <sys/types.h>

/* Allow-list: 1..64 chars, only [A-Za-z0-9_-]. Returns 1 if OK. */
int valid_name(const char *name) {
    if (name == NULL) return 0;
    size_t n = strlen(name);
    if (n == 0 || n > 64) return 0;
    for (size_t i = 0; i < n; i++) {
        unsigned char c = (unsigned char)name[i];
        if (!(isalnum(c) || c == '_' || c == '-')) return 0;
    }
    return 1;
}

/* Returns 0 on success, -1 on rejected input, -2 on launch/child error. */
int archive_secure(const char *name) {
    if (!valid_name(name)) {
        /* Security decision: rejected. Log the DECISION, not raw bytes
           you don't control (here the name is already known-safe-charset,
           but we keep it short and bounded). */
        fprintf(stderr, "archive: reject invalid name (len/charset)\n");
        return -1;
    }

    char out_path[128];
    if (snprintf(out_path, sizeof out_path, "/tmp/%s.tgz", name)
            >= (int)sizeof out_path) {
        fprintf(stderr, "archive: path too long\n");
        return -1;
    }

    /* No shell: tar receives these four args literally, then NULL. */
    char *argv[] = { "tar", "-czf", out_path, "/data", NULL };

    pid_t pid = fork();
    if (pid < 0) { perror("fork"); return -2; }
    if (pid == 0) {                 /* child */
        execvp(argv[0], argv);      /* returns only on failure */
        perror("execvp");
        _exit(127);
    }
    int status;                     /* parent: reap child */
    if (waitpid(pid, &status, 0) < 0) { perror("waitpid"); return -2; }

    if (WIFEXITED(status) && WEXITSTATUS(status) == 0) {
        fprintf(stderr, "archive: created /tmp/%s.tgz (ok)\n", name);
        return 0;
    }
    fprintf(stderr, "archive: tar failed\n");
    return -2;
}

(3) Verification harness — proves reject vs accept

#include <stdio.h>   /* printf, NULL */
#include <assert.h>

/* Compile the two files together, or paste valid_name above this. */
int valid_name(const char *name);

int main(void) {
    /* Malicious / malformed inputs MUST be rejected. */
    const char *bad[] = {
        "x; rm -rf /",      /* command separator      */
        "$(reboot)",        /* command substitution   */
        "a|b",              /* pipe                   */
        "../etc/passwd",    /* path traversal / slash */
        "",                 /* empty                  */
        NULL
    };
    for (int i = 0; bad[i]; i++)
        assert(valid_name(bad[i]) == 0);

    /* Legitimate inputs MUST be accepted. */
    const char *good[] = { "backup", "log_2026-07-08", "data-01", NULL };
    for (int i = 0; good[i]; i++)
        assert(valid_name(good[i]) == 1);

    printf("all validation checks passed\n");
    return 0;
}

Expected output of the harness: all validation checks passed (all asserts hold). If any assert fires, the program aborts and prints the failing file/line — that is the signal your validation is wrong. The secure archive_secure function, when called with a good name in a lab that has a /data directory, creates /tmp/<name>.tgz and logs archive: created ... (ok); with a bad name it logs the reject line and returns -1 without ever launching a process.

Line by line

Walking the secure path with name = "backup" and then name = "x; rm -rf /":

The crucial line is step 6. In the insecure version the entire string went through /bin/sh -c, so a ; started a new command. Here tar is handed a fixed four-element list; even if the filename contained a ;, tar would treat it as a character in a filename, not a command boundary — but because validation already ran in step 1, such input never gets this far. Two independent layers: validation (step 1) and no-shell launch (step 6).

Common mistakes

Mistake 1 — "I'll just escape the dangerous characters." WRONG: writing a function that backslash-escapes ;, |, & and keeping system(). WHY wrong: shells have many metacharacters and quoting modes; miss one (a newline, $, glob, locale quirk) and it is bypassed — the exact point of the quiz. CORRECTED: remove the shell with execvp; add an allow-list. RECOGNISE/PREVENT: any diff that keeps system()/popen() but adds an escaper should fail review.

Mistake 2 — Forgetting the NULL terminator in argv. WRONG: char *argv[] = {"tar","-czf",out,"/data"}; WHY wrong: execvp reads past the array looking for NULL, causing undefined behaviour / garbage arguments. CORRECTED: end the array with NULL. RECOGNISE/PREVENT: crashes or bogus args to the child; always eyeball the trailing NULL.

Mistake 3 — Treating execvp as if it returns on success. WRONG: code after execvp that assumes success. WHY wrong: on success execvp never returns; the following lines run only on failure. CORRECTED: put only error handling + _exit(127) after execvp.

Mistake 4 — Dropping validation because "execvp is safe now." WHY wrong: the argument vector stops shell injection but not path traversal, oversized names, or empty input. CORRECTED: keep both layers. Defence in depth.

Mistake 5 — Using exec in the child but never waitpid in the parent. WHY wrong: the child becomes a zombie; over time you leak process slots. CORRECTED: always waitpid (or handle SIGCHLD).

Debugging tips

• execvp fails with "No such file or directory". The program name is not on $PATH, or you passed an absolute path that doesn't exist. Debug: print argv[0]; try which tar; consider execv("/usr/bin/tar", argv) with an absolute path.
• Child seems to do nothing / no archive appears. Check the parent actually waitpids and inspect status with WIFEXITED/WEXITSTATUS. A non-zero exit means tar ran but errored (e.g. /data missing in your lab — create it).
• Compiler warns about implicit declarations. You forgot #include <unistd.h> / <sys/wait.h>. Compile with -Wall -Wextra and treat warnings as bugs.
• assert in the harness aborts. That's the harness working: your valid_name accepted something it should reject (or vice-versa). Print the offending string and re-examine the allow-list.
• Questions to ask when it fails: Is any part of this command attacker-influenced? Is there still a shell anywhere (system, popen, sh -c, bash -c)? Is argv NULL-terminated? Did I validate before building the path? Am I checking the child's exit status?

Memory safety

Security & safety — detection, logging, and C hygiene.

Detection & logging. For each launch decision, log: a timestamp, the source (user id / connection), the requested resource (the archive name or a hash of it), the security decision (accepted/rejected and why), the result (exit status), and a correlation id so one request can be traced across logs. A rejected value like x; rm -rf / should produce a clear reject invalid name line — a burst of these is a strong signal someone is probing for injection.

Never log: passwords, tokens, session cookies, private keys, full payment card numbers, or unnecessary PII. Do not echo raw attacker bytes verbatim into logs that a viewer's terminal or a log UI might interpret (log-injection / terminal-escape risk) — prefer a sanitised or length/charset summary. Here the name is already restricted to a safe charset before we log the success line.

False positives. A legitimate user who types a space or an accented character will be rejected. That is acceptable and expected; tune the allow-list to real valid names, and document why the rule is strict.

C memory safety for this pattern.

• snprintf bounds the buffer, but you must check its return value: if it returns >= sizeof buf, the output was truncated — treat that as an error (as the code does), never as success.
• argv must be NULL-terminated or execvp reads out of bounds (UB).
• Use _exit (not exit) after a failed execvp in the child so the parent's stdio buffers aren't flushed twice.
• Check fork, execvp, and waitpid return values; ignoring them hides zombies and launch failures.
• Cast to unsigned char before isalnum — passing a negative char to a <ctype.h> function is undefined behaviour.

Real-world uses

Authorized real-world use case. A backup/report tool on a server takes a job name from an internal web form and shells out to tar/gzip. A reviewer spots system() with the name interpolated. The remediation — deployed after testing in staging — is exactly this: switch to an argument vector (execv with an absolute path), add an allow-list on the name, and log accept/reject decisions. The team then re-tests with the malicious inputs from the harness to confirm the fix.

Professional best-practice habits.

• Input validation: allow-list at the trust boundary; reject, don't sanitise-and-hope.
• Least privilege: run the subprocess as an unprivileged user; the archiver has no reason to be root.
• Secure defaults: no shell by default; absolute program paths; set -o pipefail/error checks if a shell is truly unavoidable elsewhere.
• Logging & error handling: record security decisions, check every syscall return, fail closed.

Beginner vs advanced.

• Beginner: replace one system() with fork+execvp, add a regex-style allow-list, write the reject/accept harness.
• Advanced: use posix_spawn or execve with a controlled environment; drop privileges before exec; sandbox the child (seccomp/pledge/capabilities); add structured audit logging with correlation ids; wire the harness into CI so a regression to system() fails the build.

Practice tasks

All tasks are lab-only — run on your own machine or a container. Each ends with remediate + verify.

Beginner 1 — Spot the sink. Objective: in a given C file, list every call that reaches a shell (system, popen, execlp/execvp with a shell, sh -c). Requirements: for each, state whether any argument is attacker-influenced and mark it a finding or not. Constraints: read-only; no code changes yet. Hints: grep for system(, popen(, sh -c. Concepts: sinks, trust boundary. Conclusion: produce a short finding list you will remediate in task 3.

Beginner 2 — Write the allow-list. Objective: implement int valid_name(const char *) accepting ^[A-Za-z0-9_-]{1,64}$. Input/Output: "backup"->1, "x; rm"->0, ""->0. Constraints: cast to unsigned char before isalnum; bound the length. Hints: loop over characters; reject on first bad one. Concepts: allow-list validation. Conclusion: verify with 5 good + 5 bad strings via assert.

Intermediate 1 — Rewrite to execvp. Objective: convert the insecure archiver to fork+execvp with an argv list. Requirements: NULL-terminate argv; waitpid and check WEXITSTATUS; _exit(127) on execvp failure. Constraints: no system/popen anywhere. Hints: build argv before fork. Concepts: argument vectors. Conclusion: run with name="x; id #" and confirm no second command runs (check no unexpected output/side effect).

Intermediate 2 — Harden the launch. Objective: switch to execv("/usr/bin/tar", argv) (absolute path) and drop to an unprivileged uid before exec if running as root (lab container). Requirements: verify the binary path exists; handle execv failure. Constraints: document why absolute path + least privilege matters. Hints: setgid before setuid. Concepts: least privilege, PATH poisoning. Conclusion: verify a poisoned PATH no longer changes which binary runs.

Challenge — Detection harness + logging. Objective: build a test/log harness that feeds a list of malicious and benign names to archive_secure, asserts each is rejected/accepted correctly, and emits structured log lines (timestamp, source, name-summary, decision, result, correlation id). Requirements: prove REJECT for injection/traversal/empty inputs and ACCEPT for valid ones; ensure no secret or raw-escape bytes are logged. Constraints: lab-only; the child must never run for rejected input. Hints: reuse the verification harness; add a fake logger you can grep. Concepts: mitigation verification, detection & logging. Defensive conclusion: a reviewer reading your log can distinguish an attack (burst of rejects) from normal use, and your asserts prove the fix holds. Reset the lab afterward (see cleanup).

Authorization checklist (before any lab run): (1) It is my machine / my container / a CTF or intentionally-vulnerable VM. (2) No production data or real targets involved. (3) I have written authorization if the system isn't mine. (4) I will not point any test at a third-party host.

Lab cleanup / reset: remove created archives (rm -f /tmp/*.tgz inside the lab only), delete any test /data directory you made, restore $PATH if you altered it, and discard the container/VM snapshot so no test artifacts persist.

Summary

• Root cause: system()/popen() run their argument through /bin/sh, so any attacker-influenced part can inject extra commands (CWE-78).
• Wrong fix: hand-escaping metacharacters — shells have too many edge cases; one miss = bypass.
• Right fix: remove the shell with fork + execvp (or execv/posix_spawn), passing user input as a separate literal argument in a NULL-terminated argv.
• Defence in depth: also validate with an allow-list (^[A-Za-z0-9_-]{1,64}$) to stop traversal/oversized/empty input; the two layers are independent.
• Key syntax: char *argv[]={"tar","-czf",path,"/data",NULL}; fork(); execvp(argv[0],argv); _exit(127); waitpid(...).
• Common mistakes: missing NULL, assuming execvp returns, forgetting waitpid, dropping validation, casting char to isalnum unsafely.
• Verify + detect: a harness must prove bad input is REJECTED and good input ACCEPTED; log accept/reject decisions (never secrets) so probing is visible.
• Ethics: only ever test code you own or are authorized to test, in an isolated lab; clean up afterward. Passing a scanner is not proof of security, and nothing is ever "completely secure."

Practice with these exercises

• Does the code call system()?
• Safer alternative to system()?