[PR #1893] [MERGED] fix(managed): honor metadata ExtraData before table decoding #4885

New issue

Closed

opened 2026-10-31 00:56:24 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 00:56:24 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/1893
Author: @chqcjq
Created: 10/11/2026
Status: ✅ Merged
Merged: 10/11/2026
Merged by: @morluto

Base: main ← Head: fix/managed-metadata-extra-data

📝 Commits (2)

0016742 fix(managed): honor metadata ExtraData before table decoding
8c0e0b4 Merge remote-tracking branch 'origin/main' into fix/managed-metadata-extra-data

📊 Changes

5 files changed (+326 additions, -3 deletions)

View changed files

📝 docs/managed-code-analysis.md (+7 -0)
➕ src/dotnet/ManagedMetadataExtraData.test.ts (+208 -0)
📝 src/dotnet/ManagedMetadataLayout.ts (+12 -0)
📝 src/dotnet/ManagedPe.fixture.ts (+15 -3)
➕ tests/acceptance/managedMetadataExtraData.test.ts (+84 -0)

📄 Description

Summary

Honor the CLR metadata tables ExtraData flag before decoding table rows. The parser now checks and skips the fixed four-byte word after all row counts, rather than interpreting it as the beginning of the first metadata row.

No existing issue is linked: this was found while auditing table-header decoding, and searches for ExtraData, heapSizes and the specific header behavior found no matching issue or PR before submission.

Problem and expected behavior

When HeapSizes & 0x40 is set, Microsoft readers consume an extra four-byte word after the row counts. The compatibility extension is used by some obfuscators; it is not specified by ECMA-335. See System.Reflection.Metadata's reader and the CLR native implementation.

On base 29d2624b, REA begins decoding tables four bytes too early. A 4 KiB source-owned PE fixture with the extension reports invalid heap/blob indexes and loses usable metadata, while the ordinary fixture succeeds. Microsoft PEReader/MetadataReader independently reads both fixtures as the same module (Fixture.dll), assembly (Fixture.Managed), MVID, type and method.

Expected: align the table data correctly regardless of whether the word is zero, nonzero or 0xffffffff. Its value is opaque, not a length. An incomplete word must produce a typed coverage failure at the expected file location without borrowing bytes from adjacent heaps.

Change and scope

Add a bounded, fixed-size header step after row counts and before table construction: 12 runtime lines.
Preserve heap-index width flags, row counts, row sizes and all existing representation budgets. No input-sized allocation, copy or loop is added.
Return invalid-metadata-root / metadata.tables-extra-data for a truncated word, with an absolute file offset.
Extend the source-owned fixture, regression tests, public CLI/MCP parity test and owning documentation.

Contract and boundary impact

Semantic owner and earliest changed stage: ManagedMetadataLayout tables-header interpretation.
CLI/MCP/tool catalog: no schema or tool change; affected inspections now return correctly aligned metadata.
Provider/format/platform: portable static PE/CLI parsing; no external analysis engine or target execution.
Evidence/provenance: preserve original artifact identity; decoded row locations account for the extension. New truncation diagnostics use the original file coordinate.
Process/authorization/cleanup: unchanged.
Generated metadata/package/installation: no dependency/version change or committed generated output.

Evidence and regression coverage

Executed baseline: 6 failures and 1 passing ordinary-input control. Three extension values caused incorrect decoding; three truncated-word cases produced the wrong failure scope.
Ten focused regressions now pass: absent/zero/nonzero/maximum word, actual four-byte string-heap indexes combined with 0x40, a #- stream, row-count truncation, and ExtraData extents of 0/1/3 bytes with absolute-offset assertions.
One compiled CLI/real stdio MCP acceptance test compares artifact/member results, metadata identities and source digests.
Independent Microsoft metadata-reader check on the same generated bytes agrees with the corrected module, assembly, MVID, type and method identities. The target assembly was never loaded for execution.

For evidence-bearing changes:

Observed, derived, and inferred claims remain distinguishable.
Artifact identity, row coordinates and malformed-input evidence remain preserved.
Unsupported or incomplete outcomes remain visible.

Validation performed

On Windows x64, pinned Node 24.18.0 / npm 11.16.0:

npm run test:local -- <all 9 src/dotnet/*.test.ts files> — 147 passed, no failures or skips.
npm run test:focused -- tests/acceptance/managedMetadataExtraData.test.ts — 1 passed, compiled CLI/stdio MCP parity.
npm run verify:managed — passed, 11 portable conformance scenarios; optional operator-local manifest/ILSpy checks remain unconfigured.
npm run check — passed; reported lint warnings remain, no errors.
npm run docs:check — passed.
npm run verify:test-discovery — 844 test files, each discovered exactly once.
git diff --check — passed.
Local codex review --base origin/main, with invocation-only compatibility overrides for two legacy CLI feature settings — completed with no actionable regressions reported. It also reran 103 focused source tests and the compiled acceptance test; the full 147-test result above was run separately.

Compatibility, safety, and release

Breaking changes/migrations: none.
Scope limits: this supports the ExtraData compatibility word; it does not add Edit-and-Continue delta, #JTD, or deleted-token semantics associated with other metadata flags.
Fixtures are synthetic PE files cross-checked with an independent Microsoft reader; no arbitrary compiler/obfuscator corpus claim is made.
Linux/macOS were not run locally. No Hopper/Ghidra/Android or runtime execution claim is made.
No binary fixtures, credentials, local logs or generated artifacts are committed.

Review checklist

Concrete defect, smallest reproducible input and expected behavior documented.
One focused outcome and Conventional Commit title.
Meaningful positive, malformed-input and public-boundary regressions.
Owning documentation updated.
Final diff checked for unrelated changes 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/1893 **Author:** [@chqcjq](https://github.com/chqcjq) **Created:** 10/11/2026 **Status:** ✅ Merged **Merged:** 10/11/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/managed-metadata-extra-data` --- ### 📝 Commits (2) - [`0016742`](https://github.com/morluto/rea/commit/00167426d610e9957d1756a9387cc1012934a882) fix(managed): honor metadata ExtraData before table decoding - [`8c0e0b4`](https://github.com/morluto/rea/commit/8c0e0b43df7b694d97d417d0e1da7c578f0198db) Merge remote-tracking branch 'origin/main' into fix/managed-metadata-extra-data ### 📊 Changes **5 files changed** (+326 additions, -3 deletions) <details> <summary>View changed files</summary> 📝 `docs/managed-code-analysis.md` (+7 -0) ➕ `src/dotnet/ManagedMetadataExtraData.test.ts` (+208 -0) 📝 `src/dotnet/ManagedMetadataLayout.ts` (+12 -0) 📝 `src/dotnet/ManagedPe.fixture.ts` (+15 -3) ➕ `tests/acceptance/managedMetadataExtraData.test.ts` (+84 -0) </details> ### 📄 Description ## Summary Honor the CLR metadata tables `ExtraData` flag before decoding table rows. The parser now checks and skips the fixed four-byte word after all row counts, rather than interpreting it as the beginning of the first metadata row. No existing issue is linked: this was found while auditing table-header decoding, and searches for `ExtraData`, `heapSizes` and the specific header behavior found no matching issue or PR before submission. ## Problem and expected behavior When `HeapSizes & 0x40` is set, Microsoft readers consume an extra four-byte word after the row counts. The compatibility extension is used by some obfuscators; it is not specified by ECMA-335. See [System.Reflection.Metadata's reader](https://source.dot.net/System.Reflection.Metadata/System/Reflection/Metadata/MetadataReader.cs.html) and [the CLR native implementation](https://github.com/dotnet/runtime/blob/main/src/coreclr/md/runtime/metamodel.cpp). On base `29d2624b`, REA begins decoding tables four bytes too early. A 4 KiB source-owned PE fixture with the extension reports invalid heap/blob indexes and loses usable metadata, while the ordinary fixture succeeds. Microsoft `PEReader`/`MetadataReader` independently reads both fixtures as the same module (`Fixture.dll`), assembly (`Fixture.Managed`), MVID, type and method. Expected: align the table data correctly regardless of whether the word is zero, nonzero or `0xffffffff`. Its value is opaque, not a length. An incomplete word must produce a typed coverage failure at the expected file location without borrowing bytes from adjacent heaps. ## Change and scope - Add a bounded, fixed-size header step after row counts and before table construction: 12 runtime lines. - Preserve heap-index width flags, row counts, row sizes and all existing representation budgets. No input-sized allocation, copy or loop is added. - Return `invalid-metadata-root / metadata.tables-extra-data` for a truncated word, with an absolute file offset. - Extend the source-owned fixture, regression tests, public CLI/MCP parity test and owning documentation. ## Contract and boundary impact - Semantic owner and earliest changed stage: `ManagedMetadataLayout` tables-header interpretation. - CLI/MCP/tool catalog: no schema or tool change; affected inspections now return correctly aligned metadata. - Provider/format/platform: portable static PE/CLI parsing; no external analysis engine or target execution. - Evidence/provenance: preserve original artifact identity; decoded row locations account for the extension. New truncation diagnostics use the original file coordinate. - Process/authorization/cleanup: unchanged. - Generated metadata/package/installation: no dependency/version change or committed generated output. ## Evidence and regression coverage - Executed baseline: 6 failures and 1 passing ordinary-input control. Three extension values caused incorrect decoding; three truncated-word cases produced the wrong failure scope. - Ten focused regressions now pass: absent/zero/nonzero/maximum word, actual four-byte string-heap indexes combined with `0x40`, a `#-` stream, row-count truncation, and ExtraData extents of 0/1/3 bytes with absolute-offset assertions. - One compiled CLI/real stdio MCP acceptance test compares artifact/member results, metadata identities and source digests. - Independent Microsoft metadata-reader check on the same generated bytes agrees with the corrected module, assembly, MVID, type and method identities. The target assembly was never loaded for execution. For evidence-bearing changes: - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, row coordinates and malformed-input evidence remain preserved. - [x] Unsupported or incomplete outcomes remain visible. ## Validation performed On Windows x64, pinned Node 24.18.0 / npm 11.16.0: - `npm run test:local -- <all 9 src/dotnet/*.test.ts files>` — **147 passed**, no failures or skips. - `npm run test:focused -- tests/acceptance/managedMetadataExtraData.test.ts` — **1 passed**, compiled CLI/stdio MCP parity. - `npm run verify:managed` — passed, **11 portable conformance scenarios**; optional operator-local manifest/ILSpy checks remain unconfigured. - `npm run check` — passed; reported lint warnings remain, no errors. - `npm run docs:check` — passed. - `npm run verify:test-discovery` — **844 test files**, each discovered exactly once. - `git diff --check` — passed. - Local `codex review --base origin/main`, with invocation-only compatibility overrides for two legacy CLI feature settings — completed with no actionable regressions reported. It also reran 103 focused source tests and the compiled acceptance test; the full 147-test result above was run separately. ## Compatibility, safety, and release - Breaking changes/migrations: none. - Scope limits: this supports the ExtraData compatibility word; it does not add Edit-and-Continue delta, `#JTD`, or deleted-token semantics associated with other metadata flags. - Fixtures are synthetic PE files cross-checked with an independent Microsoft reader; no arbitrary compiler/obfuscator corpus claim is made. - Linux/macOS were not run locally. No Hopper/Ghidra/Android or runtime execution claim is made. - No binary fixtures, credentials, local logs or generated artifacts are committed. ## Review checklist - [x] Concrete defect, smallest reproducible input and expected behavior documented. - [x] One focused outcome and Conventional Commit title. - [x] Meaningful positive, malformed-input and public-boundary regressions. - [x] Owning documentation updated. - [x] Final diff checked for unrelated changes 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 00:56:24 +00:00