[PR #2981] [CLOSED] fix(ghidra): warn when Borland SEH handlers fall outside the function body #5535

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

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2981
Author: @sx4im
Created: 10/13/2026
Status: ❌ Closed

Base: main ← Head: fix/2926-seh-handler-limitations


📝 Commits (7)

  • b8b88f8 fix(ghidra): warn when Borland SEH handlers fall outside the function body
  • 8611926 fix(ghidra): tighten SEH prologue matching and stream boundary scan
  • b40ba8d fix(ghidra): gate SEH matching on 32-bit x86 and preserve handler address space
  • eb46d69 fix(ghidra): harden SEH detection against non-Win32 targets and complete reference coverage
  • b90ff8b fix(ghidra): recognize zero-valued register FS slots in SEH detection
  • a61cccd fix(ghidra): stop SEH xor proof at any control-flow instruction
  • 961672c fix(ghidra): qualify SEH attribution, hoist target check, require single-entry xor

📊 Changes

6 files changed (+388 additions, -51 deletions)

View changed files

📝 bridge/ghidra/ReaGhidraBridge.java (+347 -45)
📝 docs/ghidra-provider.md (+7 -3)
📝 src/ghidra/GhidraAnalysisProfile.ts (+2 -2)
📝 src/ghidra/GhidraFunctionValues.test.ts (+30 -0)
📝 src/ghidra/GhidraFunctionValues.ts (+1 -0)
📝 tests/boundary/mcp/ghidraEvidenceMcp.test.ts (+1 -1)

📄 Description

Summary

Fixes #2926. Delphi and C++Builder register exception handlers through the SEH chain with a push imm32; push fs:[reg]; mov fs:[reg], esp prologue. Ghidra's flow model never follows that chain, so the handler block lands outside the analyzed function body, and REA reported the function's callees, strings, and references as complete with no hint that a handler region was missing.

Problem and expected behavior

Smallest trigger: any Delphi function containing a try block. The compiler emits the SEH registration prologue at the function head; Ghidra keeps the pushed handler address out of the function body because nothing in its flow graph reaches it. REA's function views then silently presented incomplete results as complete. Expected: a reader of procedure_info, procedure_callees, or analyze_function is told when a known handler region is excluded, the same way #2326/#2360 handled truncated bodies from terminal calls.

Change and scope

bridge/ghidra/ReaGhidraBridge.java gains sehHandlerLimitations, which scans a function's instructions for the consecutive prologue triple (push with a 32-bit scalar operand, push with an fs: segment memory operand, mov to an fs: segment operand from esp). When the pushed handler address falls outside function.getBody(), the six function views that already carry terminal-call qualifications (procedure_pseudo_code, procedure_assembly, read_function_instructions, procedure_callees, procedure_callers, procedure_info, plus the analyze_function dossier) include an Evidence limitation naming the registration site and the excluded handler address.

I took the issue's report branch rather than the seed-as-code branch: the bridge only observes and reports, it does not rewrite Ghidra function bodies. Detection is strict by design. A partial match emits nothing, so a missed variant keeps the old behavior instead of producing noise.

src/ghidra/GhidraAnalysisProfile.ts versions the boundary-observation contract to ghidra-terminal-call-limitations-v3 so older snapshots cannot replay unqualified results. docs/ghidra-provider.md documents the new observation.

Follow the contribution scope. Unresolved scope, authority, side effects, or required verification block integration: not applicable, this is an ordinary fix within existing boundaries (read-only observation, no target execution, injection, mutation, or protection recovery).

Contract and boundary impact

  • Semantic owner and earliest changed stage: Ghidra provider bridge (ReaGhidraBridge.java), function-view result shaping.
  • Contribution scope fit and linked maintainer decision: ordinary bug fix within existing boundaries; no maintainer scope decision required.
  • CLI and MCP/tool-catalog contract: unchanged tool shapes; results may carry additional limitation strings.
  • Provider, bridge, target-format, or platform compatibility: bridge-only; the pattern is x86-32 specific and inert elsewhere (a 32-bit push imm32 scalar never matches in 16-bit code, and the fs: triple does not occur in 64-bit code).
  • Evidence, artifact, provenance, or reconstruction contract: function_boundary_observations v2 to v3; snapshots recorded under v2 fail profile match on replay, which is the intended invalidation.
  • Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable.
  • Generated metadata, 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: tests/boundary/mcp/ghidraEvidenceMcp.test.ts now expects the v3 contract string.
  • Base reproduction or other evidence: the updated test failed against the v2 profile (red) and passes after the bump (green). The Java detection itself could not be executed here; every Ghidra API call it uses (getMnemonicString, getNumOperands, getOpObjects, Scalar/Register checks, getDefaultOperandRepresentation, function.getBody().contains) already appears in the same file, and the file parses clean under tree-sitter's Java grammar.
  • User-visible CLI/MCP output (if applicable): a new limitation string, e.g. "Ghidra Listing shows a Borland/Delphi SEH frame registration at 0x5353ac (push 0x5355c3 followed by fs-segment stack chaining); the handler 0x5355c3 lies outside this procedure's body, so callees, strings, and cross-references may omit the handler region."
  • Remaining proof gaps: no execution against a real Ghidra instance with a Delphi binary; the real-Ghidra CI lane is the first place this runs against live analysis.

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

  • npm run test:focused -- tests/boundary/mcp/ghidraEvidenceMcp.test.ts — 4/4 passed (1 failed before the profile change, confirming the red step)
  • npm run test:focused -- src/ghidra/GhidraFunctionValues.test.ts — 50/50 passed
  • npm run test:focused -- src/ghidra/GhidraProvider.behavior.test.ts src/ghidra/GhidraSnapshotCache.test.ts — 60/60 passed
  • npm run check:fast — typecheck and lint green
  • npm run docs:check — green
  • npm run verify:test-discovery — 1112 test files, green

Compatibility, safety, and release

  • Breaking changes or migration steps: none. The v2 to v3 contract bump intentionally invalidates old analysis snapshots on replay; they re-run instead of replaying unqualified results.
  • Real Hopper/Ghidra, browser, or OS coverage: real Ghidra lane unavailable in this sandbox (no Ghidra installed). Stated plainly rather than assumed; the detection runs first in CI's real-Ghidra lane.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: read-only observation of the analyzed program; no new process, network, or mutation surface.

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/2981 **Author:** [@sx4im](https://github.com/sx4im) **Created:** 10/13/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/2926-seh-handler-limitations` --- ### 📝 Commits (7) - [`b8b88f8`](https://github.com/morluto/rea/commit/b8b88f83cec1130646cb27c77726dec2824b12bb) fix(ghidra): warn when Borland SEH handlers fall outside the function body - [`8611926`](https://github.com/morluto/rea/commit/8611926cfaae0ebcae57b6977d09ce7e255b2921) fix(ghidra): tighten SEH prologue matching and stream boundary scan - [`b40ba8d`](https://github.com/morluto/rea/commit/b40ba8d50e84893e76e43dde0f502e6e8bc21682) fix(ghidra): gate SEH matching on 32-bit x86 and preserve handler address space - [`eb46d69`](https://github.com/morluto/rea/commit/eb46d6958a1c6e8320d9c1cf3f024dea27934c96) fix(ghidra): harden SEH detection against non-Win32 targets and complete reference coverage - [`b90ff8b`](https://github.com/morluto/rea/commit/b90ff8b73975f77a507231cc97e8feb405f201f1) fix(ghidra): recognize zero-valued register FS slots in SEH detection - [`a61cccd`](https://github.com/morluto/rea/commit/a61cccdba5a1a44eddb73567cfcbd9939bf74bd2) fix(ghidra): stop SEH xor proof at any control-flow instruction - [`961672c`](https://github.com/morluto/rea/commit/961672cd5fe13026ee2f8dbf90fd4bb349fdff45) fix(ghidra): qualify SEH attribution, hoist target check, require single-entry xor ### 📊 Changes **6 files changed** (+388 additions, -51 deletions) <details> <summary>View changed files</summary> 📝 `bridge/ghidra/ReaGhidraBridge.java` (+347 -45) 📝 `docs/ghidra-provider.md` (+7 -3) 📝 `src/ghidra/GhidraAnalysisProfile.ts` (+2 -2) 📝 `src/ghidra/GhidraFunctionValues.test.ts` (+30 -0) 📝 `src/ghidra/GhidraFunctionValues.ts` (+1 -0) 📝 `tests/boundary/mcp/ghidraEvidenceMcp.test.ts` (+1 -1) </details> ### 📄 Description ## Summary Fixes #2926. Delphi and C++Builder register exception handlers through the SEH chain with a `push imm32; push fs:[reg]; mov fs:[reg], esp` prologue. Ghidra's flow model never follows that chain, so the handler block lands outside the analyzed function body, and REA reported the function's callees, strings, and references as complete with no hint that a handler region was missing. ## Problem and expected behavior Smallest trigger: any Delphi function containing a `try` block. The compiler emits the SEH registration prologue at the function head; Ghidra keeps the pushed handler address out of the function body because nothing in its flow graph reaches it. REA's function views then silently presented incomplete results as complete. Expected: a reader of `procedure_info`, `procedure_callees`, or `analyze_function` is told when a known handler region is excluded, the same way #2326/#2360 handled truncated bodies from terminal calls. ## Change and scope `bridge/ghidra/ReaGhidraBridge.java` gains `sehHandlerLimitations`, which scans a function's instructions for the consecutive prologue triple (`push` with a 32-bit scalar operand, `push` with an `fs:` segment memory operand, `mov` to an `fs:` segment operand from `esp`). When the pushed handler address falls outside `function.getBody()`, the six function views that already carry terminal-call qualifications (`procedure_pseudo_code`, `procedure_assembly`, `read_function_instructions`, `procedure_callees`, `procedure_callers`, `procedure_info`, plus the `analyze_function` dossier) include an Evidence limitation naming the registration site and the excluded handler address. I took the issue's report branch rather than the seed-as-code branch: the bridge only observes and reports, it does not rewrite Ghidra function bodies. Detection is strict by design. A partial match emits nothing, so a missed variant keeps the old behavior instead of producing noise. `src/ghidra/GhidraAnalysisProfile.ts` versions the boundary-observation contract to `ghidra-terminal-call-limitations-v3` so older snapshots cannot replay unqualified results. `docs/ghidra-provider.md` documents the new observation. Follow the contribution scope. Unresolved scope, authority, side effects, or required verification block integration: not applicable, this is an ordinary fix within existing boundaries (read-only observation, no target execution, injection, mutation, or protection recovery). ## Contract and boundary impact - Semantic owner and earliest changed stage: Ghidra provider bridge (`ReaGhidraBridge.java`), function-view result shaping. - Contribution scope fit and linked maintainer decision: ordinary bug fix within existing boundaries; no maintainer scope decision required. - CLI and MCP/tool-catalog contract: unchanged tool shapes; results may carry additional limitation strings. - Provider, bridge, target-format, or platform compatibility: bridge-only; the pattern is x86-32 specific and inert elsewhere (a 32-bit `push imm32` scalar never matches in 16-bit code, and the `fs:` triple does not occur in 64-bit code). - Evidence, artifact, provenance, or reconstruction contract: `function_boundary_observations` v2 to v3; snapshots recorded under v2 fail profile match on replay, which is the intended invalidation. - Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable. - Generated metadata, 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: `tests/boundary/mcp/ghidraEvidenceMcp.test.ts` now expects the v3 contract string. - Base reproduction or other evidence: the updated test failed against the v2 profile (red) and passes after the bump (green). The Java detection itself could not be executed here; every Ghidra API call it uses (`getMnemonicString`, `getNumOperands`, `getOpObjects`, `Scalar`/`Register` checks, `getDefaultOperandRepresentation`, `function.getBody().contains`) already appears in the same file, and the file parses clean under tree-sitter's Java grammar. - User-visible CLI/MCP output (if applicable): a new limitation string, e.g. "Ghidra Listing shows a Borland/Delphi SEH frame registration at 0x5353ac (push 0x5355c3 followed by fs-segment stack chaining); the handler 0x5355c3 lies outside this procedure's body, so callees, strings, and cross-references may omit the handler region." - Remaining proof gaps: no execution against a real Ghidra instance with a Delphi binary; the real-Ghidra CI lane is the first place this runs against live analysis. 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 - `npm run test:focused -- tests/boundary/mcp/ghidraEvidenceMcp.test.ts` — 4/4 passed (1 failed before the profile change, confirming the red step) - `npm run test:focused -- src/ghidra/GhidraFunctionValues.test.ts` — 50/50 passed - `npm run test:focused -- src/ghidra/GhidraProvider.behavior.test.ts src/ghidra/GhidraSnapshotCache.test.ts` — 60/60 passed - `npm run check:fast` — typecheck and lint green - `npm run docs:check` — green - `npm run verify:test-discovery` — 1112 test files, green ## Compatibility, safety, and release - Breaking changes or migration steps: none. The v2 to v3 contract bump intentionally invalidates old analysis snapshots on replay; they re-run instead of replaying unqualified results. - Real Hopper/Ghidra, browser, or OS coverage: real Ghidra lane unavailable in this sandbox (no Ghidra installed). Stated plainly rather than assumed; the detection runs first in CI's real-Ghidra lane. - Package or release metadata impact: none. - Security, privacy, process, or containment review: read-only observation of the analyzed program; no new process, network, or mutation surface. ## 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:07:23 +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#5535
No description provided.