morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[GH-ISSUE #4750] [Bug] NativeAOT parser reads outside declared PE header extents #2473

New issue

Closed

opened 2026-10-17 21:05:19 +00:00 by bsakaguchi · 2 comments

bsakaguchi commented

2026-10-17 21:05:19 +00:00

Owner

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

Area

Native analysis or decompilation

Observed behavior

The read-only NativeAOT parser admits an RTR directory after an eight-byte TypeManagerIndirection probe crosses SizeOfHeaders into unrelated on-disk bytes. It also accepts a declared 56-byte optional header, reading SizeOfImage/SizeOfHeaders from the following section table instead. Both inputs are reported with a detected RTR header rather than being rejected as unsupported/missing mapping.

Reproduction

At d9099190ab, ran npm run test:local -- src/domain/native/nativeAotPe.headerRanges.test.ts: 2 failed / 3 passed. Each tiny in-memory fixture is 2,560 bytes; no engine/target process or file mutation.

Existing nativeAotPeFixture: SizeOfHeaders=0x400, .text RVA=0x1000 but raw offset=0x400. Replace the optional frozen row with TypeManagerIndirection (204), start=preferred base+0x3fc. Its eight-byte read straddles the end of mapped headers. Parser still admits RTR. Start=0x3f8 (ends exactly at header end) succeeds; start=0x400 correctly rejects.
Set SizeOfOptionalHeader=56 and relocate the section table to its declared position. Mapping fields at optional offsets56/60 now belong to the section table; parser still admits RTR. A 64-byte header containing all mapping fields is a parser-boundary control, not a claim of Windows-loader validity.

Expected behavior

Honor the full requested file-backed virtual interval and declared optional-header field extent. Microsoft PE specification: SizeOfHeaders describes the combined file headers; RVA and file offset are distinct coordinates; SizeOfOptionalHeader bounds probes. https://learn.microsoft.com/en-us/windows/win32/debug/pe-format

Impact and scope

Observed false-positive metadata admission on two malformed/edge-layout synthetic parser inputs, once each in the selected baseline run. Real-file frequency, Windows loader behavior, and real Ghidra/public CLI/MCP occurrence: NOT_ENOUGH_INFO; not run. No execution, mutation, or protection recovery is proposed.

REA version / Runtime and platform

Source d9099190ab; Node24.18.0, macOS arm64.

Target and artifact context

Tiny authored x64 PE32+/RTR9.1 byte fixtures with the SHA256 recomputed after edits; no user binary.

Interface / Earliest failure stage

Other: production domain parser / Native analysis or reconstruction.

Relevant output or evidence

Both negative cases: expected summary.header_address !== null false, received true. Three exact-boundary/outside-header controls passed. Source: src/domain/native/nativeAotPe.ts PeImage constructor and read; read checks only start<SizeOfHeaders and captured-file length, while constructor validates optionalSize bytes but reads fields beyond that declared interval.

Analysis

Source-derived: the producing image owner conflates physical buffer availability with declared header ownership. Fix the constructor and read boundary, keeping downstream directory/type consumers on the owner's mapping. Update the NativeAOT profile contract revision because acceptance semantics affect profile identity. Related PE resource parser already applies full header-interval checks; it is a separate adapter and cannot be imported into the domain parser.

Additional context

Cached issue inventory distinguishes #4445 (work-unit budget), #4559 (budget/base-type indexing) and #4373/#4455 (PE resource mapping). Current NativeAOT issue/PR API refresh attempts timed out; no matching narrow report was found in the retained inventory. Source-only ambiguous section/overlay ownership remains a separate review question; not claimed fixed here.

Originally created by @morluto on GitHub (Oct 17, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/4750 ### Area Native analysis or decompilation ### Observed behavior The read-only NativeAOT parser admits an RTR directory after an eight-byte TypeManagerIndirection probe crosses SizeOfHeaders into unrelated on-disk bytes. It also accepts a declared 56-byte optional header, reading SizeOfImage/SizeOfHeaders from the following section table instead. Both inputs are reported with a detected RTR header rather than being rejected as unsupported/missing mapping. ### Reproduction At d9099190ab3a819bd3a9fd62a8714cef398016d5, ran `npm run test:local -- src/domain/native/nativeAotPe.headerRanges.test.ts`: 2 failed / 3 passed. Each tiny in-memory fixture is 2,560 bytes; no engine/target process or file mutation. 1. Existing nativeAotPeFixture: SizeOfHeaders=0x400, .text RVA=0x1000 but raw offset=0x400. Replace the optional frozen row with TypeManagerIndirection (204), start=preferred base+0x3fc. Its eight-byte read straddles the end of mapped headers. Parser still admits RTR. Start=0x3f8 (ends exactly at header end) succeeds; start=0x400 correctly rejects. 2. Set SizeOfOptionalHeader=56 and relocate the section table to its declared position. Mapping fields at optional offsets56/60 now belong to the section table; parser still admits RTR. A 64-byte header containing all mapping fields is a parser-boundary control, not a claim of Windows-loader validity. ### Expected behavior Honor the full requested file-backed virtual interval and declared optional-header field extent. Microsoft PE specification: SizeOfHeaders describes the combined file headers; RVA and file offset are distinct coordinates; SizeOfOptionalHeader bounds probes. https://learn.microsoft.com/en-us/windows/win32/debug/pe-format ### Impact and scope Observed false-positive metadata admission on two malformed/edge-layout synthetic parser inputs, once each in the selected baseline run. Real-file frequency, Windows loader behavior, and real Ghidra/public CLI/MCP occurrence: NOT_ENOUGH_INFO; not run. No execution, mutation, or protection recovery is proposed. ### REA version / Runtime and platform Source d9099190ab3a819bd3a9fd62a8714cef398016d5; Node24.18.0, macOS arm64. ### Target and artifact context Tiny authored x64 PE32+/RTR9.1 byte fixtures with the SHA256 recomputed after edits; no user binary. ### Interface / Earliest failure stage Other: production domain parser / Native analysis or reconstruction. ### Relevant output or evidence Both negative cases: expected `summary.header_address !== null` false, received true. Three exact-boundary/outside-header controls passed. Source: src/domain/native/nativeAotPe.ts PeImage constructor and read; read checks only start<SizeOfHeaders and captured-file length, while constructor validates optionalSize bytes but reads fields beyond that declared interval. ### Analysis Source-derived: the producing image owner conflates physical buffer availability with declared header ownership. Fix the constructor and read boundary, keeping downstream directory/type consumers on the owner's mapping. Update the NativeAOT profile contract revision because acceptance semantics affect profile identity. Related PE resource parser already applies full header-interval checks; it is a separate adapter and cannot be imported into the domain parser. ### Additional context Cached issue inventory distinguishes #4445 (work-unit budget), #4559 (budget/base-type indexing) and #4373/#4455 (PE resource mapping). Current NativeAOT issue/PR API refresh attempts timed out; no matching narrow report was found in the retained inventory. Source-only ambiguous section/overlay ownership remains a separate review question; not claimed fixed here.

bsakaguchi added the

triage

bug

labels

2026-10-17 21:05:19 +00:00

bsakaguchi commented

2026-10-17 21:05:21 +00:00

Author

Owner

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

Fixed in 4a076f368c (fix(native): bound NativeAOT reads to declared PE headers).

The image owner now checks that the declared optional header includes every consumed mapping field before reading it, and requires a header-backed read's entire interval to fit both SizeOfHeaders and captured bytes. Failed header reads cannot fall through to section mappings. Consumers retain the owner's parsed layout decision; no downstream repair or dependency from domain to the resource adapter was added. Profile contract revision advances to v4 so this interpretation change has distinct profile identity.

Executed validation:

Baseline: 2 failed /3 passed on tiny 2,560-byte authored fixtures.
Final: 21 parser/byte-budget controls and 15 selected Ghidra provider/profile/NativeAOT controls passed (93 unrelated cases deselected).
Both malformed layouts propagate as not_applicable with no recovered MethodTables and the exact snapshot digest through the production provider consumer.
Typecheck/lint, cached builds, diff checks and commit hooks passed. Read-only scoped boundary review found no blocker.

These are in-memory parser fixtures and a simulated Ghidra transport, not real Ghidra/Windows loader or public CLI/MCP fault reproduction. No target execution, native engine or large workload was run.

Completed valid parser summary+type-detail task against d9099190a: same 791-token cl100k_base complete reply, byte-identical across six runs. Separate first processes baseline/candidate: elapsed .03335/.03023s, CPU .02629/.02438s, peak RSS 50.39/50.45MiB. Two alternating warm-repeat medians: elapsed .03056/.03046s, CPU .02407/.02464s, peak RSS 50.43/50.44MiB. Single Node process with no children: peak process-family RSS is OS wait4 process peak; periodic samples miss most of this short task and are not representative peak measurements. No true cold-cache or performance improvement claim. Whole-agent/catalog, public CLI/MCP, real engine, large-input and malformed/rejected-workflow costs remain unmeasured.

Remaining review: ambiguous section/overlay ownership and broader NativeAOT interpretation. The 64-byte optional-header control establishes only the extent of fields this parser consumes, not Windows loader validity. Microsoft PE specification was consulted; DeepWiki runtime guidance was supplemental and its unverified pointer-size discussion was not used as runtime qualification.

@morluto commented on GitHub (Oct 17, 2026): Fixed in 4a076f368c62bdfb19415accad342e03a38d1005 (`fix(native): bound NativeAOT reads to declared PE headers`). The image owner now checks that the declared optional header includes every consumed mapping field before reading it, and requires a header-backed read's entire interval to fit both SizeOfHeaders and captured bytes. Failed header reads cannot fall through to section mappings. Consumers retain the owner's parsed layout decision; no downstream repair or dependency from domain to the resource adapter was added. Profile contract revision advances to v4 so this interpretation change has distinct profile identity. Executed validation: - Baseline: 2 failed /3 passed on tiny 2,560-byte authored fixtures. - Final: 21 parser/byte-budget controls and 15 selected Ghidra provider/profile/NativeAOT controls passed (93 unrelated cases deselected). - Both malformed layouts propagate as not_applicable with no recovered MethodTables and the exact snapshot digest through the production provider consumer. - Typecheck/lint, cached builds, diff checks and commit hooks passed. Read-only scoped boundary review found no blocker. These are in-memory parser fixtures and a simulated Ghidra transport, not real Ghidra/Windows loader or public CLI/MCP fault reproduction. No target execution, native engine or large workload was run. Completed valid parser summary+type-detail task against d9099190a: same 791-token cl100k_base complete reply, byte-identical across six runs. Separate first processes baseline/candidate: elapsed .03335/.03023s, CPU .02629/.02438s, peak RSS 50.39/50.45MiB. Two alternating warm-repeat medians: elapsed .03056/.03046s, CPU .02407/.02464s, peak RSS 50.43/50.44MiB. Single Node process with no children: peak process-family RSS is OS wait4 process peak; periodic samples miss most of this short task and are not representative peak measurements. No true cold-cache or performance improvement claim. Whole-agent/catalog, public CLI/MCP, real engine, large-input and malformed/rejected-workflow costs remain unmeasured. Remaining review: ambiguous section/overlay ownership and broader NativeAOT interpretation. The 64-byte optional-header control establishes only the extent of fields this parser consumes, not Windows loader validity. Microsoft PE specification was consulted; DeepWiki runtime guidance was supplemental and its unverified pointer-size discussion was not used as runtime qualification.

bsakaguchi commented

2026-10-17 21:05:21 +00:00

Author

Owner

@kaoru0822-kitauji commented on GitHub (Oct 17, 2026):

This appears fixed on current main by 4a076f368 (fix(native): bound NativeAOT reads to declared PE headers, which says "Refs #4750"). The PeImage constructor now rejects an optional header shorter than 64 bytes. Header reads must also end within SizeOfHeaders, and the profile contract revision moves to v4.

Executed on main 4a076f368 (macOS arm64, Node 24): npm run test:local -- src/domain/native/nativeAotPe.headerRanges.test.ts src/domain/native/nativeAotPe.test.ts → 15/15 passed. That covers this issue's cases: the probe ending exactly at 0x3f8 is admitted, the 0x3fc straddle and the probe at 0x400 are rejected, a 56-byte optional header is rejected, and a 64-byte one is admitted. Separately, on base d9099190a (before that commit), an independent regression test with the same cases failed 2 of 5, matching the report.

The separate section/overlay ownership question noted in the issue is not addressed by this. Leaving closure to a maintainer.

@kaoru0822-kitauji commented on GitHub (Oct 17, 2026): This appears fixed on current main by 4a076f368 (`fix(native): bound NativeAOT reads to declared PE headers`, which says "Refs #4750"). The `PeImage` constructor now rejects an optional header shorter than 64 bytes. Header reads must also end within `SizeOfHeaders`, and the profile contract revision moves to v4. Executed on main 4a076f368 (macOS arm64, Node 24): `npm run test:local -- src/domain/native/nativeAotPe.headerRanges.test.ts src/domain/native/nativeAotPe.test.ts` → 15/15 passed. That covers this issue's cases: the probe ending exactly at 0x3f8 is admitted, the 0x3fc straddle and the probe at 0x400 are rejected, a 56-byte optional header is rejected, and a 64-byte one is admitted. Separately, on base d9099190a (before that commit), an independent regression test with the same cases failed 2 of 5, matching the report. The separate section/overlay ownership question noted in the issue is not addressed by this. Leaving closure to a maintainer.

bsakaguchi referenced this issue

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

[GH-ISSUE #4249] [Feature] Opt-in summary delivery and retained pages for list_strings, list_procedures, list_names, and their search tools #1674

bsakaguchi closed this issue

2026-10-31 00:34:22 +00:00