From d500509297f192b633c52bca38372f285fad8169 Mon Sep 17 00:00:00 2001 From: Hugo Date: Wed, 7 Oct 2026 20:41:26 +0200 Subject: [PATCH] fix(command-injection): read tar options by implementation (#221) --- .../command_injection/command_injection.py | 28 +++++--- tests/test_command_arguments.py | 2 +- tests/test_tar_implementations.py | 69 +++++++++++++++++++ 3 files changed, 90 insertions(+), 9 deletions(-) create mode 100644 tests/test_tar_implementations.py diff --git a/src/coretrace_python/bundled/security/command_injection/command_injection.py b/src/coretrace_python/bundled/security/command_injection/command_injection.py index 56802e6..75a4243 100644 --- a/src/coretrace_python/bundled/security/command_injection/command_injection.py +++ b/src/coretrace_python/bundled/security/command_injection/command_injection.py @@ -57,8 +57,9 @@ class CommandOption: and its value; with ``rest``, every element after it is its value, and with ``opens`` the elements after it, up to ``;`` or ``+``, are a command of their own. What the program does with the value decides: with ``runs``, it runs a command the value - chooses (``examine``: a choice among code already there, such as a module, whose - exploitability is to examine); with ``inert``, it only keeps it as data (a commit + chooses (``examine``: running is possible but its exploitability is to examine, as a + choice among code already there, such as a module, or an option only some + implementations of the program run); with ``inert``, it only keeps it as data (a commit message); with neither, the effect is not established. An option that takes no value leaves the next element free.""" @@ -132,12 +133,23 @@ def _flags(program: str, *options: str) -> tuple[CommandOption, ...]: ), CommandOption("ssh", "-o", "sets options such as ProxyCommand, which runs a command"), *_flags("ssh", *(f"-{letter}" for letter in "46AaCfGgKkMNnqsTtVvXxYy")), - CommandOption( - "tar", "--to-command", "pipes each extracted file to the command it names (GNU tar)" + # GNU tar runs what these options name; bsdtar reads -I as a file of include patterns + # and rejects the others. A bare tar may be either: running is possible, its + # exploitability is to examine. + *( + CommandOption(tar, option, runs, examine=tar == "tar") + for tar in ("tar", "gtar") + for option, runs in ( + ("--to-command", "pipes each extracted file to the command it names on GNU tar"), + ("--checkpoint-action", "runs the command of an exec= action on GNU tar"), + ("-I", "runs the compressor it names on GNU tar, where bsdtar reads patterns from it"), + ) + ), + *( + CommandOption(tar, "--use-compress-program", "runs the compressor it names") + for tar in ("tar", "gtar", "bsdtar") ), - CommandOption("tar", "--checkpoint-action", "runs the command of an exec= action (GNU tar)"), - CommandOption("tar", "--use-compress-program", "runs the compressor it names"), - CommandOption("tar", "-I", "runs the compressor it names (GNU tar)"), + CommandOption("bsdtar", "-I"), CommandOption("rsync", "-e", "runs the remote shell it names"), CommandOption("rsync", "--rsh", "runs the remote shell it names"), *( @@ -272,7 +284,7 @@ def _flags(program: str, *options: str) -> tuple[CommandOption, ...]: ), ) # Programs documented to read ``--`` as the end of their options. -END_OF_OPTIONS = frozenset({"git", "tar", "rsync", "rm"}) +END_OF_OPTIONS = frozenset({"git", "tar", "gtar", "bsdtar", "rsync", "rm"}) # Interpreters run the script their first operand names, unless one of these options # gives the code or the script instead (or, for a shell's ``-s``, reads it from the # standard input). diff --git a/tests/test_command_arguments.py b/tests/test_command_arguments.py index ca492d4..066e60a 100644 --- a/tests/test_command_arguments.py +++ b/tests/test_command_arguments.py @@ -105,7 +105,7 @@ def judged(call: str) -> list[tuple[str, str, str]]: ('subprocess.run(["notmodelled", "-c", f"echo {x}"])', MEDIUM_TO_REVIEW), ('subprocess.run(["bash", "-lc", x])', HIGH), ('subprocess.run(["sh", "-ec", f"echo {x}"])', HIGH), - ('subprocess.run(["tar", f"-I{x}", "-xf", "a.tar"])', HIGH), + ('subprocess.run(["tar", f"-I{x}", "-xf", "a.tar"])', HIGH_TO_REVIEW), ('subprocess.run(["safe", "-c", x], executable="/bin/sh")', HIGH), # Operands that are a command: a remote shell, a wrapper, a batch file. ('subprocess.run(["ssh", "user@host", "--", x])', HIGH), diff --git a/tests/test_tar_implementations.py b/tests/test_tar_implementations.py new file mode 100644 index 0000000..c860ef8 --- /dev/null +++ b/tests/test_tar_implementations.py @@ -0,0 +1,69 @@ +"""Acceptance tests for issue #221: GNU tar and bsdtar read some options differently. + +GNU tar runs the command that ``-I``, ``--to-command`` and ``--checkpoint-action`` name; +bsdtar (libarchive: macOS, FreeBSD, Windows) reads ``-I`` as a file of include patterns +and rejects the other two. ``gtar`` is GNU tar, so those options run a command: +``high``. ``bsdtar`` runs nothing there: ``-I`` is the value of an option whose effect +is not a command (``medium``), and the GNU options are not its own, so the input after +one is judged as after any option the model does not describe. A bare ``tar`` may be +either: running is possible, its exploitability is to examine (``high``, hotspot). +``--use-compress-program`` runs the compressor it names on both. +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from coretrace_python import engine +from coretrace_python.source import SourceManager + +PLUGINS = Path(__file__).resolve().parent.parent / "src" / "coretrace_python" / "bundled" + +HIGH = ("high", "vulnerability", "high") +MEDIUM = ("medium", "vulnerability", "high") +HIGH_TO_REVIEW = ("high", "hotspot", "medium") + + +def judged(call: str) -> list[tuple[str, str, str]]: + """Severity, verdict and confidence of each command-injection finding on ``call``, + made with ``x`` read from the command line.""" + + text = f"import subprocess\nimport sys\n\n\ndef run():\n x = sys.argv[1]\n {call}\n" + findings = engine.check(SourceManager().add_source("app/run.py", text), [PLUGINS]) + return [ + (f.severity.value, f.metadata["verdict"], f.confidence.value) + for f in findings + if f.rule_id == "command-injection" + ] + + +@pytest.mark.parametrize( + ("call", "expected"), + [ + ('subprocess.run(["tar", f"-I{x}", "-xf", "a.tar"])', HIGH_TO_REVIEW), + ('subprocess.run(["tar", "-I", x, "-xf", "a.tar"])', HIGH_TO_REVIEW), + ('subprocess.run(["tar", "--to-command", x, "-xf", "a.tar"])', HIGH_TO_REVIEW), + ( + 'subprocess.run(["tar", f"--checkpoint-action=exec={x}", "-xf", "a.tar"])', + HIGH_TO_REVIEW, + ), + ('subprocess.run(["/usr/bin/gtar", f"-I{x}", "-xf", "a.tar"])', HIGH), + ('subprocess.run(["gtar", "--to-command", x, "-xf", "a.tar"])', HIGH), + ('subprocess.run(["gtar", f"--checkpoint-action=exec={x}", "-xf", "a.tar"])', HIGH), + ('subprocess.run(["bsdtar", f"-I{x}", "-xf", "a.tar"])', MEDIUM), + # Not an option of bsdtar: the input after it may be an option of its own, and + # --use-compress-program=CMD fits in one element. + ('subprocess.run(["bsdtar", "--to-command", x, "-xf", "a.tar"])', HIGH_TO_REVIEW), + ('subprocess.run(["tar", "--use-compress-program", x, "-xf", "a.tar"])', HIGH), + ('subprocess.run(["gtar", "--use-compress-program", x, "-xf", "a.tar"])', HIGH), + ('subprocess.run(["bsdtar", "--use-compress-program", x, "-xf", "a.tar"])', HIGH), + ('subprocess.run(["bsdtar", "-xf", "a.tar", "--", x])', None), + ('subprocess.run(["gtar", "-xf", "a.tar", "--", x])', None), + ], +) +def test_the_reading_follows_the_tar_implementation( + call: str, expected: tuple[str, str, str] | None +) -> None: + assert judged(call) == ([] if expected is None else [expected])