Command Injection Taxonomy for Code Review

This is a sink taxonomy lab, not a shell-popping note. Target is a 20-line Flask wrapper around ping. One route builds a shell string (shell=True). One route passes an argv list. Goal: show the child command line in logs, show a metacharacter reaching /bin/sh on the bad path, and show the list path treating the same bytes as a hostname. I do not spawn a reverse shell, I do not call bash -i, and I do not paste & curl attacker. The dangerous call is in the snippet so reviewers can grep it; the fix sits next to it.

1Figure 1. Metacharacters are a sh problem. argv lists still need a hostname allow-list.
2host=  -->  shell=True  -->  /bin/sh -c "ping -c 1 "+host
3host=  -->  argv list   -->  ping -c 1 -- host   (no sh)

Lab layout

1labs/cmdi_lab/
2  app.py           # two routes, ping -c 1
3  popen_wrap.c     # optional C helper for ASAN

 1# app.py — toy, 127.0.0.1 only
 2import ipaddress
 3import subprocess
 4from flask import Flask, request, abort
 5
 6app = Flask(__name__)
 7
 8def _host():
 9    h = request.args.get("host", "")
10    if not h or len(h) > 64:
11        abort(400)
12    return h
13
14@app.get("/ping_bad")
15def ping_bad():
16    h = _host()
17    cmd = "ping -c 1 " + h                 # DANGEROUS
18    print("SH", cmd)
19    p = subprocess.run(cmd, shell=True, capture_output=True, text=True, timeout=3)
20    return {"rc": p.returncode, "out": p.stdout[:200]}
21
22@app.get("/ping_ok")
23def ping_ok():
24    h = _host()
25    try:
26        ipaddress.ip_address(h)            # allow IPs only in this lab
27    except ValueError:
28        abort(400)
29    argv = ["ping", "-c", "1", h]
30    print("ARGV", argv)
31    p = subprocess.run(argv, shell=False, capture_output=True, text=True, timeout=3)
32    return {"rc": p.returncode, "out": p.stdout[:200]}

ping is the child because it is in every training slide. The taxonomy does not care: convert, ffmpeg, dig, openssl s_client are the same sink class when the wrapper is a shell string.

Sink classes (what I write on the review comment)

Shell-interpreted string — os.system, os.popen, subprocess(..., shell=True), popen(3), system(3), PowerShell Invoke-Expression. Word-splitting and metacharacters apply.
Argv without a shell — subprocess list, execve, Go exec.Command(bin, args...). Metacharacters are literal. Injection moves to which binary and which flags the child implements.
Implicit shell — CI script: blocks, Ansible shell: vs command:, Kubernetes command vs args plus a sh -c.
Second-order — filename written now, cron later does backup.sh $FILE.

Class 1 with request data is high until proven otherwise. Class 2 still needs an allow-list for hostnames, paths, and flags.

Artifact: same HTTP, two child command lines

Benign:

1curl -s 'http://127.0.0.1:5000/ping_bad?host=127.0.0.1'
2curl -s 'http://127.0.0.1:5000/ping_ok?host=127.0.0.1'

1SH   ping -c 1 127.0.0.1
2ARGV ['ping', '-c', '1', '127.0.0.1']
3# both: rc 0, one ICMP line, 127.0.0.1

Metacharacter probe. I use a comment so the extra command is inert. The point is that sh -c parsed it.

1curl -sD - 'http://127.0.0.1:5000/ping_bad?host=127.0.0.1%20%23%20lab'
2# host=127.0.0.1 # lab

1SH ping -c 1 127.0.0.1 # lab
2# ping still runs; sh ate the comment. Evidence: the shell parsed the string.
3
4$ curl -sD - 'http://127.0.0.1:5000/ping_ok?host=127.0.0.1%20%23%20lab'
5HTTP/1.0 400 BAD REQUEST
6# ipaddress.ip_address rejected it; argv never built

A second probe that would be dangerous in a real wrapper — I do not execute a useful payload. I show the log line the bad route would have built, then I do not send it:

1# NOT SENT. Reviewer grep target only.
2# SH ping -c 1 127.0.0.1; id
3# If that line appears in an access/app log from production, the sink is live.
4# This lab never calls id, bash, nc, or curl as a sibling command.

ps while the bad route pings:

1$ ps -o pid,ppid,cmd -C ping
2  PID  PPID CMD
3 4419  4410 ping -c 1 127.0.0.1
4$ ps -o pid,cmd -p 4410
5  PID CMD
6 4410 /bin/sh -c ping -c 1 127.0.0.1 # lab

PPID is sh -c. That process table dump is the artifact I attach. On the ok route PPID is the Flask worker, CMD is ping -c 1 127.0.0.1 with four argv slots (/proc/PID/cmdline null-separated).

Sanitized reproduction (failed child / crash only)

Timeouts and usage errors are enough.

1curl -s 'http://127.0.0.1:5000/ping_bad?host=not%20a%20host'
2# SH ping -c 1 not a host
3# ping: a: Name or service not known     ← word split, extra argv to ping
4# rc != 0

C helper that matches old code I still see (popen + stack buffer). Harmless child: true.

 1/* popen_wrap.c — lab crash only */
 2#include <stdio.h>
 3#include <string.h>
 4
 5int main(int argc, char **argv) {
 6    char cmd[48];
 7    snprintf(cmd, sizeof(cmd), "ping -c 1 %s", argv[1]); /* may truncate */
 8    FILE *f = popen(cmd, "r");                           /* shell */
 9    char buf[32];
10    while (fgets(buf, sizeof(buf), f)) fputs(buf, stdout);
11    return pclose(f);
12}

 1$ clang -fsanitize=address -g -o popen_wrap popen_wrap.c
 2$ ./popen_wrap 127.0.0.1
 3PING 127.0.0.1 (127.0.0.1) 56(84) bytes of data.
 4
 5$ ./popen_wrap $(python3 -c 'print("A"*80)')
 6=================================================================
 7==7731==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x[REDACTED]
 8WRITE of size 81 at 0x[REDACTED] thread T0
 9    #0 snprintf
10    #1 main /labs/cmdi_lab/popen_wrap.c:8
11# ping is not reached; we stop at the builder overflow

Two bugs, same wrapper family: shell concatenation, and an undersized stack buffer. Neither needs a reverse shell to file.

Failed-auth style log from the Flask app when I add a dummy admin gate (wrong key):

12020-09-05T14:22:11+08:00  ping_lab  GET /ping_ok?host=127.0.0.1
2  x-api-key: [REDACTED]  result: 401
3  src: 127.0.0.1
4# no SH/ARGV line — we never reached subprocess

The fix, next to the dangerous call

1# dangerous
2subprocess.run("ping -c 1 " + h, shell=True)
3
4# fix: no shell, allow-list the argument, pin the binary
5subprocess.run(["/bin/ping", "-c", "1", h], shell=False, timeout=3)

Go shape I recommend in the same review:

1cmd := exec.Command("/bin/ping", "-c", "1", host) // not exec.Command("sh", "-c", ...)
2cmd.Env = []string{"PATH=/bin"}

Never sh -c + user string. If a flag must be optional, map from an enum ({"v4": "-4", "v6": "-6"}), do not concatenate "-" + user.

Adjacent sinks I still classify here

Image pipelines (convert, ffmpeg), PDF renderers (wkhtmltopdf), and “run nmap from the admin UI” are the same taxonomy. The child binary may have its own flag injection (ffmpeg -i plus -f concat is a different bug). I still start with: is there a shell?

PowerShell in a C# service:

1// BAD — lab review comment, not a payload
2Process.Start(new ProcessStartInfo {
3    FileName = "powershell.exe",
4    Arguments = "-NoP -C ping " + host,   // shell grammar
5});
6// GOOD
7Process.Start("ping.exe", "-n 1 " + host); // still needs an IP allow-list

I do not pass host through -C. If the product must call PowerShell, the script is a file we ship, parameters are -File + -Host 127.0.0.1 with ValidatePattern.

Go, for the same ticket as the Python fix:

1cmd := exec.Command("/bin/ping", "-c", "1", host)
2out, err := cmd.CombinedOutput()

exec.Command("sh", "-c", "ping -c 1 "+host) is shell=True with extra steps. Grep sh", "-c" next to shell=True.

Mitigation / review ticks

Grep shell=True, os.system, popen(, system(, `, Invoke-Expression.
Pin absolute binary path. Reset PATH, IFS, CDPATH if a shell is unavoidable (it should be avoidable).
Allow-list argument grammar: IP via ipaddress, hostname via RFC-compliant regex, no spaces.
Timeout every child. The lab uses 3s so a hung ping is not a CPU ticket.
Do not return raw stderr to the client; the lab does, production must not.

What I file after this lab

/ping_bad: subprocess.run(..., shell=True) , ps shows /bin/sh -c
Evidence: host=127.0.0.1%20%23%20lab still pings; comment consumed by sh
/ping_ok: argv list + ipaddress allow-list; same bytes → 400
ASAN: popen_wrap stack-buffer-overflow on 80-byte host, qcrash.c:8
Fix: ["/bin/ping", "-c", "1", h], no shell, timeout, no stderr leak
Out of scope: ;, |, backticks, reverse shells, curl to an attacker

Commands appendix

1flask --app app.py run -h 127.0.0.1 -p 5000
2curl -s 'http://127.0.0.1:5000/ping_bad?host=127.0.0.1%20%23%20lab'
3curl -sD - 'http://127.0.0.1:5000/ping_ok?host=127.0.0.1%20%23%20lab'
4ps -o pid,ppid,cmd -C ping
5clang -fsanitize=address -g -o popen_wrap popen_wrap.c

Lab layout#

Sink classes (what I write on the review comment)#

Artifact: same HTTP, two child command lines#

Sanitized reproduction (failed child / crash only)#

The fix, next to the dangerous call#

Adjacent sinks I still classify here#

Mitigation / review ticks#

What I file after this lab#

Commands appendix#