diff --git a/gitgalaxy/security/security_lens.py b/gitgalaxy/security/security_lens.py index 1ae760918..926f5e9e4 100644 --- a/gitgalaxy/security/security_lens.py +++ b/gitgalaxy/security/security_lens.py @@ -197,10 +197,52 @@ def __init__(self): # reads (common benign self-inspection, e.g. version banners) -- # only the unambiguous "self-path handed straight into a mutating # call" shape qualifies. + # A worm's defining mechanical trait: duplicating or overwriting + # itself. The precision rule that shipped in #1150/#1169 is kept + # exactly -- the self-reference token has to visibly, directly feed + # a copy/write call, never merely appear nearby -- and only the + # ways it can *reach* that call are broadened (#1172, #1174). + # + # #1172: `__FILE__` was already an accepted token, but no PHP or + # Ruby copy/write function was ever paired with it, so that half of + # the alternation was dead: `__FILE__` appears only in PHP/Ruby + # source, which never calls `shutil.copy`. PowerShell and Shell are + # added for the same reason they matter in the wild -- self-copy to + # a startup folder or cron directory is the classic dropper + # persistence step. + # + # #1174: two shapes that are equally worm-like slipped through the + # literal-first-argument rule -- a single bounded path-normalization + # wrapper (`os.path.abspath(__file__)`), and read-then-write + # (`fs.writeFileSync(dest, fs.readFileSync(__filename))`), which is + # a self-copy spelled as two calls. + # + # Every quantifier here is bounded (Engine Rule 14); `$0` is only + # ever accepted as a literal argument to `cp`/`install`, since it is + # otherwise ubiquitous in usage banners and logging. "self_propagation": re.compile( - r"\b(?:fs\.(?:copyFileSync|writeFileSync|appendFileSync|renameSync)|" - r"shutil\.(?:copy2?|copyfile|move)|os\.rename)" - r"\s*\([ \t]*(?:__filename|__dirname|import\.meta\.url|__file__|__FILE__)\b", + # 1. copy/write/rename call taking the self-reference as its + # first argument, optionally through ONE normalizer. + r"\b(?:fs\.(?:copyFileSync|writeFileSync|appendFileSync|renameSync)" + r"|shutil\.(?:copy2?|copyfile|move)|os\.rename" + r"|FileUtils\.(?:cp|copy|mv)|File\.write" + r"|file_put_contents|copy|rename" + r"|Copy-Item|Move-Item" + r")\s*\(\s*" + r"(?:(?:os\.path\.(?:abspath|realpath|normpath)|path\.resolve|Resolve-Path|realpath)\s*\([ \t]*){0,1}" + r"[\"']?(?:__filename|__dirname|import\.meta\.url|__file__" + r"|\$PSCommandPath|\$MyInvocation\.MyCommand\.Path|\$0)\b" + # 2. shell / powershell command form -- no parentheses at all. + r"|\b(?:cp|install|Copy-Item|Move-Item)\b[ \t]+" + r"(?:-{1,2}[A-Za-z-]{1,20}(?:[ \t]+[A-Za-z0-9._/=-]{1,32})?[ \t]+){0,3}" + r"[\"']?(?:\$0|\$PSCommandPath|\$MyInvocation\.MyCommand\.Path)\b" + # 3. read-then-write: writing elsewhere the bytes it just read + # of itself. Bounded, lazy gap over a negated class. + r"|\b(?:fs\.(?:writeFileSync|appendFileSync)|File\.write|file_put_contents" + r"|shutil\.copyfileobj)\s*\([^)\n]{0,120}?" + r"(?:fs\.readFileSync|File\.read|file_get_contents|open)\s*\(\s*" + r"(?:(?:os\.path\.(?:abspath|realpath|normpath)|path\.resolve|realpath)\s*\([ \t]*){0,1}" + r"[\"']?(?:__filename|__dirname|import\.meta\.url|__file__)\b", re.I, ), } diff --git a/tests/ruff_audit_baseline.json b/tests/ruff_audit_baseline.json index e78c068b6..ae1e39889 100644 --- a/tests/ruff_audit_baseline.json +++ b/tests/ruff_audit_baseline.json @@ -45,7 +45,7 @@ "gitgalaxy/recorders/sbom_recorder.py:221: PERF401": "Use `list.extend` to create a transformed list", "gitgalaxy/security/security_auditor.py:359: RUF046": "Value being cast to `int` is already an integer", "gitgalaxy/security/security_auditor.py:426: PERF203": "`try`-`except` within a loop incurs performance overhead", - "gitgalaxy/security/security_lens.py:401: SIM102": "Use a single `if` statement instead of nested `if` statements", + "gitgalaxy/security/security_lens.py:443: SIM102": "Use a single `if` statement instead of nested `if` statements", "gitgalaxy/standards/config_resolver.py:254: UP045": "Use `X | None` for type annotations", "gitgalaxy/standards/config_resolver.py:255: UP045": "Use `X | None` for type annotations", "gitgalaxy/standards/config_resolver.py:256: UP045": "Use `X | None` for type annotations", diff --git a/tests/security_auditing/test_security_lens.py b/tests/security_auditing/test_security_lens.py index f62e146c8..ff5cda476 100644 --- a/tests/security_auditing/test_security_lens.py +++ b/tests/security_auditing/test_security_lens.py @@ -455,6 +455,91 @@ def test_self_propagation_detects_self_copy_and_self_overwrite(lens): assert lens.scan_content(py_self_copy)["counts"].get("self_propagation", 0) > 0 +def test_self_propagation_covers_php_ruby_powershell_and_shell(lens): + """ + [DETECTION] #1172: `__FILE__` was already an accepted token, but no PHP or + Ruby copy/write function was paired with it anywhere -- and `__FILE__` only + appears in PHP/Ruby source, which never calls `shutil.copy`. That half of + the alternation could therefore never match. PowerShell and Shell are the + two ecosystems where self-copy-to-a-startup-location is the classic dropper + persistence step. + """ + cases = { + "php copy": "copy(__FILE__, '/var/www/html/.cache.php');", + "php read-write": "file_put_contents($dest, file_get_contents(__FILE__));", + "ruby cp": "FileUtils.cp(__FILE__, dest)", + "ruby write": "File.write(dest, File.read(__FILE__))", + "powershell": "Copy-Item $PSCommandPath -Destination $startupFolder", + "powershell invocation": "Copy-Item $MyInvocation.MyCommand.Path $dest", + "shell cp": 'cp "$0" /etc/cron.hourly/update', + "shell install": "install -m 755 $0 /usr/local/bin/updater", + "shell cp with flags": 'cp -f -- "$0" "$HOME/.config/autostart/x.sh"', + } + for label, src in cases.items(): + assert lens.scan_content(src)["counts"].get("self_propagation", 0) > 0, label + + +def test_self_propagation_tolerates_normalization_wrappers_and_read_then_write(lens): + """ + [DETECTION] #1174: two shapes that are equally worm-like slipped through the + literal-first-argument rule -- one bounded path-normalization wrapper, and + a self-copy spelled as two calls instead of one. + """ + cases = { + "py abspath wrapper": "shutil.copy(os.path.abspath(__file__), dest)", + "js resolve wrapper": "fs.copyFileSync(path.resolve(__filename), dest)", + "js read-then-write": "fs.writeFileSync(dest, fs.readFileSync(__filename))", + "py realpath wrapper": "shutil.copy2(os.path.realpath(__file__), target)", + } + for label, src in cases.items(): + assert lens.scan_content(src)["counts"].get("self_propagation", 0) > 0, label + + +def test_self_propagation_does_not_fire_on_ordinary_shell_and_powershell_idioms(lens): + """ + [FALSE POSITIVE DEFENSE] `$0` is ubiquitous in usage banners and logging, + and `$PSCommandPath` in diagnostics -- precision depends entirely on + requiring the token as a literal argument to a copy call, which is the same + discipline the JS/Python half has always used. A `cp` of anything else, or + with a flag VALUE in the way, must stay silent too. + """ + benign = { + "usage banner": 'echo "usage: $0 [--force] "', + "basename logging": 'log_info "starting $(basename $0)"', + "powershell logging": 'Write-Host "running from $PSCommandPath"', + "php dirname": "$dir = dirname(__FILE__);", + "php realpath only": "$self = realpath(__FILE__);", + "ruby read only": "content = File.read(__FILE__)", + "cp unrelated": "cp /etc/hosts /tmp/hosts.bak", + "cp with flag value": "cp -m 755 /src/a /dst/b", + "install unrelated": "install -m 644 config.yml /etc/app/", + "cp other variable": 'cp -f "$SRC" /tmp/out', + "unrelated copy call": "copy(source_list, destination)", + } + for label, src in benign.items(): + assert lens.scan_content(src)["counts"].get("self_propagation", 0) == 0, label + + +def test_self_propagation_redos_immunity(lens): + """ + Every quantifier in the widened pattern is bounded (Engine Rule 14). The + adversarial inputs target each new branch's gap: the flag repeat, the + normalization wrapper, and the read-then-write span. + """ + import time + + payloads = [ + "cp " + "-a " * 20000, + "shutil.copy(" + "os.path.abspath(" * 5000, + "fs.writeFileSync(" + "x," * 20000, + "Copy-Item " + " " * 100000, + ] + for payload in payloads: + start = time.perf_counter() + lens.scan_content(payload) + assert time.perf_counter() - start < 3.0, f"pathological backtracking on {payload[:30]!r}" + + def test_self_propagation_ignores_ordinary_path_resolution_and_self_reads(lens): """ [FALSE POSITIVE DEFENSE] __filename/__file__ used for ordinary path resolution