diff --git a/bridge/ghidra/ReaGhidraBridge.java b/bridge/ghidra/ReaGhidraBridge.java index 25256fcc90..400e802a81 100644 --- a/bridge/ghidra/ReaGhidraBridge.java +++ b/bridge/ghidra/ReaGhidraBridge.java @@ -1131,12 +1131,22 @@ public final class ReaGhidraBridge extends HeadlessScript { result.addProperty("direction", direction); result.add("references", edges); result.addProperty("reference_kinds_available", true); - JsonArray unresolvedCalls = direction.equals("outgoing") - ? callBoundaryObservation( + List limitations; + JsonArray unresolvedCalls = new JsonArray(); + if (direction.equals("outgoing")) { + CallBoundaryObservation calls = callBoundaryObservation( function, currentProgram.getListing().getInstructions(function.getBody(), true), null, true, references - ).unresolvedCalls() : new JsonArray(); + ); + unresolvedCalls = calls.unresolvedCalls(); + limitations = calls.limitations(); + } else { + limitations = noReturnCallLimitations( + function, + currentProgram.getListing().getInstructions(function.getBody(), true) + ); + } result.add("unresolved_calls", unresolvedCalls); - return result; + return functionObservation(result, limitations); } private JsonObject inspectNativeDataType(JsonObject params) throws Exception { diff --git a/docs/ghidra-provider.md b/docs/ghidra-provider.md index 819d55ee56..60434f8ee2 100644 --- a/docs/ghidra-provider.md +++ b/docs/ghidra-provider.md @@ -153,8 +153,8 @@ overview merely because its source artifact is unchanged. Function extents and instruction completeness refer to Ghidra's analyzed body. An inferred no-return call can terminate that body before subsequent bytes. -Pseudocode, assembly, procedure info, instruction windows, callee and caller -reads, and dossiers report observed terminal call sites in Evidence limitations, +Pseudocode, assembly, procedure info, instruction windows, incoming and outgoing +references, callee and caller reads, and dossiers report observed terminal call sites in Evidence limitations, including resolved callee names, addresses, and FunctionManager flags when available. On x86-32, the same views also report the exact Borland/Delphi-style FS exception-registration pattern when its handler address falls outside the diff --git a/scripts/create-ghidra-windows-fixture.mjs b/scripts/create-ghidra-windows-fixture.mjs index aa22c81658..4b92eddcfc 100644 --- a/scripts/create-ghidra-windows-fixture.mjs +++ b/scripts/create-ghidra-windows-fixture.mjs @@ -75,7 +75,57 @@ for (const architecture of ["x86_64", "x86"]) { : [0xe8, 0x0b, 0, 0, 0, 0xe8, 0x16, 0, 0, 0, 0x31, 0xc0, 0xc3], 0x200, ); - bytes.set([0xb8, 0x2a, 0, 0, 0, 0xc3], 0x210); + bytes.set( + x86 && !dll + ? [0xb8, 0x2a, 0, 0, 0, 0xe8, 0x66, 0, 0, 0, 0xc3] + : [0xb8, 0x2a, 0, 0, 0, 0xc3], + 0x210, + ); + + if (x86 && !dll) { + // A statically analyzed Borland-style SEH registration whose handler + // entry is referenced but is not part of the registering function body. + bytes.set( + [ + 0x55, // PUSH EBP + 0x89, + 0xe5, // MOV EBP, ESP + 0x68, + 0xc0, + 0x10, + 0x40, + 0x00, // PUSH 0x4010c0 + 0x64, + 0xff, + 0x35, + 0, + 0, + 0, + 0, // PUSH DWORD PTR FS:[0] + 0x64, + 0x89, + 0x25, + 0, + 0, + 0, + 0, // MOV DWORD PTR FS:[0], ESP + 0x64, + 0x8f, + 0x05, + 0, + 0, + 0, + 0, // POP DWORD PTR FS:[0] + 0x83, + 0xc4, + 0x04, // ADD ESP, 4 + 0x5d, // POP EBP + 0xc3, // RET + ], + 0x280, + ); + bytes.set([0xc3], 0x2c0); + } if (!dll) { const dataSection = sectionTable + 40; diff --git a/scripts/lib/real-ghidra-borland-seh.mjs b/scripts/lib/real-ghidra-borland-seh.mjs new file mode 100644 index 0000000000..eca2a5b6f2 --- /dev/null +++ b/scripts/lib/real-ghidra-borland-seh.mjs @@ -0,0 +1,84 @@ +import assert from "node:assert/strict"; + +const limitationPattern = /Borland\/Delphi-style SEH frame registration/u; + +/** Verify Ghidra reports, and does not absorb, the fixture's handler region. */ +export async function verifyGhidraBorlandSeh(functionObservation, x86) { + assert.equal(x86, true, "The SEH control exists only in the x86 PE fixture"); + const caller = "0x401015"; + const procedure = "0x401080"; + const handler = "0x4010c0"; + + const ordinary = await functionObservation("procedure_info", { + procedure: "0x401010", + }); + assert.ok( + !ordinary.limitations.some((item) => limitationPattern.test(item)), + "The ordinary return-42 control unexpectedly reports an SEH limitation", + ); + + const info = await functionObservation("procedure_info", { procedure }); + assert.ok(info.limitations.some((item) => limitationPattern.test(item))); + + const outgoing = await functionObservation("procedure_references", { + procedure, + direction: "outgoing", + }); + assert.equal(outgoing.value.direction, "outgoing"); + assert.ok(outgoing.limitations.some((item) => limitationPattern.test(item))); + assert.ok( + outgoing.value.references.some( + (reference) => + reference.source_address === "0x401083" && + reference.target_address === handler && + reference.kind.available && + reference.kind.data && + !reference.kind.call, + ), + "The pushed handler address must remain an observed data reference, without inventing a call", + ); + + const incoming = await functionObservation("procedure_references", { + procedure, + direction: "incoming", + }); + assert.equal(incoming.value.direction, "incoming"); + assert.ok(incoming.limitations.some((item) => limitationPattern.test(item))); + assert.ok( + incoming.value.references.some( + (reference) => + reference.source_address === caller && + reference.target_address === procedure, + ), + "The caller-to-registration procedure reference was not observed", + ); + + const dossier = await functionObservation("analyze_function", { + procedure, + }); + assert.ok(dossier.limitations.some((item) => limitationPattern.test(item))); + assert.ok( + dossier.value.limitations.some((item) => limitationPattern.test(item)), + "The normalized function dossier lost the SEH limitation", + ); + assert.equal(dossier.value.procedure.body.available, true); + assert.ok(dossier.value.procedure.body.ranges.length > 0); + assert.ok( + !dossier.value.procedure.body.ranges.some( + (range) => + BigInt(range.start) <= BigInt(handler) && + BigInt(range.end) >= BigInt(handler), + ), + "The handler was incorrectly claimed as part of the procedure body", + ); + + return { + limitation: true, + ordinary_control: true, + procedure_references_incoming_and_outgoing: true, + handler_data_reference: true, + caller_reference: true, + dossier_limitation: true, + handler_body_not_claimed: true, + }; +} diff --git a/scripts/verify-real-ghidra-windows.mjs b/scripts/verify-real-ghidra-windows.mjs index 7b3fbd63f5..3466258c50 100644 --- a/scripts/verify-real-ghidra-windows.mjs +++ b/scripts/verify-real-ghidra-windows.mjs @@ -29,6 +29,7 @@ import { parseBinaryTarget } from "../dist/application/BinaryTargetResolver.js"; import { completeVerifierRun, createVerifierRun } from "./lib/verifier-run.mjs"; import { resolveGhidraVerifierTimeouts } from "./lib/ghidra-verifier-timeouts.mjs"; import { verifyGhidraCompactReferences } from "./lib/real-ghidra-compact-references.mjs"; +import { verifyGhidraBorlandSeh } from "./lib/real-ghidra-borland-seh.mjs"; import { verifyGhidraFileOffsets } from "./lib/real-ghidra-file-offsets.mjs"; import { verifyWindowsGhidraByteSearch } from "./lib/ghidra-windows-byte-search.mjs"; @@ -273,6 +274,10 @@ try { functionOperation, architecture === "x86", ); + const borlandSeh = + architecture === "x86" && !dllFixture + ? await verifyGhidraBorlandSeh(functionObservationOperation, true) + : null; report = { ok: true, provider: { id: "ghidra", version: SUPPORTED_GHIDRA_VERSION }, @@ -289,6 +294,7 @@ try { byte_search: byteSearch, cleanup: "complete", compact_references: compactReferences, + borland_seh: borlandSeh, native_controls: { job_object: true, protected_dacl: true, @@ -338,6 +344,11 @@ async function inventory(operation, parameters) { } async function functionOperation(operation, parameters) { + const observation = await functionObservationOperation(operation, parameters); + return observation.value; +} + +async function functionObservationOperation(operation, parameters) { const input = parseGhidraFunctionInput(operation, parameters); if (!input.ok) throw input.error; const called = await client.callTool(operation, input.value); @@ -345,7 +356,7 @@ async function functionOperation(operation, parameters) { const result = parseGhidraFunctionResult(operation, called.value); if (!result.ok) throw result.error; observed.add(operation); - return result.value.value; + return result.value; } function assertSession(session, targetSha256, profileDigest) { diff --git a/src/ghidra/GhidraFunctionValues.test.ts b/src/ghidra/GhidraFunctionValues.test.ts index f40cc1ca37..71c8ddb504 100644 --- a/src/ghidra/GhidraFunctionValues.test.ts +++ b/src/ghidra/GhidraFunctionValues.test.ts @@ -27,6 +27,7 @@ const observationOperations: ReadonlySet = new Set([ "procedure_callers", "procedure_info", "procedure_pseudo_code", + "procedure_references", "read_function_instructions", ]); @@ -240,6 +241,43 @@ describe("Ghidra relationship metadata validation", () => { }); }); + it("preserves handler-region limitations on procedure references", () => { + const limitation = + "Ghidra Listing shows a Borland/Delphi-style SEH frame registration at 0x401008 (push 0x4010a0 followed by fs-segment stack chaining); handler 0x4010a0 lies outside this procedure's body, so callees, strings, and cross-references may omit the handler region."; + const parsed = parseGhidraFunctionResult("procedure_references", { + value: { + procedure: ghidraFunctionIdentity(), + direction: "outgoing", + references: [ghidraReferenceEdge()], + reference_kinds_available: true, + unresolved_calls: [], + }, + limitations: [limitation], + }); + expect(parsed).toEqual({ + ok: true, + value: { + value: { + procedure: ghidraFunctionIdentity(), + direction: "outgoing", + references: [ghidraReferenceEdge()], + reference_kinds_available: true, + unresolved_calls: [], + }, + limitations: [limitation], + }, + }); + expect( + parseGhidraFunctionResult("procedure_references", { + procedure: ghidraFunctionIdentity(), + direction: "outgoing", + references: [], + reference_kinds_available: true, + unresolved_calls: [], + }), + ).toMatchObject({ ok: false, error: { _tag: "AnalysisOutputError" } }); + }); + it.each(["procedure_callees", "procedure_callers"] as const)( "requires the function-observation envelope to carry terminal-call limitations for %s", (operation) => { diff --git a/src/ghidra/GhidraFunctionValues.ts b/src/ghidra/GhidraFunctionValues.ts index d038ac82e7..c8890aae8b 100644 --- a/src/ghidra/GhidraFunctionValues.ts +++ b/src/ghidra/GhidraFunctionValues.ts @@ -418,6 +418,7 @@ const functionBoundaryObservationOperations: ReadonlySet { + const limitation = + "Ghidra Listing shows a Borland/Delphi-style SEH frame registration at 0x40108f (push 0x4010c0 followed by fs-segment stack chaining); handler 0x4010c0 lies outside this procedure's body, so callees, strings, and cross-references may omit the handler region."; + const dossier = functionDossierSchema.parse(ghidraFunctionDossier()); + const harness = await connectGhidraMcp( + "ghidra-seh-reference-evidence", + (operation, input) => { + if (operation !== "procedure_references") + throw new Error(`Unexpected operation: ${operation}`); + return Promise.resolve( + ok({ + value: jsonValueSchema.parse({ + procedure: ghidraFunctionIdentity(), + direction: input.direction ?? "outgoing", + references: [], + reference_kinds_available: true, + unresolved_calls: [], + }), + limitations: [limitation], + }), + ); + }, + ); + try { + for (const direction of ["incoming", "outgoing"] as const) { + const reply = await harness.mcp.callTool({ + name: "procedure_references", + arguments: { procedure: dossier.procedure.address, direction }, + }); + expect(reply.isError).not.toBe(true); + const evidence = sessionEvidence( + harness.session, + reply.structuredContent, + ); + expect(evidence.normalized_result).toMatchObject({ direction }); + expect(evidence.raw_result).toMatchObject({ limitations: [limitation] }); + expect(evidence.limitations).toContain(limitation); + const replay = await harness.session.execute( + "procedure_references", + evidence.parameters, + ); + if (!replay.ok) throw replay.error; + expect(replay.value.limitations).toContain(limitation); + } + expect(harness.calls).toEqual([ + "procedure_references", + "procedure_references", + "procedure_references", + "procedure_references", + ]); + } finally { + await harness.close(); + } +}); + it("rejects contradictory annotation readback across the provider and MCP boundaries", async () => { const dossier = functionDossierSchema.parse(ghidraFunctionDossier()); const annotations = { diff --git a/tests/boundary/mcp/ghidraMcpHarness.ts b/tests/boundary/mcp/ghidraMcpHarness.ts index 0fce25ebe6..aaca199d23 100644 --- a/tests/boundary/mcp/ghidraMcpHarness.ts +++ b/tests/boundary/mcp/ghidraMcpHarness.ts @@ -274,6 +274,7 @@ const functionObservationOperations: ReadonlySet = new Set([ "procedure_callers", "procedure_info", "procedure_pseudo_code", + "procedure_references", "read_function_instructions", ]);