[PR #4024] [MERGED] fix(evm): reject repeated byte-string keys in Solidity metadata #5998

New issue

Closed

opened 2026-10-31 01:11:04 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:11:04 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4024
Author: @Sriram-PR
Created: 10/15/2026
Status: ✅ Merged
Merged: 10/21/2026
Merged by: @morluto

Base: main ← Head: fix/evm-metadata-repeated-byte-keys

📝 Commits (1)

8787a02 fix(evm): reject repeated byte-string keys in Solidity metadata

📊 Changes

2 files changed (+30 additions, -2 deletions)

View changed files

📝 src/evm/SolidityMetadata.test.ts (+15 -0)
📝 src/evm/SolidityMetadata.ts (+15 -2)

📄 Description

Summary

bytecode.metadata reported a terminal CBOR root map with the same byte-string key twice as decoded, with both entries and no diagnostic, while the same map with a repeated text key is not-decoded. SolidityMetadata.ts relies on cborg's rejectDuplicateMapKeys, but with useMaps that check compares Map keys by identity, and each byte-string key decodes to its own Uint8Array, so two equal byte keys never match.

Fixes #3991

Problem and expected behavior

Smallest trigger: the bytecode 00a24101014101020007 (STOP, the 7-byte map {h'01': 1, h'01': 2}, length 0007). inspect-evm-interface <file> hex returned metadata status: "decoded", two entries with key bytes 01, diagnostic: null. Expected (docs/evm-bytecode.md: malformed candidates report not-decoded with a diagnostic): not-decoded, offset 1 and length 7 kept, no entries, no compiler version, as for repeated text keys.

Change and scope

src/evm/SolidityMetadata.ts: while collecting root map entries, compare each scalar key by kind and value and throw on a repeat. The existing catch turns that into not-decoded with the message as the diagnostic (CBOR decode error: found repeat map key {"kind":"bytes","value":"01"}), keeping the offset and length.
Keys that are nested containers (uninterpreted) are left out of the comparison: two different nested maps both project to { kind: "uninterpreted", container: "map" }, so they are not comparable from the projection. Their exact bytes stay addressable as before.
Text, integer, boolean and null repeats were already rejected by cborg and still are, with the same message.
Not included: duplicate keys inside nested containers, which this profile leaves uninterpreted.

Contribution scope: ordinary fix within existing boundaries; no scope decision needed.

Contract and boundary impact

Semantic owner and earliest changed stage: inspectSolidityMetadata in src/evm/SolidityMetadata.ts, when the root map's entries are projected.
Contribution scope fit and linked maintainer decision: not applicable (bug fix, #3991).
CLI and MCP/tool-catalog contract: no schema change. inspect-evm-interface / inspect_evm_interface report not-decoded with a diagnostic for a root map with repeated byte-string keys.
Provider, bridge, target-format, or platform compatibility: none; the EVMole engine and its raw_result are untouched.
Evidence, artifact, provenance, or reconstruction contract: bytecode.metadata only; the original bytecode and recorded range are unchanged.
Target selection, authority, upstream behavior, and cleanup details: not applicable.
Generated metadata, package, or installation impact: none.

Expected effects (check known effects; use documentation-only on its own).
Unchecked items do not establish absence; leave uncertainties visible.

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/evm/SolidityMetadata.test.ts adds duplicate byte key (a2410101410102) to the existing malformed-candidate table, and a control test that distinct byte keys still decode and that two distinct nested-container keys still give partial.
Base reproduction: on the base revision the duplicate byte key case fails (expected { ... } to match object { status: 'not-decoded', ... }); the control test passes on both.

User-visible CLI output (inspect-evm-interface <file> hex --format json, bytecode.metadata status / entry count / diagnostic, Linux x64):

bytecode               base                          PR
00a24101014102020007   decoded, 2, null              decoded, 2, null
00a26178016178020007   not-decoded, 0, repeat "x"    not-decoded, 0, repeat "x"
00a24101014101020007   decoded, 2, null              not-decoded, 0, CBOR decode error: found repeat map key {"kind":"bytes","value":"01"}

The compiled worker (dist/evm/EvmoleInterfaceWorker.js) run from the issue gives the same statuses, with offset 1 and length 7 kept for all three.

Remaining proof gaps: synthetic fixtures only; no real contract artifact with repeated byte keys was tried.

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/evm/*.test.ts src/domain/evm/*.test.ts (5 files): 53 passed (the duplicate byte key case fails on the base revision)
npm run test:focused -- tests/boundary/mcp/evmInterfaceMcp.test.ts tests/boundary/evm/solidityMetadataWorker.test.ts: 19 passed
npm run check:fast: typecheck and lint passed
CLI and worker comparison above, built with npm run build:cached on base and on this branch

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: Linux x64, Node 24.18.0; no provider involved.
Package or release metadata impact: none.
Security, privacy, process, or containment review: none.

Author readiness

Human owner: @Sriram-PR

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/4024 **Author:** [@Sriram-PR](https://github.com/Sriram-PR) **Created:** 10/15/2026 **Status:** ✅ Merged **Merged:** 10/21/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/evm-metadata-repeated-byte-keys` --- ### 📝 Commits (1) - [`8787a02`](https://github.com/morluto/rea/commit/8787a0251fc398e829ba3cbb61cb2b63ad3125fa) fix(evm): reject repeated byte-string keys in Solidity metadata ### 📊 Changes **2 files changed** (+30 additions, -2 deletions) <details> <summary>View changed files</summary> 📝 `src/evm/SolidityMetadata.test.ts` (+15 -0) 📝 `src/evm/SolidityMetadata.ts` (+15 -2) </details> ### 📄 Description ## Summary `bytecode.metadata` reported a terminal CBOR root map with the same byte-string key twice as `decoded`, with both entries and no diagnostic, while the same map with a repeated text key is `not-decoded`. `SolidityMetadata.ts` relies on cborg's `rejectDuplicateMapKeys`, but with `useMaps` that check compares Map keys by identity, and each byte-string key decodes to its own `Uint8Array`, so two equal byte keys never match. Fixes #3991 ## Problem and expected behavior Smallest trigger: the bytecode `00a24101014101020007` (STOP, the 7-byte map `{h'01': 1, h'01': 2}`, length `0007`). `inspect-evm-interface <file> hex` returned metadata `status: "decoded"`, two entries with key `bytes 01`, `diagnostic: null`. Expected (docs/evm-bytecode.md: malformed candidates report `not-decoded` with a diagnostic): `not-decoded`, offset 1 and length 7 kept, no entries, no compiler version, as for repeated text keys. ## Change and scope - `src/evm/SolidityMetadata.ts`: while collecting root map entries, compare each scalar key by kind and value and throw on a repeat. The existing catch turns that into `not-decoded` with the message as the diagnostic (`CBOR decode error: found repeat map key {"kind":"bytes","value":"01"}`), keeping the offset and length. - Keys that are nested containers (`uninterpreted`) are left out of the comparison: two different nested maps both project to `{ kind: "uninterpreted", container: "map" }`, so they are not comparable from the projection. Their exact bytes stay addressable as before. - Text, integer, boolean and null repeats were already rejected by cborg and still are, with the same message. - Not included: duplicate keys inside nested containers, which this profile leaves uninterpreted. Contribution scope: ordinary fix within existing boundaries; no scope decision needed. ## Contract and boundary impact - Semantic owner and earliest changed stage: `inspectSolidityMetadata` in `src/evm/SolidityMetadata.ts`, when the root map's entries are projected. - Contribution scope fit and linked maintainer decision: not applicable (bug fix, #3991). - CLI and MCP/tool-catalog contract: no schema change. `inspect-evm-interface` / `inspect_evm_interface` report `not-decoded` with a diagnostic for a root map with repeated byte-string keys. - Provider, bridge, target-format, or platform compatibility: none; the EVMole engine and its `raw_result` are untouched. - Evidence, artifact, provenance, or reconstruction contract: `bytecode.metadata` only; the original bytecode and recorded range are unchanged. - Target selection, authority, upstream behavior, and cleanup details: not applicable. - Generated metadata, package, or installation impact: none. Expected effects (check known effects; use documentation-only on its own). Unchecked items do not establish absence; leave uncertainties visible. - [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/evm/SolidityMetadata.test.ts` adds `duplicate byte key` (`a2410101410102`) to the existing malformed-candidate table, and a control test that distinct byte keys still decode and that two distinct nested-container keys still give `partial`. - Base reproduction: on the base revision the `duplicate byte key` case fails (`expected { ... } to match object { status: 'not-decoded', ... }`); the control test passes on both. - User-visible CLI output (`inspect-evm-interface <file> hex --format json`, `bytecode.metadata` status / entry count / diagnostic, Linux x64): ``` bytecode base PR 00a24101014102020007 decoded, 2, null decoded, 2, null 00a26178016178020007 not-decoded, 0, repeat "x" not-decoded, 0, repeat "x" 00a24101014101020007 decoded, 2, null not-decoded, 0, CBOR decode error: found repeat map key {"kind":"bytes","value":"01"} ``` The compiled worker (`dist/evm/EvmoleInterfaceWorker.js`) run from the issue gives the same statuses, with offset 1 and length 7 kept for all three. - Remaining proof gaps: synthetic fixtures only; no real contract artifact with repeated byte keys was tried. 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/evm/*.test.ts src/domain/evm/*.test.ts` (5 files): 53 passed (the `duplicate byte key` case fails on the base revision) - `npm run test:focused -- tests/boundary/mcp/evmInterfaceMcp.test.ts tests/boundary/evm/solidityMetadataWorker.test.ts`: 19 passed - `npm run check:fast`: typecheck and lint passed - CLI and worker comparison above, built with `npm run build:cached` on base and on this branch ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: Linux x64, Node 24.18.0; no provider involved. - Package or release metadata impact: none. - Security, privacy, process, or containment review: none. ## Author readiness Human owner: @Sriram-PR - [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:11:04 +00:00