[PR #1515] [MERGED] fix(apple): bound binary plist expansion before decoding keyed archives #2935

New issue

Closed

opened 2026-10-17 21:12:01 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-17 21:12:01 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/1515
Author: @kaoru0822-kitauji
Created: 10/10/2026
Status: ✅ Merged
Merged: 10/10/2026
Merged by: @kaoru0822-kitauji

Base: main ← Head: fix/keyed-archive-plist-expansion

📝 Commits (1)

af4f3db fix(apple): bound binary plist expansion before decoding keyed archives

📊 Changes

3 files changed (+158 additions, -28 deletions)

View changed files

📝 src/artifacts/apple/InterfaceBuilderDecodeBudget.ts (+56 -28)
📝 src/artifacts/apple/KeyedArchiveReader.test.ts (+85 -0)
📝 src/artifacts/apple/KeyedArchiveReader.ts (+17 -0)

📄 Description

Summary

inspect_keyed_archive passes the selected binary plist to plist.parseBinary (plist 5.0.0). That parser recurses on every object reference and builds a fresh copy for each one, so shared containers expand into a tree. The archive is caller-selected and comes from an untrusted app bundle or standalone file. Only the 64 MiB input cap applied before decoding.

Crafted standalone plists, using rea inspect-keyed-archive <file> --format json on main:

file	size	main
40 levels of `[next, next]`	484 B	RSS grew past 4.4 GB in 8 s and the process died with no output. In the MCP server this ends the server.
array containing itself	125 B	`truncated`: "Artifact is too large to process safely. Narrow the requested path…" (`Maximum call stack size exceeded`)
20,000-deep chain	140 KB	same `truncated` stack-overflow report

The self-referencing case is a malformed archive, not an oversized one, so its advice is wrong.

decode_interface_builder already has an iterative preflight for this decoder: estimateBinaryPlistExpansion handles cycles, a 128-level depth limit, and expansion counted against a budget. inspect_keyed_archive never used it.

Change

estimateBinaryPlistExpansion is exported and takes options: the budget wording, a fail factory, and bounds. Its defaults keep every Interface Builder message and error type unchanged.
decodeKeyedArchiveBytes runs it before parseBinary with a 512 MiB decode budget, the same 8:1 ratio Interface Builder uses (32 MiB input, 256 MiB decode).
- Depth or expansion over budget throws RangeError, which the existing translation reports as a limit.
- A reference cycle throws TypeError ("binary plist object references form a cycle…"), reported as a format failure.
bounds: "decoder" admits exactly the layouts plist.parseBinary reads: offset-table entries and objects anywhere in the file. The existing supplemental-binary-table fixture in keyedArchiveIntegerAmbiguity.test.ts extends its offset table into the trailer and must stay decodable. The strict Interface Builder bounds would have rejected it.

After the change:

file	this branch
40 × `[next, next]`	`truncated`: "binary plist reference expansion exceeds the keyed archive decode budget" (RSS 283 MB, immediate)
array containing itself	`artifact_operation_failed`: "binary plist object references form a cycle, which a property list cannot represent"
20,000-deep chain	`truncated`: "binary plist nesting exceeds the keyed archive decode budget"

Verification

New KeyedArchiveReader.test.ts cases cover the cycle (format), exponential expansion (limit), and shared leaf references, which still decode. The cycle case fails on main.
npx vitest run src/artifacts/apple src/domain/apple passes (163 tests).
npm run test:focused passes on keyedArchiveIntegerAmbiguity, keyedArchiveSelection, keyedArchiveEncoding and interfaceBuilderBundle. The exception is the Xcode-compiled storyboard case, which needs xcrun ibtool. This machine has only the Command Line Tools, so it also fails on main here; CI's macOS job runs it.
I ran inspect-keyed-archive on every bplist .nib/.plist (up to 40 per app) in XLD, Transmission, Anki, Sublime Text, Typora and mpv: 69 real archives, all with the same outcomes as before.
npm run check:fast and npm run docs:check pass.

🤖 Generated with Claude Code

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/1515 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/10/2026 **Status:** ✅ Merged **Merged:** 10/10/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `fix/keyed-archive-plist-expansion` --- ### 📝 Commits (1) - [`af4f3db`](https://github.com/morluto/rea/commit/af4f3db9976af54971a7f35c73dee185f0fb0384) fix(apple): bound binary plist expansion before decoding keyed archives ### 📊 Changes **3 files changed** (+158 additions, -28 deletions) <details> <summary>View changed files</summary> 📝 `src/artifacts/apple/InterfaceBuilderDecodeBudget.ts` (+56 -28) 📝 `src/artifacts/apple/KeyedArchiveReader.test.ts` (+85 -0) 📝 `src/artifacts/apple/KeyedArchiveReader.ts` (+17 -0) </details> ### 📄 Description ## Summary `inspect_keyed_archive` passes the selected binary plist to `plist.parseBinary` (plist 5.0.0). That parser recurses on every object reference and builds a fresh copy for each one, so shared containers expand into a tree. The archive is caller-selected and comes from an untrusted app bundle or standalone file. Only the 64 MiB input cap applied before decoding. Crafted standalone plists, using `rea inspect-keyed-archive <file> --format json` on `main`: | file | size | main | |---|---|---| | 40 levels of `[next, next]` | 484 B | RSS grew past **4.4 GB** in 8 s and the process died with **no output**. In the MCP server this ends the server. | | array containing itself | 125 B | `truncated`: "Artifact is too large to process safely. Narrow the requested path…" (`Maximum call stack size exceeded`) | | 20,000-deep chain | 140 KB | same `truncated` stack-overflow report | The self-referencing case is a malformed archive, not an oversized one, so its advice is wrong. `decode_interface_builder` already has an iterative preflight for this decoder: `estimateBinaryPlistExpansion` handles cycles, a 128-level depth limit, and expansion counted against a budget. `inspect_keyed_archive` never used it. ## Change - `estimateBinaryPlistExpansion` is exported and takes options: the budget wording, a `fail` factory, and `bounds`. Its defaults keep every Interface Builder message and error type unchanged. - `decodeKeyedArchiveBytes` runs it before `parseBinary` with a 512 MiB decode budget, the same 8:1 ratio Interface Builder uses (32 MiB input, 256 MiB decode). - Depth or expansion over budget throws `RangeError`, which the existing translation reports as a limit. - A reference cycle throws `TypeError` ("binary plist object references form a cycle…"), reported as a format failure. - `bounds: "decoder"` admits exactly the layouts `plist.parseBinary` reads: offset-table entries and objects anywhere in the file. The existing `supplemental-binary-table` fixture in `keyedArchiveIntegerAmbiguity.test.ts` extends its offset table into the trailer and must stay decodable. The strict Interface Builder bounds would have rejected it. After the change: | file | this branch | |---|---| | 40 × `[next, next]` | `truncated`: "binary plist reference expansion exceeds the keyed archive decode budget" (RSS 283 MB, immediate) | | array containing itself | `artifact_operation_failed`: "binary plist object references form a cycle, which a property list cannot represent" | | 20,000-deep chain | `truncated`: "binary plist nesting exceeds the keyed archive decode budget" | ## Verification - New `KeyedArchiveReader.test.ts` cases cover the cycle (format), exponential expansion (limit), and shared *leaf* references, which still decode. The cycle case fails on `main`. - `npx vitest run src/artifacts/apple src/domain/apple` passes (163 tests). - `npm run test:focused` passes on `keyedArchiveIntegerAmbiguity`, `keyedArchiveSelection`, `keyedArchiveEncoding` and `interfaceBuilderBundle`. The exception is the Xcode-compiled storyboard case, which needs `xcrun ibtool`. This machine has only the Command Line Tools, so it also fails on `main` here; CI's macOS job runs it. - I ran `inspect-keyed-archive` on every bplist `.nib`/`.plist` (up to 40 per app) in XLD, Transmission, Anki, Sublime Text, Typora and mpv: 69 real archives, all with the same outcomes as before. - `npm run check:fast` and `npm run docs:check` pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-17 21:12:01 +00:00