[GH-ISSUE #2586] fix(managed): preserve long method bodies when adding another method #902

New issue

Closed

opened 2026-10-15 18:29:26 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-15 18:29:26 +00:00

Owner

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

Summary

A managed verification fixture silently changes a supplied 40-byte method body when a second method is added. The two methods are placed only 32 bytes apart, so the second body replaces bytes inside the first. Member inspection then attributes altered instructions to the first method while reporting its body as present.

Steps to reproduce

Run this buffer-only comparison from the repository root at the environment below. The first body is a tiny method header followed by 38 nop instructions and ret. The second body is a valid tiny header followed by ret.

node --input-type=module <<'JS'
import { buildManagedPeFixture } from './scripts/fixtures/managed/pe.mjs';
const first = Buffer.concat([Buffer.from([0x9e]), Buffer.alloc(38), Buffer.from([0x2a])]);
for (const addSecond of [false, true]) {
  const methods = [{ name: 'LongBody', body: first }];
  if (addSecond) methods.push({ name: 'NextBody', body: Buffer.from([0x06, 0x2a]) });
  const bytes = buildManagedPeFixture({ methods });
  const stored = bytes.subarray(0xa00, 0xa00 + first.length);
  console.log(JSON.stringify({ addSecond, suppliedLength: first.length,
    firstDifference: [...first].findIndex((value, i) => value !== stored[i]),
    suppliedAt32: first[32], storedAt32: stored[32] }));
}
JS

Expected behavior

Adding a second method preserves the first method's complete supplied bytes and gives each method an address whose body does not overlap the other. Inspection following each MethodDef address reads the intended method body.

Actual behavior

The single-method fixture matches the supplied buffer exactly. With the second method, the first difference is at offset 32, where zero becomes 0x06, the second method's header byte. A focused source-member-inspector test reproduced MethodDef RVAs of 10240 and 10272 and still reported the first body as present. Its instruction inventory changed from 38 nop instructions and one ret to 36 nop instructions, one ldloc.0, and two ret instructions.

Evidence

Expected source: scripts/fixtures/managed/pe.d.mts:2 accepts per-method body Buffers and :17 accepts method lists without a 32-byte bound; AGENTS.md:13 requires preserving caller-selected inputs.
Failure source: scripts/fixtures/managed/pe.mjs:283 assigns consecutive method RVAs 32 bytes apart, and :381 copies bodies at that same fixed stride. The buffer comparison and source-member-inspector test reproduced alteration of the first body when the second is added.
Evidence provenance: observed
Local verification: reproduced
Reproduction completeness: complete

Both supplied bodies have valid tiny headers. The test only constructs and inspects PE bytes; it does not execute the altered methods.

Restoration check

Build the one-method and two-method cases, resolve each method's stored location from its actual MethodDef RVA, and compare its full stored body with its supplied buffer. Both bodies must remain byte-identical and their ranges must not overlap. The first method must retain its original instruction inventory when inspected. Relocating methods is acceptable; limiting or dropping their supplied bytes is not.

Runtime or environment

Repository revision 8b181716ac; Node v26.10.0; locked dependencies already installed. The member-inspector check used the repository's focused source test runner.

Triage assessment

Impact level: P2
Assessment status: supported
Impact basis: A supported fixture method list corrupts caller-supplied IL and yields altered member-inspection results; this is a consequential failure in generated verification fixtures, without tested target-execution impact.
Workaround status: unverified
Workaround basis: Shortening bodies avoids the overlap by discarding requested instructions; no workaround preserving this complete two-method fixture was verified.

Originally created by @coygeek on GitHub (Oct 13, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/2586 ## Summary A managed verification fixture silently changes a supplied 40-byte method body when a second method is added. The two methods are placed only 32 bytes apart, so the second body replaces bytes inside the first. Member inspection then attributes altered instructions to the first method while reporting its body as present. ## Steps to reproduce Run this buffer-only comparison from the repository root at the environment below. The first body is a tiny method header followed by 38 nop instructions and ret. The second body is a valid tiny header followed by ret. ```bash node --input-type=module <<'JS' import { buildManagedPeFixture } from './scripts/fixtures/managed/pe.mjs'; const first = Buffer.concat([Buffer.from([0x9e]), Buffer.alloc(38), Buffer.from([0x2a])]); for (const addSecond of [false, true]) { const methods = [{ name: 'LongBody', body: first }]; if (addSecond) methods.push({ name: 'NextBody', body: Buffer.from([0x06, 0x2a]) }); const bytes = buildManagedPeFixture({ methods }); const stored = bytes.subarray(0xa00, 0xa00 + first.length); console.log(JSON.stringify({ addSecond, suppliedLength: first.length, firstDifference: [...first].findIndex((value, i) => value !== stored[i]), suppliedAt32: first[32], storedAt32: stored[32] })); } JS ``` ## Expected behavior Adding a second method preserves the first method's complete supplied bytes and gives each method an address whose body does not overlap the other. Inspection following each MethodDef address reads the intended method body. ## Actual behavior The single-method fixture matches the supplied buffer exactly. With the second method, the first difference is at offset 32, where zero becomes `0x06`, the second method's header byte. A focused source-member-inspector test reproduced MethodDef RVAs of 10240 and 10272 and still reported the first body as present. Its instruction inventory changed from 38 nop instructions and one ret to 36 nop instructions, one ldloc.0, and two ret instructions. ## Evidence - Expected source: `scripts/fixtures/managed/pe.d.mts:2` accepts per-method body Buffers and `:17` accepts method lists without a 32-byte bound; `AGENTS.md:13` requires preserving caller-selected inputs. - Failure source: `scripts/fixtures/managed/pe.mjs:283` assigns consecutive method RVAs 32 bytes apart, and `:381` copies bodies at that same fixed stride. The buffer comparison and source-member-inspector test reproduced alteration of the first body when the second is added. - Evidence provenance: observed - Local verification: reproduced - Reproduction completeness: complete Both supplied bodies have valid tiny headers. The test only constructs and inspects PE bytes; it does not execute the altered methods. ## Restoration check Build the one-method and two-method cases, resolve each method's stored location from its actual MethodDef RVA, and compare its full stored body with its supplied buffer. Both bodies must remain byte-identical and their ranges must not overlap. The first method must retain its original instruction inventory when inspected. Relocating methods is acceptable; limiting or dropping their supplied bytes is not. ## Runtime or environment Repository revision 8b181716aca2cb467be499cba226abfb5ae19141; Node v26.10.0; locked dependencies already installed. The member-inspector check used the repository's focused source test runner. ## Triage assessment - Impact level: P2 - Assessment status: supported - Impact basis: A supported fixture method list corrupts caller-supplied IL and yields altered member-inspection results; this is a consequential failure in generated verification fixtures, without tested target-execution impact. - Workaround status: unverified - Workaround basis: Shortening bodies avoids the overlap by discarding requested instructions; no workaround preserving this complete two-method fixture was verified.

bsakaguchi

2026-10-15 18:29:26 +00:00

closed this issue
added the
area: domain

bug
labels

bsakaguchi commented

2026-10-15 18:29:27 +00:00

Author

Owner

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

Verified the reported defect is addressed on main by 537467ab9e.

Root cause and resolution: Fixture method bodies were assigned a fixed 32-byte stride, allowing a later body to overwrite earlier bytes.

Validation: src/dotnet/ManagedPeFixtureLayout.test.ts — preserves complete adjacent method bodies compares both complete script-fixture bodies at their RVAs and inspects the original opcode inventory.

Evidence: CI run 500e: owner unchanged and assertion passed.

Verification scope: Generated fixture layout only; methods are not executed.

Current main CI has separate failures under repair; this closure does not claim the complete main workflow is green.

@morluto commented on GitHub (Oct 14, 2026): Verified the reported defect is addressed on main by 537467ab9ecad4db8f984c792cd165d01a10054d. Root cause and resolution: Fixture method bodies were assigned a fixed 32-byte stride, allowing a later body to overwrite earlier bytes. Validation: `src/dotnet/ManagedPeFixtureLayout.test.ts` — `preserves complete adjacent method bodies` compares both complete script-fixture bodies at their RVAs and inspects the original opcode inventory. Evidence: CI run 500e: owner unchanged and assertion passed. Verification scope: Generated fixture layout only; methods are not executed. Current main CI has separate failures under repair; this closure does not claim the complete main workflow is green.

bsakaguchi referenced this issue

2026-10-16 21:02:34 +00:00

[GH-ISSUE #2383] [Tracker] Audit analysis and debugging coverage for the TIOBE top 100 languages (October 2026) #794

bsakaguchi referenced this issue

2026-10-16 21:17:25 +00:00

[PR #902] [CLOSED] feat(artifacts): let extraction and JavaScript analysis record integrity mismatches #2334

bsakaguchi referenced this issue

2026-10-17 21:08:26 +00:00

[PR #1083] [MERGED] fix(artifacts): keep integrity policy and extraction claims aligned #2631