morluto/rea

Fork 0

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

[GH-ISSUE #4712] [Bug] Ghidra language catalog rejects valid replacement characters #2460

New issue

Closed

opened 2026-10-17 21:05:01 +00:00 by bsakaguchi · 4 comments

bsakaguchi commented

2026-10-17 21:05:01 +00:00

Owner

Originally created by @morluto on GitHub (Oct 17, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/4712

Before submitting

Searched open and closed reports for Ghidra replacement characters. Related #4694/#4697 concern Apktool; no Ghidra catalog duplicate found.
No credentials or sensitive target data included.

Area

Provider or capability discovery

Observed behavior

A compiled parser probe rejects an otherwise usable Ghidra language-definition document when an unrelated description contains a valid literal U+FFFD. Replacing only that description with ASCII returns the expected language/compiler IDs. The failure is xmldom's advisory warning promoted by onWarningStopParsing, not an invalid source character.

Reproduction

Source 264799b0f, Node 24.18.0 on Darwin arm64. Ran parseGhidraLanguageDefinitions from the compiled module on:

<language_definitions><language id="toy:LE:32:default" description="�"><compiler id="default"/></language></language_definitions>

ASCII-description control returned [{id:"toy:LE:32:default",compilerSpecIds:["default"]}]. Literal-U+FFFD case threw a warning mentioning "Unicode replacement character detected, source encoding issues?" and onWarningStopParsing. One probe per case. No Ghidra installation or engine process was used.

Expected behavior

Keep the language/compiler facts when the XML and its original bytes are valid. Malformed UTF-8 and malformed XML must remain distinguishable refusals. XML1.0's Char production permits U+FFFD; its presence in a decoded string cannot establish byte corruption. XML1.0 character range.

Impact and scope

Confirmed at the compiled parser boundary. Source-derived: readGhidraLanguageCatalog propagates the rejection, which can prevent admission of configured language overrides even though the offending description is not a lookup key. Real Ghidra, Windows and public CLI/MCP delivery are unverified.

REA version

264799b0f (package 6.5.0).

Runtime and platform

Darwin arm64 / Node 24.18.0; @xmldom/xmldom 0.9.12. Real Ghidra not invoked.

Target and artifact context

Reduced synthetic .ldefs XML. No executable target or private installation data.

Interface

Other — language-catalog parser/provider admission boundary.

Earliest failure stage

Setup or provider admission.

Relevant output or evidence

ASCII description: one language and its default compiler returned.
Literal U+FFFD: advisory warning promoted to exception.
Catalog parser uses onWarningStopParsing.
File decoder currently uses a nonfatal TextDecoder, conflating actual malformed UTF-8 replacement with valid literal text before the XML warning check.

Analysis or suspected cause

Validity is inferred from a diagnostic warning rather than original-byte decoding plus XML grammar. Shared SAZ parsing already uses a strict grammar pass but filters the same warning by message text. Repair the owning byte/XML boundaries and remove duplicate warning heuristics while preserving DTD and line-ending policies. Context7/DeepWiki were consulted; installed source and W3C resolve the generated wiki's mistaken claim that U+FFFD is invalid XML.

Originally created by @morluto on GitHub (Oct 17, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/4712 ### Before submitting - [x] Searched open and closed reports for Ghidra replacement characters. Related #4694/#4697 concern Apktool; no Ghidra catalog duplicate found. - [x] No credentials or sensitive target data included. ### Area Provider or capability discovery ### Observed behavior A compiled parser probe rejects an otherwise usable Ghidra language-definition document when an unrelated description contains a valid literal U+FFFD. Replacing only that description with ASCII returns the expected language/compiler IDs. The failure is xmldom's advisory warning promoted by onWarningStopParsing, not an invalid source character. ### Reproduction Source 264799b0f, Node 24.18.0 on Darwin arm64. Ran parseGhidraLanguageDefinitions from the compiled module on: ```xml <language_definitions><language id="toy:LE:32:default" description="�"><compiler id="default"/></language></language_definitions> ``` ASCII-description control returned [{id:"toy:LE:32:default",compilerSpecIds:["default"]}]. Literal-U+FFFD case threw a warning mentioning "Unicode replacement character detected, source encoding issues?" and onWarningStopParsing. One probe per case. No Ghidra installation or engine process was used. ### Expected behavior Keep the language/compiler facts when the XML and its original bytes are valid. Malformed UTF-8 and malformed XML must remain distinguishable refusals. XML1.0's Char production permits U+FFFD; its presence in a decoded string cannot establish byte corruption. [XML1.0 character range](https://www.w3.org/TR/xml/#charsets). ### Impact and scope Confirmed at the compiled parser boundary. Source-derived: readGhidraLanguageCatalog propagates the rejection, which can prevent admission of configured language overrides even though the offending description is not a lookup key. Real Ghidra, Windows and public CLI/MCP delivery are unverified. ### REA version 264799b0f (package 6.5.0). ### Runtime and platform Darwin arm64 / Node 24.18.0; @xmldom/xmldom 0.9.12. Real Ghidra not invoked. ### Target and artifact context Reduced synthetic .ldefs XML. No executable target or private installation data. ### Interface Other — language-catalog parser/provider admission boundary. ### Earliest failure stage Setup or provider admission. ### Relevant output or evidence - ASCII description: one language and its default compiler returned. - Literal U+FFFD: advisory warning promoted to exception. - [Catalog parser](https://github.com/morluto/rea/blob/264799b0f/src/ghidra/GhidraLanguageCatalog.ts#L79) uses onWarningStopParsing. - [File decoder](https://github.com/morluto/rea/blob/264799b0f/src/ghidra/GhidraLanguageCatalog.ts#L150) currently uses a nonfatal TextDecoder, conflating actual malformed UTF-8 replacement with valid literal text before the XML warning check. ### Analysis or suspected cause Validity is inferred from a diagnostic warning rather than original-byte decoding plus XML grammar. Shared SAZ parsing already uses a strict grammar pass but filters the same warning by message text. Repair the owning byte/XML boundaries and remove duplicate warning heuristics while preserving DTD and line-ending policies. Context7/DeepWiki were consulted; installed source and W3C resolve the generated wiki's mistaken claim that U+FFFD is invalid XML.

bsakaguchi added the

triage

bug

labels

2026-10-17 21:05:01 +00:00

bsakaguchi commented

2026-10-17 21:05:03 +00:00

Author

Owner

@morluto commented on GitHub (Oct 17, 2026):

Published on main: 623339a7a (shared XML owner; Apktool/SAZ) and 3cfa00a29 (Ghidra byte/syntax boundaries). Follow-up to merged #4697 / #4694.

Root cause: xmldom emits an advisory warning for literal U+FFFD, while Ghidra rejected all warnings and decoded malformed bytes nonfatally. The shared owner now validates syntax with saxes, then projects the source-located DOM; callers select DTD and line-ending policies. Removed duplicated warning-message filters. Ghidra reuses fatal BOM/declaration-aware byte decoding. No byte substitutions or provider authority changes.

Validation executed: 92 focused checks across six files; 3 selected fake-Apktool production-boundary checks (22 skipped); 6 final filesystem encoding controls with typed failure assertions; check:fast; docs:check (5 checks); commit and push hooks. Valid UTF-8/UTF-16, literal U+FFFD, malformed bytes, declaration mismatch, malformed XML and existing descriptor cleanup controls passed. Original reduced baseline parser probe failed with literal U+FFFD. No real Ghidra/Apktool process, Windows qualification, or large-catalog/stress run.

Context7 and DeepWiki informed the investigation, verified against pinned xmldom 0.9.12 / saxes 6.0.0 and XML 1.0 Char. DeepWiki incorrectly described U+FFFD as invalid; that claim was rejected. DeepWiki has no indexed saxes repository. saxes documentation explicitly excludes full DTD validation; the DOM stage retains its DTD checks. This is not a validating-DTD claim.

Bounded equivalent completed task: tiny actual Ghidra file read plus SAZ and Apktool XML decode; same complete JSON replies in all six runs, 155 raw cl100k tokens each. Baseline c0b3cd14b; first process separate from two alternating warm repeats.

Phase/build	Elapsed s	CPU s	Sampled peak process-family RSS bytes
First/baseline	0.084134	0.073464	60358656
First/candidate	0.082337	0.063016	56410112
Warm median/baseline	0.083311	0.073279	55746560
Warm median/candidate	0.081568	0.064602	56811520

No performance improvement claim. No true cold-cache, whole-agent/schema/retry, public CLI/MCP, native-engine, or large-output costs measured. wait4 CPU includes reaped children; PPID/RSS sampling may miss transients. Heavy profiling remained stopped.

Remaining audit: other XML/plist consumer policies and broader codebase areas still require review; this batch does not establish all XML consumers have been audited.

@morluto commented on GitHub (Oct 17, 2026): Published on main: `623339a7a` (shared XML owner; Apktool/SAZ) and `3cfa00a29` (Ghidra byte/syntax boundaries). Follow-up to merged #4697 / #4694. Root cause: xmldom emits an advisory warning for literal U+FFFD, while Ghidra rejected all warnings and decoded malformed bytes nonfatally. The shared owner now validates syntax with saxes, then projects the source-located DOM; callers select DTD and line-ending policies. Removed duplicated warning-message filters. Ghidra reuses fatal BOM/declaration-aware byte decoding. No byte substitutions or provider authority changes. Validation executed: 92 focused checks across six files; 3 selected fake-Apktool production-boundary checks (22 skipped); 6 final filesystem encoding controls with typed failure assertions; check:fast; docs:check (5 checks); commit and push hooks. Valid UTF-8/UTF-16, literal U+FFFD, malformed bytes, declaration mismatch, malformed XML and existing descriptor cleanup controls passed. Original reduced baseline parser probe failed with literal U+FFFD. No real Ghidra/Apktool process, Windows qualification, or large-catalog/stress run. Context7 and DeepWiki informed the investigation, verified against pinned xmldom 0.9.12 / saxes 6.0.0 and [XML 1.0 Char](https://www.w3.org/TR/xml/#charsets). DeepWiki incorrectly described U+FFFD as invalid; that claim was rejected. DeepWiki has no indexed saxes repository. [saxes documentation](https://github.com/lddubeau/saxes#readme) explicitly excludes full DTD validation; the DOM stage retains its DTD checks. This is not a validating-DTD claim. Bounded equivalent completed task: tiny actual Ghidra file read plus SAZ and Apktool XML decode; same complete JSON replies in all six runs, 155 raw cl100k tokens each. Baseline `c0b3cd14b`; first process separate from two alternating warm repeats. | Phase/build | Elapsed s | CPU s | Sampled peak process-family RSS bytes | | --- | ---: | ---: | ---: | | First/baseline | 0.084134 | 0.073464 | 60358656 | | First/candidate | 0.082337 | 0.063016 | 56410112 | | Warm median/baseline | 0.083311 | 0.073279 | 55746560 | | Warm median/candidate | 0.081568 | 0.064602 | 56811520 | No performance improvement claim. No true cold-cache, whole-agent/schema/retry, public CLI/MCP, native-engine, or large-output costs measured. wait4 CPU includes reaped children; PPID/RSS sampling may miss transients. Heavy profiling remained stopped. Remaining audit: other XML/plist consumer policies and broader codebase areas still require review; this batch does not establish all XML consumers have been audited.

bsakaguchi commented

2026-10-17 21:05:03 +00:00

Author

Owner

@yadavd4631 commented on GitHub (Oct 17, 2026):

Opened #4715 with the fix and test coverage.

@yadavd4631 commented on GitHub (Oct 17, 2026): Opened #4715 with the fix and test coverage.

bsakaguchi commented

2026-10-17 21:05:04 +00:00

Author

Owner

@XinwangTao commented on GitHub (Oct 17, 2026):

Agreed with the root cause — U+FFFD is explicitly allowed by the XML 1.0 Char production, so promoting xmldom's advisory warning to a fatal rejection was over-strict. Switching the shared owner to saxes-based syntax validation and then projecting the source-located DOM (with callers picking DTD/line-ending policy) is a clean split, and reusing the fatal BOM/declaration-aware byte decoding for Ghidra keeps the provider boundary honest. One suggestion: make the advisory-vs-fatal distinction explicit in the shared owner's API (e.g. a warnings collection on the returned document) so future callers do not have to guess which warnings are safe to ignore — that is exactly the class of bug that reappears when a new parser gets swapped in.

@XinwangTao commented on GitHub (Oct 17, 2026): Agreed with the root cause — U+FFFD is explicitly allowed by the XML 1.0 Char production, so promoting xmldom's advisory warning to a fatal rejection was over-strict. Switching the shared owner to saxes-based syntax validation and then projecting the source-located DOM (with callers picking DTD/line-ending policy) is a clean split, and reusing the fatal BOM/declaration-aware byte decoding for Ghidra keeps the provider boundary honest. One suggestion: make the advisory-vs-fatal distinction explicit in the shared owner's API (e.g. a `warnings` collection on the returned document) so future callers do not have to guess which warnings are safe to ignore — that is exactly the class of bug that reappears when a new parser gets swapped in.

bsakaguchi closed this issue

2026-10-31 00:34:13 +00:00

bsakaguchi commented

2026-10-31 00:34:14 +00:00

Author

Owner

@morluto commented on GitHub (Oct 18, 2026):

Closing as fixed on main after reconciling this report with the landed owner/consumer changes.

3cfa00a29: fix(ghidra): preserve valid Unicode language metadata. Validation: 92 focused checks, 3 selected fake Apktool boundary checks, 6 final encoding controls, check:fast and docs:check. Ghidra coverage uses tiny real files and descriptors, without a real engine or Windows qualification.

Evidence scope: this closure uses the landed source, regression coverage and validation already recorded in the linked commits/thread. No new local tests or native-provider reproduction were run during this backlog pass; previously documented platform and real-provider gaps still apply.

@morluto commented on GitHub (Oct 18, 2026):  Closing as fixed on `main` after reconciling this report with the landed owner/consumer changes. - [3cfa00a29](https://github.com/morluto/rea/commit/3cfa00a29c1eb531faca9542935446e6d751ff4c): fix(ghidra): preserve valid Unicode language metadata. Validation: 92 focused checks, 3 selected fake Apktool boundary checks, 6 final encoding controls, check:fast and docs:check. Ghidra coverage uses tiny real files and descriptors, without a real engine or Windows qualification. Evidence scope: this closure uses the landed source, regression coverage and validation already recorded in the linked commits/thread. No new local tests or native-provider reproduction were run during this backlog pass; previously documented platform and real-provider gaps still apply.

bsakaguchi referenced this issue

2026-10-31 00:36:35 +00:00

[GH-ISSUE #5038] [Feature] IDA provider: declare read_bytes with faithful loaded-byte semantics #3094

bsakaguchi referenced this issue

2026-10-31 01:05:28 +00:00

[PR #2513] [CLOSED] fix(ida): keep binary_overview available without segment enumeration #5288

bsakaguchi referenced this issue

2026-10-31 01:05:37 +00:00

[PR #2680] [MERGED] fix(application): accept alternative document selection in binary_ove… #5307