[PR #6155] [CLOSED] fix(domain): reject unwalkable Mach-O load-command tables at admission #7169

New issue

Closed

opened 2026-11-01 21:45:53 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6155
Author: @charan-rathore
Created: 10/21/2026
Status: ❌ Closed

Base: main ← Head: fix/6154-macho-load-command-walk

📝 Commits (2)

4379253 fix(domain): reject unwalkable Mach-O load-command tables at admission
693c4bb fix(domain): reject unwalkable Mach-O load-command tables at admission

📊 Changes

2 files changed (+112 additions, -0 deletions)

View changed files

📝 src/domain/binaryTarget.test.ts (+78 -0)
📝 src/domain/binaryTarget.ts (+34 -0)

📄 Description

Fixes #6154.

Problem

parseThinMachO in src/domain/binaryTarget.ts only checked that header + sizeofcmds fits in the file. A thin Mach-O whose header is valid but whose load-command table is corrupt was admitted, then failed inside a provider as a generic execution_failure whose remediation is rea doctor - even though the environment is fine and the target is not.

Fix

Walk the declared table when its bytes are present, with the same rules the loader enforces: each command needs an 8-byte header, cmdsize >= 8, cmdsize aligned (8 for 64-bit, 4 for 32-bit), no command running past sizeofcmds, and all commands together adding up to exactly sizeofcmds. Violations are rejected at admission like the existing truncated case, so the CLI reports target_unavailable with a specific reason instead of a misleading environment failure. A declared table that outlives the probed prefix keeps the existing commitment-only behavior.

Scope note: #5170 covers misalignment in the Go/Rust readers (#5076) and does not touch this admission path; this PR is the admission-side complement for the cases it does not cover (cmdsize below the minimum, over/under-run of the declared table).

Tests

New cases pin the reporter's exact fixture shape (ncmds=17, sizeofcmds=1056, table bytes 0xff) plus cmdsize below the minimum, cmdsize running past sizeofcmds, commands not adding up to sizeofcmds, a clean-walking table, and the probed-prefix case. All 4 rejection cases fail without the fix and pass with it; src/domain/binaryTarget.test.ts is 43/43 green. The heavier gates (turbo typecheck/lint/knip) were not run locally.

Prepared with AI assistance (Instinct assisted).

_{🔄 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/6155 **Author:** [@charan-rathore](https://github.com/charan-rathore) **Created:** 10/21/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/6154-macho-load-command-walk` --- ### 📝 Commits (2) - [`4379253`](https://github.com/morluto/rea/commit/43792530330ae70b3da024ca476e1e513d6c02e1) fix(domain): reject unwalkable Mach-O load-command tables at admission - [`693c4bb`](https://github.com/morluto/rea/commit/693c4bbb67940c5fe2915a17c7e0b75106a01590) fix(domain): reject unwalkable Mach-O load-command tables at admission ### 📊 Changes **2 files changed** (+112 additions, -0 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/binaryTarget.test.ts` (+78 -0) 📝 `src/domain/binaryTarget.ts` (+34 -0) </details> ### 📄 Description Fixes #6154. ## Problem `parseThinMachO` in `src/domain/binaryTarget.ts` only checked that `header + sizeofcmds` fits in the file. A thin Mach-O whose header is valid but whose load-command table is corrupt was admitted, then failed inside a provider as a generic `execution_failure` whose remediation is `rea doctor` - even though the environment is fine and the target is not. ## Fix Walk the declared table when its bytes are present, with the same rules the loader enforces: each command needs an 8-byte header, `cmdsize >= 8`, `cmdsize` aligned (8 for 64-bit, 4 for 32-bit), no command running past `sizeofcmds`, and all commands together adding up to exactly `sizeofcmds`. Violations are rejected at admission like the existing truncated case, so the CLI reports `target_unavailable` with a specific reason instead of a misleading environment failure. A declared table that outlives the probed prefix keeps the existing commitment-only behavior. Scope note: #5170 covers misalignment in the Go/Rust readers (#5076) and does not touch this admission path; this PR is the admission-side complement for the cases it does not cover (cmdsize below the minimum, over/under-run of the declared table). ## Tests New cases pin the reporter's exact fixture shape (`ncmds=17`, `sizeofcmds=1056`, table bytes `0xff`) plus cmdsize below the minimum, cmdsize running past `sizeofcmds`, commands not adding up to `sizeofcmds`, a clean-walking table, and the probed-prefix case. All 4 rejection cases fail without the fix and pass with it; `src/domain/binaryTarget.test.ts` is 43/43 green. The heavier gates (turbo typecheck/lint/knip) were not run locally. Prepared with AI assistance (Instinct assisted). --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

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

closed this issue
added the
pull-request
label

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#7169

No description provided.

Rows
Columns