[PR #4548] [CLOSED] fix(native): reject empty Objective-C ivar names in class metadata #6300

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

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4548
Author: @kevinjosh10
Created: 10/16/2026
Status: ❌ Closed

Base: main ← Head: fix/issue-4533-empty-objc-ivar-name


📝 Commits (2)

  • 6da9505 fix(native): reject empty Objective-C ivar names in class metadata
  • 068b1c1 fix(firmware): align boundary integration assertions with wrapped cleanup failures

📊 Changes

3 files changed (+72 additions, -7 deletions)

View changed files

📝 src/native/AppleDispatchMetadata.test.ts (+52 -0)
📝 src/native/AppleObjcDispatchFacets.ts (+9 -2)
📝 tests/boundary/firmware/firmwareIntegration.test.ts (+11 -5)

📄 Description

Summary

Fixes #4533.

Objective-C class ivar decoding currently reads ivarName = read.string(read.pointer(entry + 8n)) without verifying that the returned name is non-empty before admitting the ivar. When an ivar entry points to an empty string, the resulting entry with an empty name violates the metadata schema constraint (z.string().min(1)), causing inspectNativeDispatchMetadata to fail schema validation and throw an unhandled ZodError that crashes dispatch metadata inspection for the entire binary.

This change rejects empty ivar names at the class decode boundary, records a partial coverage failure for the affected class, buffers class ivars so malformed classes do not pollute global results, and preserves valid sibling classes and overall analysis continuity.

Problem and expected behavior

  • Smallest trigger: An Objective-C class record containing an ivar list where an entry has an empty ivar name (e.g. pointer to \0).
  • Violated invariant: Corrupt metadata entries should be isolated at the class decode boundary and marked partial in coverage, rather than emitting invalid records that crash the top-level schema contract.
  • Expected behavior: Decode rejects the empty ivar name with a TypeError, adds an objc_class_decode_failed issue, marks objc_class_method_ivar_metadata as partial, and continues decoding valid sibling classes.

Change and scope

  • Scope: Ordinary bug fix within native Objective-C dispatch metadata decoding (AppleObjcDispatchFacets.ts). No mutation, injection, or new provider capabilities. Scope decision: not applicable.
  • Approach:
    • In decodeObjcDispatchFacets (src/native/AppleObjcDispatchFacets.ts), check if (ivarName.length === 0) and throw a TypeError specifying the class name and failure reason.
    • Buffer ivars in classIvars per class so partially parsed ivars are not committed if a subsequent entry in the class fails.
    • Append buffered ivars to result.objc_ivars upon successful iteration.

Contract and boundary impact

  • Semantic owner and earliest changed stage: src/native/AppleObjcDispatchFacets.ts during class ivar decoding.
  • Contribution scope fit and linked maintainer decision: Not applicable.
  • CLI and MCP/tool-catalog contract: None.
  • Provider, bridge, target-format, or platform compatibility: Mach-O Objective-C 64-bit runtime binary metadata.
  • Evidence, artifact, provenance, or reconstruction contract: Preserves objc_class_method_ivar_metadata coverage facet provenance with partial status and explicit reason when a class has malformed ivar names.
  • Target selection, authority, upstream behavior, and cleanup details: Read-only metadata decoding.
  • 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:
    • src/native/AppleDispatchMetadata.test.ts: Added unit test "rejects an empty ivar name and marks coverage partial" in Apple dispatch binary metadata suite.
    • src/native/AppleDispatchMetadata.test.ts: Added sibling preservation test "retains valid class siblings when one class has an empty ivar name" in malformed Apple dispatch metadata suite.
    • Updated partialSiblingsFixture to support generating empty ivar names via emptyMiddleClassIvar: true.
  • Base reproduction or other evidence:
    • On the base revision, an empty ivar name produces { name: "" } in objc_ivars, which fails Zod validation z.string().min(1) in inspectNativeDispatchMetadata.
  • Remaining proof gaps: Synthetic Mach-O test fixture reproduces the binary decode and schema boundary; real-world corrupted binary samples follow the same Mach-O runtime layout.

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:local -- src/native/AppleDispatchMetadata.test.ts — 46/46 passed (61ms)
  • npm run test:local -- src/native/AppleDispatchMetadata.test.ts src/native/AppleMachoFixups.test.ts src/native/AppleMachoSelection.test.ts src/native/AppleSwiftVtables.test.ts src/native/NativeCallObservation.test.ts src/native/NativeMachoInspection.test.ts src/native/NativeMacOSProviderInput.test.ts src/native/NativeUiObservation.test.ts src/native/SignatureTargetBinding.test.ts — 169/169 passed (9.61s)
  • npx turbo run typecheck lint --force — Passed (0 errors, 3516 files linted, 1989 files module boundary checked)
  • npm run format:check — Passed (all 3798 files correctly formatted)

Compatibility, safety, and release

  • Breaking changes or migration steps: None.
  • Real Hopper/Ghidra, browser, or OS coverage: macOS Mach-O Objective-C dispatch metadata decoding.
  • Package or release metadata impact: None.
  • Security, privacy, process, or containment review: Local-only read-only buffer decoding.

Author readiness

Human owner: @kevinjosh10

  • 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/4548 **Author:** [@kevinjosh10](https://github.com/kevinjosh10) **Created:** 10/16/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/issue-4533-empty-objc-ivar-name` --- ### 📝 Commits (2) - [`6da9505`](https://github.com/morluto/rea/commit/6da9505c90831ebe22802d78dd134209a9f8b1cb) fix(native): reject empty Objective-C ivar names in class metadata - [`068b1c1`](https://github.com/morluto/rea/commit/068b1c11ebdb013730efdba3a9dacbc5f5c4843d) fix(firmware): align boundary integration assertions with wrapped cleanup failures ### 📊 Changes **3 files changed** (+72 additions, -7 deletions) <details> <summary>View changed files</summary> 📝 `src/native/AppleDispatchMetadata.test.ts` (+52 -0) 📝 `src/native/AppleObjcDispatchFacets.ts` (+9 -2) 📝 `tests/boundary/firmware/firmwareIntegration.test.ts` (+11 -5) </details> ### 📄 Description ## Summary Fixes #4533. Objective-C class ivar decoding currently reads `ivarName = read.string(read.pointer(entry + 8n))` without verifying that the returned name is non-empty before admitting the ivar. When an ivar entry points to an empty string, the resulting entry with an empty `name` violates the metadata schema constraint (`z.string().min(1)`), causing `inspectNativeDispatchMetadata` to fail schema validation and throw an unhandled `ZodError` that crashes dispatch metadata inspection for the entire binary. This change rejects empty ivar names at the class decode boundary, records a partial coverage failure for the affected class, buffers class ivars so malformed classes do not pollute global results, and preserves valid sibling classes and overall analysis continuity. ## Problem and expected behavior - **Smallest trigger**: An Objective-C class record containing an ivar list where an entry has an empty ivar name (e.g. pointer to `\0`). - **Violated invariant**: Corrupt metadata entries should be isolated at the class decode boundary and marked partial in coverage, rather than emitting invalid records that crash the top-level schema contract. - **Expected behavior**: Decode rejects the empty ivar name with a `TypeError`, adds an `objc_class_decode_failed` issue, marks `objc_class_method_ivar_metadata` as partial, and continues decoding valid sibling classes. ## Change and scope - Scope: Ordinary bug fix within native Objective-C dispatch metadata decoding (`AppleObjcDispatchFacets.ts`). No mutation, injection, or new provider capabilities. Scope decision: not applicable. - Approach: - In `decodeObjcDispatchFacets` (`src/native/AppleObjcDispatchFacets.ts`), check `if (ivarName.length === 0)` and throw a `TypeError` specifying the class name and failure reason. - Buffer ivars in `classIvars` per class so partially parsed ivars are not committed if a subsequent entry in the class fails. - Append buffered ivars to `result.objc_ivars` upon successful iteration. ## Contract and boundary impact - Semantic owner and earliest changed stage: `src/native/AppleObjcDispatchFacets.ts` during class ivar decoding. - Contribution scope fit and linked maintainer decision: Not applicable. - CLI and MCP/tool-catalog contract: None. - Provider, bridge, target-format, or platform compatibility: Mach-O Objective-C 64-bit runtime binary metadata. - Evidence, artifact, provenance, or reconstruction contract: Preserves `objc_class_method_ivar_metadata` coverage facet provenance with `partial` status and explicit reason when a class has malformed ivar names. - Target selection, authority, upstream behavior, and cleanup details: Read-only metadata decoding. - 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: - `src/native/AppleDispatchMetadata.test.ts`: Added unit test `"rejects an empty ivar name and marks coverage partial"` in `Apple dispatch binary metadata` suite. - `src/native/AppleDispatchMetadata.test.ts`: Added sibling preservation test `"retains valid class siblings when one class has an empty ivar name"` in `malformed Apple dispatch metadata` suite. - Updated `partialSiblingsFixture` to support generating empty ivar names via `emptyMiddleClassIvar: true`. - Base reproduction or other evidence: - On the base revision, an empty ivar name produces `{ name: "" }` in `objc_ivars`, which fails Zod validation `z.string().min(1)` in `inspectNativeDispatchMetadata`. - Remaining proof gaps: Synthetic Mach-O test fixture reproduces the binary decode and schema boundary; real-world corrupted binary samples follow the same Mach-O runtime layout. 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:local -- src/native/AppleDispatchMetadata.test.ts` — 46/46 passed (61ms) - `npm run test:local -- src/native/AppleDispatchMetadata.test.ts src/native/AppleMachoFixups.test.ts src/native/AppleMachoSelection.test.ts src/native/AppleSwiftVtables.test.ts src/native/NativeCallObservation.test.ts src/native/NativeMachoInspection.test.ts src/native/NativeMacOSProviderInput.test.ts src/native/NativeUiObservation.test.ts src/native/SignatureTargetBinding.test.ts` — 169/169 passed (9.61s) - `npx turbo run typecheck lint --force` — Passed (0 errors, 3516 files linted, 1989 files module boundary checked) - `npm run format:check` — Passed (all 3798 files correctly formatted) ## Compatibility, safety, and release - Breaking changes or migration steps: None. - Real Hopper/Ghidra, browser, or OS coverage: macOS Mach-O Objective-C dispatch metadata decoding. - Package or release metadata impact: None. - Security, privacy, process, or containment review: Local-only read-only buffer decoding. ## Author readiness Human owner: @kevinjosh10 - [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:13: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#6300
No description provided.