Token table chain_id is uint8_t, so Base/Arbitrum/Avalanche cannot be represented at all

[BitHighlander]

Triaged during the 7.14.2 residual pass and deferred out of 7.14.2 as low. Still present at release/7.14.2 head.

Evidence

include/keepkey/firmware/ethereum_tokens.h:42 (uint8_t chain_id;) and :54 (tokenByChainAddress(uint8_t chain_id, ...)), narrowed from the uint32_t at lib/firmware/ethereum.c:71 at call sites :116, :733 and :738

Assessment

Still present - neither ethereum_tokens.h nor ethereum_tokens.c received any commit on this branch. TokenType::chain_id is uint8_t (ethereum_tokens.h:42), tokenByChainAddress takes uint8_t (:54), and the transaction's chain id is uint32_t (ethereum.c:71) assigned straight from the host's protobuf with no bound (ethereum.c:628). There are now three narrowing call sites, one more than the finding recorded: ethereum_getStandardERC20Coin (ethereum.c:116) plus the transfer and approve lookups (ethereum.c:733, :738). The reachable path is concrete: chain_id = 257 with to set to a mainnet token address matches that mainnet entry (257 & 0xFF == 1) at ethereum_tokens.c:48, so layoutEthereumConfirmTx renders the mainnet ticker and decimals for a transaction that is EIP-155-bound to chain 257.

I re-rated from the code and low holds, for two reasons I verified rather than assumed. First, the dangerous half of the original impact is gone: the UnknownToken sentinel no longer skips the blind-sign gate - ethereum.c:780 now reads if ((token == NULL || token == UnknownToken) && data_total > 0 && data_needs_confirm) and routes into the AdvancedMode block plus raw-data disclosure (GH #433), so a token on a chain >= 256 that fails to resolve fails CLOSED to raw-data review rather than to silent narration. Second, the false-match half requires the same contract address to carry different meaning on two chains congruent mod 256, and I enumerated the table: every chain in deps/python-keepkey/keepkeylib/eth/ethereum_networks.json is < 256 (max 137) and the uniswap generator hardcodes chain_id = 1, so no table entry is itself truncated at build time and no real chain of interest collides with a populated net (Base 8453 -> 5/Goerli, Linea 59144 -> 8/UBIQ, neither populated). For the record it should also be stated that the confirm screen never names the network at all (layoutEthereumConfirmTx, ethereum.c:407-462, renders only amount and address), so chain misrepresentation on the ERC-20 screen is pre-existing and not created by this narrowing. File it: the fix (uint32_t chain_id in ethereum_tokens.{c,h}, already written off-line as d54e8fc) is correct but changes the generated table format, which is not a change to make late in a tag for a low finding.

---

Deferred because 7.14.2 is a minimal security release; this did not clear the bar for a late change. Filed so it is not lost.

[BitHighlander]

This is worse than a display-quality bug, and it now has a near-term mitigation.

Why it is a security defect, not cosmetics

The uint8_t chain id doesn't just mislabel a token — it removes the entire justification for clear-signing 0x calls.

zx_confirmZxTransERC20() shows four values and hides the transformations[] body. That is only defensible because the input amount and minimum output amount bound the outcome. On the three chains where the id truncates:

chain	id	as uint8_t
Base	8453	5
Arbitrum	42161	177
Avalanche	43114	106

…the lookup misses, returns UnknownToken, and ethereumFormatAmount() short-circuits to the literal "Unknown token value" (lib/firmware/ethereum.c:347) for both operands. The screen reads:

Transform ERC20
Input Unknown token value
Output Unknown token value

No amounts. No bound. The transformations[] body executes regardless. That is a blind signature presented under a decoder's title — the exact thing this release exists to stop.

zxswap.c:116-117,127,129 is the same pattern, so 0x zxSwap degrades identically on those chains.

Near-term mitigation: #470

Drops Base, Arbitrum and Avalanche from zx_isExchangeProxyChain(), so both 0x decoders fall through to the raw-calldata path there — AdvancedMode-gated, bytes on screen. The owner can still sign; the device stops claiming it understood. Ethereum, BNB Chain and Polygon keep decoded signing, where the amounts render.

What this issue still owns

Widening the representation: TokenType.chain_id (include/keepkey/firmware/ethereum_tokens.h:42) and the tokenByChainAddress() / tokenByTicker() signatures at :54,67. Once the table carries a full-width chain id, the three excluded chains can be restored to the allowlist with their amounts actually rendering — the test in #470 states that condition explicitly so the restoration is deliberate rather than accidental.

Worth checking during that work whether any other consumer of the token table is silently truncating the same way.

[BitHighlander]

Correcting my earlier comment on this issue. I said the uint8_t truncation was why Base/Arbitrum/Avalanche render "Unknown token value". It isn't, and the distinction matters for what this issue is for.

Measured

Generated tables, both files, 1944 entries:

chain	token entries
Ethereum (1)	1924
BSC (56)	3
Polygon (137)	3
Base (8453)	0
Arbitrum (42161)	0
Avalanche (43114)	0

Widening the type would not restore a single amount. There are no tokens for those three chains, so the lookup misses on the data, not on the representation. Both defects independently produce UnknownToken.

It also shows the problem is not confined to those three: BSC and Polygon carry three tokens each, so any 0x swap there involving a fourth token is equally unnameable. Even chain 1 fails for tokens outside its 1924.

What that changes

The signing-safety fix is #472 — clear-sign only when both token lookups resolve — which is chain- and data-agnostic and needs neither this issue nor an allowlist.

This issue remains open and is still a prerequisite, just for a different reason. It is not "fix this to restore the amounts". It is: uint8_t cannot represent 8453, 42161 or 43114 at all, so there is no correct way to add token entries for those chains until the representation is widened. Adding tables is blocked on this; the blank screens are not.

On the representation, since it is now scoped work

uint16_t is not sufficient — Blast (81457), Scroll (534352) and Sepolia (11155111) exceed it, and ethereum.c already accepts up to MAX_CHAIN_ID = 2147483630. Picking 16 bits would bake in the same class of assumption again.

Widening the field to uint32_t costs ~7.8KB of flash: TokenType is {const char*, const char*, uint8_t, uint8_t} = 10 bytes padded to 12 on ARM, and {ptr, ptr, uint32_t, uint8_t} = 13 padded to 16, across 1944 entries. Worth weighing against the ROM budget.

A cheaper shape, if the growth is unwelcome: keep a uint8_t chain_index in TokenType pointing into a small table of full uint32_t chain ids. The struct stays 12 bytes, arbitrary 32-bit chain ids become representable, and the 1924 chain-1 entries stop each carrying a field they all share. That also scales better if the token set keeps growing.

[BitHighlander]

Resolved on alpha by accd11d. TokenType.chain_id and token lookup APIs use uint32_t, eliminating modulo-256 chain collisions.