[PR #5216] fix: allow Tiny PE e_lfanew inside DOS header #6821

New issue

Open

opened 2026-11-01 21:43:16 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:43:16 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5216
Author: @vanshsharma3777
Created: 10/19/2026
Status: 🔄 Open

Base: main ← Head: fix/tiny-pe-e-lfanew

📝 Commits (5)

8cda450 fix: allow Tiny PE e_lfanew inside DOS header
b5c2d5f test(binary): cover complete overlapped-header PE; keep helper doc attached
68da92a fix(binary): apply the overlapped-header PE offset rule to inventory, Go and Electron consumers
0a345db merge origin/main into fix/tiny-pe-e-lfanew
cf0fdab fix(binary): apply PE offset rule to NativeAOT parser

📊 Changes

14 files changed (+225 additions, -76 deletions)

View changed files

📝 src/application/BinaryTargetResolver.ts (+8 -2)
📝 src/artifacts/inventory/ArtifactGraphConstruction.ts (+3 -1)
📝 src/domain/binaryTarget.test.ts (+18 -0)
📝 src/domain/binaryTarget.ts (+9 -2)
📝 src/domain/dosMz.ts (+8 -0)
📝 src/domain/flutter/flutterBuildAnalysis.ts (+1 -1)
📝 src/domain/native/nativeAotPe.headerRanges.test.ts (+36 -0)
📝 src/domain/native/nativeAotPe.ts (+5 -1)
📝 src/electron/ElectronFuseProvider.test.ts (+17 -0)
📝 src/electron/ElectronFuseReader.ts (+2 -1)
📝 src/go/GoPeImage.ts (+6 -2)
📝 src/server/http/NodeHttpRequestOwner.ts (+2 -2)
📝 tests/boundary/filesystem/artifactMzInventory.test.ts (+71 -64)
📝 tests/boundary/filesystem/binaryTarget.test.ts (+39 -0)

📄 Description

Summary

Part of #5168

REA refused MZ images with e_lfanew < 64 even when a PE signature sits at that
offset (e.g. Sotirov's 97-byte Tiny PE, e_lfanew = 4). A signature-confirmed
"PE" now uses a minimum offset of 4. NE/LE/LX keep 64.

Partial fix: tiny.97.exe (sha256 e872a508a28fe08ad4d309d6949e3dbe26efad305c2c114303e01af1241a6a67)
now passes the offset guards but is still refused later by the SizeOfOptionalHeader check.
Before (per #5168): "invalid Windows new-header offset in MZ image"
After: "truncated PE optional header"

Change and scope

Added minimumWindowsHeaderOffset(signature) in src/domain/dosMz.ts (4 for PE, else 64).
Used in src/domain/binaryTarget.ts and src/application/BinaryTargetResolver.ts.
Not included: relaxing peManagedStatus. Left for a follow-up, pending maintainer guidance.

Contract and boundary impact

Semantic owner and earliest changed stage: domain/dosMz, target resolution
Contribution scope fit: ordinary fix within existing boundaries
CLI/MCP contract, package, generated metadata: none
Provider/format compatibility: PE only; NE/LE/LX unchanged
Other lines: not applicable

Expected effects:

Read existing files or captured data.

Evidence and regression coverage

Tests added or updated: src/domain/binaryTarget.test.ts,
tests/boundary/filesystem/binaryTarget.test.ts
Base reproduction: fixture and steps in #5168
After-state is exercised by the added tests (parseBinaryTarget and
parseExecutableHeader on the 97-byte fixture); not run end-to-end via rea inspect.
Remaining proof gaps: tiny.97.exe still refused at peManagedStatus; the
97-byte file also does not import in Ghidra (file-length limit, separate issue)

Validation performed

npx oxfmt --check on the 5 changed files — all matched files use the correct format
pre-push precheck:fast (typecheck + lint) — 2 successful, 2 total
npx vitest run src/domain/binaryTarget.test.ts tests/boundary/filesystem/binaryTarget.test.ts — 2 files passed, 94 tests passed, 10 skipped

Compatibility, safety, and release

Breaking changes: none
Real Hopper/Ghidra coverage: not run
Security/privacy review: not applicable (read-only header validation)

Author readiness

Human owner: @vanshsharma3777

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

_{🔄 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/5216 **Author:** [@vanshsharma3777](https://github.com/vanshsharma3777) **Created:** 10/19/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/tiny-pe-e-lfanew` --- ### 📝 Commits (5) - [`8cda450`](https://github.com/morluto/rea/commit/8cda4505397f292675ceefa79bd5ce046b9f04f9) fix: allow Tiny PE e_lfanew inside DOS header - [`b5c2d5f`](https://github.com/morluto/rea/commit/b5c2d5fe4055870be77a4e3e2841ee0814c2c369) test(binary): cover complete overlapped-header PE; keep helper doc attached - [`68da92a`](https://github.com/morluto/rea/commit/68da92a8a817f039459d287f26585518f43cb852) fix(binary): apply the overlapped-header PE offset rule to inventory, Go and Electron consumers - [`0a345db`](https://github.com/morluto/rea/commit/0a345db9993d4659afea4385bd0bddd605fd47d4) merge origin/main into fix/tiny-pe-e-lfanew - [`cf0fdab`](https://github.com/morluto/rea/commit/cf0fdab1dc4624dc5db7b7b56bcf1a8750d4396a) fix(binary): apply PE offset rule to NativeAOT parser ### 📊 Changes **14 files changed** (+225 additions, -76 deletions) <details> <summary>View changed files</summary> 📝 `src/application/BinaryTargetResolver.ts` (+8 -2) 📝 `src/artifacts/inventory/ArtifactGraphConstruction.ts` (+3 -1) 📝 `src/domain/binaryTarget.test.ts` (+18 -0) 📝 `src/domain/binaryTarget.ts` (+9 -2) 📝 `src/domain/dosMz.ts` (+8 -0) 📝 `src/domain/flutter/flutterBuildAnalysis.ts` (+1 -1) 📝 `src/domain/native/nativeAotPe.headerRanges.test.ts` (+36 -0) 📝 `src/domain/native/nativeAotPe.ts` (+5 -1) 📝 `src/electron/ElectronFuseProvider.test.ts` (+17 -0) 📝 `src/electron/ElectronFuseReader.ts` (+2 -1) 📝 `src/go/GoPeImage.ts` (+6 -2) 📝 `src/server/http/NodeHttpRequestOwner.ts` (+2 -2) 📝 `tests/boundary/filesystem/artifactMzInventory.test.ts` (+71 -64) 📝 `tests/boundary/filesystem/binaryTarget.test.ts` (+39 -0) </details> ### 📄 Description ## Summary Part of #5168 REA refused MZ images with e_lfanew < 64 even when a PE signature sits at that offset (e.g. Sotirov's 97-byte Tiny PE, e_lfanew = 4). A signature-confirmed "PE" now uses a minimum offset of 4. NE/LE/LX keep 64. Partial fix: tiny.97.exe (sha256 e872a508a28fe08ad4d309d6949e3dbe26efad305c2c114303e01af1241a6a67) now passes the offset guards but is still refused later by the SizeOfOptionalHeader check. Before (per #5168): "invalid Windows new-header offset in MZ image" After: "truncated PE optional header" ## Change and scope - Added minimumWindowsHeaderOffset(signature) in src/domain/dosMz.ts (4 for PE, else 64). - Used in src/domain/binaryTarget.ts and src/application/BinaryTargetResolver.ts. - Not included: relaxing peManagedStatus. Left for a follow-up, pending maintainer guidance. ## Contract and boundary impact - Semantic owner and earliest changed stage: domain/dosMz, target resolution - Contribution scope fit: ordinary fix within existing boundaries - CLI/MCP contract, package, generated metadata: none - Provider/format compatibility: PE only; NE/LE/LX unchanged - Other lines: not applicable Expected effects: - [x] Read existing files or captured data. ## Evidence and regression coverage - Tests added or updated: src/domain/binaryTarget.test.ts, tests/boundary/filesystem/binaryTarget.test.ts - Base reproduction: fixture and steps in #5168 - After-state is exercised by the added tests (parseBinaryTarget and parseExecutableHeader on the 97-byte fixture); not run end-to-end via rea inspect. - Remaining proof gaps: tiny.97.exe still refused at peManagedStatus; the 97-byte file also does not import in Ghidra (file-length limit, separate issue) ## Validation performed - `npx oxfmt --check` on the 5 changed files — all matched files use the correct format - pre-push precheck:fast (typecheck + lint) — 2 successful, 2 total - `npx vitest run src/domain/binaryTarget.test.ts tests/boundary/filesystem/binaryTarget.test.ts` — 2 files passed, 94 tests passed, 10 skipped ## Compatibility, safety, and release - Breaking changes: none - Real Hopper/Ghidra coverage: not run - Security/privacy review: not applicable (read-only header validation) ## Author readiness Human owner: @vanshsharma3777 - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-11-01 21:43:16 +00:00

bsakaguchi referenced this issue

2026-11-01 21:47:45 +00:00

[PR #6833] [CLOSED] fix(apk): bind certificate metadata to the carried DER bytes #7421