fix(scan): require XML evidence in the file before reporting XXE at parse() - #86
Merged
ralyodio merged 1 commit intoAug 10, 2026
Conversation
…arse() `java-xxe-parse-call` matched on the receiver's *name* — anything suffixed Builder, Parser or Reader calling `.parse(`. That shape is right for the vulnerability, because the real XXE is `builder.parse(is)` and the declared type is rarely on that line, but the suffix says nothing about XML. Any parser that parses something else matched too, at high severity under CWE-611. On ionic-team/capacitor that meant `HostMask.Parser.parse(origins)` — a hostname mask — reported three times in Bridge.java and UriMatcher.java, plus three more in a unit test asserting on "*.example.org" strings. Adds `fileRequires`, a precondition tested against the whole file rather than the guard window, and points the rule at the XML packages and types. A file that parses XML imports javax.xml or org.xml.sax at the top; one that never mentions XML is not parsing it. The window could not answer this — an import sits hundreds of lines from the match, and widening guardBack far enough would drag unrelated evidence into every other rule. The whole-file text is memoised on the lines array so the check costs one join per file rather than one per line. Verified against ionic-team/capacitor: 19 findings to 13, removing all six java-xxe-parse-call reports and nothing else. Combined with the scoped typosquat fix, 21 to 13. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ThreatCrush Security Scan144 finding(s) HIGH/CRITICAL: 12 | MEDIUM: 96 | LOW: 36
…and 94 more. Full results in the Security tab. Snippets are redacted; ThreatCrush never prints matched credential material. |
This was referenced Aug 10, 2026
ralyodio
added a commit
that referenced
this pull request
Aug 10, 2026
…ge guard (#88) 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>
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
1 participant
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.
Stacked on #85 — review that one first; this PR's diff is the second commit.
The bug
java-xxe-parse-callmatches on the receiver's name:pattern: /\b\w*(?:[Bb]uilder|[Pp]arser|[Rr]eader)\s*\.\s*parse\s*\(/That shape is right for the vulnerability — real XXE is
builder.parse(is), and the declared type is rarely on the same line — but a name suffix says nothing about XML. Every parser that parses something else matched too, at high severity under CWE-611.On
ionic-team/capacitor, all six hits wereHostMask.Parser.parse(...), a hostname-mask parser:Bridge.java:254,:1582UriMatcher.java:148HostMaskTest.java:12–14"*.example.org"in a unit testThose three files contain zero references to
javax.xml,org.xml.sax,org.w3c.dom, or any XML type.The fix
Adds
fileRequires, a precondition tested against the whole file rather than the guard window, and points the rule at the XML packages and types.A file that parses XML says so at the top; one that never mentions XML is not parsing it. The existing
requirescould not express this — it searches a ±6-line window, and an import sits hundreds of lines from the match. WideningguardBackfar enough to reach it would drag unrelated evidence into every other rule that shares the window.The rule still fires on the shape it was written for —
builder.parse(is)in a file that importsDocumentBuilderFactory— which is covered by a new positive test, so this narrows scope without giving up the detection.Cost
The whole-file text is memoised on the
linesarray via aWeakMap, so the check costs onejoinper file rather than one per line. Without that, a file-level regex would make scanning quadratic in file length.Verification
Against
ionic-team/capacitorat5e5bb3b: 19 findings → 13, removing all sixjava-xxe-parse-callreports and nothing else. Combined with #85, 21 → 13.Three tests added. The two negative cases fail on
masterwithjava-xxe-parse-callfiring; the positive case pins that a genuinely unhardened XML parse is still caught.vitest runinapps/cli: 68 passed.tsc --noEmit: clean.