fix(command-injection): read tar options by implementation (#221)

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).

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),

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])