From dcae3c6c148582d235f5650aace6f9744eeb153c Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Fri, 9 Oct 2026 03:53:28 +0800 Subject: [PATCH] fix(ghidra): reject address truncation before reads and edits --- bridge/ghidra/ReaGhidraBridge.java | 11 ++- docs/ghidra-dos.md | 8 +- docs/testing.md | 8 +- .../lib/real-ghidra-address-boundaries.mjs | 85 +++++++++++++++++++ scripts/verify-real-ghidra-com.mjs | 3 + scripts/verify-real-ghidra-dos.mjs | 7 ++ 6 files changed, 117 insertions(+), 5 deletions(-) create mode 100644 scripts/lib/real-ghidra-address-boundaries.mjs diff --git a/bridge/ghidra/ReaGhidraBridge.java b/bridge/ghidra/ReaGhidraBridge.java index 784a33da8..d5f6cd9e4 100644 --- a/bridge/ghidra/ReaGhidraBridge.java +++ b/bridge/ghidra/ReaGhidraBridge.java @@ -3004,8 +3004,17 @@ public final class ReaGhidraBridge extends HeadlessScript { AddressSpace space = spaceName == null ? currentProgram.getAddressFactory().getDefaultAddressSpace() : currentProgram.getAddressFactory().getAddressSpace(spaceName); - return space == null ? null : space.getAddress(offset); + if (space == null) return null; + Address address = space.getAddress(offset); + BigInteger requestedOffset = new BigInteger(offset, 16); + BigInteger parsedOffset = new BigInteger(Long.toUnsignedString(address.getOffset())); + if (!requestedOffset.equals(parsedOffset)) + throw new RequestFailure("invalid_request", + "Ghidra address offset cannot be represented without truncation in space " + + space.getName() + ": requested " + value + ", parsed " + canonicalAddress(address)); + return address; } + catch (RequestFailure failure) { throw failure; } catch (Exception exception) { return null; } diff --git a/docs/ghidra-dos.md b/docs/ghidra-dos.md index 893c3506d..808303681 100644 --- a/docs/ghidra-dos.md +++ b/docs/ghidra-dos.md @@ -1,7 +1,7 @@ # DOS MZ and COM analysis with Ghidra REA can analyze DOS MZ executables and explicitly selected COM images through the bring-your-own Ghidra adapter. -The Linux x64 verification lane exercises actual 16-bit disassembly and +The Linux x64 and macOS arm64 verification lanes exercise actual 16-bit disassembly and decompilation through the CLI and stdio MCP. Provider admission accepts Ghidra 12.1.x and that installation's declared JDK range. The verification lane uses Ghidra 12.1.4 and JDK 21; no DOS emulator or cross-compiler is required. @@ -87,6 +87,10 @@ address for subsequent lookups and leaves instruction decoding context with Ghidra. These coordinates are analysis load addresses, not file offsets or observations of a running DOS system. +Offsets that Ghidra would truncate to a different address are rejected before +reading or annotating a function. This also applies to explicit or encoded +address-space names; leading zeros preserve the numeric offset. + `procedure_info` and function dossiers include `body` evidence: ```json @@ -150,7 +154,7 @@ are separate from the initialized MZ module. Raw COM files and DOS extenders without an admitted MZ real-mode entry are outside this import boundary. DOS MZ is available through Ghidra, with no Hopper support claim. It is outside -the Windows native PE P0 boundary. macOS DOS verification remains unverified. +the Windows native PE P0 boundary. macOS x64 DOS verification remains unverified. ## Real verification diff --git a/docs/testing.md b/docs/testing.md index 051122f00..4c14a0c17 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -420,14 +420,18 @@ a DOS emulator or compiler, then checks real 16-bit decoding, segment relocation, near/far calls, decompilation, disjoint function body ranges, stable CLI/MCP observations, unchanged source bytes, and owned process/project cleanup. Raw p-code address-space selector tokens are reported separately from -the stable observation comparison. Linux x64 is verified; macOS DOS remains -unverified. This lane is separate from host-native and optional cross-format +the stable observation comparison. Linux x64 and macOS arm64 are verified; +macOS x64 remains unverified. This lane is separate from host-native and optional cross-format verification. See [DOS analysis](ghidra-dos.md). `npm run verify:ghidra:com` uses a generated headerless fixture with no compiler, DOS emulator or game data. It exercises explicit admission, BinaryLoader entry preparation, measured register context, whole-file byte readback, source offsets, unmapped PSP/partial reads, actual decompilation, CLI/MCP parity and owned cleanup. +Both segmented-address lanes reject oversized default, explicit-space and encoded-space +coordinates through real reads, function queries and annotation attempts. Rejected +annotations must preserve the live function dossier; CLI and MCP must report the +truncation constraint, while leading-zero coordinates still resolve correctly. It has the same Ghidra/JDK prerequisites as the MZ lane. Neither lane claims DOS runtime or PC-98 device execution. diff --git a/scripts/lib/real-ghidra-address-boundaries.mjs b/scripts/lib/real-ghidra-address-boundaries.mjs new file mode 100644 index 000000000..032e65be8 --- /dev/null +++ b/scripts/lib/real-ghidra-address-boundaries.mjs @@ -0,0 +1,85 @@ +import assert from "node:assert/strict"; + +/** Exercise segmented-address truncation through real CLI/MCP reads and edits. */ +export async function verifyGhidraAddressBoundaries(client, entry, cli) { + const query = async (name, args) => { + const reply = await client.callTool( + { name, arguments: args }, + { timeout: 240000 }, + ); + assert.notEqual(reply.isError, true, JSON.stringify(reply)); + return reply.structuredContent.result; + }; + const before = await query("analyze_function", { procedure: entry }); + // The segmented parser used to discard the high 32 bits and alias this entry. + const overflow = (0x100000000n + BigInt(entry)).toString(16); + const representations = [ + `0x${overflow}`, + `0X${overflow}`, + overflow, + `ram:0x${overflow}`, + `%72am:0x${overflow}`, + ]; + for (const address of representations) { + for (const [name, args] of [ + ["read_bytes", { address, length: 1 }], + ["inspect_native_instruction", { address }], + ["resolve_containing_procedure", { address }], + ["procedure_info", { procedure: address }], + [ + "annotate_native_function", + { + procedure: address, + name: "MUST_NOT_ALIAS", + comment: "MUST NOT WRITE", + }, + ], + ]) { + const reply = await client.callTool( + { name, arguments: args }, + { timeout: 240000 }, + ); + assert.equal(reply.isError, true, `${name} accepted ${address}`); + const error = reply.structuredContent?.error; + assert.equal(error?.code, "invalid_request", JSON.stringify(reply)); + assert.equal(error.details.operation, name); + const issues = JSON.stringify(error.details.issues); + assert.match(issues, /cannot be represented without truncation/u); + assert.ok(issues.includes(address), issues); + assert.equal(reply.structuredContent.evidence, undefined); + } + } + assert.deepEqual( + await query("analyze_function", { procedure: entry }), + before, + "Rejected oversized annotations changed the aliased function", + ); + // Leading zeros and an encoded explicit space preserve the requested offset. + const padded = `%72am:0x0000${BigInt(entry).toString(16)}`; + assert.deepEqual( + await query("read_bytes", { address: padded, length: 1 }), + await query("read_bytes", { address: entry, length: 1 }), + ); + for (const [command, flags, operation] of [ + ["read-bytes", ["--length", "1"], "read_bytes"], + [ + "annotate-native-function", + ["--name", "MUST_NOT_ALIAS"], + "annotate_native_function", + ], + ]) { + let failure; + try { + await cli(command, representations[0], flags); + } catch (error) { + assert.equal(error.code, 1); + failure = JSON.parse(error.stdout); + } + assert.equal(failure?.code, "invalid_request"); + assert.equal(failure.details.operation, operation); + assert.match( + JSON.stringify(failure.details.issues), + /cannot be represented without truncation/u, + ); + } +} diff --git a/scripts/verify-real-ghidra-com.mjs b/scripts/verify-real-ghidra-com.mjs index 455c0e735..0db71b486 100644 --- a/scripts/verify-real-ghidra-com.mjs +++ b/scripts/verify-real-ghidra-com.mjs @@ -14,6 +14,7 @@ import { functionDossierSchema } from "../dist/domain/hopperValues.js"; import { inspectGhidraInstallation } from "../dist/ghidra/GhidraInstallation.js"; import { buildDosComFixture } from "../tests/conformance/ghidra/dos-com-fixture.mjs"; import { createVerifierRun, completeVerifierRun } from "./lib/verifier-run.mjs"; +import { verifyGhidraAddressBoundaries } from "./lib/real-ghidra-address-boundaries.mjs"; // This lane intentionally has no game paths, binary seeds or compiler dependency. if (process.argv.length !== 2) @@ -228,6 +229,7 @@ try { ); assert.equal(retained.annotations.inline_comment, changes.inline_comment); assert.equal(retained.dossier.procedure.name, changes.name); + await verifyGhidraAddressBoundaries(client, fixture.entry, cli); const cleared = await call("annotate_native_function", { procedure: fixture.entry, comment: "", @@ -294,6 +296,7 @@ try { cli_mcp_parity: true, }, original_unchanged: true, + address_truncation_rejected: true, limitations: image.limitations, }; } catch (error) { diff --git a/scripts/verify-real-ghidra-dos.mjs b/scripts/verify-real-ghidra-dos.mjs index 1a786992a..114f920e2 100644 --- a/scripts/verify-real-ghidra-dos.mjs +++ b/scripts/verify-real-ghidra-dos.mjs @@ -24,6 +24,7 @@ import { import { inspectGhidraInstallation } from "../dist/ghidra/GhidraInstallation.js"; import { buildDosMzFixture } from "../tests/conformance/ghidra/dos-mz-fixture.mjs"; import { createVerifierRun, completeVerifierRun } from "./lib/verifier-run.mjs"; +import { verifyGhidraAddressBoundaries } from "./lib/real-ghidra-address-boundaries.mjs"; // This lane intentionally has no game paths, binary seeds or compiler dependency. if (process.argv.length !== 2) @@ -367,6 +368,11 @@ try { arguments: { procedure: "0xffffffffffffffff" }, }); assert.equal(invalid.isError, true, "Unmapped address did not fail"); + await verifyGhidraAddressBoundaries( + client, + identities.entry.address, + cliEvidence, + ); const cliWindow = await cliEvidence("instructions", identities.entry.address); assert.deepEqual( cliWindow.normalized_result, @@ -428,6 +434,7 @@ try { far_target: expected.far, }, invalid_address_rejected: true, + address_truncation_rejected: true, original_unchanged: true, limitations: [ "Read-only import/decompilation evidence, not DOS runtime execution or whole-program semantic equivalence.",