Skip to content

fix: detect pickle binunicode8 setitem abuse - #1524

Merged
mldangelo-oai merged 2 commits into
mainfrom
mdangelo/codex/fix-pickle-binunicode8
Jun 9, 2026
Merged

mldangelo-oai merged 2 commits into
mainfrom
mdangelo/codex/fix-pickle-binunicode8

Conversation

@mldangelo-oai

@mldangelo-oai mldangelo-oai commented Jun 6, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • add BINUNICODE8 to shared pickle literal analysis without treating byte literals as STACK_GLOBAL names
  • scope CVE-2026-24747 SETITEM / SETITEMS evidence to the actual mutation target and to bounded individual pickle streams
  • analyze concatenated and incomplete streams across padding and unrecognized separators, including protocol-less scalar/container roots and memo-get prefixes
  • preserve fail-closed coverage after 64 streams with a single-buffer, linear stream probe
  • structurally route raw Joblib pickles, including truncated persistent-ID payloads and large leading literals, without decoding file-sized opcode arguments
  • register malicious and benign regressions for reduced CI lanes

False-positive / false-negative audit

  • FN closed: a non-pickle separator can no longer hide a malicious later stream
  • FN closed: later incomplete streams and escaped STACK_GLOBAL operands retain stream-local attribution
  • FN closed: malicious extra streams beyond the 64-stream cap make the scan inconclusive
  • FN closed: truncated Joblib PERSID / BINPERSID payloads reach pickle analysis and emit S212
  • FP closed: direct globals and plain strings named _rebuild_tensor* are not treated as executable mutation targets
  • FP closed: ordinary dict/list/mapping keys and values, raw trailing text, byte literals, and bare STOP-prefixed text remain clean
  • DoS closed: Joblib protocol-0 classification skips large operands structurally instead of materializing them through pickletools.genops

Validation

  • associated tests: 270 passed
    • tests/detectors/test_cve_detection.py_cases/test_pickle_binunicode8_setitem.py
    • tests/scanners/test_joblib_scanner_codecs.py
    • tests/scanners/test_pickle_scanner.py
  • scoped Ruff format/check, mypy, and git diff --check: clean
  • 100 MiB leading Unicode operand probe: 0 KiB additional peak RSS, about 0.004s
  • 3,000 generated valid pickles across protocols 0-5: zero routing misses
  • full test suite intentionally left to CI

Published state

  • current main integrated: 6f4407d7f28f7dd61a6f520a493bf85c1a68123f
  • head: 69a449ac45b93099e3a6f7cfc572012e7692128b
  • exact tested tree: ed6eb9e62e47dfcb53de3bbb50c6c21e071f107a

All inline review threads are resolved.

@github-actions

github-actions Bot commented Jun 6, 2026 •

Copy link
Copy Markdown
Contributor

Workflow run and artifacts

Performance Benchmarks

Compared 12 shared benchmarks with a regression threshold of 15%.
Status: 4 regressions, 0 improved, 8 stable, 0 new, 0 missing.
Aggregate shared-benchmark median: 1.354s -> 1.810s (+33.6%).

Top regressions:

  • tests/benchmarks/test_scan_benchmarks.py::test_scan_single_checkpoint_before_load +64.6% (67.59ms -> 111.24ms, single-checkpoint-preflight, single_checkpoint.pkl, size=183.0 KiB, files=1)
  • tests/benchmarks/test_scan_benchmarks.py::test_scan_duplicate_registry_snapshot +60.6% (367.18ms -> 589.74ms, duplicate-heavy-registry, registry-snapshot, size=915.2 KiB, files=13)
  • tests/benchmarks/test_scan_benchmarks.py::test_scan_suspicious_pickle_intake +33.4% (139.30ms -> 185.76ms, suspicious-pickle-intake, suspicious-intake, size=183.8 KiB, files=4)
Workload Benchmark Target Size Files Baseline Current Change Status
single-checkpoint-preflight tests/benchmarks/test_scan_benchmarks.py::test_scan_single_checkpoint_before_load single_checkpoint.pkl 183.0 KiB 1 67.59ms 111.24ms +64.6% regression
duplicate-heavy-registry tests/benchmarks/test_scan_benchmarks.py::test_scan_duplicate_registry_snapshot registry-snapshot 915.2 KiB 13 367.18ms 589.74ms +60.6% regression
suspicious-pickle-intake tests/benchmarks/test_scan_benchmarks.py::test_scan_suspicious_pickle_intake suspicious-intake 183.8 KiB 4 139.30ms 185.76ms +33.4% regression
mixed-model-repository tests/benchmarks/test_scan_benchmarks.py::test_scan_release_candidate_repository release-candidate 547.3 KiB 32 464.51ms 598.42ms +28.8% regression
chunked-upload-stream tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_chunked_upload_stream chunked_stream 278.2 KiB 1 107.75ms 113.19ms +5.1% stable
nested-payload-review tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_nested_payload_review[nested_base64] nested_base64 98 B 1 541.9us 552.3us +1.9% stable
nested-payload-review tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_nested_payload_review[nested_hex] nested_hex 130 B 1 555.5us 565.4us +1.8% stable
clean-training-checkpoint tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_clean_training_checkpoint safe_large 278.2 KiB 1 107.13ms 109.04ms +1.8% stable
nested-payload-review tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_nested_payload_review[nested_raw] nested_raw 78 B 1 526.8us 536.0us +1.7% stable
warm-cache-rescan tests/benchmarks/test_scan_benchmarks.py::test_scan_warm_cached_repository_rescan release-candidate 547.3 KiB 32 98.12ms 99.82ms +1.7% stable
direct-malicious-upload tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_direct_malicious_upload malicious_reduce 52 B 1 498.6us 506.6us +1.6% stable
padded-multi-stream-upload tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_padded_multi_stream_upload multi_stream_padded 4.1 KiB 1 611.6us 614.2us +0.4% stable

@mldangelo-oai
mldangelo-oai marked this pull request as ready for review June 6, 2026 19:18

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 77e84e81ce

ℹ️ 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".

Comment thread CHANGELOG.md Outdated
Comment thread modelaudit/scanners/pickle_scanner.py

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f6a32cd1cf

ℹ️ 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".

Comment thread modelaudit/scanners/pickle_scanner.py Outdated

Copy link
Copy Markdown
Contributor Author

Verified the concurrent target-scoping fix at 3231794a instead of overwriting it. SETITEM/SETITEMS now classify abuse from the mutated target, with additional BUILD, legacy object, opaque-key, and benign mapping/dict/list regressions. Focused CVE validation: 36 passed; scoped Ruff, mypy, and diff checks are clean. The fresh false-positive thread is resolved.

mldangelo-oai commented Jun 8, 2026 •

Copy link
Copy Markdown
Contributor Author

@codex please publish local reviewed commit 0470cfad69a1485d44f6b8df60d144170204b479 onto this PR's head branch. It supersedes the stale local hash previously mentioned here, which is no longer present in the checkout.

Critical follow-up on the current-main merge reproduced and fixed additional false positives and false negatives:

  • a malicious _rebuild_tensor target in a second concatenated pickle stream was missed
  • a malicious os.system target in a later stream was missed
  • raw text after a complete pickle could combine with first-stream SETITEM metadata and manufacture CVE-2026-24747 attribution
  • an incomplete ordinary dictionary containing an _rebuild_tensor key was falsely attributed to CVE-2026-24747
  • incomplete unknown mutation targets still fail closed
  • NUL/newline-padded later streams are analyzed independently
  • benign later-stream dictionaries remain clean
  • CVE analysis is bounded to 64 streams; additional parseable streams mark the result explicitly inconclusive with an S902 coverage check

The branch also includes current public main at 532ae5969abe887bd67f67ba71c75d80ae2821c6 through merge commit b4131c16.

Validation, associated tests only:

  • PROMPTFOO_DISABLE_TELEMETRY=1 .venv/bin/pytest tests/scanners/test_pickle_scanner.py tests/detectors/test_cve_detection.py_cases/test_pickle_binunicode8_setitem.py -> 181 passed
  • .venv/bin/ruff check modelaudit/scanners/pickle_scanner.py tests/detectors/test_cve_detection.py_cases/test_pickle_binunicode8_setitem.py
  • .venv/bin/ruff format --check modelaudit/scanners/pickle_scanner.py tests/detectors/test_cve_detection.py_cases/test_pickle_binunicode8_setitem.py
  • .venv/bin/mypy modelaudit/scanners/pickle_scanner.py tests/detectors/test_cve_detection.py_cases/test_pickle_binunicode8_setitem.py
  • git diff --check

Shell GitHub authentication rejected the push. Do not merge remote head 3231794a0348e2667bea29847c654a623511bac3 without this fix.

Copy link
Copy Markdown
Contributor Author

Published the reviewed concatenated-stream fix requested in #issuecomment-4654449191 at 4349db17a4f080743f313872530a2722bb19e6e7.

The remote tree d5816141a3a28e366bb98d39c49795a79488f97e exactly matches the locally tested tree. This closes the later-stream false negatives, raw-tail state bleed, incomplete ordinary-dict false positive, protocol-0 follow-on bypass, and 64-stream coverage boundary. Associated validation is green: 198 tests plus scoped Ruff, format, mypy, and diff checks.

Copy link
Copy Markdown
Contributor Author

Fixed the Python CI regressions at ead71eda81f517261db5af580ed4cfce0820c485. The broader pickle prefix sniff had caused Joblib to treat BZ2 (BZh) and ordinary This... text as raw pickle opcodes. Joblib routing now requires bounded structural pickle evidence for protocol-less payloads while retaining explicit protocol headers as fail-closed raw input. The exact published tree is ebcbdbd593848fa7983d046a3816731cc34fd113; associated validation is 212 passed, broader Joblib QA is 48 passed, 18 skipped, and scoped Ruff/format/mypy/diff checks are clean. All existing review threads remain resolved.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ead71eda81

ℹ️ 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".

Comment thread modelaudit/scanners/pickle_scanner.py
Comment thread modelaudit/scanners/joblib_scanner.py Outdated

Copy link
Copy Markdown
Contributor Author

Addressed both new P1 review findings in 4527d3fc.

  • Complete pickle streams now retain stream-scoped, non-SETITEM CVE-2026-24747 attribution (including tensor metadata mismatch/storage-context evidence) while SETITEM attribution remains target-sensitive.
  • Truncated protocol-0 Joblib payloads are routed to pickle scanning once pickletools has parsed at least one real opcode, preserving RCE detection without reclassifying the existing opcode-looking text negatives.
  • Added malicious regressions for both paths.

Focused validation: Joblib codec and BINUNICODE8/CVE regression files passed; neighboring pickle/CVE selection 11 passed; scoped Ruff, format, mypy, and git diff --check are clean. Full suite left to CI as requested.

Copy link
Copy Markdown
Contributor Author

Published the final Joblib routing hardening at 8e80c70e74054c18219d1a87e5a904e552b72b86.

The follow-up closes both newly found edges:

  • malformed printable text beginning with opcode-like bytes such as R, N, ], or } no longer routes as a truncated pickle
  • a malicious protocol-0 pickle with a >4 KB leading literal still reaches its later GLOBAL/REDUCE detection

The exact published tree is f66df5ccb45ec8abbae3058bafbe3d3b93df71a7. Associated validation: 31 focused Joblib tests, scoped Ruff/format/mypy/diff checks, and a deterministic 20,000-sample printable-text fuzz pass with no incomplete-pickle routes. The full suite remains delegated to CI.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8e80c70e74

ℹ️ 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".

Comment thread modelaudit/scanners/pickle_scanner.py
Comment thread modelaudit/scanners/pickle_scanner.py
@mldangelo-oai
mldangelo-oai marked this pull request as draft June 9, 2026 08:19
@mldangelo-oai
mldangelo-oai marked this pull request as ready for review June 9, 2026 08:59
@mldangelo-oai
mldangelo-oai enabled auto-merge (squash) June 9, 2026 08:59
@mldangelo-oai
mldangelo-oai requested a review from mldangelo June 9, 2026 08:59
Scope CVE-2026-24747 SETITEM evidence to bounded pickle streams, preserve incomplete-stream coverage, and avoid near-match global false positives. Harden Joblib routing for truncated persistent-ID pickles.
@mldangelo-oai
mldangelo-oai force-pushed the mdangelo/codex/fix-pickle-binunicode8 branch from fff5735 to 3e2b7c0 Compare June 9, 2026 09:26

Copy link
Copy Markdown
Contributor Author

Audit follow-up pushed in 3e2b7c085ac52e532d745f8ab913ceaa240efffa (exact tested tree 9a36cf8efb07f9cb01ad59968fa057a85829e0fd). Closed incomplete later-stream coverage gaps, exact-matched dangerous globals to remove near-match CVE false positives, preserved 65th-stream fail-closed behavior, and routed truncated Joblib persistent IDs through S212. Associated validation: 237 tests passed; scoped Ruff, format, mypy, and diff checks are clean. Full-suite coverage is left to CI.

@mldangelo-oai
mldangelo-oai marked this pull request as draft June 9, 2026 10:24
auto-merge was automatically disabled June 9, 2026 10:24

Pull request was converted to draft

@mldangelo-oai
mldangelo-oai marked this pull request as ready for review June 9, 2026 14:53
@mldangelo-oai
mldangelo-oai merged commit 43d2fb8 into main Jun 9, 2026
23 of 27 checks passed
@mldangelo-oai
mldangelo-oai deleted the mdangelo/codex/fix-pickle-binunicode8 branch June 9, 2026 14:53

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 69a449ac45

ℹ️ 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".

Comment thread modelaudit/scanners/pickle_scanner.py
Comment thread modelaudit/scanners/joblib_scanner.py

Copy link
Copy Markdown
Contributor Author

Addressed both post-merge P1 findings in #1603, now merged as 76352d787e712d52833a72b985321d09094e3bfb.

  • Failed zero-opcode B/X stream probes now advance one byte and reprobe, while genuinely parsed incomplete streams remain whole. Malicious-positive and benign incomplete-stream regressions were added.
  • Protocol-0 BINSTRING/LONG4 routing now uses the required four-byte length prefix, with a later os.system REDUCE regression.

The same follow-up also restores the scan-time regression from about 110.8 ms to 73.2 ms against a 71.4 ms pre-regression baseline. Focused validation: 278 tests passed; targeted Ruff and mypy clean.

@github-actions github-actions Bot mentioned this pull request Jun 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant