[GH-ISSUE #4372] [Bug] Malformed metadata stream names retain root-relative file offsets #1730

New issue

Closed

opened 2026-10-16 21:09:33 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-16 21:09:33 +00:00

Owner

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

Before submitting

I searched the open and closed issues for a related report.
I removed credentials, tokens, private paths, and sensitive target data.

Area

Evidence, artifacts, or reconstruction

Observed behavior

Both source trees returned offset: 40 for the non-printable and unterminated names, although the metadata root begins at file offset 768 and the name begins at 808. The same incorrect value appears in classification.evidence[0].file_offset, checked by the script. The valid control still returns complete coverage.

Reproduction

This is an executed source-boundary reproduction, using the production inspectManagedArtifactBytes parser and the repository's synthetic PE fixture. A built CLI/MCP command and real provider were not run. The mutations affect in-memory synthetic metadata names only; no target code runs. Each malformed-name case reproduced once per tested source tree; the unchanged control returned complete coverage in each tree.

For a clean checkout, use the pinned toolchain and locked dependencies:

git clone https://github.com/morluto/rea.git rea-repro
cd rea-repro
git checkout --detach 8ab5024397ddbd3b37ab68a13cabedc2eb7ce7dd
npm ci
export REA_CHECKOUT="$PWD"

Save the complete script below as repro.mts in the checkout and run ./node_modules/.bin/tsx repro.mts. These checkout/preparation commands describe clean setup; the executed probe used already-installed locked dependencies. For the executed release comparison, a separate checkout at the release revision from the version field had its locked dependencies installed, and the main checkout’s locked tsx runner executed this same script with REA_CHECKOUT pointing to that release checkout. The release checkout does not itself include tsx.

import assert from "node:assert/strict";
import { pathToFileURL } from "node:url";
const load = (name: string) => import(pathToFileURL(
  `${process.env.REA_CHECKOUT}/src/dotnet/${name}.ts`).href);
const { buildManagedPeFixture, managedPeFixtureTarget } = await load("ManagedPe.fixture");
const { readManagedPeLayout } = await load("ManagedPeReader");
const { inspectManagedArtifactBytes: inspect } = await load("ManagedArtifactInspector");
const valid = buildManagedPeFixture();
const pe = readManagedPeLayout(valid);
assert(pe.cli);
const root = pe.rvaToOffset(pe.cli.metadata.rva, pe.cli.metadata.size, "cli.metadata");
const relative = Math.ceil((16 + valid.readUInt32LE(root + 12)) / 4) * 4 + 12;
const expected = root + relative;
const control = inspect(valid, managedPeFixtureTarget(valid));
assert.deepEqual(control.coverage, { state: "complete", issues: [] });
console.log(JSON.stringify({ case: "valid", root, expected, coverage: control.coverage }));
for (const mode of ["non-printable", "unterminated"]) {
  const bytes = Buffer.from(valid);
  if (mode === "non-printable") bytes[expected] = 1;
  else bytes.fill(0x41, expected, expected + 32);
  const result = inspect(bytes, managedPeFixtureTarget(bytes));
  const { coverage } = result;
  assert.equal(result.classification.evidence[0].file_offset, relative);
  const issue = coverage.issues[0];
  assert.equal(coverage.state, "partial");
  assert.equal(issue.scope, "metadata.stream-name");
  assert.equal(issue.offset, relative);
  assert.notEqual(issue.offset, expected);
  console.log(JSON.stringify({ case: mode, expected, issue }));
}

Expected behavior

The retained metadata.stream-name issue must use the artifact's file coordinate, so this input should report offset: 808; the classification evidence's file_offset must agree. Malformed metadata should still produce partial coverage and preserve its failure details. The inspector projects this issue directly into a field named file_offset (projection), establishing the coordinate contract independently of the older range-check fix.

Impact and scope

The diagnostic directs an analyst to a PE-header location instead of the malformed stream name. Rejection and valid-input reading remain intact. For these cases, resolving the metadata root from the CLI directory and adding it to the returned name offset recovers 768 + 40 = 808 without changing the artifact. Other root placements, member/native-boundary inspection, built CLI/MCP, compiler-produced assemblies, and external engines were not tested.

REA version

Main source 8ab5024397ddbd3b37ab68a13cabedc2eb7ce7dd; latest published rea-agents 6.5.0 source 3b79af28e80785afce9f5998d8f03289402a1847.

Runtime and platform

Node.js v24.18.0; npm 11.16.0; macOS 27.0.1 arm64; TypeScript source imported through the main checkout’s locked tsx runner on both source trees.

Target and artifact context

Synthetic managed PE from the repository's fixture builder, held in memory. Its metadata root starts at file offset 768; the first stream name starts at file offset 808. No private target or external analysis engine is involved.

Interface

Other

Earliest failure stage

Native analysis or reconstruction

Relevant output or evidence

Exact output on both tested source trees; each process exited 0 with all assertions passing:

{"case":"valid","root":768,"expected":808,"coverage":{"state":"complete","issues":[]}}
{"case":"non-printable","expected":808,"issue":{"code":"invalid-stream","scope":"metadata.stream-name","offset":40,"detail":"Metadata stream name is not printable ASCII"}}
{"case":"unterminated","expected":808,"issue":{"code":"invalid-stream","scope":"metadata.stream-name","offset":40,"detail":"Metadata stream name is missing a bounded terminator"}}

Typed outcome or recovery expected

Repeat both malformed-name cases and the valid control. Both name failures should retain their current codes, scopes, details, and partial coverage while identifying file offset 808 in both coverage and classification evidence; the valid control should remain complete. Also check a different metadata-root placement to distinguish artifact coordinates from a hard-coded value. This additional placement check is proposed, not executed.

Analysis or suspected cause

Source-derived reasoning: stream-name failures receive the metadata-root-relative cursor + 8 at the call site. The metadata entry point supplies a sliced root, so that relative value needs interpretation in the artifact coordinate system. The production managed static provider dispatch calls the same artifact parser for inspect_managed_artifact; only the parser boundary was executed here.

Additional context

Related closed issue #3364 and merged PR #3508 concern requireRange failures. The PR identifies seven range-check scopes and changes that helper and its callers; it does not change readStreamName or metadata.stream-name. Both name-failure branches remain incorrect after that fix, with a different trigger and failing scope. The full open/closed issue and PR bodies were searched for the name, terminator, ASCII, relative-offset, and managed-metadata concepts; no matching name-failure report was found.

Triage assessment

Impact level: P2
Assessment status: supported
Impact basis: Two bounded malformed-name cases retain incorrect file coordinates in production parser coverage and classification evidence; malformed-input rejection and valid reading remain intact.
Workaround status: available
Workaround basis: The executed fixture calculation resolves the metadata root and name location; adding the root offset to the returned relative value recovers the actual file location without changing the artifact.

Originally created by @coygeek on GitHub (Oct 16, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/4372 ### Before submitting - [x] I searched the open and closed issues for a related report. - [x] I removed credentials, tokens, private paths, and sensitive target data. ### Area Evidence, artifacts, or reconstruction ### Observed behavior Both source trees returned `offset: 40` for the non-printable and unterminated names, although the metadata root begins at file offset `768` and the name begins at `808`. The same incorrect value appears in `classification.evidence[0].file_offset`, checked by the script. The valid control still returns complete coverage. ### Reproduction This is an executed source-boundary reproduction, using the production `inspectManagedArtifactBytes` parser and the repository's synthetic PE fixture. A built CLI/MCP command and real provider were not run. The mutations affect in-memory synthetic metadata names only; no target code runs. Each malformed-name case reproduced once per tested source tree; the unchanged control returned complete coverage in each tree. For a clean checkout, use the pinned toolchain and locked dependencies: ```sh git clone https://github.com/morluto/rea.git rea-repro cd rea-repro git checkout --detach 8ab5024397ddbd3b37ab68a13cabedc2eb7ce7dd npm ci export REA_CHECKOUT="$PWD" ``` Save the complete script below as `repro.mts` in the checkout and run `./node_modules/.bin/tsx repro.mts`. These checkout/preparation commands describe clean setup; the executed probe used already-installed locked dependencies. For the executed release comparison, a separate checkout at the release revision from the version field had its locked dependencies installed, and the main checkout’s locked `tsx` runner executed this same script with `REA_CHECKOUT` pointing to that release checkout. The release checkout does not itself include `tsx`. ```ts import assert from "node:assert/strict"; import { pathToFileURL } from "node:url"; const load = (name: string) => import(pathToFileURL( `${process.env.REA_CHECKOUT}/src/dotnet/${name}.ts`).href); const { buildManagedPeFixture, managedPeFixtureTarget } = await load("ManagedPe.fixture"); const { readManagedPeLayout } = await load("ManagedPeReader"); const { inspectManagedArtifactBytes: inspect } = await load("ManagedArtifactInspector"); const valid = buildManagedPeFixture(); const pe = readManagedPeLayout(valid); assert(pe.cli); const root = pe.rvaToOffset(pe.cli.metadata.rva, pe.cli.metadata.size, "cli.metadata"); const relative = Math.ceil((16 + valid.readUInt32LE(root + 12)) / 4) * 4 + 12; const expected = root + relative; const control = inspect(valid, managedPeFixtureTarget(valid)); assert.deepEqual(control.coverage, { state: "complete", issues: [] }); console.log(JSON.stringify({ case: "valid", root, expected, coverage: control.coverage })); for (const mode of ["non-printable", "unterminated"]) { const bytes = Buffer.from(valid); if (mode === "non-printable") bytes[expected] = 1; else bytes.fill(0x41, expected, expected + 32); const result = inspect(bytes, managedPeFixtureTarget(bytes)); const { coverage } = result; assert.equal(result.classification.evidence[0].file_offset, relative); const issue = coverage.issues[0]; assert.equal(coverage.state, "partial"); assert.equal(issue.scope, "metadata.stream-name"); assert.equal(issue.offset, relative); assert.notEqual(issue.offset, expected); console.log(JSON.stringify({ case: mode, expected, issue })); } ``` ### Expected behavior The retained `metadata.stream-name` issue must use the artifact's file coordinate, so this input should report `offset: 808`; the classification evidence's `file_offset` must agree. Malformed metadata should still produce partial coverage and preserve its failure details. The inspector projects this issue directly into a field named `file_offset` ([projection](https://github.com/morluto/rea/blob/8ab5024397ddbd3b37ab68a13cabedc2eb7ce7dd/src/dotnet/ManagedArtifactInspector.ts#L307-L340)), establishing the coordinate contract independently of the older range-check fix. ### Impact and scope The diagnostic directs an analyst to a PE-header location instead of the malformed stream name. Rejection and valid-input reading remain intact. For these cases, resolving the metadata root from the CLI directory and adding it to the returned name offset recovers `768 + 40 = 808` without changing the artifact. Other root placements, member/native-boundary inspection, built CLI/MCP, compiler-produced assemblies, and external engines were not tested. ### REA version Main source `8ab5024397ddbd3b37ab68a13cabedc2eb7ce7dd`; latest published rea-agents 6.5.0 source `3b79af28e80785afce9f5998d8f03289402a1847`. ### Runtime and platform Node.js v24.18.0; npm 11.16.0; macOS 27.0.1 arm64; TypeScript source imported through the main checkout’s locked `tsx` runner on both source trees. ### Target and artifact context Synthetic managed PE from the repository's fixture builder, held in memory. Its metadata root starts at file offset `768`; the first stream name starts at file offset `808`. No private target or external analysis engine is involved. ### Interface Other ### Earliest failure stage Native analysis or reconstruction ### Relevant output or evidence Exact output on both tested source trees; each process exited 0 with all assertions passing: ```json {"case":"valid","root":768,"expected":808,"coverage":{"state":"complete","issues":[]}} {"case":"non-printable","expected":808,"issue":{"code":"invalid-stream","scope":"metadata.stream-name","offset":40,"detail":"Metadata stream name is not printable ASCII"}} {"case":"unterminated","expected":808,"issue":{"code":"invalid-stream","scope":"metadata.stream-name","offset":40,"detail":"Metadata stream name is missing a bounded terminator"}} ``` ### Typed outcome or recovery expected Repeat both malformed-name cases and the valid control. Both name failures should retain their current codes, scopes, details, and partial coverage while identifying file offset `808` in both coverage and classification evidence; the valid control should remain complete. Also check a different metadata-root placement to distinguish artifact coordinates from a hard-coded value. This additional placement check is proposed, not executed. ### Analysis or suspected cause Source-derived reasoning: [stream-name failures](https://github.com/morluto/rea/blob/8ab5024397ddbd3b37ab68a13cabedc2eb7ce7dd/src/dotnet/ManagedMetadataLayout.ts#L144-L165) receive the metadata-root-relative `cursor + 8` at the [call site](https://github.com/morluto/rea/blob/8ab5024397ddbd3b37ab68a13cabedc2eb7ce7dd/src/dotnet/ManagedMetadataLayout.ts#L249-L253). The [metadata entry point](https://github.com/morluto/rea/blob/8ab5024397ddbd3b37ab68a13cabedc2eb7ce7dd/src/dotnet/ManagedMetadataLayout.ts#L428-L445) supplies a sliced root, so that relative value needs interpretation in the artifact coordinate system. The production [managed static provider dispatch](https://github.com/morluto/rea/blob/8ab5024397ddbd3b37ab68a13cabedc2eb7ce7dd/src/dotnet/ManagedStaticProvider.ts#L201-L221) calls the same artifact parser for `inspect_managed_artifact`; only the parser boundary was executed here. ### Additional context Related closed issue [#3364](https://github.com/morluto/rea/issues/3364) and merged [PR #3508](https://github.com/morluto/rea/pull/3508) concern `requireRange` failures. The PR identifies seven range-check scopes and changes that helper and its callers; it does not change `readStreamName` or `metadata.stream-name`. Both name-failure branches remain incorrect after that fix, with a different trigger and failing scope. The full open/closed issue and PR bodies were searched for the name, terminator, ASCII, relative-offset, and managed-metadata concepts; no matching name-failure report was found. ## Triage assessment - Impact level: P2 - Assessment status: supported - Impact basis: Two bounded malformed-name cases retain incorrect file coordinates in production parser coverage and classification evidence; malformed-input rejection and valid reading remain intact. - Workaround status: available - Workaround basis: The executed fixture calculation resolves the metadata root and name location; adding the root offset to the returned relative value recovers the actual file location without changing the artifact.

bsakaguchi closed this issue

2026-10-16 21:09:33 +00:00

bsakaguchi referenced this issue

2026-10-31 00:54:58 +00:00

[PR #1730] [MERGED] fix(bun): report the real runtime and enforce worker memory budgets on Windows #4779