Skip to content

transformERC20 clear-signs up to 860 bytes of transformations[] that the decoder never reads #473

Description

@BitHighlander

Split out of #444, which was scoped to the streamed tail. This is the in-buffer half, and it currently has no owning issue — I previously attributed it to #414, which is wrong: #414 is about the unread ABI offset word (arguments placed elsewhere so fixed positions show plausible-but-wrong values). That is misdirection, and it is fixed. This is non-disclosure, and it is not.

The defect

ethereum_contractHandled() gates on data_total != msg->data_initial_chunk.size (lib/firmware/ethereum_contracts.c:63). That proves the calldata was fully received. It does not prove any of it was displayed.

transformERC20(address,address,uint256,uint256,(uint32,bytes)[]) has a 164-byte ABI head — selector(4) + inputToken(32) + outputToken(32) + inputTokenAmount(32) + minOutputAmount(32) + offset-to-transformations[](32). zx_confirmZxTransERC20() reads only inside that head (zxtransERC20.c, the four reads at +4+12, +4+32+12, +4+2*32, +4+3*32).

So for any payload that fits one 1024-byte chunk:

data_total == data_initial_chunk.size   ->  gate passes
zx_isZxTransformERC20()                 ->  claims the transaction
displayed                               ->  bytes 0..163
signed, never shown                     ->  bytes 164..data_total   (up to 860)

The transformations[] body selects transformers and carries their payloads. None of it reaches the screen.

Why it is not already covered

The bounded-outcome argument is what makes clear-signing transformERC20 defensible at all, and it depends on the 0x Exchange Proxy enforcing the final balance check. The Proxy is upgradeable, so that is an assumption this firmware cannot verify — which is the reason this residual is worth tracking rather than accepting silently.

Options

  1. Paginate the transformations[] body through confirm_bytes() so the tail is disclosed. Costs screens and needs a pager for streamed calldata (see EVM: contract-call data display shows only the first chunk of what is signed #400).
  2. Decode transformations[] and name the transformers, so the screen describes the route rather than hiding it. Most work, best outcome.
  3. Accept it in writing, on the record that the bound is contract-enforced by an upgradeable third party, and that the four displayed values are correct.

Not a blocker for 7.14.2 — it is unchanged from the base, neither introduced nor worsened by this release. Filed so it does not disappear when #444 and #414 close.

Metadata

Metadata

Assignees

No one assigned

    Labels

    evmEthereum / EVM signingsecuritySecurity-relevant defect or hardening

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions