Skip to content

fix(vm): resolve function selectors with leading zero bytes - #708

Open
Jon-Becker wants to merge 1 commit into
mainfrom
jon-becker/fix-polymarket-decomp
Open

fix(vm): resolve function selectors with leading zero bytes#708
Jon-Becker wants to merge 1 commit into
mainfrom
jon-becker/fix-polymarket-decomp

Conversation

@Jon-Becker

Copy link
Copy Markdown
Owner

What changed? Why?

Observed failure. While decompiling an unverified Polygon contract (a
participant-linked, bot-like contract adjacent to Polymarket, selected from
PolygonScan; heimdall-cli v0.9.2 on ARM64), any externally-dispatched function
whose 4-byte selector begins with a zero byte (e.g. 0x00aabbcc) was silently
dropped from the output. Those selectors never appeared in the recovered ABI or
Solidity, so the contract's routing/dispatch logic was incomplete and could not
be trusted.

Root cause. In resolve_entry_point (crates/vm/src/ext/selectors.rs), the
dispatcher's PUSH4 constant is solidified through encode_hex_reduced, which
strips leading zero bytes (0x00aabbcc0xaabbcc). The matcher, however,
compared jump_condition.contains(selector) against the un-reduced selector
string (00aabbcc). The substring never matched, the entry point resolved to
0, and find_function_selectors skipped the selector entirely. Only selectors
without a leading zero byte were affected — which is why the bug was intermittent
and easy to miss.

Fix. Compare against the same reduced form the solidifier emits
(encode_hex_reduced(U256::from_be_slice(&selector_bytes))). Selectors with a
leading zero byte now resolve; all other selectors are byte-for-byte unaffected.

Notes to reviewers

  • crates/vm/src/ext/selectors.rsresolve_entry_point: match the reduced
    selector form; added encode_hex_reduced / U256 imports.
  • crates/core/tests/test_decompile.rs — regression test.

How has it been tested?

  • Added test_decompile_leading_zero_byte_selector: a minimal synthetic
    dispatcher routing 0x00aabbcc (leading zero byte, previously dropped) and
    0x11223344 (control). It fails before the fix (only 11223344 recovered) and
    passes after (both recovered). Verified the same before/after behavior directly
    via the CLI on the fixture bytecode.
  • cargo test -p heimdall-vm: 153 + 34 tests pass.
  • Bytecode-based decompile integration tests pass; cargo +nightly fmt --check
    and cargo clippy -p heimdall-vm are clean. RPC/address-based tests were not
    run (no RPC_URL in this environment).

Limitations

  • The regression uses a minimal synthetic fixture that reproduces the exact
    failing dispatcher pattern (leading-zero-byte selector); the original
    unverified contract is not committed, and no PolygonScan/RPC dependency is
    added. This change only restores discovery of these selectors — it does not
    claim to recover the contract's higher-level business logic.

Function selectors whose first byte is `0x00` (e.g. `0x00aabbcc`) were
silently dropped from decompilation. The dispatcher's `PUSH4` constant is
solidified via `encode_hex_reduced`, which strips leading zero bytes
(`0xaabbcc`), but `resolve_entry_point` matched `jump_condition.contains`
against the un-reduced selector string (`00aabbcc`). The comparison never
matched, the entry point resolved to 0, and the function was skipped.

Match against the same reduced form the solidifier produces so these
selectors resolve correctly, while all other selectors are unaffected.

Add a regression test with a minimal two-selector dispatcher
(`0x00aabbcc` + `0x11223344`) asserting both functions are recovered.
@github-actions

Copy link
Copy Markdown
Contributor

❌ Eval Report for 6ca7ea8

Test Case CFG Decompilation
TransientStorage 100 18
NestedMappings 100 40
NestedMapping 100 12
Mapping 100 88
SimpleLoop 85 10
WhileLoop 68 10
SimpleStorage 100 93
WETH9 100 68
NestedLoop 65 10
Events 100 68
Average 91 41
⚠️ 8 eval(s) scoring <70%

TransientStorage (CFG: 100, Decompilation: 18)

Decompilation

{
  "score": 18,
  "summary": "The decompiler failed to reconstruct the transient-storage (EIP-1153 TLOAD/TSTORE) based contract. Five of the six functions were collapsed into meaningless constant declarations that discard their logic entirely, and the one function actually decompiled has the wrong mutability. Fundamental program behavior is not preserved.",
  "differences": [
    "incrementCounter is emitted as an empty `bytes public constant` instead of a function; the transient counter read, +1 arithmetic, and write-back are entirely lost.",
    "lock is emitted as an empty constant instead of a function that writes locked = true to transient storage; state change is lost.",
    "unlock is emitted as an empty constant instead of a function that writes locked = false to transient storage; state change is lost.",
    "getCounter is emitted as `uint256 public constant getCounter = 1` instead of a view function returning the transient counter value; the storage read and return of the actual value are lost.",
    "isLocked is emitted as a constant `bool` instead of a view function returning the transient `locked` flag; the storage read is lost.",
    "setTempOwner is marked `public pure` although it performs a transient storage write (TSTORE), which is incorrect mutability; it also introduces a spurious require(arg0 == address(arg0)) check not present in the source.",
    "All transient-storage semantics (TLOAD/TSTORE per EIP-1153) are only faintly represented for one slot; the remaining transient variables (counter, locked) have no read/write representation at all."
  ]
}

NestedMappings (CFG: 100, Decompilation: 40)

Decompilation

{
  "score": 40,
  "summary": "The decompilation captures a storage write in approve but flattens the nested mapping (dropping the msg.sender key dimension), and both view getters (allowance and allowances) lose their entire read-and-return logic, appearing as empty stubs that only validate the address argument.",
  "differences": [
    "The nested mapping structure (mapping(address => mapping(address => uint256))) is lost: approve writes to a single-level mapping keyed only by the spender (arg0), and the msg.sender key of the outer mapping is entirely absent from the slot computation.",
    "The allowance getter (selector 0xdd62ed3e) does not read from storage or return the stored value; it only contains an address validation require and no return statement, so the original 'return allowances[owner][spender]' is missing.",
    "The allowances public auto-getter (selector 0x55b6ed5c) likewise omits the storage read and return of the uint256 value, reduced to an address validation stub.",
    "Function mutability is misrepresented: the view getters are marked 'pure' (with no storage read) and approve is marked 'payable' instead of a non-payable state-mutating function."
  ]
}

NestedMapping (CFG: 100, Decompilation: 12)

Decompilation

{
  "score": 12,
  "summary": "The decompilation fails to capture the fundamental behavior of the contract. Every original function performs a nested-mapping storage write (setAllowance, setGrid, setDeepNested) or read (getAllowance / auto-generated getters), but the decompiled functions contain none of these SSTORE/SLOAD operations. Instead, each function is reduced to a meaningless tautological check (e.g. require(arg0 == arg0)) and incorrectly marked `pure`. Only the approximate function count and argument types are recovered; the actual logic, state changes, storage slot computations, and return values are entirely absent.",
  "differences": [
    "No storage writes are captured: setAllowance, setGrid, and setDeepNested all write to nested mappings, but the decompiled equivalents perform no SSTORE and only contain no-op require checks.",
    "Storage reads are missing: getAllowance and the auto-generated mapping getters return values loaded from nested mapping slots, but no decompiled function performs SLOAD or returns any value.",
    "Nested mapping slot computation (keccak-based hashing of keys with the mapping base slot) is entirely absent from all functions.",
    "Mutability is wrong: state-changing setters and view getters are all rendered as `pure`, which does not match the original storage-accessing behavior.",
    "Return values are lost: getAllowance and the getters return uint256/bool, but the decompiled functions declare no return type and return nothing.",
    "Function logic is replaced by tautological guards (require(arg0 == arg0), require(arg0 == address(arg0))) that correspond to nothing in the source."
  ]
}

SimpleLoop (CFG: 85, Decompilation: 10)

Decompilation

{
  "score": 10,
  "summary": "The decompilation completely fails to capture the core behavior of the loop function. The original iterates `loops` times, incrementing the storage variable `number` on each pass. The decompiled output contains no loop, no storage write, and marks the function as `view` when it is actually state-mutating. The body is a set of nonsensical require statements that bear no relation to the increment logic or control flow.",
  "differences": [
    "The for-loop control flow (iterating from 0 to `loops`) is entirely missing from the decompiled output",
    "The storage write `number++` is not represented; there is no SSTORE/state change in the decompiled body",
    "Function mutability is wrong: decompiled marks `loop` as `view`, but the original mutates the `number` storage variable",
    "The increment arithmetic on `number` is absent, replaced by meaningless require expressions (`require(arg0 == arg0)`, `require(!0 < arg0)`, and a require on `number - 0xfff...ff`)",
    "No functional equivalent of the loop bound comparison `i < loops` driving repeated execution is preserved"
  ]
}

WhileLoop (CFG: 68, Decompilation: 10)

Decompilation

{
  "score": 10,
  "summary": "The decompilation fails to capture the fundamental behavior of the loop function. The original iterates a while loop that increments the storage variable `number` by 1 on each iteration. The decompiled output contains no loop, no storage write to `number`, and incorrectly marks the function as `view` (non-state-changing). The require statements produced are nonsensical (e.g. `require(arg0 == arg0)`) and do not represent the original control flow or state mutation.",
  "differences": [
    "The while loop control flow is entirely missing — no iteration structure is present",
    "The storage write `number = number + 1` is not captured; there is no SSTORE to the `number` slot",
    "Function is incorrectly marked `view`, indicating no state change, whereas the original mutates storage",
    "The loop counter increment (i = i + 1) and loop condition (i < loops) are not represented",
    "Generated require statements (require(arg0 == arg0), require(!0 < arg0), require(!number > (number + 0x01))) are meaningless and do not correspond to any original logic"
  ]
}

CFG

{
  "score": 68,
  "summary": "The CFG captures both function entry points (loop and the implicit number getter), the dispatcher, all revert paths, and the while-loop condition test with its two-way branch (enter body vs. exit loop). However, the loop's defining back-edge is missing: the body node only branches to the overflow-panic revert, and the normal (no-overflow) continuation that stores `number`, increments `i`, and jumps back to the condition check is not represented as a node/edge. As a result the graph shows the loop as a linear condition->partial-body->revert / condition->exit shape rather than a true cycle.",
  "missing_paths": [
    "Loop back-edge: the iteration path from the loop body back to the condition check at 0x77 is not present, so the CFG does not represent that the while loop repeats.",
    "Loop body continuation: the normal path after 'number = number + 1' (jump target 0x0193 that stores number, computes 'i = i + 1', and re-enters the loop) is absent; the body node (8) only has an edge to the panic-revert node (9)."
  ],
  "extra_paths": [
    "CALLVALUE non-zero revert at function entry (0 -> 1) - compiler-added payable check.",
    "CALLDATASIZE < 4 revert (2 -> 14) - compiler-added selector-length check.",
    "No-matching-selector fallback revert (12 -> 14).",
    "Calldata decode SLT/length checks reverting (4 -> 11, 5 -> 6) - compiler-added ABI decoding validation.",
    "Arithmetic overflow Panic(0x11) revert (8 -> 9) - compiler-added checked-arithmetic guard for number + 1."
  ],
  "observations": [
    "The while-condition conditional branch is correctly captured: node 7 (0x77 JUMPDEST, LT/ISZERO) forks to the body (node 8) and to the loop exit (node 10 -> STOP).",
    "Both external entry points and their exits are present: loop() (selector 0x0b7d796e) via node 4 and the public number() getter (selector 0x8381f58a) via node 12 -> 13 with a RETURN.",
    "All require/revert-style termination points from the compiled contract are represented, so error paths are complete even though the loop iteration cycle is not.",
    "The single most important structural feature of this source - that the loop iterates - is not visible because no edge returns to the loop head, which is why the score is held below the 'minor paths missing' band."
  ]
}

WETH9 (CFG: 100, Decompilation: 68)

Decompilation

{
  "score": 68,
  "summary": "Core value-transfer logic, ETH deposit/withdraw, totalSupply, and all four events are preserved, but the ERC20 allowance mechanics are significantly garbled: approve writes to the wrong storage mapping, the nested allowance key structure is lost, and the allowance check/decrement in transfer/transferFrom is incorrect. Balance operations and event emissions are otherwise faithful.",
  "differences": [
    "approve() writes the allowance value into storage_map_c (the balanceOf mapping) instead of the allowance mapping, and drops the nested [msg.sender][guy] two-key indexing, so approvals corrupt/misplace state.",
    "The allowance getter reads only a single key (arg1) from storage_map_d, ignoring arg0; the nested-mapping slot computation for allowance[src][spender] is not represented.",
    "In transfer/transferFrom the original 'allowance[src][msg.sender] != uint(-1)' check is rendered as 'require(storage_map_c[...] == 0xff..ff)' — it reads from the balance mapping, uses the wrong (inverted, ==) comparison, and the subsequent 'allowance -= wad' decrement is missing entirely, so the allowance-consumption path is not preserved.",
    "The uint8 constant decimals = 18 is represented as a 'bool public decimals' with no value, losing the mutability/type and constant value.",
    "transfer/transferFrom contain large duplicated, unreachable code blocks after the first return statement, obscuring the actual single control-flow path."
  ]
}

NestedLoop (CFG: 65, Decompilation: 10)

Decompilation

{
  "score": 10,
  "summary": "The decompilation fails to capture the fundamental behavior of the contract. The original performs a nested loop that increments the storage variable `number` by 1 on each inner iteration (loops*loops times). The decompiled output contains no loop structure, no storage write, and incorrectly marks the function as `view`. It reduces to a series of degenerate require() checks that do not represent the arithmetic, control flow, or state change of the original.",
  "differences": [
    "Nested for-loops (outer and inner iteration up to `loops`) are entirely absent from the decompiled output",
    "The storage write `number += 1` is not preserved; the decompiled `require(!number > (number + 0x01))` reads state but never persists any change",
    "Function is marked `public view`, but the original mutates state (`number` is written), so mutability behavior is incorrect",
    "The accumulation semantics (incrementing `number` by loops*loops) are completely lost",
    "The comparison conditions are rendered as require() reverts (e.g. `require(!0 < arg0)`) rather than loop-continuation branches, misrepresenting the control flow"
  ]
}

CFG

{
  "score": 65,
  "summary": "The CFG correctly captures the function dispatch, both function entry/exit points, all top-level revert paths, and the entry+body decision nodes for both the outer and inner for-loops. However, the loops' iterative structure is incompletely represented: the inner loop's exit branch is missing, the loop back-edges (i++/j++ returning to the condition checks) are absent, and the increment-side of the loop body is not shown. Main control flow is intact but the looping semantics are only partially captured.",
  "missing_paths": [
    "Inner loop condition false-branch / exit path: node 8 (0x84 condition 'j < loops') only branches to the body (node 9) via fall-through; its conditional jump target 0xb1 (inner loop exit) is not represented as a node/edge.",
    "Inner loop back-edge: after 'number += 1' and j++ the control returns to the inner loop condition (0x84); this iteration edge is not present (graph is acyclic).",
    "Outer loop back-edge: after the inner loop completes and i++ executes, control returns to the outer condition (0x77); this iteration edge is missing.",
    "Loop body success continuation: node 9 only shows the arithmetic-panic revert branch (node 10); the non-overflow continuation that performs the SSTORE and increment (target 0x01ac) is not represented."
  ],
  "extra_paths": [
    "0->1: CALLVALUE non-zero revert (compiler-added payable guard)",
    "4->12 and 5->6: calldata SLT/decode validation reverts (compiler-added ABI decoding checks)",
    "9->10: PUSH32 0x4e487b71 Panic(0x11) arithmetic overflow revert (compiler-added checked-arithmetic guard)",
    "2->15 and 13->15: calldatasize < 4 / unknown-selector fallback revert (compiler dispatch logic)"
  ],
  "observations": [
    "Function dispatch is well captured: node 3 tests selector 0x0b7d796e (loop) and node 13 tests 0x8381f58a (number getter), each with a match branch and a continue/revert branch.",
    "The outer for-loop is the most complete structure: condition node (7 at 0x77), body-entry (8), and exit-to-STOP (11 at 0xbf) are all present.",
    "The extracted graph is entirely acyclic (a DAG) with no back-edges, so the repeated-iteration nature of both nested loops is not visually captured, though each loop's condition check appears as a distinct decision node.",
    "The auto-generated public getter for 'number' (node 14: SLOAD, MSTORE, RETURN) is fully represented with its entry and return."
  ]
}

Events (CFG: 100, Decompilation: 68)

Decompilation

{
  "score": 68,
  "summary": "The decompilation correctly preserves 5 of the 7 event-emitting functions (emitDeposit, emitWithdrawal, emitLog, emitLogBytes, and emitMultiple with all three of its emissions), including string/bytes payloads and correct event topic structure. However, the two single-emit functions emitTransfer and emitApproval — whose entire purpose is emitting one event — fail completely: neither emits its event, both are reduced to a single address argument, and the Approval event is not even represented in the output.",
  "differences": [
    "emitTransfer (selector 0x5687f2b8) does not emit the Transfer event at all; its body contains only an argument-validation require, and it is decompiled with a single address parameter instead of (address, address, uint256), so the entire function behavior (emit Transfer(from, to, value)) is lost.",
    "emitApproval (selector 0x23de6651) does not emit the Approval event at all; the Approval event is absent from the output entirely, the function body is only a require, and it is decompiled with a single address parameter instead of (address, address, uint256).",
    "All functions are marked pure, but emitting events is not a pure operation; the original functions are non-view/non-pure state-observing external functions (minor mutability representation issue)."
  ]
}

@github-actions

Copy link
Copy Markdown
Contributor

✅ Coverage Report for 6ca7ea8

Metric Value
Base branch 66.39%
PR branch 66.40%
Diff +0.01%

@github-actions

Copy link
Copy Markdown
Contributor

Benchmark for 6ca7ea8

Click to view benchmark
Test Base PR %
heimdall_cfg/complex 14.2±0.92ms 14.2±0.94ms 0.00%
heimdall_cfg/simple 1375.9±51.24µs 1377.8±25.35µs +0.14%
heimdall_decoder/seaport 58.0±4.33µs 58.8±4.57µs +1.38%
heimdall_decoder/transfer 3.9±0.49µs 4.1±0.40µs +5.13%
heimdall_decoder/uniswap 15.9±1.06µs 16.0±0.94µs +0.63%
heimdall_decompiler/abi_complex 58.8±0.87ms 54.5±0.57ms -7.31%
heimdall_decompiler/abi_simple 1457.1±30.44µs 1450.3±65.28µs -0.47%
heimdall_decompiler/sol_complex 82.8±1.74ms 79.0±1.59ms -4.59%
heimdall_decompiler/sol_simple 2.2±0.02ms 2.2±0.08ms 0.00%
heimdall_decompiler/yul_complex 63.9±1.02ms 61.8±1.16ms -3.29%
heimdall_decompiler/yul_simple 1620.2±101.06µs 1592.1±46.35µs -1.73%
heimdall_disassembler/complex 1301.6±78.38µs 1314.5±90.97µs +0.99%
heimdall_disassembler/simple 64.6±6.58µs 64.5±5.70µs -0.15%
heimdall_vm/erc20_transfer 264.9±14.28µs 271.2±18.85µs +2.38%
heimdall_vm/fib 863.8±8.17µs 861.4±6.99µs -0.28%
heimdall_vm/ten_thousand_hashes 750.8±19.55ms 1347.0±1779.00ms +79.41%

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant