fix(scanner): discover pickle payloads by content, not file extension - #92
Conversation
Discovery decided what to open from a file's suffix alone, which is the whole of CVE-2025-1889: name the payload `config.p` and nothing ever opens it, so the scan reports "No AI models found" and exits 0 — a clean bill of health on a directory carrying a reverse shell. The inspector was never the problem; it already decides what a file is from its bytes. Only discovery still trusted the name. Adding `.p` to the extension list was rejected. It would have flipped the published corpus case while protecting nobody, because the technique is *any* unexpected suffix, not that one — the attacker picks the name. That is exactly the laundering the scorecard floor exists to prevent. Unclaimed files are now sniffed by content. The bar is deliberately high in both directions: a missed payload is the bug being closed, but claiming ordinary repository files as models would be worse, filling every SBOM with phantom components until findings stop meaning anything. Parsing as opcodes turned out to be far too weak a test. `.` is the STOP opcode, so on a "reaches STOP" rule every stylesheet opening with a class selector is a one-opcode pickle; measured against a real node_modules, that rule claimed 5 JavaScript/TypeScript files, a stylesheet and a man page. The sniff therefore validates the pickle stack via pickletools.dis, and those six filenames are now regression tests. Validation runs on the prefix ending at the first STOP rather than the whole buffer, so a payload followed by a corrupt tail — the nullifAI shape — is still recognized instead of being discarded along with its own garbage. Reads are bounded: 64KB per unclaimed file, escalating only when opcodes parsed cleanly and no STOP appeared (a real pickle larger than the window), capped at the variant inspector's own budget so discovery is exactly as far-sighted as the inspection that follows it. A walk over 5,101 files takes 1.52s and claims nothing. Known limit, stated rather than hidden: a pickle whose first opcode carries an argument longer than the entire sniff budget parses nothing here and is not discovered by content. Reaching that needs a single 16MB-plus literal ahead of the payload. Scorecard: 8/11 -> 9/11. cve-2025-1889-nonstandard-extension moves from missed to detected in both modes and the floor is raised accordingly, locking it in.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b0b631b8f5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| elif self._sniff_is_pickle(full_path): | ||
| # Nothing claimed this file by name, but its bytes read as a | ||
| # pickle. This is the CVE-2025-1889 class: `config.p`, `weights.dat`, | ||
| # a file with no extension at all. The inspector below already | ||
| # decides what a file *is* from its contents rather than its | ||
| # suffix — only discovery was still trusting the name. | ||
| self.artifacts.append(self._inspect_pickle_variant(full_path)) |
There was a problem hiding this comment.
This fallback is only called from the local-file dispatcher, so the same bare pickle remains invisible when scanning remotely. In the scan() remote branch, direct HTTP targets are still dispatched solely by suffix, while resolve_huggingface_repo() filters unsupported suffixes before returning URLs; consequently, an hf:// repository or direct URL containing the CVE's config.p payload produces no artifact or error. Remote targets need an equivalent bounded content-sniffing path.
Useful? React with 👍 / 👎.
| try: | ||
| for opcode, _arg, pos in pickletools.genops(io.BytesIO(data)): | ||
| parsed += 1 | ||
| if opcode.name == "STOP": | ||
| stop_at = pos + 1 | ||
| break |
There was a problem hiding this comment.
When an attacker renames a supported wrapped format, such as a zlib-compressed joblib model or an object-array .npy, this opcode walk starts at the wrapper header and rejects the file even though _inspect_pickle_variant() can identify and scan those formats by content. For example, changing a malicious model.joblib to model.dat still leaves it undiscovered, preserving the same extension-confusion bypass this change is intended to close. The sniff should recognize supported container magics and route them to the variant inspector as well.
Useful? React with 👍 / 👎.
| if stop_at is None: | ||
| # No complete pickle in view. Worth a bigger read only if something | ||
| # actually parsed; a file that yielded nothing is simply not a pickle. | ||
| return (False, parsed > 0) |
There was a problem hiding this comment.
Requiring a STOP opcode makes content discovery reject malformed streams that execute before failing. For example, the repository's nullifAI shape removes STOP after an os.system REDUCE and appends an invalid opcode; scan_pickle_stream() deliberately salvages and flags that payload, but head_looks_like_pickle() returns only (False, True), and _sniff_is_pickle() ultimately rejects it after reading the whole file. Naming that payload with an unlisted suffix therefore combines the two documented evasions and restores a zero-artifact scan; dangerous globals recovered from an incomplete stream should be enough to claim the file.
Useful? React with 👍 / 👎.
1 participant
The gap
Discovery decided what to open from a file's suffix alone. That is the whole of CVE-2025-1889: name the payload
config.pand nothing ever opens it, so the scan reportsNo AI models foundand exits 0 — a clean bill of health on a directory carrying a reverse shell.The inspector was never the problem.
_inspect_pickle_variantalready decides what a file is from its bytes. Only discovery still trusted the name.Why not just add
.pBecause it protects nobody. The technique is any unexpected suffix — the attacker picks the name — so a one-line extension addition would flip the published corpus case while leaving
weights.datwide open. That is exactly the launderingtests/corpus/floor.jsonexists to prevent.What landed
Unclaimed files are sniffed by content, and the bar is deliberately high in both directions. A missed payload is the bug being closed; claiming ordinary repository files as models would be worse, filling every SBOM with phantom components until findings stop meaning anything.
Parsing as opcodes proved far too weak a test.
.is the STOP opcode, so on a "reaches STOP" rule every stylesheet opening with a class selector is a one-opcode pickle. Measured against a real 5,101-filenode_modules, that first rule claimed 11 files — 5 JavaScript/TypeScript files, a stylesheet, a man page. The sniff now validates the pickle stack viapickletools.dis, and those six filenames are regression tests intests/test_content_discovery.py.Validation runs on the prefix ending at the first STOP rather than the whole buffer, so a payload followed by a corrupt tail — the nullifAI shape — is still recognized instead of being discarded along with its own garbage.
Reads are bounded: 64KB per unclaimed file, escalating only when opcodes parsed cleanly and no STOP appeared, capped at the variant inspector's own budget so discovery is exactly as far-sighted as the inspection that follows it.
Known limit, stated rather than hidden: a pickle whose first opcode carries an argument longer than the entire sniff budget parses nothing here and is not discovered by content. Reaching that needs a single 16MB-plus literal ahead of the payload.
Scorecard
8/11 → 9/11.
cve-2025-1889-nonstandard-extensionmoves frommissedtodetectedin both modes; the floor is raised accordingly, locking it in.Verification
poetry run pytest— 824 passed, 91.38% coverage (gate 85%).aisbom bypass-scorecard --check— gate green at 9/11.node_modules: 11 → 0.config.p,payload.dat,weights.modelandblobreports all four asCRITICAL (RCE Detected: os.system)and exits 2;README.mdandsettings.jsonbeside them stay unclaimed.