[PR #4101] [CLOSED] fix(ghidra): compare DOS relocation windows with the final loaded memory #6056

Closed
opened 2026-10-31 01:11:31 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4101
Author: @pablodiazjorge
Created: 10/15/2026
Status: ❌ Closed

Base: main ← Head: fix/dos-overlapping-relocation-windows


📝 Commits (2)

  • 7c8dc39 fix(ghidra): compare DOS relocation windows with the final loaded memory
  • 268147e Merge upstream main into fix/dos-overlapping-relocation-windows

📊 Changes

3 files changed (+147 additions, -12 deletions)

View changed files

📝 src/ghidra/GhidraLoadImage.fixture.ts (+83 -0)
📝 src/ghidra/GhidraLoadImageValues.test.ts (+44 -0)
📝 src/ghidra/GhidraLoadImageValues.ts (+20 -12)

📄 Description

Summary

Fixes #3981. inspect_native_load_image reports mismatch for a DOS MZ image whose relocation words overlap, even when Ghidra's observations describe the correct final image. The attestation applies the fixups in table order and recorded each record's expected memory_bytes_hex right after its own fixup, but the bridge reads every record's window from the final loaded Program (ReaGhidraBridge.java, getMemory().getBytes(location, …) after loading). When a later fixup rewrites part of an earlier record's word, the expected window was the intermediate value. Now the fixups are still applied in order, which keeps original_bytes_hex and values as the loader saw them, and every record's window is taken from the final modified image.

Problem and expected behavior

Smallest trigger, from the issue: an MZ with two relocation entries at module offsets 9 then 8, both original words zero. The fixup at 9 writes 00 10 at bytes 9-10, then the fixup at 8 writes 00 10 at bytes 8-9, so the final bytes 8-10 are 00 10 10 and the first record's window is 10 10. On main, relocation_records expects 00 10 for that record and the whole result is mismatch, although the modified-file and mapping digests all match. Expected, as the issue asks: verified for the final-memory observations, and mismatch for the intermediate 00 10 window, keeping both records and their addresses.

Change and scope

  • src/ghidra/GhidraLoadImageValues.ts: the loop that applies fixups now keeps each record's file offset, and the expected memory_bytes_hex of every record is read from the final modified buffer after all fixups. Addresses, segments, offsets, values, original_bytes_hex, multiplicity and the comparison itself are unchanged.
  • src/ghidra/GhidraLoadImage.fixture.ts: fixtureOverlappingDosLoadImage, an MZ with a 48-byte header, two overlapping relocations and its final bytes written independently (00 10 10), not computed by the code under test.
  • src/ghidra/GhidraLoadImageValues.test.ts: two tests in their own block. The final-memory observations verify, with both records and addresses retained. The intermediate window for the first record fails, and relocation_records is the only failing check.

Images without overlapping relocations give the same expected records as before, since a window that no later fixup touches has the same bytes after its own fixup and at the end.

Contribution scope: not applicable (verification semantics of an existing tool).

Contract and boundary impact

  • Semantic owner and earliest changed stage: the DOS MZ load-image attestation in GhidraLoadImageValues.ts.
  • CLI and MCP/tool-catalog contract: no schema change; only the expected relocation windows of overlapping fixups change.
  • Provider, bridge, target-format, or platform compatibility: the bridge is unchanged; the attestation now matches its current-memory read.
  • Evidence, artifact, provenance, or reconstruction contract: a load image that was wrongly mismatch becomes verified; incorrect modified-file, mapping or relocation observations still fail.
  • Generated metadata (docs/public/product-catalog.json), package, or installation impact: none.

Expected effects:

  • 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: the two tests above. With main's GhidraLoadImageValues.ts in place both fail: the final-memory case is mismatch, and the intermediate window is accepted.
  • Base reproduction or other evidence: the issue's reduction (expected_memory_bytes_hex: "0010", observed_memory_bytes_hex: "1010" at 0x10009) is the first test's failure on main.
  • Remaining proof gaps: no real Ghidra session loaded an overlapping MZ; the observations are simulated, as in the issue, and the bridge's current-memory read is source-derived.

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

Windows 11 x64 (10.0.26300), Node 24.18.0, branch based on c7697595d.

  • npx vitest run src/ghidra/GhidraLoadImageValues.test.ts src/ghidra/GhidraDosComLoadImage.test.ts src/ghidra/GhidraRawBinaryLoadImage.test.ts: 66 passed.
  • npx vitest run src/ghidra/GhidraProvider.behavior.test.ts src/contracts/toolEffects.test.ts: 78 passed.
  • The two new tests with main's attestation file: both failed, as expected.
  • npm run check:fast: passed.
  • npx oxfmt --check on the three files: passed.
  • Not run here: tests/boundary/cli/nativeEfficiencyComparison.test.ts, whose DOS cases call python3, which this Windows host does not provide.

Compatibility, safety, and release

  • Breaking changes or migration steps: none.
  • Real Hopper/Ghidra, browser, or OS coverage: none; simulated observations only.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: no new input is read and nothing runs.

Author readiness

Human owner: @pablodiazjorge

  • 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/4101 **Author:** [@pablodiazjorge](https://github.com/pablodiazjorge) **Created:** 10/15/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/dos-overlapping-relocation-windows` --- ### 📝 Commits (2) - [`7c8dc39`](https://github.com/morluto/rea/commit/7c8dc39afc110e26f3be7f47da71250b00c3232f) fix(ghidra): compare DOS relocation windows with the final loaded memory - [`268147e`](https://github.com/morluto/rea/commit/268147e0c13311c5a7bc2f93480503a63729873d) Merge upstream main into fix/dos-overlapping-relocation-windows ### 📊 Changes **3 files changed** (+147 additions, -12 deletions) <details> <summary>View changed files</summary> 📝 `src/ghidra/GhidraLoadImage.fixture.ts` (+83 -0) 📝 `src/ghidra/GhidraLoadImageValues.test.ts` (+44 -0) 📝 `src/ghidra/GhidraLoadImageValues.ts` (+20 -12) </details> ### 📄 Description ## Summary Fixes #3981. `inspect_native_load_image` reports `mismatch` for a DOS MZ image whose relocation words overlap, even when Ghidra's observations describe the correct final image. The attestation applies the fixups in table order and recorded each record's expected `memory_bytes_hex` right after its own fixup, but the bridge reads every record's window from the final loaded Program (`ReaGhidraBridge.java`, `getMemory().getBytes(location, …)` after loading). When a later fixup rewrites part of an earlier record's word, the expected window was the intermediate value. Now the fixups are still applied in order, which keeps `original_bytes_hex` and `values` as the loader saw them, and every record's window is taken from the final modified image. ## Problem and expected behavior Smallest trigger, from the issue: an MZ with two relocation entries at module offsets 9 then 8, both original words zero. The fixup at 9 writes `00 10` at bytes 9-10, then the fixup at 8 writes `00 10` at bytes 8-9, so the final bytes 8-10 are `00 10 10` and the first record's window is `10 10`. On main, `relocation_records` expects `00 10` for that record and the whole result is `mismatch`, although the modified-file and mapping digests all match. Expected, as the issue asks: `verified` for the final-memory observations, and `mismatch` for the intermediate `00 10` window, keeping both records and their addresses. ## Change and scope - `src/ghidra/GhidraLoadImageValues.ts`: the loop that applies fixups now keeps each record's file offset, and the expected `memory_bytes_hex` of every record is read from the final `modified` buffer after all fixups. Addresses, segments, offsets, `values`, `original_bytes_hex`, multiplicity and the comparison itself are unchanged. - `src/ghidra/GhidraLoadImage.fixture.ts`: `fixtureOverlappingDosLoadImage`, an MZ with a 48-byte header, two overlapping relocations and its final bytes written independently (`00 10 10`), not computed by the code under test. - `src/ghidra/GhidraLoadImageValues.test.ts`: two tests in their own block. The final-memory observations verify, with both records and addresses retained. The intermediate window for the first record fails, and `relocation_records` is the only failing check. Images without overlapping relocations give the same expected records as before, since a window that no later fixup touches has the same bytes after its own fixup and at the end. Contribution scope: not applicable (verification semantics of an existing tool). ## Contract and boundary impact - Semantic owner and earliest changed stage: the DOS MZ load-image attestation in `GhidraLoadImageValues.ts`. - CLI and MCP/tool-catalog contract: no schema change; only the expected relocation windows of overlapping fixups change. - Provider, bridge, target-format, or platform compatibility: the bridge is unchanged; the attestation now matches its current-memory read. - Evidence, artifact, provenance, or reconstruction contract: a load image that was wrongly `mismatch` becomes `verified`; incorrect modified-file, mapping or relocation observations still fail. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact: none. Expected effects: - [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: the two tests above. With main's `GhidraLoadImageValues.ts` in place both fail: the final-memory case is `mismatch`, and the intermediate window is accepted. - Base reproduction or other evidence: the issue's reduction (`expected_memory_bytes_hex: "0010"`, `observed_memory_bytes_hex: "1010"` at `0x10009`) is the first test's failure on main. - Remaining proof gaps: no real Ghidra session loaded an overlapping MZ; the observations are simulated, as in the issue, and the bridge's current-memory read is source-derived. For evidence-bearing changes: - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed Windows 11 x64 (10.0.26300), Node 24.18.0, branch based on `c7697595d`. - `npx vitest run src/ghidra/GhidraLoadImageValues.test.ts src/ghidra/GhidraDosComLoadImage.test.ts src/ghidra/GhidraRawBinaryLoadImage.test.ts`: 66 passed. - `npx vitest run src/ghidra/GhidraProvider.behavior.test.ts src/contracts/toolEffects.test.ts`: 78 passed. - The two new tests with main's attestation file: both failed, as expected. - `npm run check:fast`: passed. - `npx oxfmt --check` on the three files: passed. - Not run here: `tests/boundary/cli/nativeEfficiencyComparison.test.ts`, whose DOS cases call `python3`, which this Windows host does not provide. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: none; simulated observations only. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no new input is read and nothing runs. ## Author readiness Human owner: @pablodiazjorge - [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. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] 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-10-31 01:11:31 +00:00
bsakaguchi changed title from [PR #4101] [CLOSED] fix(ghidra): compare DOS relocation windows with the final loaded memory to [PR #4101] fix(ghidra): compare DOS relocation windows with the final loaded memory 2026-11-03 13:31:42 +00:00
bsakaguchi changed title from [PR #4101] fix(ghidra): compare DOS relocation windows with the final loaded memory to [PR #4101] [CLOSED] fix(ghidra): compare DOS relocation windows with the final loaded memory 2026-11-03 14:33:24 +00:00
Sign in to join this conversation.
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#6056
No description provided.