[GH-ISSUE #5057] [Bug] LLDB PID validation reads unbounded file contents before rejecting them #3101

New issue

Closed

opened 2026-10-31 00:36:39 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-31 00:36:39 +00:00

Owner

Originally created by @morluto on GitHub (Oct 18, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/5057

Observed behavior

LLDB PID readback allocates and reads the entire file before validating one positive safe decimal integer. runLldb retries unavailable PID readback during launch, so a persistently invalid large file can repeat that work. This does not affect normal tiny PID publication; prevalence and large-file memory impact are NOT_ENOUGH_INFO.

Executed evidence

At main 36d156ff05f48f9f0b4c4abb8f7c899249a49ebf, a production-boundary test wraps actual FileHandle reads and uses tiny real temporary files. Valid safe-integer/LF/CRLF controls pass. A 64-byte invalid decimal fixture transfers all 64 bytes before rejection, exceeding the 19-byte bound sufficient to represent the accepted PID protocol plus an overflow sentinel. The unchanged-main test fails expected 64 <= 19; candidate passes. No giant file, native LLDB or target execution.

Root cause and expected behavior

src/native/LldbReadbackFiles.ts:readTargetPid uses readFile("utf8") then trim/regex/Number, so validation occurs after unbounded allocation. The owned Python bridge publishes only str(process.GetProcessID()) (bridge/native/rea_lldb_tracer.py); the consumer accepts positive safe integers. Read at most 16 decimal digits plus tolerated CRLF and one overflow sentinel, inside the existing descriptor owner. Keep short-read handling, valid PID caching and absent/invalid retry semantics. The producer currently publishes via open/write, so empty/partial publication must not be reclassified terminal based on file presence alone.

Existing capability only; no change to identity proof, signaling authority, ACK or process launch. Bounded short inputs that previously trim remain supported; arbitrary whitespace padding is outside the producer representation. Searched open/closed LLDB PID/bounded issues and related lifecycle reports; no matching read-allocation defect. Related investigation #2397 and lifecycle #4227 are broader. Validation is source and controlled descriptor workflow only; real-provider/platform/whole-agent costs remain unverified.

Originally created by @morluto on GitHub (Oct 18, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/5057 ### Observed behavior LLDB PID readback allocates and reads the entire file before validating one positive safe decimal integer. `runLldb` retries unavailable PID readback during launch, so a persistently invalid large file can repeat that work. This does not affect normal tiny PID publication; prevalence and large-file memory impact are NOT_ENOUGH_INFO. ### Executed evidence At main `36d156ff05f48f9f0b4c4abb8f7c899249a49ebf`, a production-boundary test wraps actual FileHandle reads and uses tiny real temporary files. Valid safe-integer/LF/CRLF controls pass. A 64-byte invalid decimal fixture transfers all 64 bytes before rejection, exceeding the 19-byte bound sufficient to represent the accepted PID protocol plus an overflow sentinel. The unchanged-main test fails `expected 64 <= 19`; candidate passes. No giant file, native LLDB or target execution. ### Root cause and expected behavior `src/native/LldbReadbackFiles.ts:readTargetPid` uses readFile("utf8") then trim/regex/Number, so validation occurs after unbounded allocation. The owned Python bridge publishes only `str(process.GetProcessID())` (`bridge/native/rea_lldb_tracer.py`); the consumer accepts positive safe integers. Read at most 16 decimal digits plus tolerated CRLF and one overflow sentinel, inside the existing descriptor owner. Keep short-read handling, valid PID caching and absent/invalid retry semantics. The producer currently publishes via open/write, so empty/partial publication must not be reclassified terminal based on file presence alone. ### Scope and related search Existing capability only; no change to identity proof, signaling authority, ACK or process launch. Bounded short inputs that previously trim remain supported; arbitrary whitespace padding is outside the producer representation. Searched open/closed LLDB PID/bounded issues and related lifecycle reports; no matching read-allocation defect. Related investigation #2397 and lifecycle #4227 are broader. Validation is source and controlled descriptor workflow only; real-provider/platform/whole-agent costs remain unverified.

bsakaguchi closed this issue

2026-10-31 00:36:40 +00:00

bsakaguchi commented

2026-10-31 00:36:40 +00:00

Author

Owner

@morluto commented on GitHub (Oct 18, 2026):

Fixed on main in 292e7bef2, with typed fault-control cleanup in eb2c3b589. Root cause: readFile allocated the entire PID file before validation. The descriptor owner now reads only the maximum accepted safe decimal PID plus tolerated CRLF and an overflow sentinel (19 bytes total), with short-read handling and valid-fact caching. Normal producer representation and existing absent/invalid retry and ACK behavior remain intact. A 64-byte baseline fixture actually transferred all 64 bytes; the candidate transfers at most 19. No large allocation was required to demonstrate the boundary defect.

Validation: final affected selection passed 162 tests in 13 files; one existing 8 MiB journal allocation test skipped. Full npm run check, independent Luna reviews and commit hooks passed. Controlled production readback/provider/MCP and error-owner workflows; no actual LLDB, ADB device, GUI or storage fault. Bounded completed LLDB trace/close warm medians (two repeats/build plus separate first processes): baseline → candidate elapsed 0.16447 → 0.16405 s, CPU 0.25947 → 0.25865 s, sampled family RSS 193.8 → 194.2 MB, reply estimate 307 → 307 cl100k tokens. No true cold-cache, whole-agent or injected-failure cost measurement; no performance claim. Exact replies and probe records retained locally.

Remaining separate question: publication uses open/write, so transient empty/partial files must not be treated as terminal invalid state merely from file presence. This change does not introduce that interpretation.

@morluto commented on GitHub (Oct 18, 2026): Fixed on main in 292e7bef2, with typed fault-control cleanup in eb2c3b589. Root cause: `readFile` allocated the entire PID file before validation. The descriptor owner now reads only the maximum accepted safe decimal PID plus tolerated CRLF and an overflow sentinel (19 bytes total), with short-read handling and valid-fact caching. Normal producer representation and existing absent/invalid retry and ACK behavior remain intact. A 64-byte baseline fixture actually transferred all 64 bytes; the candidate transfers at most 19. No large allocation was required to demonstrate the boundary defect. Validation: final affected selection passed 162 tests in 13 files; one existing 8 MiB journal allocation test skipped. Full `npm run check`, independent Luna reviews and commit hooks passed. Controlled production readback/provider/MCP and error-owner workflows; no actual LLDB, ADB device, GUI or storage fault. Bounded completed LLDB trace/close warm medians (two repeats/build plus separate first processes): baseline → candidate elapsed 0.16447 → 0.16405 s, CPU 0.25947 → 0.25865 s, sampled family RSS 193.8 → 194.2 MB, reply estimate 307 → 307 cl100k tokens. No true cold-cache, whole-agent or injected-failure cost measurement; no performance claim. Exact replies and probe records retained locally. Remaining separate question: publication uses open/write, so transient empty/partial files must not be treated as terminal invalid state merely from file presence. This change does not introduce that interpretation.

bsakaguchi referenced this issue

2026-10-31 01:08:01 +00:00

[PR #3101] [CLOSED] fix(browser): settle retained cleanup after cancelled opening #5617

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#3101

No description provided.

Rows
Columns