diff --git a/docs/release/7.14.2.md b/docs/release/7.14.2.md index 6f30acd7f..2f866b12e 100644 --- a/docs/release/7.14.2.md +++ b/docs/release/7.14.2.md @@ -166,6 +166,7 @@ landed, so the table below is the current state, not what #440 originally set. | TRON `SignTx` | AdvancedMode | `raw_data` still cannot be parsed on this line (#405) | | EVM `SignTx` with **no `chain_id`** | **REFUSED** | produced a pre-EIP-155 signature replayable on every EVM chain, and the confirm screen named no network (#445) | | 0x `transformERC20` over 1024 bytes | **REFUSED** by default, AdvancedMode to sign | was clear-signed with an undisclosed `transformations[]` tail; now falls through to the raw-hex path (#444) | +| 0x swaps on **Base / Arbitrum / Avalanche** | **REFUSED** by default, AdvancedMode to sign | the token table's `uint8_t` chain id truncates on these three, so the decoder rendered "Unknown token value" for both amounts — no bound, no disclosure (#455) | **On the 0x change — this affects real swaps, not an edge case.** `EthereumSignTx.data_initial_chunk` holds 1024 bytes and 0x aggregates across liquidity diff --git a/lib/firmware/ethereum_contracts.c b/lib/firmware/ethereum_contracts.c index 02cdda856..304a4b6d0 100644 --- a/lib/firmware/ethereum_contracts.c +++ b/lib/firmware/ethereum_contracts.c @@ -35,13 +35,36 @@ bool zx_isExchangeProxyChain(uint32_t chain_id) { for ZXSWAP_ADDRESS would let the 0x decoder narrate an unrelated contract — exactly the confusion the chain scoping exists to prevent. Verified against 0xProject/protocol packages/contract-addresses/addresses.json. */ + /* Base (8453), Arbitrum (42161) and Avalanche (43114) are deliberately absent + even though 0x deploys the same Exchange Proxy there. The decoder can find + the contract on those chains but cannot describe the trade, so allowing + them would produce a screen that asserts understanding it does not have. + + TokenType.chain_id and tokenByChainAddress() are uint8_t + (include/keepkey/firmware/ethereum_tokens.h:42,54), so those three chain + ids truncate to 5, 177 and 106 -- values matching no table entry. The + lookup returns UnknownToken and ethereumFormatAmount() short-circuits to + the literal "Unknown token value" (lib/firmware/ethereum.c:347) for BOTH + operands, so zx_confirmZxTransERC20() renders + + Transform ERC20 + Input Unknown token value + Output Unknown token value + + while the transformations[] body executes. The whole justification for + clear-signing transformERC20 is that the input amount and minimum output + amount shown on screen bound the outcome; on these chains no amount is + shown, so there is no bound and no disclosure -- a blind signature wearing + a decoder's title. + + Denying them here routes those swaps to the generic raw-calldata path, + which is AdvancedMode-gated and shows the bytes. That is the honest answer + until the token table carries a full-width chain id (#455), at which point + these three can be restored with their amounts actually rendering. */ switch (chain_id) { - case 1: /* Ethereum */ - case 56: /* BNB Chain */ - case 137: /* Polygon */ - case 8453: /* Base */ - case 42161: /* Arbitrum */ - case 43114: /* Avalanche */ + case 1: /* Ethereum */ + case 56: /* BNB Chain */ + case 137: /* Polygon */ return true; default: /* Including chain_id 0 / absent, which callers treat as unknown. */ diff --git a/unittests/firmware/ethereum.cpp b/unittests/firmware/ethereum.cpp index 0b09d7199..4755b0770 100644 --- a/unittests/firmware/ethereum.cpp +++ b/unittests/firmware/ethereum.cpp @@ -135,12 +135,28 @@ extern "C" { // (0xdef1abe32c034e558cdd535791643c58a13acc10), so allowing chain 10 for // ZXSWAP_ADDRESS would narrate an unrelated contract. TEST(Ethereum, ZxExchangeProxyChainAllowlist) { - EXPECT_TRUE(zx_isExchangeProxyChain(1)); // Ethereum - EXPECT_TRUE(zx_isExchangeProxyChain(56)); // BNB Chain - EXPECT_TRUE(zx_isExchangeProxyChain(137)); // Polygon - EXPECT_TRUE(zx_isExchangeProxyChain(8453)); // Base - EXPECT_TRUE(zx_isExchangeProxyChain(42161)); // Arbitrum - EXPECT_TRUE(zx_isExchangeProxyChain(43114)); // Avalanche + // Chains where the token table can actually name the tokens, so the input + // and minimum-output amounts that bound the trade are rendered. + EXPECT_TRUE(zx_isExchangeProxyChain(1)); // Ethereum + EXPECT_TRUE(zx_isExchangeProxyChain(56)); // BNB Chain + EXPECT_TRUE(zx_isExchangeProxyChain(137)); // Polygon + + // 0x deploys the same Exchange Proxy on these three, but TokenType.chain_id + // and tokenByChainAddress() are uint8_t, so the ids truncate to values that + // match no table entry: 8453->5, 42161->177, 43114->106. Both operands then + // render "Unknown token value" and the screen shows no bound at all, which + // is a blind signature with a decoder's title. They must fall through to the + // AdvancedMode raw-calldata path until #455 widens the chain id. + EXPECT_FALSE(zx_isExchangeProxyChain(8453)) + << "Base truncates to 5 and renders no amounts"; + EXPECT_FALSE(zx_isExchangeProxyChain(42161)) + << "Arbitrum truncates to 177 and renders no amounts"; + EXPECT_FALSE(zx_isExchangeProxyChain(43114)) + << "Avalanche truncates to 106 and renders no amounts"; + + // The truncated values themselves must not be a way back in. + EXPECT_FALSE(zx_isExchangeProxyChain(177)); + EXPECT_FALSE(zx_isExchangeProxyChain(106)); EXPECT_FALSE(zx_isExchangeProxyChain(10)) << "Optimism uses a different 0x proxy";