feat(scan): shebang scripts, shell and PHP rule sets, language-coverage guard (v0.6.0) - #88
Merged
Conversation
…ge guard Release 0.6.0. Two languages the scanner claimed to support had no rules at all. `shell` and `php` were both in `ScanLanguage`, both mapped from extensions, and between them zero of the 46 code rules targeted either. From the outside they looked supported: files were read, matched against the secret rules, and reported clean whatever the code did. Language detection made it worse — it was extension-only, so an executable named for the command it provides rather than the language it is written in was never opened. `ralyodio/debtap`, 3,511 lines of bash in a file called `debtap`, scanned clean by scanning nothing and exited 0 while doing it. - Shebang detection for extensionless files, from a 128-byte prefix so a checked-in blob costs one small read rather than a megabyte decoded and discarded. - Seven shell rules: remote script execution, eval on an expansion, unquoted expansion in a recursive remove, disabled certificate verification, plain-HTTP download, world-writable permissions, predictable temp paths. - Eight PHP rules: SQL interpolation, shell interpolation, dynamic code execution, dynamic include, unserialize on request data, unescaped output, request-driven path traversal, extract() variable injection. - A language-coverage test that fails when a language the scanner claims has no rule targeting it. Both gaps above existed because nothing checked the two lists against each other; now something does. Also carries the two fixes stranded on intermediate branches when the stack was merged out of order: scoped packages are no longer reduced to the part after the slash (#85), and XXE requires XML evidence in the file (#86). Every rule is built against the corrected shape as well as the vulnerable one. The eval rule matches eval's argument rather than the whole line: the line-wide form counts bash's dynamic-range idiom, where every expansion is arithmetic and cannot carry a command, and reported it 355 times in debtap alone. debtap: 0 findings (nothing scanned) to 8, all genuine. capacitor: unchanged at 13. Tests: 100, up from 68. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| }); | ||
|
|
||
| it('flags dynamic code execution', () => { | ||
| expect(ruleIds('a.php', 'eval($code);')).toContain('php-dynamic-code-execution'); |
|
|
||
| it('flags dynamic code execution', () => { | ||
| expect(ruleIds('a.php', 'eval($code);')).toContain('php-dynamic-code-execution'); | ||
| expect(ruleIds('a.php', 'eval("return $expr;");')).toContain('php-dynamic-code-execution'); |
| * line of every installer. | ||
| */ | ||
| const UNTRUSTED_SH = | ||
| /\$\{?[1-9]\d*\b|\$[@*]|\$\{@\}|\bread\s+(?:-\S+\s+)*[A-Za-z_]\w*|\$\{?REPLY\b|\$\{?QUERY_STRING\b/; |
| languages: ['shell'], | ||
| // The pipe must be the *next* thing: `curl -o f url && sh f` is a different | ||
| // (and checkable) shape, and `curl url | jq` is not an execution at all. | ||
| pattern: /\b(?:curl|wget)\b[^|\n]*\|\s*(?:sudo\s+(?:-\S+\s+)*)?(?:\/bin\/|\/usr\/bin\/)?(?:ba|da|k|z|a)?sh\b/, |
| // actually re-parses untrusted text as source. `eval echo $x` is not | ||
| // covered; catching it without also catching the range idiom needs to know | ||
| // which expansions are arithmetic, which is parsing, not matching. | ||
| pattern: /\beval\s+(?:-\S+\s+)*(?:"\s*)?\$(?:\{?[A-Za-z_]\w*|\((?!\())/, |
| // never reaches the `$` and never matches. Only a genuinely bare expansion | ||
| // does. Restricted to recursive/forced removal: a bare `$f` in `rm $f` is | ||
| // sloppy, but it is not the shape that erases a filesystem. | ||
| pattern: /\brm\s+(?:-[a-zA-Z-]*[rRf][a-zA-Z-]*\s+)+[^"'\n]*?\$\{?[A-Za-z_]/, |
| cwe: 'CWE-732', | ||
| severity: 'medium', | ||
| languages: ['shell'], | ||
| pattern: /\bchmod\s+(?:-[a-zA-Z-]+\s+)*(?:0?777|a\+rwx|ugo\+rwx|a=rwx)\b/, |
| // Redirection or an explicit write into a literal `/tmp` path. A `$$` or | ||
| // `$RANDOM` suffix is still predictable, so it is not treated as a fix; | ||
| // `mktemp` is, and it is the guard below. | ||
| pattern: /(?:>{1,2}\s*|\b(?:tee|touch|cp|mv|install)\s+(?:-\S+\s+)*)\/tmp\/[\w.$-]+/, |
ThreatCrush Security Scan152 finding(s) HIGH/CRITICAL: 12 | MEDIUM: 104 | LOW: 36
…and 102 more. Full results in the Security tab. Snippets are redacted; ThreatCrush never prints matched credential material. |
| // never reaches the `$` and never matches. Only a genuinely bare expansion | ||
| // does. Restricted to recursive/forced removal: a bare `$f` in `rm $f` is | ||
| // sloppy, but it is not the shape that erases a filesystem. | ||
| pattern: /\brm\s+(?:-[a-zA-Z-]*[rRf][a-zA-Z-]*\s+)+[^"'\n]*?\$\{?[A-Za-z_]/, |
| expect(ruleIds('i.sh', 'curl -fsSL https://example.invalid/i.sh | bash')).toContain( | ||
| 'sh-remote-script-execution', | ||
| ); | ||
| expect(ruleIds('i.sh', 'wget -qO- https://example.invalid/i.sh | su' + 'do sh')).toContain( |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Release 0.6.0.
Recovers work stranded by the stack merge
#85, #86 and #87 were stacked. #85 merged to
master; #86 and #87 merged into their intermediate base branches, so their content never reachedmaster—mastertoday has neitherfileRequiresnor shebang detection nor the shell rules. This branch is cut frommasterand carries all of it.Two languages had no rules
shellandphpwere both inScanLanguage, both mapped from extensions, and between them zero of the 46 code rules targeted either. From the outside they looked supported: files were read, matched against the secret rules, and reported clean whatever the code did.Detection made it worse — extension-only, so an executable named for the command it provides rather than the language it is written in was never opened at all:
A clean scan that scanned nothing is the one result a security tool must never produce.
What's in it
.pngis not a script.curl | bash(CWE-494),evalon an expansion (CWE-78), unquoted expansion inrm -rf(CWE-78), disabled cert verification (CWE-295), plain-HTTP download (CWE-319),chmod 777(CWE-732), predictable temp paths (CWE-377).eval(CWE-95), dynamicinclude(CWE-98),unserializeon request data (CWE-502), unescaped output (CWE-79), request-driven path traversal (CWE-22),extract()variable injection (CWE-621).Every rule is built against the corrected shape as well as the vulnerable one, per this file's existing contract:
rm -rf "$DIR"doesn't match,escapeshellarg($f)doesn't match,include basename($p)doesn't match, a prepared statement doesn't match,curl … | jqdoesn't match.The eval rule matches eval's argument, not the line
The obvious spelling,
\beval\b.*\$, is wrong. Bash builds a numeric range withEvery expansion sits inside
$((…)), which the shell parses as an arithmetic expression — a;there is a syntax error, not a second command. A line-wide search still finds$kinside it and fires: 355 findings in debtap alone, all one safe loop. Anchoring to the argument keepseval "$cmd"and drops the idiom.Verification
curl -k, written to/var/cache/debtap/, then used to build packages.5e5bb3b: unchanged at 13. PHP and shell rules fire on none of it; shebang detection adds no files.tsc --noEmitclean.Tuning was measured, not guessed: debtap went 368 → 13 (eval anchored) → 8 (plain-HTTP double-report removed).
After merge
Tag
v0.6.0to publish. ralyodio/debtap#1 pins0.6.0and is red until then.