fix: discover hidden executorch pickle members - #1547
Conversation
Performance BenchmarksCompared
|
|
Critical review complete and fixes published at |
![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: f5249a9766
ℹ️ 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".
|
I addressed all three review findings locally in commit Validation on the integrated head:
Shell push is blocked by an invalid repository credential in this environment. @codex please address the three unresolved review threads using the resolutions described in my thread replies, preserve the malicious-positive and benign-negative regressions, add the routed exit-code/cache regression, merge current |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f5249a9766
ℹ️ 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".
|
@codex review All five prior review threads are resolved on |
|
Codex Review: Didn't find any major issues. Delightful! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Pull request was converted to draft
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e3330cad0
ℹ️ 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".
| candidate = sample | ||
| removed_token_count = 0 | ||
|
|
||
| while (token_index := candidate.find(b"#\n")) > 0: |
There was a problem hiding this comment.
When a hidden protocol-1 member contains an earlier binary string payload with the bytes #\n before the malicious GLOBAL ... #\n ... REDUCE sequence, this find stops on the data bytes first; parsing the truncated BINSTRING fails and _without_global_comment_tokens() returns None. The member then fails discovery and never reaches PickleScanner, so the existing comment-token evasion can still bypass hidden-member scanning by prepending a harmless BINSTRING containing #\n.
Useful? React with 👍 / 👎.
|
|
||
| if last_opcode_name != "GLOBAL": |
There was a problem hiding this comment.
Hidden protocol-0 payloads can use the same inserted #\n token after an INST opcode instead of GLOBAL; this branch only strips the token when the preceding opcode is GLOBAL, so a stream like (S'cmd'\ntiposix\nsystem\n#\n. is rejected during discovery and never reaches PickleScanner. Since INST carries the same module/name execution target, this leaves another extensionless ExecuTorch pickle evasion path.
Useful? React with 👍 / 👎.
| has_binary_protocol = _looks_like_binary_pickle_protocol(data_start) | ||
| has_protocolless_binary_start = data_start[0] in _PICKLE_PROTOCOLLESS_BINARY_START_BYTES | ||
| has_proto0_or_1_start = data_start[0] in PROTO0_1_START_BYTES | ||
| if not ( | ||
| incomplete_protocol_prefix | ||
| or has_binary_protocol | ||
| or has_protocolless_binary_start | ||
| or has_proto0_or_1_start | ||
| ): |
There was a problem hiding this comment.
A valid pickle can start with the PROTO opcode carrying protocol 0 (\x80\x00...), but this hidden-member gate only treats \x80 as a pickle when _looks_like_binary_pickle_protocol() accepts the following byte, which starts at protocol 1. An extensionless ExecuTorch ZIP member using \x80\x00 before a dangerous GLOBAL/REDUCE stream is therefore omitted from pickle_files and never scanned, even though the same payload would be analyzed if the member were named .pkl.
Useful? React with 👍 / 👎.
1 participant
Summary
GLOBALcomment-token evasions for embedded pickle analysisCritical review findings addressed
#\ntokens adjacent to parsedGLOBALopcodesThe final review found an additional false negative on the prior head: two consecutive
#\ntokens caused an extensionless malicious protocol-0 payload to be omitted, while the same bytes produced five findings when passed directly toPickleScanner. The current revision detects that payload, keeps repeated-token text near-matches unselected, and refuses inconclusively after 64 comment tokens.Validation
PROMPTFOO_DISABLE_TELEMETRY=1 uv run --frozen pytest -q tests/scanners/test_executorch_scanner.py tests/test_cve_2025_10155_bin_pickle.py(125 passed)git diff --check(clean)Published revision
9e3330cad03288a046aee9264c59cd6d682f6e9d1acb9886f5e9eb5a1751ebae8ce7b954d860f911mainat423913409542aa6e4c3fc5eccffb2d6b1cb1f218Review state