morluto/rea

Fork 0

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

[PR #5170] [CLOSED] fix(apple): reject misaligned 64-bit Mach-O load commands #6795

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5170
Author: @Cassian433
Created: 10/19/2026
Status: ❌ Closed

Base: main ← Head: fix-macho-cmdsize-alignment

📝 Commits (1)

9731e88 fix(apple): reject misaligned 64-bit Mach-O load commands

📊 Changes

7 files changed (+79 additions, -5 deletions)

View changed files

📝 src/artifacts/apple/MachoLoadCommandReader.ts (+6 -3)
📝 src/domain/apple/machoContainer.ts (+4 -0)
📝 src/go/GoMachoImage.ts (+6 -1)
📝 src/native/AppleMachoSelection.test.ts (+21 -0)
📝 src/native/AppleMachoSelection.ts (+6 -1)
📝 tests/boundary/go/goBinaryImage.test.ts (+22 -0)
📝 tests/boundary/rust/rustBinaryImage.test.ts (+14 -0)

📄 Description

Summary

Fixes #5076.

readGoMachoImage in src/go/GoMachoImage.ts checked cmdsize % 4 for both widths, so a 64-bit thin Mach-O with a 12-byte load command came back from inspect_go_binary as successful Evidence (format: "macho", bits: 64, build_info: null). inspect_rust_binary reads containers through the same function (src/rust/RustBinaryImage.ts), so it had the same gap. The Apple reader already rejects these bytes (MachoLoadCommandReader.ts uses 8 for 64-bit images and 4 for 32-bit).

Problem and expected behavior

loader.h requires cmdsize to be a multiple of 8 in 64-bit images and 4 in 32-bit ones. A 64-bit image with cmdsize 12 should be an input-format failure, as docs/go-binaries.md says for malformed supported containers. A 32-bit image with cmdsize 12 is valid and should still parse.

Change and scope

The loader.h rule now lives once, as machoLoadCommandAlignment in src/domain/apple/machoContainer.ts, which all three walkers already import. MachoLoadCommandReader.ts uses it instead of its local constant, the Go walker uses it instead of % 4, and parseMachoLayout in src/native/AppleMachoSelection.ts (64-bit little-endian only) had no alignment check at all and now uses it too. No error messages or types changed.

Not included: moving the Go walker onto readMachoImage. That reader is async over a read callback and rejects big-endian slices (MachoLoadCommandReader.ts:305), while readGoMachoImage reads a Buffer synchronously and parses big-endian images. src/native/parsers/otool.ts walks otool's text output, not the bytes, so it is left alone.

Follow the contribution scope. Not applicable: an ordinary parsing fix inside existing boundaries.

Contract and boundary impact

Semantic owner and earliest changed stage: Mach-O container parsing in GoMachoImage.ts (Go and Rust inspection) and AppleMachoSelection.ts (native layout).
Contribution scope fit and linked maintainer decision: format inspection fix, no decision needed.
CLI and MCP/tool-catalog contract: unchanged. Misaligned 64-bit images now fail: inspect_go_binary and inspect_rust_binary with the existing invalid_format input failure, the native dispatch metadata path with its existing Invalid load command size error.
Provider, bridge, target-format, or platform compatibility: only 64-bit images whose cmdsize is not a multiple of 8 change outcome. readMachoImage already rejected those.
Evidence, artifact, provenance, or reconstruction contract: inspect_go_binary and inspect_rust_binary no longer return observed Evidence for these images. Evidence schema unchanged.
Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable.
Generated metadata (docs/public/product-catalog.json), package, or installation impact: none.

Expected effects (check known effects; use documentation-only on its own).
Unchecked items do not establish absence; leave uncertainties visible.

Read existing files or captured data.
Launch or attach to a process.
Inject instrumentation or run scripts/code.
Change process memory, registers, files, or configuration.
Contact network services.
Recover protected code or work around protections.
Documentation or usability only; no target interaction.
Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

Tests added or updated:
- enforces width-dependent Mach-O load command alignment in tests/boundary/go/goBinaryImage.test.ts: 64-bit cmdsize 12 is a malformed GoBinaryFormatFailure, 32-bit cmdsize 12 still parses.
- rejects misaligned load commands in 64-bit Mach-O images in tests/boundary/rust/rustBinaryImage.test.ts: the same image through readRustBinaryImage throws RustAuditableFormatFailure.
- rejects misaligned load commands and accepts aligned commands of the same kind in src/native/AppleMachoSelection.test.ts: cmdsize 12 throws, 16 parses.
All three fail on dfe8378c (Go and Rust: expected function to throw an error, but it didn't; native: expected [Function] to throw an error) and pass with the fix.
Base reproduction or other evidence: the probe from #5076, run against GoBinaryService before and after:
```
main {"bits":64,"cmdsize":12,"go_ok":true,"apple_status":"malformed","reason":"load command 0 at offset 0 has invalid cmdsize 12"}
fix  {"bits":64,"cmdsize":12,"go_ok":false,"apple_status":"malformed","reason":"load command 0 at offset 0 has invalid cmdsize 12"}
```
The other three cases (64/16, 64/10, 32/12) are the same before and after. With the fix the 64/12 case returns AnalysisInputError with reason: "invalid_format" and "Mach-O load command size is invalid or truncated".
Real binaries: Go 1.26.5 darwin/amd64 and darwin/arm64 builds (default, -s -w, -buildmode=pie) still read on the branch with go1.26.5 build info. Every cmdsize in them is a multiple of 8 (14 to 16 commands each).
User-visible CLI/MCP output (if applicable): the packaged CLI and MCP transport were not run against the misaligned fixture. The Go and Rust MCP and CLI acceptance tests pass.
Remaining proof gaps: no misaligned binary from the wild, only synthetic fixtures. No macOS-built (ld64) binaries checked, only Go's own linker output. No Hopper, Ghidra or IDA run.

For evidence-bearing changes:

Observed, derived, and inferred claims remain distinguishable.
Artifact identity, source provenance, and failed attempts remain preserved.
Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

After npm run build:cached:

npx vitest run --project=boundary --project=mcp-boundary --project=process-global --project=cli-boundary --project=acceptance tests/boundary/go/ tests/boundary/rust/ tests/boundary/mcp/goBinaryMcp.test.ts tests/boundary/mcp/rustBinaryMcp.test.ts tests/process-global/stableArtifactConsumers.test.ts tests/boundary/cli/callerPathSymlinkParentCli.test.ts tests/acceptance/goBinaryCli.test.ts tests/acceptance/rustBinaryCli.test.ts: 11 files, 201 passed
npm run test:local -- src/native/AppleMachoSelection.test.ts src/native/AppleDispatchMetadata.test.ts: 70 passed
npx vitest run --project=adapters src/artifacts/apple/ src/domain/apple/: 87 passed
npm run check:fast (typecheck and lint): passed

Node 24.18.1, npm 11.16.0, Linux x64.

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: none needed, the change is in the built-in readers.
Package or release metadata impact: none.
Security, privacy, process, or containment review: parsing only, no new reads.

Author readiness

Human owner:

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.

Before merge

This PR addresses a concrete problem or an agreed enhancement.
Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
The PR has one focused outcome and the title follows type(scope): outcome.
Related issue is linked, or the reason for not linking one is stated above.
Tests cover changed observable behavior and meaningful failure paths.
Owning docs, contracts, and generated metadata are updated where needed.
User-visible CLI/MCP changes include representative output.
I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

_{🔄 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/5170 **Author:** [@Cassian433](https://github.com/Cassian433) **Created:** 10/19/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix-macho-cmdsize-alignment` --- ### 📝 Commits (1) - [`9731e88`](https://github.com/morluto/rea/commit/9731e884ff0001aa5e99813e76acc62fc4ea267c) fix(apple): reject misaligned 64-bit Mach-O load commands ### 📊 Changes **7 files changed** (+79 additions, -5 deletions) <details> <summary>View changed files</summary> 📝 `src/artifacts/apple/MachoLoadCommandReader.ts` (+6 -3) 📝 `src/domain/apple/machoContainer.ts` (+4 -0) 📝 `src/go/GoMachoImage.ts` (+6 -1) 📝 `src/native/AppleMachoSelection.test.ts` (+21 -0) 📝 `src/native/AppleMachoSelection.ts` (+6 -1) 📝 `tests/boundary/go/goBinaryImage.test.ts` (+22 -0) 📝 `tests/boundary/rust/rustBinaryImage.test.ts` (+14 -0) </details> ### 📄 Description ## Summary Fixes #5076. `readGoMachoImage` in `src/go/GoMachoImage.ts` checked `cmdsize % 4` for both widths, so a 64-bit thin Mach-O with a 12-byte load command came back from `inspect_go_binary` as successful Evidence (`format: "macho"`, `bits: 64`, `build_info: null`). `inspect_rust_binary` reads containers through the same function (`src/rust/RustBinaryImage.ts`), so it had the same gap. The Apple reader already rejects these bytes (`MachoLoadCommandReader.ts` uses 8 for 64-bit images and 4 for 32-bit). ## Problem and expected behavior loader.h requires cmdsize to be a multiple of 8 in 64-bit images and 4 in 32-bit ones. A 64-bit image with cmdsize 12 should be an input-format failure, as docs/go-binaries.md says for malformed supported containers. A 32-bit image with cmdsize 12 is valid and should still parse. ## Change and scope The loader.h rule now lives once, as `machoLoadCommandAlignment` in `src/domain/apple/machoContainer.ts`, which all three walkers already import. `MachoLoadCommandReader.ts` uses it instead of its local constant, the Go walker uses it instead of `% 4`, and `parseMachoLayout` in `src/native/AppleMachoSelection.ts` (64-bit little-endian only) had no alignment check at all and now uses it too. No error messages or types changed. Not included: moving the Go walker onto `readMachoImage`. That reader is async over a read callback and rejects big-endian slices (`MachoLoadCommandReader.ts:305`), while `readGoMachoImage` reads a Buffer synchronously and parses big-endian images. `src/native/parsers/otool.ts` walks otool's text output, not the bytes, so it is left alone. Follow the [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope). Not applicable: an ordinary parsing fix inside existing boundaries. ## Contract and boundary impact - Semantic owner and earliest changed stage: Mach-O container parsing in `GoMachoImage.ts` (Go and Rust inspection) and `AppleMachoSelection.ts` (native layout). - Contribution scope fit and linked maintainer decision: format inspection fix, no decision needed. - CLI and MCP/tool-catalog contract: unchanged. Misaligned 64-bit images now fail: `inspect_go_binary` and `inspect_rust_binary` with the existing `invalid_format` input failure, the native dispatch metadata path with its existing `Invalid load command size` error. - Provider, bridge, target-format, or platform compatibility: only 64-bit images whose cmdsize is not a multiple of 8 change outcome. `readMachoImage` already rejected those. - Evidence, artifact, provenance, or reconstruction contract: `inspect_go_binary` and `inspect_rust_binary` no longer return observed Evidence for these images. Evidence schema unchanged. - Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact: none. Expected effects (check known effects; use documentation-only on its own). Unchecked items do not establish absence; leave uncertainties visible. - [x] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - Tests added or updated: - `enforces width-dependent Mach-O load command alignment` in `tests/boundary/go/goBinaryImage.test.ts`: 64-bit cmdsize 12 is a malformed `GoBinaryFormatFailure`, 32-bit cmdsize 12 still parses. - `rejects misaligned load commands in 64-bit Mach-O images` in `tests/boundary/rust/rustBinaryImage.test.ts`: the same image through `readRustBinaryImage` throws `RustAuditableFormatFailure`. - `rejects misaligned load commands and accepts aligned commands of the same kind` in `src/native/AppleMachoSelection.test.ts`: cmdsize 12 throws, 16 parses. All three fail on dfe8378c (Go and Rust: `expected function to throw an error, but it didn't`; native: `expected [Function] to throw an error`) and pass with the fix. - Base reproduction or other evidence: the probe from #5076, run against `GoBinaryService` before and after: ``` main {"bits":64,"cmdsize":12,"go_ok":true,"apple_status":"malformed","reason":"load command 0 at offset 0 has invalid cmdsize 12"} fix {"bits":64,"cmdsize":12,"go_ok":false,"apple_status":"malformed","reason":"load command 0 at offset 0 has invalid cmdsize 12"} ``` The other three cases (64/16, 64/10, 32/12) are the same before and after. With the fix the 64/12 case returns `AnalysisInputError` with `reason: "invalid_format"` and "Mach-O load command size is invalid or truncated". - Real binaries: Go 1.26.5 darwin/amd64 and darwin/arm64 builds (default, `-s -w`, `-buildmode=pie`) still read on the branch with go1.26.5 build info. Every cmdsize in them is a multiple of 8 (14 to 16 commands each). - User-visible CLI/MCP output (if applicable): the packaged CLI and MCP transport were not run against the misaligned fixture. The Go and Rust MCP and CLI acceptance tests pass. - Remaining proof gaps: no misaligned binary from the wild, only synthetic fixtures. No macOS-built (ld64) binaries checked, only Go's own linker output. No Hopper, Ghidra or IDA run. For evidence-bearing changes: - [ ] Observed, derived, and inferred claims remain distinguishable. - [ ] Artifact identity, source provenance, and failed attempts remain preserved. - [ ] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed After `npm run build:cached`: - `npx vitest run --project=boundary --project=mcp-boundary --project=process-global --project=cli-boundary --project=acceptance tests/boundary/go/ tests/boundary/rust/ tests/boundary/mcp/goBinaryMcp.test.ts tests/boundary/mcp/rustBinaryMcp.test.ts tests/process-global/stableArtifactConsumers.test.ts tests/boundary/cli/callerPathSymlinkParentCli.test.ts tests/acceptance/goBinaryCli.test.ts tests/acceptance/rustBinaryCli.test.ts`: 11 files, 201 passed - `npm run test:local -- src/native/AppleMachoSelection.test.ts src/native/AppleDispatchMetadata.test.ts`: 70 passed - `npx vitest run --project=adapters src/artifacts/apple/ src/domain/apple/`: 87 passed - `npm run check:fast` (typecheck and lint): passed Node 24.18.1, npm 11.16.0, Linux x64. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: none needed, the change is in the built-in readers. - Package or release metadata impact: none. - Security, privacy, process, or containment review: parsing only, no new reads. ## Author readiness Human owner:  - [ ] 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. ## Before merge - [ ] This PR addresses a concrete problem or an agreed enhancement. - [ ] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [ ] The PR has one focused outcome and the title follows `type(scope): outcome`. - [ ] Related issue is linked, or the reason for not linking one is stated above. - [ ] Tests cover changed observable behavior and meaningful failure paths. - [ ] Owning docs, contracts, and generated metadata are updated where needed. - [ ] User-visible CLI/MCP changes include representative output. - [ ] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-11-01 21:43:06 +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#6795

No description provided.

Rows
Columns