From 2a8f8a937b81f55c6fc8be2b99467b2dcc6fc3a5 Mon Sep 17 00:00:00 2001 From: highlander Date: Thu, 6 Aug 2026 12:30:21 -0300 Subject: [PATCH] feat(clearsign): describe EVM transactions the device can verify MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Uses the signed-metadata engine on the Ethereum path so a user sees what a contract call actually does instead of a hex blob: - EthereumTxMetadata carries the signed description and EthereumMetadataAck acknowledges it; LoadClearsignSigner installs a trust anchor for the current session only. Metadata that fails verification is dropped and the transaction falls back to raw display — it never degrades to a friendlier screen. - EIP-712 typed data: domain/primary-type handling is corrected, the policy gate decides when a typed hash may be signed at all, and the recursion and length limits are enforced rather than assumed. - EIP-7702 authorization lists are recognised and shown. - ERC-20 and the bundled contract decoders (THORChain router, liquidity and swap proxies) check the selector against the argument shape before claiming to know what a call means. - A factory reset clears loaded signers. - unittests/firmware/eip712.cpp and the extended ethereum.cpp cover the typed data limits, selector mismatches and the metadata accept/reject paths. --- include/keepkey/firmware/eip712.h | 14 +- include/keepkey/firmware/ethereum.h | 3 + .../firmware/ethereum_contracts/thortx.h | 28 +- .../firmware/ethereum_contracts/zxliquidtx.h | 3 + include/keepkey/firmware/ethereum_tokens.h | 10 +- include/keepkey/firmware/fsm.h | 2 + lib/firmware/eip712.c | 600 +++++++++++------- lib/firmware/ethereum.c | 245 +++++-- lib/firmware/ethereum_contracts.c | 24 +- lib/firmware/ethereum_contracts/saproxy.c | 3 +- lib/firmware/ethereum_contracts/thortx.c | 232 +++++-- lib/firmware/ethereum_contracts/zxappliquid.c | 196 +++--- lib/firmware/ethereum_contracts/zxliquidtx.c | 258 ++++---- lib/firmware/ethereum_contracts/zxswap.c | 20 +- .../ethereum_contracts/zxtransERC20.c | 4 + lib/firmware/ethereum_tokens.c | 5 +- lib/firmware/fsm.c | 1 + lib/firmware/fsm_msg_common.h | 3 + lib/firmware/fsm_msg_ethereum.h | 252 +++++--- lib/firmware/messagemap.def | 4 + unittests/firmware/CMakeLists.txt | 1 + unittests/firmware/eip712.cpp | 67 ++ unittests/firmware/ethereum.cpp | 216 ++++++- 23 files changed, 1578 insertions(+), 613 deletions(-) create mode 100644 unittests/firmware/eip712.cpp diff --git a/include/keepkey/firmware/eip712.h b/include/keepkey/firmware/eip712.h index 686acd6de..165a52fd6 100644 --- a/include/keepkey/firmware/eip712.h +++ b/include/keepkey/firmware/eip712.h @@ -25,9 +25,9 @@ Parser wants to see C strings, not javascript strings: requires all complete json message strings to be enclosed by braces, i.e., { ... } Cannot have entire json string quoted, i.e., "{ ... }" will not - work. Remove all quote escape chars, e.g., {"types": not {\"types\": int - values must be hex. Negative sign indicates negative value, e.g., -5, -8a67 - Note: Do not prefix ints or uints with 0x + work. Remove all quote escape chars, e.g., {"types": not {\"types\": + Integer values must use canonical base-10 digits. Negative values use a + leading minus sign. Do not prefix ints or uints with 0x. All hex and byte strings must be big-endian Byte strings and address should be prefixed by 0x */ @@ -95,10 +95,16 @@ typedef enum { DOMAIN = 1, MESSAGE } dm; #define JSON_TYPE_T_NOVAL 31 #define ADDR_STRING_NULL 32 #define JSON_TYPE_WNOVAL 33 +#define USER_CANCELLED 34 -#define LAST_ERROR JSON_TYPE_WNOVAL +#define LAST_ERROR USER_CANCELLED int encode(const json_t* jsonTypes, const json_t* jsonVals, const char* typeS, uint8_t* hashRet); +/* Exposed for strict-value regression tests. */ +int encAddress(const char* string, uint8_t* encoded); +int encodeBytes(const char* string, uint8_t* encoded); +int encodeBytesN(const char* typeT, const char* string, uint8_t* encoded); + #endif diff --git a/include/keepkey/firmware/ethereum.h b/include/keepkey/firmware/ethereum.h index 77c8f4775..e9ccbbb91 100644 --- a/include/keepkey/firmware/ethereum.h +++ b/include/keepkey/firmware/ethereum.h @@ -36,6 +36,7 @@ typedef struct _CoinType CoinType; void ethereum_signing_init(EthereumSignTx* msg, const HDNode* node, bool needs_confirm); void ethereum_signing_abort(void); +bool ethereum_signing_isInProgress(void); void ethereum_signing_txack(EthereumTxAck* tx); void format_ethereum_address(const uint8_t* to, char* destination_str, uint32_t destination_str_len); @@ -70,6 +71,8 @@ void bn_from_bytes(const uint8_t* value, size_t value_len, bignum256* val); void ethereum_typed_hash_sign(const EthereumSignTypedHash* msg, const HDNode* node, EthereumTypedDataSignature* resp); +bool ethereum_typed_hash_policy_allows(bool advanced_mode); +bool ethereum_eip712_is_domain_primary_type(const char* primary_type); bool ethereum_path_check(uint32_t address_n_count, const uint32_t* address_n, bool pubkey_export, uint64_t chain); void e712_types_values(Ethereum712TypesValues* msg, diff --git a/include/keepkey/firmware/ethereum_contracts/thortx.h b/include/keepkey/firmware/ethereum_contracts/thortx.h index 8f61c27f3..8f71c7fcc 100644 --- a/include/keepkey/firmware/ethereum_contracts/thortx.h +++ b/include/keepkey/firmware/ethereum_contracts/thortx.h @@ -30,7 +30,31 @@ "\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee\xee" \ "\xee\xee" -#define THOR_ROUTER "42a5ed456650a09dc10ebc6361a7480fdd61f27b" +/* THORChain ETH router (mainnet), current v4.1.1. + * NOTE: THORChain migrates this router periodically (v1 42a5ed.. -> v3 + * 3624525.. -> v4 d37bbe..). A hardcoded pin must be updated on each migration; + * the durable path is the signed-metadata clear-sign protocol (host-signed, + * key-pinned) which needs no firmware update per router change. */ +#define THOR_ROUTER "d37bbe5744d730a1d98d8dc97c42f0ca46ad7146" + +/* THORChain deploys its Router at a DIFFERENT address on every EVM chain, so + * the pin must be chain-scoped (see thor_router_for_chain): a deposit on any + * chain but mainnet can never match THOR_ROUTER and would fall to the + * blind-sign gate. Avalanche C-Chain router, verified live against THORChain + * /inbound_addresses via a Pioneer quote (2026-07). Lowercase, no 0x, to match + * thor_format_to_addr's output. Same migration caveat as THOR_ROUTER. + * ponytail: BSC (chainId 56) and Base (8453) routers also exist on-chain but + * are omitted until verified against a live node — the shipped Pioneer catalog + * lists STALE addresses (its AVAX entry 8f66c4ae.. is already wrong vs the live + * 00dc6100..), and Pioneer currently routes BSC/Base swaps via Relay, not a + * THORChain deposit, so no such tx reaches the device today. Add each here once + * verified live. */ +#define THOR_ROUTER_AVAX "00dc6100103bc402d490aee3f9a5560cbd91f1d4" + +/* Maya Protocol ETH router v4 (mainnet), verified on Etherscan + * (0xe3985e6b61b814f7cdb188766562ba71b446b46d). The prior pin + * d89dce57.. has never held contract code on mainnet. */ +#define MAYA_ROUTER "e3985e6b61b814f7cdb188766562ba71b446b46d" /* deposit(address,address,uint256,string) — legacy selector */ #define THOR_SELECTOR_DEPOSIT "\x1f\xec\xe7\xb4" @@ -43,6 +67,8 @@ typedef struct _EthereumSignTx EthereumSignTx; bool thor_has_deposit_selector(const EthereumSignTx* msg); bool thor_is_expiry_variant(const EthereumSignTx* msg); bool thor_isThorchainTx(const EthereumSignTx* msg); +bool thor_isMayachainTx(const EthereumSignTx* msg); bool thor_confirmThorTx(uint32_t data_total, const EthereumSignTx* msg); +bool thor_confirmMayaTx(uint32_t data_total, const EthereumSignTx* msg); #endif diff --git a/include/keepkey/firmware/ethereum_contracts/zxliquidtx.h b/include/keepkey/firmware/ethereum_contracts/zxliquidtx.h index 6287da2bd..1a4c6046d 100644 --- a/include/keepkey/firmware/ethereum_contracts/zxliquidtx.h +++ b/include/keepkey/firmware/ethereum_contracts/zxliquidtx.h @@ -22,6 +22,7 @@ #include #include +#include #define UNISWAP_ROUTER_ADDRESS \ "\x7a\x25\x0d\x56\x30\xB4\xcF\x53\x97\x39\xdF\x2C\x5d\xAc\xb4\xc6\x59\xF2" \ @@ -31,5 +32,7 @@ typedef struct _EthereumSignTx EthereumSignTx; bool zx_isZxLiquidTx(const EthereumSignTx* msg); bool zx_confirmZxLiquidTx(uint32_t data_total, const EthereumSignTx* msg); +bool zx_formatZxLiquidityPrimaryAmount(const EthereumSignTx* msg, char* out, + size_t out_len); #endif diff --git a/include/keepkey/firmware/ethereum_tokens.h b/include/keepkey/firmware/ethereum_tokens.h index 7e26bd164..0f5c040f8 100644 --- a/include/keepkey/firmware/ethereum_tokens.h +++ b/include/keepkey/firmware/ethereum_tokens.h @@ -25,6 +25,9 @@ #include #include +#if BITCOIN_ONLY +#define TOKENS_COUNT 0 // no ERC-20 tokens in the bitcoin-only image +#else enum { #define X(CHAIN_ID, CONTRACT_ADDR, TICKER, DECIMALS) \ CONCAT(TokenIndex, __COUNTER__), @@ -35,11 +38,12 @@ enum { }; #define TOKENS_COUNT ((int)TokenIndexLast - (int)TokenIndexFirst) +#endif typedef struct _TokenType { const char* const address; const char* const ticker; - uint8_t chain_id; + uint32_t chain_id; uint8_t decimals; } TokenType; @@ -51,7 +55,7 @@ extern const TokenType* UnknownToken; const TokenType* tokenIter(int32_t* ctr); -const TokenType* tokenByChainAddress(uint8_t chain_id, const uint8_t* address); +const TokenType* tokenByChainAddress(uint32_t chain_id, const uint8_t* address); /// Tokens don't have unique tickers, so this might not return the one you're /// looking for :/ @@ -64,7 +68,7 @@ const TokenType* tokenByChainAddress(uint8_t chain_id, const uint8_t* address); /// \param[out] token The found token, assuming it was uniquely determinable. /// \returns true iff the token can be uniquely found in the list of known /// tokens. -bool tokenByTicker(uint8_t chain_id, const char* ticker, +bool tokenByTicker(uint32_t chain_id, const char* ticker, const TokenType** token); void coinFromToken(CoinType* coin, const TokenType* token); diff --git a/include/keepkey/firmware/fsm.h b/include/keepkey/firmware/fsm.h index d0411d2a2..530a414e3 100644 --- a/include/keepkey/firmware/fsm.h +++ b/include/keepkey/firmware/fsm.h @@ -85,6 +85,8 @@ void fsm_msgEthereumSignMessage(EthereumSignMessage* msg); void fsm_msgEthereumVerifyMessage(const EthereumVerifyMessage* msg); void fsm_msgEthereumSignTypedHash(const EthereumSignTypedHash* msg); void fsm_msgEthereum712TypesValues(Ethereum712TypesValues* msg); +void fsm_msgEthereumTxMetadata(const EthereumTxMetadata* msg); +void fsm_msgLoadClearsignSigner(const LoadClearsignSigner* msg); void fsm_msgNanoGetAddress(NanoGetAddress* msg); void fsm_msgNanoSignTx(NanoSignTx* msg); diff --git a/lib/firmware/eip712.c b/lib/firmware/eip712.c index 1fec75f4b..84da58083 100644 --- a/lib/firmware/eip712.c +++ b/lib/firmware/eip712.c @@ -31,11 +31,13 @@ strings and address should be prefixed by 0x */ +#include #include #include #include #include "keepkey/board/confirm_sm.h" #include "keepkey/board/memory.h" +#include "keepkey/firmware/app_confirm.h" #include "keepkey/firmware/eip712.h" #include "keepkey/firmware/ethereum_tokens.h" #include "keepkey/firmware/tiny-json.h" @@ -47,38 +49,117 @@ static dm confirmProp; static const char* nameForValue; +static bool append_type_string(char* dest, const char* value) { + if (!dest || !value) return false; + const size_t used = strnlen(dest, STRBUFSIZE + 1); + const size_t added = strlen(value); + if (used > STRBUFSIZE || added > STRBUFSIZE - used) return false; + memcpy(dest + used, value, added + 1); + return true; +} + +static bool type_array_suffix_is_valid(const char* suffix) { + if (*suffix == '\0') return true; + if (*suffix++ != '[') return false; + while (*suffix >= '0' && *suffix <= '9') suffix++; + return suffix[0] == ']' && suffix[1] == '\0'; +} + +static bool type_matches(const char* type, const char* base) { + const size_t len = strlen(base); + return strncmp(type, base, len) == 0 && + type_array_suffix_is_valid(type + len); +} + +static bool type_is_integer(const char* type, const char* prefix) { + const size_t prefix_len = strlen(prefix); + if (strncmp(type, prefix, prefix_len) != 0) return false; + const char* p = type + prefix_len; + unsigned bits = 0; + bool has_bits = false; + while (*p >= '0' && *p <= '9') { + has_bits = true; + bits = bits * 10 + (unsigned)(*p++ - '0'); + } + if (has_bits && (bits < 8 || bits > 256 || (bits % 8) != 0)) return false; + return type_array_suffix_is_valid(p); +} + +static unsigned integer_type_width(const char* type, const char* prefix) { + const char* p = type + strlen(prefix); + if (*p < '0' || *p > '9') return 256; + unsigned bits = 0; + while (*p >= '0' && *p <= '9') { + bits = bits * 10 + (unsigned)(*p++ - '0'); + } + return bits; +} + +static bool type_is_bytes(const char* type, unsigned* byte_size, + bool* dynamic) { + if (strncmp(type, "bytes", 5) != 0) return false; + const char* p = type + 5; + if (*p == '\0' || *p == '[') { + if (!type_array_suffix_is_valid(p)) return false; + *byte_size = 0; + *dynamic = true; + return true; + } + unsigned size = 0; + bool has_size = false; + while (*p >= '0' && *p <= '9') { + has_size = true; + size = size * 10 + (unsigned)(*p++ - '0'); + } + if (!has_size || size == 0 || size > 32 || !type_array_suffix_is_valid(p)) + return false; + *byte_size = size; + *dynamic = false; + return true; +} + +static int hex_nibble(char c) { + if (c >= '0' && c <= '9') return c - '0'; + if (c >= 'a' && c <= 'f') return c - 'a' + 10; + if (c >= 'A' && c <= 'F') return c - 'A' + 10; + return -1; +} + +static bool hex_string_is_valid(const char* string, size_t expected_bytes, + bool exact_size) { + if (!string || string[0] != '0' || string[1] != 'x') return false; + const size_t chars = strlen(string + 2); + if ((chars & 1) != 0 || (exact_size && chars != 2 * expected_bytes)) + return false; + for (size_t i = 0; i < chars; i++) { + if (hex_nibble(string[i + 2]) < 0) return false; + } + return true; +} + int encodableType(const char* typeStr) { int ctr; - if (0 == strncmp(typeStr, "address", sizeof("address") - 1)) { + if (!typeStr || typeStr[0] == '\0') return NOT_ENCODABLE; + + if (type_matches(typeStr, "address")) { return ADDRESS; } - if (0 == strncmp(typeStr, "string", sizeof("string") - 1)) { + if (type_matches(typeStr, "string")) { return STRING; } - if (0 == strncmp(typeStr, "int", sizeof("int") - 1)) { - // This could be 'int8', 'int16', ..., 'int256' + if (type_is_integer(typeStr, "int")) { return INT; } - if (0 == strncmp(typeStr, "uint", sizeof("uint") - 1)) { - // This could be 'uint8', 'uint16', ..., 'uint256' + if (type_is_integer(typeStr, "uint")) { return UINT; } - if (0 == strncmp(typeStr, "bytes", sizeof("bytes") - 1)) { - // This could be 'bytes', 'bytes1', ..., 'bytes32' - if (0 == strcmp(typeStr, "bytes")) { - return BYTES; - } else { - // parse out the length val - uint8_t byteTypeSize = (uint8_t)(strtol((typeStr + 5), NULL, 10)); - if (byteTypeSize > 32) { - return NOT_ENCODABLE; - } else { - return BYTES_N; - } - } + unsigned byte_size = 0; + bool dynamic = false; + if (type_is_bytes(typeStr, &byte_size, &dynamic)) { + return dynamic ? BYTES : BYTES_N; } - if (0 == strcmp(typeStr, "bool")) { + if (type_matches(typeStr, "bool")) { return BOOL; } @@ -90,10 +171,11 @@ int encodableType(const char* typeStr) { strtok(typeNoArrTok, "["); // eliminate the array tokens if there if (udefList[ctr] != 0) { - if (0 == strncmp(udefList[ctr], typeNoArrTok, - strlen(udefList[ctr]) - strlen(typeNoArrTok))) { + const size_t previous_len = strcspn(udefList[ctr], "["); + const size_t candidate_len = strlen(typeNoArrTok); + if (previous_len == candidate_len && + strncmp(udefList[ctr], typeNoArrTok, candidate_len) == 0) { return PREV_USERDEF; - } else { } } else { @@ -134,8 +216,10 @@ int parseType(const json_t* eip712Types, const char* typeS, char* typeStr) { return JSON_TYPE_S_NAMEERR; } - strncat(typeStr, nameTest, STRBUFSIZE - strlen((const char*)typeStr)); - strncat(typeStr, "(", STRBUFSIZE - strlen((const char*)typeStr)); + if (!append_type_string(typeStr, nameTest) || + !append_type_string(typeStr, "(")) { + return UDEF_NAME_ERROR; + } tarray = json_getChild(jType); while (tarray != 0) { @@ -189,10 +273,12 @@ int parseType(const json_t* eip712Types, const char* typeS, char* typeStr) { if (NULL == pVal) { return JSON_NOPAIRVAL; } - strncat(typeStr, typeType, STRBUFSIZE - strlen((const char*)typeStr)); - strncat(typeStr, " ", STRBUFSIZE - strlen((const char*)typeStr)); - strncat(typeStr, pVal, STRBUFSIZE - strlen((const char*)typeStr)); - strncat(typeStr, ",", STRBUFSIZE - strlen((const char*)typeStr)); + if (!append_type_string(typeStr, typeType) || + !append_type_string(typeStr, " ") || + !append_type_string(typeStr, pVal) || + !append_type_string(typeStr, ",")) { + return UDEF_NAME_ERROR; + } } tarray = json_getSibling(tarray); } @@ -202,32 +288,28 @@ int parseType(const json_t* eip712Types, const char* typeS, char* typeStr) { typeStr[strlen(typeStr) - 1] = ')'; } else { // append paren, there are no parameters - strncat(typeStr, ")", STRBUFSIZE - 1); + if (!append_type_string(typeStr, ")")) return UDEF_NAME_ERROR; } if (strlen(append) > 0) { - strncat(typeStr, append, STRBUFSIZE - strlen((const char*)append)); + if (!append_type_string(typeStr, append)) return UDEF_NAME_ERROR; } return SUCCESS; } int encAddress(const char* string, uint8_t* encoded) { - unsigned ctr; - char byteStrBuf[3] = {0}; - - if (string == NULL) { + if (!string) { return ADDR_STRING_NULL; } - if (ADDRESS_SIZE < strlen(string)) { + if (strlen(string) != ADDRESS_SIZE || + !hex_string_is_valid(string, 20, true)) { return ADDR_STRING_VFLOW; } - for (ctr = 0; ctr < 12; ctr++) { - encoded[ctr] = '\0'; - } - for (ctr = 12; ctr < 32; ctr++) { - strncpy(byteStrBuf, &string[2 * ((ctr - 12)) + 2], 2); - encoded[ctr] = (uint8_t)(strtol(byteStrBuf, NULL, 16)); + memset(encoded, 0, 12); + for (size_t i = 0; i < 20; i++) { + encoded[12 + i] = (uint8_t)((hex_nibble(string[2 + 2 * i]) << 4) | + hex_nibble(string[3 + 2 * i])); } return SUCCESS; } @@ -242,17 +324,15 @@ int encString(const char* string, uint8_t* encoded) { } int encodeBytes(const char* string, uint8_t* encoded) { + if (!hex_string_is_valid(string, 0, false)) return GENERAL_ERROR; struct SHA3_CTX byteCtx; const char* valStrPtr = string + 2; - uint8_t valByte[1]; - char byteStrBuf[3] = {0}; sha3_256_Init(&byteCtx); while (*valStrPtr != '\0') { - strncpy(byteStrBuf, valStrPtr, 2); - valByte[0] = (uint8_t)(strtol(byteStrBuf, NULL, 16)); - sha3_Update(&byteCtx, (const unsigned char*)valByte, - (size_t)sizeof(uint8_t)); + const uint8_t valByte = + (uint8_t)((hex_nibble(valStrPtr[0]) << 4) | hex_nibble(valStrPtr[1])); + sha3_Update(&byteCtx, &valByte, sizeof(valByte)); valStrPtr += 2; } keccak_Final(&byteCtx, encoded); @@ -260,44 +340,39 @@ int encodeBytes(const char* string, uint8_t* encoded) { } int encodeBytesN(const char* typeT, const char* string, uint8_t* encoded) { - char byteStrBuf[3] = {0}; - unsigned ctr; - - if (MAX_ENCBYTEN_SIZE < strlen(string)) { - return BYTESN_STRING_ERROR; - } - - // parse out the length val - uint8_t byteTypeSize = (uint8_t)(strtol((typeT + 5), NULL, 10)); - if (32 < byteTypeSize) { + unsigned byteTypeSize = 0; + bool dynamic = false; + if (!type_is_bytes(typeT, &byteTypeSize, &dynamic) || dynamic) { return BYTESN_SIZE_ERROR; } - for (ctr = 0; ctr < 32; ctr++) { - // zero padding - encoded[ctr] = 0; + if (!hex_string_is_valid(string, byteTypeSize, true)) { + return BYTESN_STRING_ERROR; } - unsigned zeroFillLen = 32 - ((strlen(string) - 2 /* skip '0x' */) / 2); - // bytesN are zero padded on the right - for (ctr = zeroFillLen; ctr < 32; ctr++) { - strncpy(byteStrBuf, &string[2 + 2 * (ctr - zeroFillLen)], 2); - encoded[ctr - zeroFillLen] = (uint8_t)(strtol(byteStrBuf, NULL, 16)); + memset(encoded, 0, 32); + for (size_t i = 0; i < byteTypeSize; i++) { + encoded[i] = (uint8_t)((hex_nibble(string[2 + 2 * i]) << 4) | + hex_nibble(string[3 + 2 * i])); } return SUCCESS; } int confirmName(const char* name, bool valAvailable) { - if (valAvailable) { - nameForValue = name; - } else { - (void)review(ButtonRequestType_ButtonRequest_Other, "MESSAGE DATA", - "Press button to continue for\n\"%s\" values", name); + (void)valAvailable; + if (!name) return GENERAL_ERROR; + nameForValue = name; + if (!confirm_bytes(ButtonRequestType_ButtonRequest_Other, "EIP-712 Field", + (const uint8_t*)name, strlen(name))) { + return USER_CANCELLED; } return SUCCESS; } int confirmValue(const char* value) { - (void)review(ButtonRequestType_ButtonRequest_Other, "MESSAGE DATA", "%s %s", - nameForValue, value); + if (!value || !confirm_bytes(ButtonRequestType_ButtonRequest_Other, + nameForValue ? "EIP-712 Value" : "MESSAGE DATA", + (const uint8_t*)value, strlen(value))) { + return USER_CANCELLED; + } return SUCCESS; } @@ -320,15 +395,18 @@ void marshallDsVals(const char* value) { return; } -void dsConfirm(void) { +static int confirmTypedValue(bool ds_vals, const char* value) { + if (ds_vals) marshallDsVals(value); + return confirmValue(value); +} + +int dsConfirm(void) { // First check if we recognize the contract - const TokenType* assetToken; uint8_t addrHexStr[20] = {0}; char name[41] = {0}; char version[11] = {0}; uint32_t chainInt; bool noChain = true; - int ctr; IconType iconNum = NO_ICON; char title[64] = {0}; char* fillerStr = ""; @@ -343,9 +421,13 @@ void dsConfirm(void) { } if (dsverifyingContract != NULL) { - for (ctr = 2; ctr < 42; ctr += 2) { - sscanf((char*)&dsverifyingContract[ctr], "%2hhx", - &addrHexStr[(ctr - 2) / 2]); + // Same two-chars-then-strtol idiom as encAddress(). sscanf("%2hhx") did + // this before, and it was the firmware's only caller of newlib's scanf + // engine — ~6KB of ROM on a part with none to spare. + char byteStrBuf[3] = {0}; + for (int ctr = 2; ctr < 42; ctr += 2) { + strncpy(byteStrBuf, (char*)&dsverifyingContract[ctr], 2); + addrHexStr[(ctr - 2) / 2] = (uint8_t)strtol(byteStrBuf, NULL, 16); } strcat(verifyingContract, "Verifying Contract: "); strncat(verifyingContract, dsverifyingContract, @@ -354,11 +436,7 @@ void dsConfirm(void) { if (NULL != dschainId) { noChain = false; -#ifdef EMULATOR - sscanf((char*)dschainId, "%u", &chainInt); -#else - sscanf((char*)dschainId, "%ld", &chainInt); -#endif + chainInt = (uint32_t)strtoul((const char*)dschainId, NULL, 10); // As more chains are supported, add icon choice below // TBD: not implemented for first release // if (chainInt == 1) { @@ -366,7 +444,8 @@ void dsConfirm(void) { // } } if (noChain == false && dsverifyingContract != NULL) { - assetToken = tokenByChainAddress(chainInt, (uint8_t*)addrHexStr); + const TokenType* assetToken = + tokenByChainAddress(chainInt, (uint8_t*)addrHexStr); (void)assetToken; fillerStr = ""; } @@ -380,12 +459,17 @@ void dsConfirm(void) { snprintf(chainStr, 32, "chain %s, ", dschainId); } // snprintf(contractStr, 64, "verifyingContract: %s", verifyingContract); - (void)review_with_icon(ButtonRequestType_ButtonRequest_Other, iconNum, title, - "%s %s%s", chainStr, verifyingContract, fillerStr); + bool approved = + review_with_icon(ButtonRequestType_ButtonRequest_Other, iconNum, title, + "%s %s%s", chainStr, verifyingContract, fillerStr); dsname = NULL; dsversion = NULL; dschainId = NULL; dsverifyingContract = NULL; + if (!approved) { + return USER_CANCELLED; + } + return SUCCESS; } /* @@ -438,7 +522,6 @@ int parseVals(const json_t* eip712Types, const json_t* jType, walkVals = nextVal; while (0 != walkVals) { if (0 == strcmp(json_getName(walkVals), typeName)) { - valStr = json_getValue(walkVals); break; } else { // keep looking for val @@ -446,28 +529,35 @@ int parseVals(const json_t* eip712Types, const json_t* jType, } } - bool hasValue = (JSON_TEXT == json_getType(walkVals) || - JSON_INTEGER == json_getType(walkVals)); - confirmName(typeName, hasValue); - if (walkVals == 0) { return JSON_TYPE_WNOVAL; - } else { - if (0 == strncmp("address", typeType, strlen("address") - 1)) { + } + const jsonType_t value_type = json_getType(walkVals); + const bool hasValue = value_type == JSON_TEXT || + value_type == JSON_INTEGER || + value_type == JSON_BOOLEAN; + valStr = hasValue ? json_getValue(walkVals) : NULL; + if (SUCCESS != (errRet = confirmName(typeName, hasValue))) { + return errRet; + } + + { + if (type_matches(typeType, "address")) { if (']' == typeType[strlen(typeType) - 1]) { // array of addresses + if (value_type != JSON_ARRAY) return GENERAL_ERROR; json_t const* addrVals = json_getChild(walkVals); sha3_256_Init(&valCtx); // hash of concatenated encoded strings while (0 != addrVals) { + if (json_getType(addrVals) != JSON_TEXT) return GENERAL_ERROR; + const char* address = json_getValue(addrVals); // just walk the string values assuming, for fixed sizes, all // values are there. - if (ds_vals) { - marshallDsVals(json_getValue(addrVals)); - } else { - confirmValue(json_getValue(addrVals)); + if (SUCCESS != (errRet = confirmTypedValue(ds_vals, address))) { + return errRet; } - errRet = encAddress(json_getValue(addrVals), encBytes); + errRet = encAddress(address, encBytes); if (SUCCESS != errRet) { return errRet; } @@ -476,10 +566,9 @@ int parseVals(const json_t* eip712Types, const json_t* jType, } keccak_Final(&valCtx, encBytes); } else { - if (ds_vals) { - marshallDsVals(valStr); - } else { - confirmValue(valStr); + if (value_type != JSON_TEXT) return GENERAL_ERROR; + if (SUCCESS != (errRet = confirmTypedValue(ds_vals, valStr))) { + return errRet; } errRet = encAddress(valStr, encBytes); if (SUCCESS != errRet) { @@ -487,21 +576,23 @@ int parseVals(const json_t* eip712Types, const json_t* jType, } } - } else if (0 == strncmp("string", typeType, strlen("string") - 1)) { + } else if (type_matches(typeType, "string")) { if (']' == typeType[strlen(typeType) - 1]) { // array of strings + if (value_type != JSON_ARRAY) return GENERAL_ERROR; json_t const* stringVals = json_getChild(walkVals); uint8_t strEncBytes[32]; sha3_256_Init(&valCtx); // hash of concatenated encoded strings while (0 != stringVals) { + if (json_getType(stringVals) != JSON_TEXT) return GENERAL_ERROR; + const char* string_value = json_getValue(stringVals); // just walk the string values assuming, for fixed sizes, all // values are there. - if (ds_vals) { - marshallDsVals(json_getValue(stringVals)); - } else { - confirmValue(json_getValue(stringVals)); + if (SUCCESS != + (errRet = confirmTypedValue(ds_vals, string_value))) { + return errRet; } - errRet = encString(json_getValue(stringVals), strEncBytes); + errRet = encString(string_value, strEncBytes); if (SUCCESS != errRet) { return errRet; } @@ -510,10 +601,9 @@ int parseVals(const json_t* eip712Types, const json_t* jType, } keccak_Final(&valCtx, encBytes); } else { - if (ds_vals) { - marshallDsVals(valStr); - } else { - confirmValue(valStr); + if (value_type != JSON_TEXT) return GENERAL_ERROR; + if (SUCCESS != (errRet = confirmTypedValue(ds_vals, valStr))) { + return errRet; } errRet = encString(valStr, encBytes); if (SUCCESS != errRet) { @@ -521,18 +611,19 @@ int parseVals(const json_t* eip712Types, const json_t* jType, } } - } else if ((0 == strncmp("uint", typeType, strlen("uint") - 1)) || - (0 == strncmp("int", typeType, strlen("int") - 1))) { + } else if (type_is_integer(typeType, "uint") || + type_is_integer(typeType, "int")) { if (']' == typeType[strlen(typeType) - 1]) { return INT_ARRAY_ERROR; } else { - if (ds_vals) { - marshallDsVals(valStr); - } else { - confirmValue(valStr); + if (value_type != JSON_TEXT && value_type != JSON_INTEGER) + return GENERAL_ERROR; + if (SUCCESS != (errRet = confirmTypedValue(ds_vals, valStr))) { + return errRet; } + const bool is_uint = type_is_integer(typeType, "uint"); uint8_t negInt = 0; // 0 is positive, 1 is negative - if (0 == strncmp("int", typeType, strlen("int") - 1)) { + if (!is_uint) { if (*valStr == '-') { negInt = 1; } @@ -548,147 +639,184 @@ int parseVals(const json_t* eip712Types, const json_t* jType, } } // all int strings are assumed to be base 10 and fit into 64 bits - long long intVal = strtoll(valStr, NULL, 10); + const char* digits = valStr + (negInt ? 1 : 0); + if (*digits == '\0') return GENERAL_ERROR; + for (const char* p = digits; *p; p++) { + if (*p < '0' || *p > '9') return GENERAL_ERROR; + } + errno = 0; + char* endptr = NULL; + long long intVal = strtoll(valStr, &endptr, 10); + if (errno == ERANGE || endptr == valStr || *endptr != '\0') { + return GENERAL_ERROR; + } + if (is_uint && intVal < 0) { + return GENERAL_ERROR; + } + const unsigned declared_bits = + integer_type_width(typeType, is_uint ? "uint" : "int"); + if (declared_bits < 64) { + if (is_uint) { + const uint64_t max_value = (UINT64_C(1) << declared_bits) - 1; + if ((uint64_t)intVal > max_value) return GENERAL_ERROR; + } else { + const int64_t min_value = -(INT64_C(1) << (declared_bits - 1)); + const int64_t max_value = + (INT64_C(1) << (declared_bits - 1)) - 1; + if (intVal < min_value || intVal > max_value) + return GENERAL_ERROR; + } + } // Needs to be big endian, so add to encBytes appropriately - encBytes[24] = (intVal >> 56) & 0xff; - encBytes[25] = (intVal >> 48) & 0xff; - encBytes[26] = (intVal >> 40) & 0xff; - encBytes[27] = (intVal >> 32) & 0xff; - encBytes[28] = (intVal >> 24) & 0xff; - encBytes[29] = (intVal >> 16) & 0xff; - encBytes[30] = (intVal >> 8) & 0xff; - encBytes[31] = (intVal) & 0xff; + const uint64_t intBits = (uint64_t)intVal; + encBytes[24] = (intBits >> 56) & 0xff; + encBytes[25] = (intBits >> 48) & 0xff; + encBytes[26] = (intBits >> 40) & 0xff; + encBytes[27] = (intBits >> 32) & 0xff; + encBytes[28] = (intBits >> 24) & 0xff; + encBytes[29] = (intBits >> 16) & 0xff; + encBytes[30] = (intBits >> 8) & 0xff; + encBytes[31] = intBits & 0xff; } - } else if (0 == strncmp("bytes", typeType, strlen("bytes"))) { - if (']' == typeType[strlen(typeType) - 1]) { - return BYTESN_ARRAY_ERROR; - } else { - // This could be 'bytes', 'bytes1', ..., 'bytes32' - if (ds_vals) { - marshallDsVals(valStr); + } else { + unsigned byte_size = 0; + bool dynamic_bytes = false; + if (type_is_bytes(typeType, &byte_size, &dynamic_bytes)) { + if (']' == typeType[strlen(typeType) - 1]) { + return BYTESN_ARRAY_ERROR; } else { - confirmValue(valStr); - } - if (0 == strcmp(typeType, "bytes")) { - errRet = encodeBytes(valStr, encBytes); - if (SUCCESS != errRet) { + if (value_type != JSON_TEXT) return GENERAL_ERROR; + // This could be 'bytes', 'bytes1', ..., 'bytes32' + if (SUCCESS != (errRet = confirmTypedValue(ds_vals, valStr))) { return errRet; } + if (dynamic_bytes) { + errRet = encodeBytes(valStr, encBytes); + if (SUCCESS != errRet) { + return errRet; + } - } else { - errRet = encodeBytesN(typeType, valStr, encBytes); - if (SUCCESS != errRet) { - return errRet; + } else { + errRet = encodeBytesN(typeType, valStr, encBytes); + if (SUCCESS != errRet) { + return errRet; + } } } - } - } else if (0 == strncmp("bool", typeType, strlen(typeType))) { - if (']' == typeType[strlen(typeType) - 1]) { - return BOOL_ARRAY_ERROR; - } else { - if (ds_vals) { - marshallDsVals(valStr); + } else if (type_matches(typeType, "bool")) { + if (']' == typeType[strlen(typeType) - 1]) { + return BOOL_ARRAY_ERROR; } else { - confirmValue(valStr); - } - for (ctr = 0; ctr < 32; ctr++) { - // leading zeros in bool - encBytes[ctr] = 0; - } - if (0 == strncmp(valStr, "true", sizeof("true"))) { - encBytes[31] = 0x01; + if (value_type != JSON_BOOLEAN && value_type != JSON_TEXT) + return GENERAL_ERROR; + if (SUCCESS != (errRet = confirmTypedValue(ds_vals, valStr))) { + return errRet; + } + if (strcmp(valStr, "true") != 0 && strcmp(valStr, "false") != 0) + return GENERAL_ERROR; + for (ctr = 0; ctr < 32; ctr++) { + // leading zeros in bool + encBytes[ctr] = 0; + } + if (strcmp(valStr, "true") == 0) { + encBytes[31] = 0x01; + } } - } - - } else { - // encode user defined type - char encSubTypeStr[STRBUFSIZE + 1] = {0}; - // clear out the user-defined types list - for (ctr = 0; ctr < MAX_USERDEF_TYPES; ctr++) { - udefList[ctr] = NULL; - } - char typeNoArrTok[MAX_TYPESTRING] = {0}; - // need to get typehash of type first - if (']' == typeType[strlen(typeType) - 1]) { - // array of structs. To parse name, remove array tokens. - strncpy(typeNoArrTok, typeType, sizeof(typeNoArrTok) - 1); - if (strlen(typeNoArrTok) < strlen(typeType)) { - return UDEF_ARRAY_NAME_ERR; - } - strtok(typeNoArrTok, "["); - if (STACK_GOOD != (errRet = memcheck(STACK_SIZE_GUARD))) { - return errRet; - } - if (SUCCESS != (errRet = parseType(eip712Types, typeNoArrTok, - encSubTypeStr))) { - return errRet; - } } else { - if (STACK_GOOD != (errRet = memcheck(STACK_SIZE_GUARD))) { - return errRet; - } - if (SUCCESS != - (errRet = parseType(eip712Types, typeType, encSubTypeStr))) { - return errRet; + // encode user defined type + char encSubTypeStr[STRBUFSIZE + 1] = {0}; + // clear out the user-defined types list + for (ctr = 0; ctr < MAX_USERDEF_TYPES; ctr++) { + udefList[ctr] = NULL; } - } - sha3_256_Init(&valCtx); - sha3_Update(&valCtx, (const unsigned char*)encSubTypeStr, - (size_t)strlen(encSubTypeStr)); - keccak_Final(&valCtx, encBytes); - if (']' == typeType[strlen(typeType) - 1]) { - // array of udefs - struct SHA3_CTX eleCtx = {0}; // local hash context - struct SHA3_CTX arrCtx = {0}; // array elements hash context - uint8_t eleHashBytes[32]; + char typeNoArrTok[MAX_TYPESTRING] = {0}; + // need to get typehash of type first + if (']' == typeType[strlen(typeType) - 1]) { + // array of structs. To parse name, remove array tokens. + if (value_type != JSON_ARRAY) return GENERAL_ERROR; + strncpy(typeNoArrTok, typeType, sizeof(typeNoArrTok) - 1); + if (strlen(typeNoArrTok) < strlen(typeType)) { + return UDEF_ARRAY_NAME_ERR; + } + strtok(typeNoArrTok, "["); + if (STACK_GOOD != (errRet = memcheck(STACK_SIZE_GUARD))) { + return errRet; + } + if (SUCCESS != (errRet = parseType(eip712Types, typeNoArrTok, + encSubTypeStr))) { + return errRet; + } + } else { + if (STACK_GOOD != (errRet = memcheck(STACK_SIZE_GUARD))) { + return errRet; + } + if (SUCCESS != + (errRet = parseType(eip712Types, typeType, encSubTypeStr))) { + return errRet; + } + } + sha3_256_Init(&valCtx); + sha3_Update(&valCtx, (const unsigned char*)encSubTypeStr, + (size_t)strlen(encSubTypeStr)); + keccak_Final(&valCtx, encBytes); - sha3_256_Init(&arrCtx); + if (']' == typeType[strlen(typeType) - 1]) { + // array of udefs + struct SHA3_CTX eleCtx = {0}; // local hash context + struct SHA3_CTX arrCtx = {0}; // array elements hash context + uint8_t eleHashBytes[32]; + + sha3_256_Init(&arrCtx); + + json_t const* udefVals = json_getChild(walkVals); + while (0 != udefVals) { + if (json_getType(udefVals) != JSON_OBJ) return GENERAL_ERROR; + sha3_256_Init(&eleCtx); + sha3_Update(&eleCtx, (const unsigned char*)encBytes, 32); + if (STACK_GOOD != (errRet = memcheck(STACK_SIZE_GUARD))) { + return errRet; + } + if (SUCCESS != + (errRet = parseVals( + eip712Types, + json_getProperty(eip712Types, + strtok(typeNoArrTok, "]")), + json_getChild(udefVals), // where to get the values + &eleCtx // encode hash happens in parse, this is the + // return + ))) { + return errRet; + } + keccak_Final(&eleCtx, eleHashBytes); + sha3_Update(&arrCtx, (const unsigned char*)eleHashBytes, 32); + // just walk the udef values assuming, for fixed sizes, all + // values are there. + udefVals = json_getSibling(udefVals); + } + keccak_Final(&arrCtx, encBytes); - json_t const* udefVals = json_getChild(walkVals); - while (0 != udefVals) { - sha3_256_Init(&eleCtx); - sha3_Update(&eleCtx, (const unsigned char*)encBytes, 32); + } else { + if (value_type != JSON_OBJ) return GENERAL_ERROR; + sha3_256_Init(&valCtx); + sha3_Update(&valCtx, (const unsigned char*)encBytes, + (size_t)sizeof(encBytes)); if (STACK_GOOD != (errRet = memcheck(STACK_SIZE_GUARD))) { return errRet; } if (SUCCESS != (errRet = parseVals( - eip712Types, - json_getProperty(eip712Types, strtok(typeNoArrTok, "]")), - json_getChild(udefVals), // where to get the values - &eleCtx // encode hash happens in parse, this is the - // return + eip712Types, json_getProperty(eip712Types, typeType), + json_getChild(walkVals), // where to get the values + &valCtx // val hash happens in parse, this is the return ))) { return errRet; } - keccak_Final(&eleCtx, eleHashBytes); - sha3_Update(&arrCtx, (const unsigned char*)eleHashBytes, 32); - // just walk the udef values assuming, for fixed sizes, all values - // are there. - udefVals = json_getSibling(udefVals); + keccak_Final(&valCtx, encBytes); } - keccak_Final(&arrCtx, encBytes); - - } else { - sha3_256_Init(&valCtx); - sha3_Update(&valCtx, (const unsigned char*)encBytes, - (size_t)sizeof(encBytes)); - if (STACK_GOOD != (errRet = memcheck(STACK_SIZE_GUARD))) { - return errRet; - } - if (SUCCESS != - (errRet = parseVals( - eip712Types, json_getProperty(eip712Types, typeType), - json_getChild(walkVals), // where to get the values - &valCtx // val hash happens in parse, this is the return - ))) { - return errRet; - } - keccak_Final(&valCtx, encBytes); } } } @@ -699,7 +827,9 @@ int parseVals(const json_t* eip712Types, const json_t* jType, tarray = json_getSibling(tarray); } if (ds_vals) { - dsConfirm(); + if (SUCCESS != (errRet = dsConfirm())) { + return errRet; + } } return SUCCESS; diff --git a/lib/firmware/ethereum.c b/lib/firmware/ethereum.c index b7c12c1b5..37cae37e3 100644 --- a/lib/firmware/ethereum.c +++ b/lib/firmware/ethereum.c @@ -33,6 +33,7 @@ #include "keepkey/firmware/eip712.h" #include "keepkey/firmware/ethereum_contracts.h" #include "keepkey/firmware/ethereum_contracts/makerdao.h" +#include "keepkey/firmware/signed_metadata.h" #include "keepkey/firmware/ethereum_tokens.h" #include "keepkey/firmware/storage.h" #include "keepkey/firmware/thorchain.h" @@ -48,6 +49,14 @@ #define _(X) (X) +bool ethereum_typed_hash_policy_allows(bool advanced_mode) { + return advanced_mode; +} + +bool ethereum_eip712_is_domain_primary_type(const char* primary_type) { + return primary_type && strcmp(primary_type, "EIP712Domain") == 0; +} + #define MAX_CHAIN_ID 2147483630 #define ETHEREUM_TX_TYPE_LEGACY 0UL @@ -204,6 +213,20 @@ static void hash_rlp_number(uint32_t number) { hash_rlp_field(data + offset, 4 - offset); } +/* Strip leading zero bytes before RLP-encoding an integer field. + * Per the Ethereum yellow paper, integer fields (nonce, gas, value, etc.) + * must not have leading zeros. Addresses are NOT integers and must not use + * this function. */ +static void hash_rlp_bytes_stripped(const uint8_t* buf, size_t size) { + size_t offset = 0; + while (offset < size && buf[offset] == 0) offset++; + if (offset == size) { + hash_rlp_field(buf, 0); + } else { + hash_rlp_field(buf + offset, size - offset); + } +} + /* * Calculate the number of bytes needed for an RLP length header. * NOTE: supports up to 16MB of data (how unlikely...) @@ -223,6 +246,21 @@ static int rlp_calculate_length(int length, uint8_t firstbyte) { } } +/* Length of an RLP-encoded integer field AFTER stripping leading zero bytes. + * MUST mirror hash_rlp_bytes_stripped(): the Stage-1 list-length header + * (hash_rlp_list_length) and the Stage-2 bytes actually hashed have to agree, + * or the keccak pre-image is malformed and the signature recovers to a garbage + * address (looks like a "random signer" / dropped tx). Any integer field whose + * big-endian form has a leading zero byte hits this. */ +static int rlp_calculate_length_stripped(const uint8_t* buf, size_t size) { + size_t offset = 0; + while (offset < size && buf[offset] == 0) offset++; + if (offset == size) { + return rlp_calculate_length(0, 0); + } + return rlp_calculate_length(size - offset, buf[offset]); +} + static int rlp_calculate_number_length(uint32_t number) { if (number <= 0x7f) { return 1; @@ -265,6 +303,19 @@ static void send_signature(void) { } keccak_Final(&keccak_ctx, hash); + + /* Insight clear-signing binding. If a verified metadata blob suppressed the + * raw-data confirmation, the actual signed digest MUST equal the tx hash the + * metadata committed to. This is the first point that digest exists, so the + * check reuses it rather than re-deriving the RLP pre-image. Fail closed — + * never emit a signature the displayed decoded screen did not cover. */ + if (!signed_metadata_enforce(hash)) { + fsm_sendFailure(FailureType_Failure_Other, + "Metadata does not match signed transaction"); + ethereum_signing_abort(); + return; + } + if (ecdsa_sign_digest(&secp256k1, privkey, hash, sig, &v, ethereum_is_canonic) != 0) { fsm_sendFailure(FailureType_Failure_Other, "Signing failed"); @@ -388,6 +439,9 @@ void ethereumFormatAmount(const bignum256* amnt, const TokenType* token, case 137: suffix = " MATIC"; break; // Polygon Mainnet + case 43114: + suffix = " AVAX"; + break; // Avalanche C-Chain } } } @@ -585,8 +639,13 @@ static bool ethereum_signing_check(const EthereumSignTx* msg) { return false; } - if (msg->gas_price.size + msg->gas_limit.size > 30) { - // sanity check that fee doesn't overflow + // Sanity-bound the fee field that this tx type actually uses, so the + // on-screen fee (fee_per_gas * gas_limit) cannot overflow into the modular + // bn_multiply and display a wrong value. EIP-1559 uses max_fee_per_gas; + // legacy uses gas_price (which is 0 for EIP-1559 and vice versa). + size_t fee_per_gas_size = msg->has_max_fee_per_gas ? msg->max_fee_per_gas.size + : msg->gas_price.size; + if (fee_per_gas_size + msg->gas_limit.size > 30) { return false; } @@ -648,17 +707,30 @@ void ethereum_signing_init(EthereumSignTx* msg, const HDNode* node, ethereum_tx_type = ETHEREUM_TX_TYPE_LEGACY; } - if (ethereum_tx_type == ETHEREUM_TX_TYPE_EIP_1559 && chain_id == 0) { - fsm_sendFailure(FailureType_Failure_SyntaxError, - _("EIP-1559 transactions require chain_id")); - ethereum_signing_abort(); - return; - } - - if (ethereum_tx_type == ETHEREUM_TX_TYPE_EIP_1559 && - !msg->has_max_fee_per_gas) { + /* The typed prefix (0x02) and access list are emitted based on + * ethereum_tx_type, while the fee fields are selected by has_max_fee_per_gas. + * If those two disagree, Stage 1 (rlp_length) and Stage 2 (hashed bytes) + * describe different field lists and the signature recovers to a wrong + * address. Enforce a consistent shape up front. */ + if (ethereum_tx_type == ETHEREUM_TX_TYPE_EIP_1559) { + if (chain_id == 0) { + /* chain_id is the mandatory first RLP field of an EIP-1559 tx; absent + * chain_id is counted (1 byte) in Stage 1 but hash_rlp_number(0) hashes + * nothing in Stage 2. */ + fsm_sendFailure(FailureType_Failure_SyntaxError, + _("EIP-1559 transactions require chain_id")); + ethereum_signing_abort(); + return; + } + if (!msg->has_max_fee_per_gas) { + fsm_sendFailure(FailureType_Failure_SyntaxError, + _("EIP-1559 transactions require max_fee_per_gas")); + ethereum_signing_abort(); + return; + } + } else if (msg->has_max_fee_per_gas) { fsm_sendFailure(FailureType_Failure_SyntaxError, - _("EIP-1559 transactions require max_fee_per_gas")); + _("max_fee_per_gas requires an EIP-1559 (type 2) tx")); ethereum_signing_abort(); return; } @@ -718,6 +790,40 @@ void ethereum_signing_init(EthereumSignTx* msg, const HDNode* node, data_needs_confirm = false; } + // Signed metadata clear signing (backwards compatible). + // Only fires if host sent EthereumTxMetadata before this EthereumSignTx. + if (data_needs_confirm && data_total > 0 && signed_metadata_available()) { + if (signed_metadata_matches_tx(msg)) { + if (signed_metadata_confirm()) { + if (signed_metadata_from_loaded_signer()) { + /* A self-service signer is annotation-only. Its decoded screens are + * followed by the same amount and raw-calldata review an Advanced + * transaction would have received without metadata. A lying runtime + * schema therefore cannot conceal transaction bytes. */ + needs_confirm = true; + data_needs_confirm = true; + } else { + /* A future firmware-pinned signer may replace the raw-data screen. + * Payable calls still show amount/recipient because a v2 schema + * describes calldata only and cannot bind msg->value. */ + needs_confirm = signed_metadata_schema_moves_value(); + data_needs_confirm = false; + } + } else { + fsm_sendFailure(FailureType_Failure_ActionCancelled, + "Signing cancelled by user"); + ethereum_signing_abort(); // clears metadata + return; + } + } + } + // Drop metadata now UNLESS we relied on it to suppress the raw-data confirm + // (then it must survive to bind the signature). Prevents stale reuse when the + // contractHandled / ERC-20 paths bypass the metadata check above. + if (!signed_metadata_relied()) { + signed_metadata_clear(); + } + // detect ERC-20 token if (data_total == 68 && ethereum_isStandardERC20Transfer(msg)) { token = tokenByChainAddress(chain_id, msg->to.bytes); @@ -763,15 +869,15 @@ void ethereum_signing_init(EthereumSignTx* msg, const HDNode* node, memset(confirm_body_message, 0, sizeof(confirm_body_message)); if (token == NULL && data_total > 0 && data_needs_confirm) { - // KeepKey custom: warn the user that they're trying to do something - // that is potentially dangerous. People (generally) aren't great at - // parsing raw transaction data, and we can't effectively show them - // what they're about to do in the general case. + // AdvancedMode policy: hard gate for blind-signing arbitrary contract data if (!storage_isPolicyEnabled("AdvancedMode")) { - (void)review( - ButtonRequestType_ButtonRequest_Other, "Warning", - "Signing of arbitrary ETH contract data is recommended only for " - "experienced users. Enable 'AdvancedMode' policy to dismiss."); + (void)review(ButtonRequestType_ButtonRequest_Other, "Blocked", + "Blind signing requires AdvancedMode. " + "Enable in device settings."); + fsm_sendFailure(FailureType_Failure_ActionCancelled, + "Blind signing disabled by policy"); + ethereum_signing_abort(); + return; } layoutEthereumData(msg->data_initial_chunk.bytes, @@ -808,24 +914,24 @@ void ethereum_signing_init(EthereumSignTx* msg, const HDNode* node, rlp_length += rlp_calculate_number_length(chain_id); } - rlp_length += rlp_calculate_length(msg->nonce.size, msg->nonce.bytes[0]); - if (ethereum_tx_type == ETHEREUM_TX_TYPE_EIP_1559) { + rlp_length += + rlp_calculate_length_stripped(msg->nonce.bytes, msg->nonce.size); + if (msg->has_max_fee_per_gas) { rlp_length += - rlp_calculate_length(msg->max_priority_fee_per_gas.size, - msg->max_priority_fee_per_gas.size - ? msg->max_priority_fee_per_gas.bytes[0] - : 0); - rlp_length += rlp_calculate_length(msg->max_fee_per_gas.size, - msg->max_fee_per_gas.bytes[0]); + rlp_calculate_length_stripped(msg->max_priority_fee_per_gas.bytes, + msg->max_priority_fee_per_gas.size); + rlp_length += rlp_calculate_length_stripped(msg->max_fee_per_gas.bytes, + msg->max_fee_per_gas.size); } else { - rlp_length += - rlp_calculate_length(msg->gas_price.size, msg->gas_price.bytes[0]); + rlp_length += rlp_calculate_length_stripped(msg->gas_price.bytes, + msg->gas_price.size); } rlp_length += - rlp_calculate_length(msg->gas_limit.size, msg->gas_limit.bytes[0]); + rlp_calculate_length_stripped(msg->gas_limit.bytes, msg->gas_limit.size); rlp_length += rlp_calculate_length(msg->to.size, msg->to.bytes[0]); - rlp_length += rlp_calculate_length(msg->value.size, msg->value.bytes[0]); + rlp_length += + rlp_calculate_length_stripped(msg->value.bytes, msg->value.size); rlp_length += rlp_calculate_length(data_total, msg->data_initial_chunk.bytes[0]); @@ -872,19 +978,26 @@ void ethereum_signing_init(EthereumSignTx* msg, const HDNode* node, hash_rlp_number(chain_id); } - hash_rlp_field(msg->nonce.bytes, msg->nonce.size); + hash_rlp_bytes_stripped(msg->nonce.bytes, msg->nonce.size); - if (ethereum_tx_type == ETHEREUM_TX_TYPE_EIP_1559) { - hash_rlp_field(msg->max_priority_fee_per_gas.bytes, - msg->max_priority_fee_per_gas.size); - hash_rlp_field(msg->max_fee_per_gas.bytes, msg->max_fee_per_gas.size); + if (msg->has_max_fee_per_gas) { + /* max_priority_fee_per_gas is a mandatory EIP-1559 field; when absent it + * encodes as the empty integer (0x80). Stage 1 always counts it + * (unconditionally, above), so Stage 2 must always hash it too -- guarding + * on has_max_priority_fee_per_gas here would under-hash and leave the list + * header over-declared (the same wrong-signer class this commit fixes). + * .size is 0 when unset, which hash_rlp_bytes_stripped emits as 0x80. */ + hash_rlp_bytes_stripped(msg->max_priority_fee_per_gas.bytes, + msg->max_priority_fee_per_gas.size); + hash_rlp_bytes_stripped(msg->max_fee_per_gas.bytes, + msg->max_fee_per_gas.size); } else { - hash_rlp_field(msg->gas_price.bytes, msg->gas_price.size); + hash_rlp_bytes_stripped(msg->gas_price.bytes, msg->gas_price.size); } - hash_rlp_field(msg->gas_limit.bytes, msg->gas_limit.size); - hash_rlp_field(msg->to.bytes, msg->to.size); - hash_rlp_field(msg->value.bytes, msg->value.size); + hash_rlp_bytes_stripped(msg->gas_limit.bytes, msg->gas_limit.size); + hash_rlp_field(msg->to.bytes, msg->to.size); /* address: no strip */ + hash_rlp_bytes_stripped(msg->value.bytes, msg->value.size); hash_rlp_length(data_total, msg->data_initial_chunk.bytes[0]); hash_data(msg->data_initial_chunk.bytes, msg->data_initial_chunk.size); data_left = data_total - msg->data_initial_chunk.size; @@ -932,11 +1045,14 @@ void ethereum_signing_txack(EthereumTxAck* tx) { void ethereum_signing_abort(void) { if (ethereum_signing) { memzero(privkey, sizeof(privkey)); + signed_metadata_clear(); layoutHome(); ethereum_signing = false; } } +bool ethereum_signing_isInProgress(void) { return ethereum_signing; } + static void ethereum_message_hash(const uint8_t* message, size_t message_len, uint8_t hash[32]) { struct SHA3_CTX ctx; @@ -1089,6 +1205,19 @@ void ethereum_typed_hash_sign(const EthereumSignTypedHash* msg, resp->signature.bytes[64] = 27 + v; resp->signature.size = 65; + /* Populate response-only fields after every confirmation. Emulator debug + * requests (including screenshot capture) share msg_resp and can clear data + * prepared before the confirmation callbacks complete. */ + uint8_t pubkeyhash[20] = {0}; + if (!hdnode_get_ethereum_pubkeyhash(node, pubkeyhash)) { + fsm_sendFailure(FailureType_Failure_Other, + _("Ethereum address derivation failed")); + return; + } + resp->address[0] = '0'; + resp->address[1] = 'x'; + ethereum_address_checksum(pubkeyhash, resp->address + 2, false, 0); + msg_write(MessageType_MessageType_EthereumTypedDataSignature, resp); } @@ -1126,9 +1255,14 @@ const char* failMsgReturn[LAST_ERROR - 2] = { "EIP-712 typeType has no name in parseVals", "EIP-712 address string is NULL", "EIP-712 no value for type during walkVals", // 33 + "EIP-712 cancelled", // 34 (USER_CANCELLED) }; void failMessage(int err) { + if (err == USER_CANCELLED) { + fsm_sendFailure(FailureType_Failure_ActionCancelled, NULL); + return; + } if (err < GENERAL_ERROR || err > LAST_ERROR) { // unknown error number fsm_sendFailure(FailureType_Failure_Other, _("EIP-712 unknown failure")); @@ -1205,14 +1339,23 @@ void e712_types_values(Ethereum712TypesValues* msg, failMessage(JSON_PTYPENAMEERR); return; } + if (json_getType(obTest) != JSON_TEXT) { + failMessage(JSON_PTYPEVALERR); + return; + } const char* primeType; - if (0 == (primeType = json_getValue(obTest))) { + if (0 == (primeType = json_getValue(obTest)) || primeType[0] == '\0') { failMessage(JSON_PTYPEVALERR); return; } - if (0 != strncmp(primeType, "EIP712Domain", - strlen(primeType))) { // if primaryType is "EIP712Domain", - // message hash is NULL + if (!confirm_bytes(ButtonRequestType_ButtonRequest_Other, + "EIP-712 Primary Type", (const uint8_t*)primeType, + strlen(primeType))) { + failMessage(USER_CANCELLED); + return; + } + if (!ethereum_eip712_is_domain_primary_type( + primeType)) { // domain-only signatures have no message hash errRet = encode(jsonT, jsonV, primeType, resp->message_hash.bytes); if (!(SUCCESS == errRet || NULL_MSG_HASH == errRet)) { failMessage(errRet); @@ -1251,5 +1394,17 @@ void e712_types_values(Ethereum712TypesValues* msg, have_ds = false; } + /* Debug-link reads during confirmation reuse msg_resp, so populate the + * returned address only after the final confirmation has completed. */ + uint8_t pubkeyhash[20] = {0}; + if (!hdnode_get_ethereum_pubkeyhash(node, pubkeyhash)) { + fsm_sendFailure(FailureType_Failure_Other, + _("Ethereum address derivation failed")); + return; + } + resp->address[0] = '0'; + resp->address[1] = 'x'; + ethereum_address_checksum(pubkeyhash, resp->address + 2, false, 0); + msg_write(MessageType_MessageType_EthereumTypedDataSignature, resp); } diff --git a/lib/firmware/ethereum_contracts.c b/lib/firmware/ethereum_contracts.c index 87fb61488..deec3b4eb 100644 --- a/lib/firmware/ethereum_contracts.c +++ b/lib/firmware/ethereum_contracts.c @@ -20,6 +20,7 @@ #include "keepkey/firmware/ethereum_contracts.h" +#include "keepkey/firmware/ethereum.h" // completes EthereumSignTx (msg fields) #include "keepkey/firmware/ethereum_contracts/saproxy.h" #include "keepkey/firmware/ethereum_contracts/thortx.h" #include "keepkey/firmware/ethereum_contracts/zxappliquid.h" @@ -32,12 +33,32 @@ bool ethereum_contractHandled(uint32_t data_total, const EthereumSignTx* msg, const HDNode* node) { (void)node; - if (sa_isWithdrawFromSalary(msg)) return true; + /* Only a CALL to a contract may be clear-signed, never a CREATE + * (to.size == 0 must reach the deploy screen). */ + if (msg->to.size != 20) { + return false; + } + + /* 0x transformERC20 is pinned to the ExchangeProxy and bounded by its + * displayed input/min-output amounts, so it is safe to clear-sign at ANY + * calldata size; its transformations[] tail legitimately exceeds one 1024- + * byte chunk. (It guards its own fixed-offset reads against + * data_initial_chunk.size.) */ if (zx_isZxTransformERC20(msg)) return true; + + /* Every other handler must have the ENTIRE calldata in the first chunk, so + * the fields it parses and displays are the whole transaction and nothing + * unshown streams in afterwards. */ + if (data_total != msg->data_initial_chunk.size) { + return false; + } + + if (sa_isWithdrawFromSalary(msg)) return true; if (zx_isZxSwap(msg)) return true; if (zx_isZxLiquidTx(msg)) return true; if (zx_isZxApproveLiquid(msg)) return true; + if (thor_isMayachainTx(msg)) return true; if (thor_isThorchainTx(msg)) return true; if (makerdao_isMakerDAO(data_total, msg)) return true; @@ -62,6 +83,7 @@ bool ethereum_contractConfirmed(uint32_t data_total, const EthereumSignTx* msg, if (zx_isZxApproveLiquid(msg)) return zx_confirmApproveLiquidity(data_total, msg); + if (thor_isMayachainTx(msg)) return thor_confirmMayaTx(data_total, msg); if (thor_isThorchainTx(msg)) return thor_confirmThorTx(data_total, msg); if (makerdao_isMakerDAO(data_total, msg)) diff --git a/lib/firmware/ethereum_contracts/saproxy.c b/lib/firmware/ethereum_contracts/saproxy.c index ccab30ba6..c7514995f 100644 --- a/lib/firmware/ethereum_contracts/saproxy.c +++ b/lib/firmware/ethereum_contracts/saproxy.c @@ -45,7 +45,8 @@ bool sa_isWithdrawFromSalary(const EthereumSignTx* msg) { bool sa_confirmWithdrawFromSalary(uint32_t data_total, const EthereumSignTx* msg) { - (void)data_total; + /* reads selector + 2 32-byte words (salaryId, withdrawAmount) */ + if (data_total < 4 + 2 * 32) return false; char confStr[41]; bignum256 salaryId, withdrawAmount; diff --git a/lib/firmware/ethereum_contracts/thortx.c b/lib/firmware/ethereum_contracts/thortx.c index 6f48d1ab5..d86e98e52 100644 --- a/lib/firmware/ethereum_contracts/thortx.c +++ b/lib/firmware/ethereum_contracts/thortx.c @@ -41,28 +41,123 @@ bool thor_is_expiry_variant(const EthereumSignTx* msg) { THOR_SELECTOR_DEPOSIT_WITH_EXPIRY, 4) == 0; } -bool thor_isThorchainTx(const EthereumSignTx* msg) { - if (msg->has_to && msg->to.size == 20 && thor_has_deposit_selector(msg)) { - return true; +/* Format msg->to as lowercase hex string (40 chars + NUL) */ +static void thor_format_to_addr(const EthereumSignTx* msg, char out[41]) { + for (uint32_t i = 0; i < 20; i++) { + snprintf(&out[i * 2], 3, "%02x", msg->to.bytes[i]); } - return false; + out[40] = '\0'; } -bool thor_confirmThorTx(uint32_t data_total, const EthereumSignTx* msg) { +bool thor_isMayachainTx(const EthereumSignTx* msg) { + if (!msg->has_to || msg->to.size != 20) return false; + /* MAYA_ROUTER is an Ethereum-mainnet identity; the same address on another + * EVM chain may hold unrelated attacker code. Bind to mainnet so a + * host-selected chain_id cannot borrow the trusted router UX. */ + if (!msg->has_chain_id || msg->chain_id != 1) return false; + if (!thor_has_deposit_selector(msg)) return false; + char toStr[41]; + thor_format_to_addr(msg, toStr); + return strncmp(toStr, MAYA_ROUTER, 40) == 0; +} + +/* The THORChain router address for this tx's chain, or NULL if the chain has + * no pinned router (then the deposit is not clear-signed and falls to the + * blind-sign gate). Each router address is a per-chain identity — the same + * address on another chain may hold unrelated attacker code — so the pin is + * (chain_id, address) together. A tx with NO chain_id gets no router at all: + * ethereum.c would default it to mainnet for hashing, but an identity pin + * must never be inherited from a default the host simply omitted. */ +static const char* thor_router_for_chain(const EthereumSignTx* msg) { + if (!msg->has_chain_id) return NULL; + switch (msg->chain_id) { + case 1: + return THOR_ROUTER; /* Ethereum */ + case 43114: + return THOR_ROUTER_AVAX; /* Avalanche C-Chain */ + default: + return NULL; + } +} + +bool thor_isThorchainTx(const EthereumSignTx* msg) { + if (!msg->has_to || msg->to.size != 20) return false; + if (!thor_has_deposit_selector(msg)) return false; + /* Pin to the THORChain router FOR THIS CHAIN. Without the pin, ANY contract + * carrying the deposit selector would get the THORChain clear-sign UX and + * bypass the AdvancedMode blind-sign gate, letting an attacker contract + * drain while the device shows a benign deposit. Without the chain scope, + * only mainnet deposits ever match (the AVAX->ETH blind-sign bug). */ + const char* router = thor_router_for_chain(msg); + if (!router) return false; + char toStr[41]; + thor_format_to_addr(msg, toStr); + return strncmp(toStr, router, 40) == 0; +} + +static bool thor_confirm_deposit_tx(uint32_t data_total, + const EthereumSignTx* msg, + const char* protocol_label, + const char* router_label) { (void)data_total; - /* Minimum calldata: selector(4) + vault(32) + asset(32) + amount(32) + - * memo_offset(32) + memo_length(32) = 164 bytes for deposit(), - * + expiry(32) = 196 bytes for depositWithExpiry(). */ + /* Minimum calldata to read the fixed head through the memo_length word: + * selector(4) + vault(32) + asset(32) + amount(32) + memo_offset(32) + + * memo_length(32) = 164 bytes for deposit(), + expiry(32) = 196 for + * depositWithExpiry(). The exact memo bounds are enforced below from the ABI + * memo length, so a short memo (e.g. "ADD:ETH.ETH") still clear-signs rather + * than being rejected by an over-tight fixed floor. */ const bool is_expiry = thor_is_expiry_variant(msg); - const size_t min_chunk = is_expiry ? 260 : 228; + const size_t min_chunk = is_expiry ? 196 : 164; if (msg->data_initial_chunk.size < min_chunk) return false; - char confStr[41], *conf; - const TokenType* assetToken; + /* The memo is a dynamic `string`; its ABI head pointer (word 3, offset + * 4+3*32) must be canonical (0x80 for deposit's 4 head words, 0xa0 for + * depositWithExpiry's 5), else abi.decode on the router reads the memo from a + * different location than we display from the fixed offset below -> the + * executed swap destination can differ from what the user approved. */ + { + static const uint8_t MEMO_OFF_DEPOSIT[32] = {[31] = 0x80}; + static const uint8_t MEMO_OFF_EXPIRY[32] = {[31] = 0xa0}; + const uint8_t* expected = is_expiry ? MEMO_OFF_EXPIRY : MEMO_OFF_DEPOSIT; + if (memcmp(msg->data_initial_chunk.bytes + 4 + 3 * 32, expected, 32) != 0) { + return false; + } + } + + /* The memo is a dynamic `string`: read its ABI length word instead of + * assuming a fixed 64 bytes. A longer memo places router-executed fields + * (destination, affiliate, aggregator, min-out) past byte 64 that a fixed + * parse never displays. Reject dirty high bytes, cap at THORChain's 256-byte + * memo max, require the whole calldata to be in this chunk, and require the + * padded memo to end exactly at the calldata end so no trailing bytes hide. + */ + const uint8_t* memo_len_word = + msg->data_initial_chunk.bytes + 4 + (is_expiry ? 5 : 4) * 32; + for (int i = 0; i < 28; i++) { + if (memo_len_word[i] != 0) return false; + } + const uint32_t memo_len = ((uint32_t)memo_len_word[28] << 24) | + ((uint32_t)memo_len_word[29] << 16) | + ((uint32_t)memo_len_word[30] << 8) | + (uint32_t)memo_len_word[31]; + if (memo_len > 256) return false; + const size_t memo_off = (size_t)(4 + (is_expiry ? 6 : 5) * 32); + const size_t memo_padded = ((memo_len + 31u) / 32u) * 32u; + if (msg->has_data_length && + msg->data_length != msg->data_initial_chunk.size) { + return false; /* whole calldata must be in the initial chunk to bound it */ + } + if (memo_off + memo_padded != msg->data_initial_chunk.size) { + return false; /* trailing bytes after the memo would be executed but hidden + */ + } + + char confStr[41]; + const char* conf; uint8_t* thorchainData; const uint8_t* contractAssetAddress; - const uint8_t *vaultAddress, *assetAddress; + const uint8_t* vaultAddress; uint32_t ctr; bignum256 Amount; @@ -75,15 +170,16 @@ bool thor_confirmThorTx(uint32_t data_total, const EthereumSignTx* msg) { (uint8_t*)(msg->data_initial_chunk.bytes + 4 + (is_expiry ? 6 : 5) * 32); // Start confirmations - for (ctr = 0; ctr < 20; ctr++) { - snprintf(&confStr[ctr * 2], 3, "%02x", msg->to.bytes[ctr]); - } - if (strncmp(confStr, THOR_ROUTER, sizeof(THOR_ROUTER)) == 0) { + thor_format_to_addr(msg, confStr); + const char* thor_router = thor_router_for_chain(msg); + if (thor_router && strncmp(confStr, thor_router, 40) == 0) { conf = "Thorchain router"; + } else if (strncmp(confStr, MAYA_ROUTER, 40) == 0) { + conf = router_label; } else { conf = confStr; } - if (!confirm(ButtonRequestType_ButtonRequest_ConfirmOutput, "Thorchain data", + if (!confirm(ButtonRequestType_ButtonRequest_ConfirmOutput, protocol_label, "Routing through %s", conf)) { return false; } @@ -92,50 +188,92 @@ bool thor_confirmThorTx(uint32_t data_total, const EthereumSignTx* msg) { for (ctr = 0; ctr < 20; ctr++) { snprintf(&confStr[ctr * 2], 3, "%02x", vaultAddress[ctr]); } - if (!confirm(ButtonRequestType_ButtonRequest_ConfirmOutput, "Thorchain data", + if (!confirm(ButtonRequestType_ButtonRequest_ConfirmOutput, protocol_label, "Using Asgard vault %s", confStr)) { return false; } - if (memcmp(contractAssetAddress, ETH_ADDRESS, sizeof(ETH_ADDRESS)) == 0) { - assetAddress = (const uint8_t*) - ETH_NATIVE; // get eth native parameters if asset is not a token - } else { - assetAddress = contractAssetAddress; - } - - assetToken = tokenByChainAddress(msg->chain_id, assetAddress); + /* Both pinned routers treat ONLY address(0) as native (and require + * msg.value == 0 for any other asset), so the 0xEeee..Ee sentinel is NOT + * native here — accepting it would clear-sign a tx that reverts on-chain and + * burns gas. Match address(0) exactly (20 bytes, not sizeof, whose literal + * NUL would over-read into the amount word). */ + const bool is_native = memcmp(contractAssetAddress, ETH_ADDRESS, 20) == 0; + bignum256 Value; + bn_from_bytes(msg->value.bytes, msg->value.size, &Value); + if (is_native) { + /* Display msg.value — the amount the router actually forwards — not the ABI + * amount word it ignores. That alone closes the "display 0.01 while sending + * 100" gap; we do NOT additionally require amount == value, since the ABI + * amount is a router-ignored hint that legitimately differs. Format with a + * NULL token so the ticker is the CHAIN's native asset (ETH on mainnet, + * AVAX on Avalanche); the 0xEE pseudo-token entry is pinned to " ETH" and + * would mislabel every other chain's native deposit. */ + ethereumFormatAmount(&Value, NULL, msg->chain_id, confStr, sizeof(confStr)); - if (strncmp(assetToken->ticker, " UNKN", 5) == 0) { - // just display token address and amount as string - for (ctr = 0; ctr < 20; ctr++) { - snprintf(&confStr[ctr * 2], 3, "%02x", assetAddress[ctr]); - } - if (!confirm(ButtonRequestType_ButtonRequest_ConfirmOutput, - "Thorchain data", "from asset %s", confStr)) { + if (!confirm(ButtonRequestType_ButtonRequest_ConfirmOutput, protocol_label, + "Confirm sending %s", confStr)) { return false; } - // We don't know what the exponent should be so just confirm raw unformatted - // number - bn_format(&Amount, NULL, " unformatted", 0, 0, false, confStr, - sizeof(confStr)); - - if (!confirm(ButtonRequestType_ButtonRequest_ConfirmOutput, - "Thorchain data", "amount %s", confStr)) { + } else { + /* A token deposit must not also carry native value (the router pulls tokens + * via transferFrom); nonzero msg.value would be swept and never shown. */ + if (!bn_is_zero(&Value)) { return false; } + const uint8_t* assetAddress = contractAssetAddress; - } else { - ethereumFormatAmount(&Amount, assetToken, msg->chain_id, confStr, - sizeof(confStr)); + const TokenType* assetToken = + tokenByChainAddress(msg->chain_id, assetAddress); - if (!confirm(ButtonRequestType_ButtonRequest_ConfirmOutput, - "Thorchain data", "Confirm sending %s", confStr)) { - return false; + if (strncmp(assetToken->ticker, " UNKN", 5) == 0) { + // just display token address and amount as string + for (ctr = 0; ctr < 20; ctr++) { + snprintf(&confStr[ctr * 2], 3, "%02x", assetAddress[ctr]); + } + if (!confirm(ButtonRequestType_ButtonRequest_ConfirmOutput, + protocol_label, "from asset %s", confStr)) { + return false; + } + // We don't know what the exponent should be so just confirm raw + // unformatted number + bn_format(&Amount, NULL, " unformatted", 0, 0, false, confStr, + sizeof(confStr)); + + if (!confirm(ButtonRequestType_ButtonRequest_ConfirmOutput, + protocol_label, "amount %s", confStr)) { + return false; + } + + } else { + ethereumFormatAmount(&Amount, assetToken, msg->chain_id, confStr, + sizeof(confStr)); + + if (!confirm(ButtonRequestType_ButtonRequest_ConfirmOutput, + protocol_label, "Confirm sending %s", confStr)) { + return false; + } } } - if (!thorchain_parseConfirmMemo((const char*)thorchainData, 64)) return false; + if (!thorchain_parseConfirmMemo((const char*)thorchainData, memo_len)) + return false; + + /* Page the complete raw memo as the authoritative disclosure: a long + * structured field (dest/affiliate/aggregator) would otherwise truncate in + * its single confirm and hide the tail that the router still executes. */ + if (!thorchain_confirm_full_memo("Memo", (const char*)thorchainData, + memo_len)) + return false; return true; } + +bool thor_confirmThorTx(uint32_t data_total, const EthereumSignTx* msg) { + return thor_confirm_deposit_tx(data_total, msg, "Thorchain data", + "Thorchain router"); +} + +bool thor_confirmMayaTx(uint32_t data_total, const EthereumSignTx* msg) { + return thor_confirm_deposit_tx(data_total, msg, "Maya data", "Maya router"); +} diff --git a/lib/firmware/ethereum_contracts/zxappliquid.c b/lib/firmware/ethereum_contracts/zxappliquid.c index 76d7d3cda..841fd5fe2 100644 --- a/lib/firmware/ethereum_contracts/zxappliquid.c +++ b/lib/firmware/ethereum_contracts/zxappliquid.c @@ -7,108 +7,138 @@ * it under the terms of the GNU Lesser General Public License as published by * the Free Software Foundation, either version 3 of the License, or * (at your option) any later version. - * - * This library is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with this library. If not, see . */ #include "keepkey/firmware/ethereum_contracts/zxappliquid.h" #include "keepkey/firmware/ethereum_contracts/zxliquidtx.h" #include "keepkey/board/confirm_sm.h" +#include "keepkey/board/font.h" +#include "keepkey/board/layout.h" #include "keepkey/board/util.h" -#include "keepkey/firmware/app_confirm.h" -#include "keepkey/firmware/coins.h" #include "keepkey/firmware/ethereum.h" #include "keepkey/firmware/ethereum_tokens.h" -#include "keepkey/firmware/fsm.h" -#include "keepkey/firmware/storage.h" -#include "trezor/crypto/address.h" -#include "trezor/crypto/bip32.h" -#include "trezor/crypto/curves.h" -#include "trezor/crypto/memzero.h" +#include "trezor/crypto/bignum.h" #include "trezor/crypto/sha3.h" -bool zx_confirmApproveLiquidity(uint32_t data_total, - const EthereumSignTx *msg) { - (void)data_total; - const char *to, *tikstr, *poolstr, *allowance, *amt; - unsigned char data[40]; - uint8_t digest[SHA3_256_DIGEST_LENGTH] = {0}; - uint8_t tokdigest[SHA3_256_DIGEST_LENGTH] = {0}; - char digestStr[2 * SHA3_256_DIGEST_LENGTH + 1], amtStr[2 * 32 + 1] = {0}; - int32_t ctr, tokctr; - uint32_t wethord; - const TokenType *WETH, *ttoken; - - if (!tokenByTicker(msg->chain_id, "WETH", &WETH)) return false; - wethord = read_be((const uint8_t *)WETH->address); - to = (const char *)msg->to.bytes; - tokctr = 0; - while (tokctr != -1) { - ttoken = tokenIter(&tokctr); - - // https://uniswap.org/docs/v2/smart-contract-integration/getting-pair-addresses/ - uint32_t ttokenord = read_be((const uint8_t *)ttoken->address); - if (ttokenord < wethord) { - memcpy(data, ttoken->address, 20); - memcpy(&data[20], WETH->address, 20); - } else { - memcpy(data, WETH->address, 20); - memcpy(&data[20], ttoken->address, 20); - } - keccak_256(data, sizeof(data), tokdigest); - SHA3_CTX ctx = {0}; - keccak_256_Init(&ctx); - keccak_Update(&ctx, (unsigned char *)"\xff", 1); - keccak_Update(&ctx, (unsigned char *)"\x5C\x69\xbE\xe7\x01\xef\x81\x4a\x2B\x6a\x3E\xDD\x4B\x16\x52\xCB\x9c\xc5\xaA\x6f", 20); - keccak_Update(&ctx, tokdigest, sizeof(tokdigest)); - keccak_Update(&ctx, (unsigned char *)"\x96\xe8\xac\x42\x77\x19\x8f\xf8\xb6\xf7\x85\x47\x8a\xa9\xa3\x9f\x40\x3c\xb7\x68\xdd\x02\xcb\xee\x32\x6c\x3e\x7d\xa3\x48\x84\x5f", 32); - keccak_Final(&ctx, digest); - if (memcmp(to, &digest[12], 20) == 0) break; +#include +#include + +#define UNISWAP_APPROVE_CALL_SIZE (4 + 2 * 32) +#define UNISWAP_AMOUNT_TEXT_SIZE 96 + +static const uint8_t UNISWAP_FACTORY_ADDRESS[20] = { + 0x5c, 0x69, 0xbe, 0xe7, 0x01, 0xef, 0x81, 0x4a, 0x2b, 0x6a, + 0x3e, 0xdd, 0x4b, 0x16, 0x52, 0xcb, 0x9c, 0xc5, 0xaa, 0x6f}; +static const uint8_t UNISWAP_PAIR_INIT_CODE_HASH[32] = { + 0x96, 0xe8, 0xac, 0x42, 0x77, 0x19, 0x8f, 0xf8, 0xb6, 0xf7, 0x85, + 0x47, 0x8a, 0xa9, 0xa3, 0x9f, 0x40, 0x3c, 0xb7, 0x68, 0xdd, 0x02, + 0xcb, 0xee, 0x32, 0x6c, 0x3e, 0x7d, 0xa3, 0x48, 0x84, 0x5f}; +static const uint8_t WETH_MAINNET_ADDRESS[20] = { + 0xc0, 0x2a, 0xaa, 0x39, 0xb2, 0x23, 0xfe, 0x8d, 0x0a, 0x0e, + 0x5c, 0x4f, 0x27, 0xea, 0xd9, 0x08, 0x3c, 0x75, 0x6c, 0xc2}; + +static bool tx_value_is_zero(const EthereumSignTx* msg) { + if (!msg->has_value && msg->value.size != 0) return false; + for (size_t i = 0; i < msg->value.size; i++) { + if (msg->value.bytes[i] != 0) return false; } + return true; +} + +static bool spender_word_is_router(const EthereumSignTx* msg) { + const uint8_t* word = msg->data_initial_chunk.bytes + 4; + for (size_t i = 0; i < 12; i++) { + if (word[i] != 0) return false; + } + return memcmp(word + 12, UNISWAP_ROUTER_ADDRESS, 20) == 0; +} - if (tokctr != -1) { - for (ctr = 0; ctr < SHA3_256_DIGEST_LENGTH; ctr++) { - snprintf(&digestStr[ctr * 2], 3, "%02x", digest[ctr]); - } - tikstr = ttoken->ticker; - poolstr = &digestStr[12 * 2]; +static void derive_pair_address(const uint8_t* token_a, const uint8_t* token_b, + uint8_t pair[20]) { + uint8_t ordered[40]; + if (memcmp(token_a, token_b, 20) < 0) { + memcpy(ordered, token_a, 20); + memcpy(ordered + 20, token_b, 20); } else { - for (ctr = 0; ctr < 20; ctr++) { - snprintf(&digestStr[ctr * 2], 3, "%02x", to[ctr]); - } - tikstr = ""; - poolstr = digestStr; + memcpy(ordered, token_b, 20); + memcpy(ordered + 20, token_a, 20); } - allowance = (char *)(msg->data_initial_chunk.bytes + 4 + 32); - if (memcmp(allowance, (uint8_t *)&MAX_ALLOWANCE, 32) == 0) { - amt = "full balance"; + uint8_t salt[SHA3_256_DIGEST_LENGTH]; + uint8_t digest[SHA3_256_DIGEST_LENGTH]; + keccak_256(ordered, sizeof(ordered), salt); + SHA3_CTX ctx = {0}; + keccak_256_Init(&ctx); + const uint8_t prefix = 0xff; + keccak_Update(&ctx, &prefix, 1); + keccak_Update(&ctx, UNISWAP_FACTORY_ADDRESS, sizeof(UNISWAP_FACTORY_ADDRESS)); + keccak_Update(&ctx, salt, sizeof(salt)); + keccak_Update(&ctx, UNISWAP_PAIR_INIT_CODE_HASH, + sizeof(UNISWAP_PAIR_INIT_CODE_HASH)); + keccak_Final(&ctx, digest); + memcpy(pair, digest + 12, 20); +} + +static const TokenType* pool_underlying_token(const EthereumSignTx* msg) { + int32_t token_index = 0; + while (token_index >= 0) { + const TokenType* token = tokenIter(&token_index); + if (token == UnknownToken) break; + if (token->chain_id != 1 || + memcmp(token->address, WETH_MAINNET_ADDRESS, 20) == 0) + continue; + uint8_t pair[20]; + derive_pair_address((const uint8_t*)token->address, WETH_MAINNET_ADDRESS, + pair); + if (memcmp(msg->to.bytes, pair, 20) == 0) return token; + } + return NULL; +} + +static bool approve_shape_is_clear_signable(const EthereumSignTx* msg) { + if (!msg->has_chain_id || msg->chain_id != 1 || !msg->has_to || + msg->to.size != 20 || !msg->has_data_initial_chunk || + msg->data_initial_chunk.size != UNISWAP_APPROVE_CALL_SIZE || + memcmp(msg->data_initial_chunk.bytes, "\x09\x5e\xa7\xb3", 4) != 0 || + msg->value.size > 32 || !tx_value_is_zero(msg) || + !spender_word_is_router(msg)) + return false; + return pool_underlying_token(msg) != NULL; +} + +bool zx_confirmApproveLiquidity(uint32_t data_total, + const EthereumSignTx* msg) { + if (data_total != UNISWAP_APPROVE_CALL_SIZE || + !approve_shape_is_clear_signable(msg)) + return false; + + const TokenType* token = pool_underlying_token(msg); + const uint8_t* allowance = msg->data_initial_chunk.bytes + 4 + 32; + char amount_text[UNISWAP_AMOUNT_TEXT_SIZE]; + if (memcmp(allowance, (const uint8_t*)MAX_ALLOWANCE, 32) == 0) { + strlcpy(amount_text, "full LP balance", sizeof(amount_text)); } else { - for (ctr = 0; ctr < 32; ctr++) { - snprintf(&amtStr[ctr * 2], 3, "%02x", allowance[ctr]); - } - amt = amtStr; + bignum256 amount; + bn_from_bytes(allowance, 32, &amount); + if (bn_format(&amount, NULL, " LP", 18, 0, false, amount_text, + sizeof(amount_text)) == 0 || + calc_str_line(get_body_font(), amount_text, BODY_WIDTH) > BODY_ROWS) + return false; } - const char *appStr = "uniswap approve liquidity"; - confirm(ButtonRequestType_ButtonRequest_ConfirmOutput, appStr, "Amount: %s", - amt); - confirm(ButtonRequestType_ButtonRequest_ConfirmOutput, appStr, - "approve for pool %s %s", tikstr, poolstr); - return true; + if (!confirm(ButtonRequestType_ButtonRequest_ConfirmOutput, + "Uniswap LP Approval", "%s", amount_text)) + return false; + + char pair_text[43] = {'0', 'x', '\0'}; + for (size_t i = 0; i < 20; i++) { + snprintf(pair_text + 2 + i * 2, 3, "%02x", msg->to.bytes[i]); + } + return confirm(ButtonRequestType_ButtonRequest_ConfirmOutput, + "Uniswap LP Pool", "%s\n%s", token->ticker, pair_text); } -bool zx_isZxApproveLiquid(const EthereumSignTx *msg) { - if (memcmp(msg->data_initial_chunk.bytes, "\x09\x5e\xa7\xb3", 4) == 0) - if (memcmp((uint8_t *)(msg->data_initial_chunk.bytes + 4 + 32 - 20), - UNISWAP_ROUTER_ADDRESS, 20) == 0) - return true; - return false; +bool zx_isZxApproveLiquid(const EthereumSignTx* msg) { + return approve_shape_is_clear_signable(msg); } diff --git a/lib/firmware/ethereum_contracts/zxliquidtx.c b/lib/firmware/ethereum_contracts/zxliquidtx.c index e70ccd4e3..568080722 100644 --- a/lib/firmware/ethereum_contracts/zxliquidtx.c +++ b/lib/firmware/ethereum_contracts/zxliquidtx.c @@ -20,164 +20,200 @@ #include "keepkey/firmware/ethereum_contracts/zxliquidtx.h" #include "keepkey/board/confirm_sm.h" -#include "keepkey/board/util.h" -#include "keepkey/firmware/app_confirm.h" -#include "keepkey/firmware/coins.h" +#include "keepkey/board/font.h" +#include "keepkey/board/layout.h" #include "keepkey/firmware/ethereum.h" #include "keepkey/firmware/ethereum_tokens.h" -#include "keepkey/firmware/fsm.h" #include "keepkey/firmware/storage.h" #include "trezor/crypto/address.h" +#include "trezor/crypto/bignum.h" #include "trezor/crypto/bip32.h" #include "trezor/crypto/curves.h" #include "trezor/crypto/memzero.h" -#include "trezor/crypto/sha3.h" -#include +#include +#include -static HDNode* zx_getDerivedNode(const char* curve, const uint32_t* address_n, - size_t address_n_count, - uint32_t* fingerprint) { - static HDNode CONFIDENTIAL node; - if (fingerprint) { - *fingerprint = 0; - } +#define UNISWAP_LIQUIDITY_CALL_SIZE (4 + 6 * 32) +#define UNISWAP_TOKEN_WORD 0 +#define UNISWAP_PRIMARY_AMOUNT_WORD 1 +#define UNISWAP_TOKEN_MIN_WORD 2 +#define UNISWAP_NATIVE_MIN_WORD 3 +#define UNISWAP_RECIPIENT_WORD 4 +#define UNISWAP_DEADLINE_WORD 5 +#define UNISWAP_AMOUNT_TEXT_SIZE 96 - if (!get_curve_by_name(curve)) { - return 0; - } +static const uint8_t* abi_word(const EthereumSignTx* msg, size_t word) { + return msg->data_initial_chunk.bytes + 4 + word * 32; +} - if (!storage_getRootNode(curve, true, &node)) { - return 0; +static bool abi_address_is_canonical(const uint8_t* word) { + for (size_t i = 0; i < 12; i++) { + if (word[i] != 0) return false; } + return true; +} - if (!address_n || address_n_count == 0) { - return &node; +static bool uint256_fits_u64(const uint8_t* word) { + for (size_t i = 0; i < 24; i++) { + if (word[i] != 0) return false; } + return true; +} - if (hdnode_private_ckd_cached(&node, address_n, address_n_count, - fingerprint) == 0) { - return 0; +static bool tx_value_is_zero(const EthereumSignTx* msg) { + if (!msg->has_value && msg->value.size != 0) return false; + for (size_t i = 0; i < msg->value.size; i++) { + if (msg->value.bytes[i] != 0) return false; } - - return &node; + return true; } static bool isAddLiquidityEthCall(const EthereumSignTx* msg) { - if (memcmp(msg->data_initial_chunk.bytes, "\xf3\x05\xd7\x19", 4) == 0) - return true; - - return false; + return memcmp(msg->data_initial_chunk.bytes, "\xf3\x05\xd7\x19", 4) == 0; } static bool isRemoveLiquidityEthCall(const EthereumSignTx* msg) { - if (memcmp(msg->data_initial_chunk.bytes, "\x02\x75\x1c\xec", 4) == 0) - return true; + return memcmp(msg->data_initial_chunk.bytes, "\x02\x75\x1c\xec", 4) == 0; +} - return false; +static const TokenType* liquidity_token(const EthereumSignTx* msg) { + const uint8_t* token_address = abi_word(msg, UNISWAP_TOKEN_WORD) + 12; + const TokenType* token = tokenByChainAddress(1, token_address); + return token == UnknownToken ? NULL : token; } -static bool confirmFromAccountMatch(const EthereumSignTx* msg, - const char* addremStr) { - // Determine withdrawal address - char addressStr[43] = {'0', 'x', '\0'}; - const char* fromSrc; - const uint8_t* fromAddress; - uint8_t addressBytes[20]; +static bool liquidity_shape_is_clear_signable(const EthereumSignTx* msg) { + if (!msg->has_chain_id || msg->chain_id != 1 || !msg->has_to || + msg->to.size != 20 || + memcmp(msg->to.bytes, UNISWAP_ROUTER_ADDRESS, 20) != 0 || + !msg->has_data_initial_chunk || + msg->data_initial_chunk.size != UNISWAP_LIQUIDITY_CALL_SIZE || + msg->value.size > 32 || (!msg->has_value && msg->value.size != 0)) + return false; - HDNode* node = zx_getDerivedNode(SECP256K1_NAME, msg->address_n, - msg->address_n_count, NULL); - if (!node) return false; + if (!isAddLiquidityEthCall(msg) && !isRemoveLiquidityEthCall(msg)) + return false; + if (!abi_address_is_canonical(abi_word(msg, UNISWAP_TOKEN_WORD)) || + !abi_address_is_canonical(abi_word(msg, UNISWAP_RECIPIENT_WORD)) || + !uint256_fits_u64(abi_word(msg, UNISWAP_DEADLINE_WORD))) + return false; + if (liquidity_token(msg) == NULL) return false; + if (isRemoveLiquidityEthCall(msg) && !tx_value_is_zero(msg)) return false; + return true; +} - if (!hdnode_get_ethereum_pubkeyhash(node, addressBytes)) { - memzero(node, sizeof(*node)); +static bool format_amount(const bignum256* amount, const char* suffix, + unsigned int decimals, char* out, size_t out_len) { + if (bn_format(amount, NULL, suffix, decimals, 0, false, out, out_len) == 0) + return false; + return calc_str_line(get_body_font(), out, BODY_WIDTH) <= BODY_ROWS; +} + +bool zx_formatZxLiquidityPrimaryAmount(const EthereumSignTx* msg, char* out, + size_t out_len) { + if (!out || out_len == 0 || !liquidity_shape_is_clear_signable(msg)) + return false; + + bignum256 amount; + bn_from_bytes(abi_word(msg, UNISWAP_PRIMARY_AMOUNT_WORD), 32, &amount); + if (isAddLiquidityEthCall(msg)) { + const TokenType* token = liquidity_token(msg); + return format_amount(&amount, token->ticker, token->decimals, out, out_len); } + return format_amount(&amount, " LP", 18, out, out_len); +} - fromAddress = - (const uint8_t*)(msg->data_initial_chunk.bytes + 4 + 5 * 32 - 20); +static HDNode* zx_getDerivedNode(const char* curve, const uint32_t* address_n, + size_t address_n_count, + uint32_t* fingerprint) { + static HDNode CONFIDENTIAL node; + if (fingerprint) *fingerprint = 0; + if (!get_curve_by_name(curve)) return NULL; + if (!storage_getRootNode(curve, true, &node)) return NULL; + if (!address_n || address_n_count == 0) return &node; + if (hdnode_private_ckd_cached(&node, address_n, address_n_count, + fingerprint) == 0) + return NULL; + return &node; +} + +static bool confirmFromAccountMatch(const EthereumSignTx* msg) { + char address_str[43] = {'0', 'x', '\0'}; + uint8_t address_bytes[20]; - if (memcmp(fromAddress, addressBytes, 20) == 0) { - fromSrc = "self"; - } else { - fromSrc = "NOT this wallet"; + HDNode* node = zx_getDerivedNode(SECP256K1_NAME, msg->address_n, + msg->address_n_count, NULL); + if (!node) return false; + if (!hdnode_get_ethereum_pubkeyhash(node, address_bytes)) { + memzero(node, sizeof(*node)); + return false; } + memzero(node, sizeof(*node)); - for (uint32_t ctr = 0; ctr < 20; ctr++) { - snprintf(&addressStr[2 + ctr * 2], 3, "%02x", fromAddress[ctr]); + const uint8_t* recipient = abi_word(msg, UNISWAP_RECIPIENT_WORD) + 12; + bool is_self = memcmp(recipient, address_bytes, 20) == 0; + for (uint32_t i = 0; i < 20; i++) { + snprintf(&address_str[2 + i * 2], 3, "%02x", recipient[i]); } - if (!confirm(ButtonRequestType_ButtonRequest_ConfirmOutput, addremStr, - "Confirming ETH address is %s: %s", fromSrc, addressStr)) { + if (!confirm(ButtonRequestType_ButtonRequest_ConfirmOutput, + "Uniswap Recipient", "%s\n%s", + is_self ? "this wallet" : "NOT this wallet", address_str)) return false; - } - return true; + return is_self; } bool zx_isZxLiquidTx(const EthereumSignTx* msg) { - if (memcmp(msg->to.bytes, UNISWAP_ROUTER_ADDRESS, 20) == - 0) { // correct contract address? - - if (isAddLiquidityEthCall(msg)) return true; - - if (isRemoveLiquidityEthCall(msg)) return true; - } - return false; + return liquidity_shape_is_clear_signable(msg); } bool zx_confirmZxLiquidTx(uint32_t data_total, const EthereumSignTx* msg) { - (void)data_total; - const TokenType* token; - char constr1[40], constr2[40], tokbuf[32]; - const char* arStr = ""; - const uint8_t *tokenAddress, *deadlineBytes; - bignum256 Amount; - uint64_t deadline; + if (data_total != UNISWAP_LIQUIDITY_CALL_SIZE || + !liquidity_shape_is_clear_signable(msg)) + return false; - if (isAddLiquidityEthCall(msg)) { - arStr = "uniswap add liquidity"; - } else if (isRemoveLiquidityEthCall(msg)) { - arStr = "uniswap remove liquidity"; - } else { + const TokenType* token = liquidity_token(msg); + bignum256 amount; + char amount_text[UNISWAP_AMOUNT_TEXT_SIZE]; + + if (!zx_formatZxLiquidityPrimaryAmount(msg, amount_text, + sizeof(amount_text)) || + !confirm(ButtonRequestType_ButtonRequest_ConfirmOutput, + isAddLiquidityEthCall(msg) ? "Uniswap Token" : "Uniswap LP Burn", + "%s", amount_text)) return false; - } - tokenAddress = (const uint8_t*)(msg->data_initial_chunk.bytes + 4 + 32 - 20); - token = tokenByChainAddress(msg->chain_id, tokenAddress); - deadlineBytes = - (const uint8_t*)(msg->data_initial_chunk.bytes + 4 + 6 * 32 - 8); - deadline = ((uint64_t)deadlineBytes[0] << 8 * 7) | - ((uint64_t)deadlineBytes[1] << 8 * 6) | - ((uint64_t)deadlineBytes[2] << 8 * 5) | - ((uint64_t)deadlineBytes[3] << 8 * 4) | - ((uint64_t)deadlineBytes[4] << 8 * 3) | - ((uint64_t)deadlineBytes[5] << 8 * 2) | - ((uint64_t)deadlineBytes[6] << 8 * 1) | - ((uint64_t)deadlineBytes[7]); - - bn_from_bytes(msg->data_initial_chunk.bytes + 4 + 32, 32, - &Amount); // token amount - ethereumFormatAmount(&Amount, token, msg->chain_id, tokbuf, sizeof(tokbuf)); - snprintf(constr1, 32, "%s", tokbuf); - bn_from_bytes(msg->data_initial_chunk.bytes + 4 + 2 * 32, 32, - &Amount); // token min amount - ethereumFormatAmount(&Amount, token, msg->chain_id, tokbuf, sizeof(tokbuf)); - snprintf(constr2, 32, "%s", tokbuf); - confirm(ButtonRequestType_ButtonRequest_ConfirmOutput, arStr, - "%s\nMinimum %s", constr1, constr2); - if (!confirmFromAccountMatch(msg, arStr)) { + bn_from_bytes(abi_word(msg, UNISWAP_TOKEN_MIN_WORD), 32, &amount); + if (!format_amount(&amount, token->ticker, token->decimals, amount_text, + sizeof(amount_text)) || + !confirm(ButtonRequestType_ButtonRequest_ConfirmOutput, + "Uniswap Token Min", "%s", amount_text)) return false; - } - bn_from_bytes(msg->data_initial_chunk.bytes + 4 + 3 * 32, 32, - &Amount); // eth min amount - ethereumFormatAmount(&Amount, NULL, msg->chain_id, tokbuf, sizeof(tokbuf)); + if (!confirmFromAccountMatch(msg)) return false; - snprintf(constr1, 32, "%s", tokbuf); - confirm(ButtonRequestType_ButtonRequest_ConfirmOutput, arStr, "Minimum %s", - constr1); + if (isAddLiquidityEthCall(msg)) { + bn_from_bytes(msg->value.bytes, msg->value.size, &amount); + if (!format_amount(&amount, " ETH", 18, amount_text, sizeof(amount_text)) || + !confirm(ButtonRequestType_ButtonRequest_ConfirmOutput, "Uniswap ETH", + "%s", amount_text)) + return false; + } - confirm(ButtonRequestType_ButtonRequest_ConfirmOutput, arStr, "Deadline %s", - ctime((const time_t*)&deadline)); + bn_from_bytes(abi_word(msg, UNISWAP_NATIVE_MIN_WORD), 32, &amount); + if (!format_amount(&amount, " ETH", 18, amount_text, sizeof(amount_text)) || + !confirm(ButtonRequestType_ButtonRequest_ConfirmOutput, "Uniswap ETH Min", + "%s", amount_text)) + return false; - return true; + const uint8_t* deadline_word = abi_word(msg, UNISWAP_DEADLINE_WORD); + uint64_t deadline = 0; + for (size_t i = 24; i < 32; i++) { + deadline = (deadline << 8) | deadline_word[i]; + } + char deadline_text[21]; + snprintf(deadline_text, sizeof(deadline_text), "%" PRIu64, deadline); + return confirm(ButtonRequestType_ButtonRequest_ConfirmOutput, + "Uniswap Deadline", "%s", deadline_text); } diff --git a/lib/firmware/ethereum_contracts/zxswap.c b/lib/firmware/ethereum_contracts/zxswap.c index 5d033e804..15dccb699 100644 --- a/lib/firmware/ethereum_contracts/zxswap.c +++ b/lib/firmware/ethereum_contracts/zxswap.c @@ -44,7 +44,22 @@ bool zx_isZxSwap(const EthereumSignTx* msg) { } bool zx_confirmZxSwap(uint32_t data_total, const EthereumSignTx* msg) { - (void)data_total; + /* fixed reads run through the fromAddress word at offset 4 + 5*32 + 12 .. +20 + * (== 4 + 6*32); the toAddress word is bounds-checked below once its + * position is known from numOfTokens. */ + if (data_total < 4 + 6 * 32) return false; + + /* The first head word is the ABI offset pointer to the dynamic `tokens[]` + * array. We read numOfTokens / tokens[] at FIXED offsets that are only + * correct when this pointer is canonical (0x80 = 4 head words). If it is + * not, the Solidity decoder follows it elsewhere, so what we display would + * differ from what executes (a drain via display/execution mismatch). Reject + * the non-canonical encoding -> falls through to the blind-sign path. */ + static const uint8_t TOKENS_OFFSET_CANON[32] = {[31] = 0x80}; + if (memcmp(msg->data_initial_chunk.bytes + 4, TOKENS_OFFSET_CANON, 32) != 0) { + return false; + } + const TokenType *from, *to; const uint8_t *fromAddress, *toAddress; char constr1[40], constr2[40]; @@ -71,6 +86,9 @@ bool zx_confirmZxSwap(uint32_t data_total, const EthereumSignTx* msg) { break; } + /* toAddress word ends at offset 4 + (6 + adder + 1) * 32 */ + if (data_total < 4 + (7 + adder) * 32) return false; + fromAddress = (const uint8_t*)(msg->data_initial_chunk.bytes + 4 + 5 * 32 + 12); toAddress = (const uint8_t*)(msg->data_initial_chunk.bytes + 4 + diff --git a/lib/firmware/ethereum_contracts/zxtransERC20.c b/lib/firmware/ethereum_contracts/zxtransERC20.c index 75f64c160..83074443e 100644 --- a/lib/firmware/ethereum_contracts/zxtransERC20.c +++ b/lib/firmware/ethereum_contracts/zxtransERC20.c @@ -45,6 +45,10 @@ bool zx_isZxTransformERC20(const EthereumSignTx* msg) { bool zx_confirmZxTransERC20(uint32_t data_total, const EthereumSignTx* msg) { (void)data_total; + /* Reads selector + 4 static head words (in/out token, in/out amount). This + * handler is allowed at any data_total (large transformations[] tail), so + * guard the read extent against the RECEIVED chunk, not the total length. */ + if (msg->data_initial_chunk.size < 4 + 4 * 32) return false; const TokenType *in, *out; const uint8_t *inAddress, *outAddress; char constr1[40], constr2[40]; diff --git a/lib/firmware/ethereum_tokens.c b/lib/firmware/ethereum_tokens.c index 15e5f0678..dd7f57f60 100644 --- a/lib/firmware/ethereum_tokens.c +++ b/lib/firmware/ethereum_tokens.c @@ -42,7 +42,8 @@ const TokenType* tokenIter(int32_t* ctr) { return &(tokens[*ctr - 1]); } -const TokenType* tokenByChainAddress(uint8_t chain_id, const uint8_t* address) { +const TokenType* tokenByChainAddress(uint32_t chain_id, + const uint8_t* address) { if (!address) return 0; for (int i = 0; i < TOKENS_COUNT; i++) { if (chain_id == tokens[i].chain_id && @@ -57,7 +58,7 @@ const TokenType* tokenByChainAddress(uint8_t chain_id, const uint8_t* address) { return UnknownToken; } -bool tokenByTicker(uint8_t chain_id, const char* ticker, +bool tokenByTicker(uint32_t chain_id, const char* ticker, const TokenType** token) { *token = NULL; diff --git a/lib/firmware/fsm.c b/lib/firmware/fsm.c index 01629abbf..5faf631e0 100644 --- a/lib/firmware/fsm.c +++ b/lib/firmware/fsm.c @@ -56,6 +56,7 @@ #include "keepkey/firmware/ripple.h" #include "keepkey/firmware/signing.h" #include "keepkey/firmware/signtx_tendermint.h" +#include "keepkey/firmware/signed_metadata.h" #include "keepkey/firmware/solana.h" #include "keepkey/firmware/zcash.h" #include "keepkey/firmware/storage.h" diff --git a/lib/firmware/fsm_msg_common.h b/lib/firmware/fsm_msg_common.h index e913acc84..289e6b4b5 100644 --- a/lib/firmware/fsm_msg_common.h +++ b/lib/firmware/fsm_msg_common.h @@ -463,6 +463,9 @@ void fsm_msgWipeDevice(WipeDevice* msg) { storage_reset(); storage_resetUuid(); storage_commit(); + /* Factory reset drops runtime trust anchors too: loaded clearsign + * signers (and any metadata they verified) must not survive a wipe. */ + signed_metadata_clear_signers(); fsm_sendSuccess("Device wiped"); layoutHome(); diff --git a/lib/firmware/fsm_msg_ethereum.h b/lib/firmware/fsm_msg_ethereum.h index 5c9827164..46ad0fcf9 100644 --- a/lib/firmware/fsm_msg_ethereum.h +++ b/lib/firmware/fsm_msg_ethereum.h @@ -19,6 +19,136 @@ * along with this library. If not, see . */ +#include "keepkey/firmware/signed_metadata.h" + +void fsm_msgEthereumTxMetadata(const EthereumTxMetadata* msg) { + CHECK_INITIALIZED + CHECK_PIN + + /* Metadata must arrive before signing starts. signed_metadata_process() + * clears the binding on entry, so accepting metadata mid-signing would + * drop the tx<->metadata binding without aborting: a host could approve a + * benign decode (suppressing the blind-sign gate), then inject metadata to + * clear the binding and stream attacker-chosen calldata for the rest. + * Refuse and abort any in-progress signing session. */ + if (ethereum_signing_isInProgress()) { + ethereum_signing_abort(); + fsm_sendFailure(FailureType_Failure_UnexpectedMessage, + _("Metadata not allowed during signing")); + layoutHome(); + return; + } + + CHECK_PARAM(storage_isPolicyEnabled("AdvancedMode"), + _("AdvancedMode required for clearsign metadata")); + + RESP_INIT(EthereumMetadataAck); + + MetadataClassification result = signed_metadata_process( + msg->signed_payload.bytes, msg->signed_payload.size, + msg->has_key_id ? msg->key_id : 0); + + resp->classification = (uint32_t)result; + resp->has_display_summary = true; + + switch (result) { + case METADATA_VERIFIED: + strlcpy(resp->display_summary, "Verified", sizeof(resp->display_summary)); + break; + case METADATA_OPAQUE: + strlcpy(resp->display_summary, "Unverified", + sizeof(resp->display_summary)); + break; + case METADATA_MALFORMED: + default: + strlcpy(resp->display_summary, "Invalid", sizeof(resp->display_summary)); + break; + } + + msg_write(MessageType_MessageType_EthereumMetadataAck, resp); +} + +void fsm_msgLoadClearsignSigner(const LoadClearsignSigner* msg) { + CHECK_INITIALIZED + CHECK_PIN + CHECK_PARAM(storage_isPolicyEnabled("AdvancedMode"), + _("AdvancedMode required for clearsign signers")); + + CHECK_PARAM(msg->has_key_id && msg->has_pubkey && msg->has_alias, + _("key_id, pubkey and alias required")); + /* Range-check as uint32 BEFORE narrowing: (uint8_t)256 would alias slot 0 */ + CHECK_PARAM(msg->key_id < METADATA_MAX_KEYS, _("key_id out of range")); + CHECK_PARAM( + signed_metadata_signer_valid((uint8_t)msg->key_id, msg->pubkey.bytes, + msg->pubkey.size, msg->alias), + _("Invalid clearsign signer")); + + /* Optional identity icon (1bpp mono RLE). The proto caps icon at 384 bytes; + * bound the dims too so the render path never scans a bogus geometry. An icon + * with zero/oversized dims is rejected rather than silently dropped so a + * malformed upload is visible, not a mystery text-only identity. */ + const uint8_t* icon = NULL; + uint16_t icon_len = 0; + uint8_t icon_w = 0, icon_h = 0; + if (msg->has_icon && msg->icon.size > 0) { + CHECK_PARAM(msg->icon.size <= METADATA_ICON_MAX, _("icon too large")); + /* Width is capped at the confirm screen's icon column + * (LEFT_MARGIN_WITH_ICON = 40), NOT at the 64px height. Title/body text + * begins at x=40 and the icon is drawn AFTER the text, so a wider + * host-supplied icon would paint over the alias, fingerprint and the + * "NOT verified by KeepKey" warning — on the very screen that exists to + * carry that warning. This is the trust boundary for icons arriving on the + * wire; signed_metadata_signer_icon() rechecks the session copy at use. */ + CHECK_PARAM(msg->has_icon_width && msg->has_icon_height && + msg->icon_width > 0 && + msg->icon_width <= LEFT_MARGIN_WITH_ICON && + msg->icon_height > 0 && msg->icon_height <= 64, + _("icon dimensions out of range")); + /* Reject a malformed RLE stream HERE rather than discovering it at draw + * time. The render path returns a bool that layout_add_icon() discards, so + * an undecodable icon would otherwise show no logo while still returning + * Success — the user would consent to an identity + * whose logo silently does not exist. Validation is exact (every packet + * well-formed, no run straddling the image, whole input consumed) and + * side-effect-free. + */ + CHECK_PARAM(draw_bitmap_mono_rle_valid( + msg->icon.bytes, (uint32_t)msg->icon.size, + (uint16_t)msg->icon_width, (uint16_t)msg->icon_height), + _("invalid icon encoding")); + icon = msg->icon.bytes; + icon_len = (uint16_t)msg->icon.size; + icon_w = (uint8_t)msg->icon_width; + icon_h = (uint8_t)msg->icon_height; + } + bool persist = msg->has_persist && msg->persist; + CHECK_PARAM(!persist, _("Persistent clearsign signers are disabled")); + + /* Mandatory on-device consent — leads with the identity's logo (if any) + + * alias + fingerprint. The whole trust model hangs on this confirm; the same + * fingerprint reappears on every per-tx identity screen. */ + char fingerprint[METADATA_FINGERPRINT_LEN]; + signed_metadata_pubkey_fingerprint(msg->pubkey.bytes, fingerprint); + if (!signed_metadata_confirm_load(msg->alias, fingerprint, icon, icon_w, + icon_h, icon_len)) { + fsm_sendFailure(FailureType_Failure_ActionCancelled, + _("Load clearsign signer cancelled")); + layoutHome(); + return; + } + + if (!signed_metadata_store_signer((uint8_t)msg->key_id, msg->pubkey.bytes, + msg->alias, icon, icon_w, icon_h, icon_len, + persist)) { + fsm_sendFailure(FailureType_Failure_Other, + _("Clearsign signer could not be loaded")); + layoutHome(); + return; + } + fsm_sendSuccess(_("Clearsign signer loaded")); + layoutHome(); +} + static int process_ethereum_xfer(const CoinType* coin, EthereumSignTx* msg) { if (!ethereum_isStandardERC20Transfer(msg) && msg->data_length != 0) return TXOUT_COMPILE_ERROR; @@ -191,44 +321,16 @@ void fsm_msgEthereumGetAddress(EthereumGetAddress* msg) { layoutHome(); } -#define MSG_MAX (38 * 3) // 38 chars per line, three lines max void fsm_msgEthereumSignMessage(EthereumSignMessage* msg) { - char msgBuf[MSG_MAX + 1] = {0}; - const char* typeIndicator; - unsigned ctr; - unsigned msgLen = 0; - bool canPrint = true; - RESP_INIT(EthereumMessageSignature); CHECK_INITIALIZED CHECK_PIN - // truncate to display size if too long - msgLen = msg->message.size * 2; - if (msgLen > MSG_MAX) { - msgLen = MSG_MAX; - } - for (ctr = 0; ctr < msg->message.size; ctr++) { - if (isprint(msg->message.bytes[ctr]) == false) { - canPrint = false; - break; - } - } - if (canPrint) { - typeIndicator = "Sign Message"; - strncpy(msgBuf, (char*)msg->message.bytes, MSG_MAX + 1); - msgBuf[MSG_MAX] = '\0'; - } else { - typeIndicator = "Sign Bytes"; - for (ctr = 0; ctr < msgLen / 2; ctr++) { - snprintf(&msgBuf[2 * ctr], 3, "%02x", msg->message.bytes[ctr]); - } - } - - if (!confirm(ButtonRequestType_ButtonRequest_ProtectCall, _(typeIndicator), - "%s", msgBuf)) { + if (!confirm_bytes(ButtonRequestType_ButtonRequest_ProtectCall, + "Sign Ethereum Message", msg->message.bytes, + msg->message.size)) { fsm_sendFailure(FailureType_Failure_ActionCancelled, NULL); layoutHome(); return; @@ -244,12 +346,6 @@ void fsm_msgEthereumSignMessage(EthereumSignMessage* msg) { } void fsm_msgEthereumVerifyMessage(const EthereumVerifyMessage* msg) { - char msgBuf[MSG_MAX + 1] = {0}; - const char* typeIndicator; - unsigned ctr; - unsigned msgLen = 0; - bool canPrint = true; - CHECK_PARAM(msg->has_address, _("No address provided")); CHECK_PARAM(msg->has_message, _("No message provided")); @@ -266,29 +362,9 @@ void fsm_msgEthereumVerifyMessage(const EthereumVerifyMessage* msg) { return; } - // truncate to display size if too long - msgLen = msg->message.size; - if (msgLen > MSG_MAX) { - msgLen = MSG_MAX; - } - for (ctr = 0; ctr < msgLen; ctr++) { - if (isprint(msg->message.bytes[ctr]) == false) { - canPrint = false; - break; - } - } - if (canPrint) { - typeIndicator = "Message Verified"; - strncpy(msgBuf, (char*)msg->message.bytes, MSG_MAX + 1); - msgBuf[MSG_MAX] = '\0'; - } else { - typeIndicator = "Bytes Verified"; - for (ctr = 0; ctr < msgLen / 2; ctr++) { - snprintf(&msgBuf[2 * ctr], 3, "%02x", msg->message.bytes[ctr]); - } - } - if (!confirm(ButtonRequestType_ButtonRequest_Other, _(typeIndicator), "%s", - msgBuf)) { + if (!confirm_bytes(ButtonRequestType_ButtonRequest_Other, + "Ethereum Message Verified", msg->message.bytes, + msg->message.size)) { fsm_sendFailure(FailureType_Failure_ActionCancelled, NULL); layoutHome(); return; @@ -305,6 +381,20 @@ void fsm_msgEthereumSignTypedHash(const EthereumSignTypedHash* msg) { CHECK_PIN + /* This endpoint receives only precomputed hashes, so the device cannot bind + * them to the typed data the host claims they represent. Treat it exactly + * like every other blind-signing path. */ + if (!ethereum_typed_hash_policy_allows( + storage_isPolicyEnabled("AdvancedMode"))) { + (void)review(ButtonRequestType_ButtonRequest_Other, "Blocked", + "Typed-hash signing requires AdvancedMode. " + "Enable in device settings."); + fsm_sendFailure(FailureType_Failure_ActionCancelled, + _("Typed-hash signing disabled by policy")); + layoutHome(); + return; + } + if (msg->domain_separator_hash.size != 32 || (msg->has_message_hash && msg->message_hash.size != 32)) { fsm_sendFailure(FailureType_Failure_Other, @@ -331,24 +421,40 @@ void fsm_msgEthereumSignTypedHash(const EthereumSignTypedHash* msg) { char str[64 + 1]; int ctr; - confirm(ButtonRequestType_ButtonRequest_Other, "Verify Address", - "Confirm address: %s", resp->address); + if (!confirm(ButtonRequestType_ButtonRequest_Other, "Verify Address", + "Confirm address: %s", resp->address)) { + fsm_sendFailure(FailureType_Failure_ActionCancelled, NULL); + layoutHome(); + return; + } for (ctr = 0; ctr < 64 / 2; ctr++) { snprintf(&str[2 * ctr], 3, "%02x", msg->domain_separator_hash.bytes[ctr]); } - confirm(ButtonRequestType_ButtonRequest_Other, "Typed Data domain", - "Confirm hash digest: %s", str); + if (!confirm(ButtonRequestType_ButtonRequest_Other, "Typed Data domain", + "Confirm hash digest: %s", str)) { + fsm_sendFailure(FailureType_Failure_ActionCancelled, NULL); + layoutHome(); + return; + } if (msg->has_message_hash) { for (ctr = 0; ctr < 64 / 2; ctr++) { snprintf(&str[2 * ctr], 3, "%02x", msg->message_hash.bytes[ctr]); } - confirm(ButtonRequestType_ButtonRequest_Other, "Typed Data message", - "Confirm hash digest: %s", str); + if (!confirm(ButtonRequestType_ButtonRequest_Other, "Typed Data message", + "Confirm hash digest: %s", str)) { + fsm_sendFailure(FailureType_Failure_ActionCancelled, NULL); + layoutHome(); + return; + } } else { - confirm(ButtonRequestType_ButtonRequest_Other, "Typed Data message", - "Confirm: No message"); + if (!confirm(ButtonRequestType_ButtonRequest_Other, "Typed Data message", + "Confirm: No message")) { + fsm_sendFailure(FailureType_Failure_ActionCancelled, NULL); + layoutHome(); + return; + } } ethereum_typed_hash_sign(msg, node, resp); @@ -372,16 +478,6 @@ void fsm_msgEthereum712TypesValues(Ethereum712TypesValues* msg) { msg->address_n_count, NULL); if (!node) return; - uint8_t pubkeyhash[20] = {0}; - if (!hdnode_get_ethereum_pubkeyhash(node, pubkeyhash)) { - layoutHome(); - return; - } - - resp->address[0] = '0'; - resp->address[1] = 'x'; - ethereum_address_checksum(pubkeyhash, resp->address + 2, false, 0); - e712_types_values(msg, resp, node); layoutHome(); diff --git a/lib/firmware/messagemap.def b/lib/firmware/messagemap.def index 8cd70d881..295cc441c 100644 --- a/lib/firmware/messagemap.def +++ b/lib/firmware/messagemap.def @@ -198,3 +198,7 @@ DEBUG_OUT(MessageType_MessageType_DebugLinkLog, DebugLinkLog, NO_PROCESS_FUNC) DEBUG_OUT(MessageType_MessageType_DebugLinkFlashDumpResponse, DebugLinkFlashDumpResponse, NO_PROCESS_FUNC) #endif + + MSG_IN(MessageType_MessageType_EthereumTxMetadata, EthereumTxMetadata, fsm_msgEthereumTxMetadata) + MSG_OUT(MessageType_MessageType_EthereumMetadataAck, EthereumMetadataAck, NO_PROCESS_FUNC) + MSG_IN(MessageType_MessageType_LoadClearsignSigner, LoadClearsignSigner, fsm_msgLoadClearsignSigner) diff --git a/unittests/firmware/CMakeLists.txt b/unittests/firmware/CMakeLists.txt index 691f2a832..f8df17d09 100644 --- a/unittests/firmware/CMakeLists.txt +++ b/unittests/firmware/CMakeLists.txt @@ -5,6 +5,7 @@ set(sources binance.cpp cosmos.cpp eos.cpp + eip712.cpp ethereum.cpp mayachain.cpp nano.cpp diff --git a/unittests/firmware/eip712.cpp b/unittests/firmware/eip712.cpp new file mode 100644 index 000000000..c1c8d4447 --- /dev/null +++ b/unittests/firmware/eip712.cpp @@ -0,0 +1,67 @@ +extern "C" { +#include "keepkey/firmware/eip712.h" +} + +#include "gtest/gtest.h" + +#include + +TEST(EIP712, AddressRequiresCanonicalTwentyByteHex) { + uint8_t encoded[32] = {0}; + ASSERT_EQ(SUCCESS, + encAddress("0x00112233445566778899aabbccddeeff00112233", encoded)); + for (size_t i = 0; i < 12; i++) EXPECT_EQ(0, encoded[i]); + EXPECT_EQ(0x00, encoded[12]); + EXPECT_EQ(0x11, encoded[13]); + EXPECT_EQ(0x33, encoded[31]); + + EXPECT_NE(SUCCESS, encAddress("0x112233", encoded)); + EXPECT_NE(SUCCESS, + encAddress("00112233445566778899aabbccddeeff00112233", encoded)); + EXPECT_NE(SUCCESS, + encAddress("0x00112233445566778899aabbccddeeff0011223g", encoded)); + EXPECT_NE(SUCCESS, encAddress("0x00112233445566778899aabbccddeeff0011223344", + encoded)); +} + +TEST(EIP712, DynamicBytesRequireCompleteHexOctets) { + uint8_t encoded[32] = {0}; + EXPECT_EQ(SUCCESS, encodeBytes("0x", encoded)); + EXPECT_EQ(SUCCESS, encodeBytes("0x00a1FF", encoded)); + EXPECT_NE(SUCCESS, encodeBytes("00a1", encoded)); + EXPECT_NE(SUCCESS, encodeBytes("0x0", encoded)); + EXPECT_NE(SUCCESS, encodeBytes("0x0z", encoded)); +} + +TEST(EIP712, FixedBytesRequireExactDeclaredLength) { + uint8_t encoded[32]; + memset(encoded, 0xa5, sizeof(encoded)); + ASSERT_EQ(SUCCESS, encodeBytesN("bytes4", "0x0011aAff", encoded)); + EXPECT_EQ(0x00, encoded[0]); + EXPECT_EQ(0x11, encoded[1]); + EXPECT_EQ(0xaa, encoded[2]); + EXPECT_EQ(0xff, encoded[3]); + for (size_t i = 4; i < sizeof(encoded); i++) EXPECT_EQ(0, encoded[i]); + + EXPECT_NE(SUCCESS, encodeBytesN("bytes4", "0x0011aa", encoded)); + EXPECT_NE(SUCCESS, encodeBytesN("bytes4", "0x0011aaff00", encoded)); + EXPECT_NE(SUCCESS, encodeBytesN("bytes0", "0x", encoded)); + EXPECT_NE(SUCCESS, encodeBytesN("bytes33", "0x", encoded)); + EXPECT_NE(SUCCESS, encodeBytesN("bytes4x", "0x0011aaff", encoded)); +} + +TEST(EIP712, MissingTypedValueFailsWithoutDereferencingNull) { + char types_json[] = + "{\"types\":{\"Mail\":[{\"name\":\"from\",\"type\":\"address\"}," + "{\"name\":\"note\",\"type\":\"string\"}]}}"; + char values_json[] = "{\"message\":{\"note\":\"hello\"}}"; + json_t type_nodes[16] = {}; + json_t value_nodes[8] = {}; + const json_t* types = json_create(types_json, type_nodes, 16); + const json_t* values = json_create(values_json, value_nodes, 8); + ASSERT_NE(nullptr, types); + ASSERT_NE(nullptr, values); + + uint8_t hash[32] = {}; + EXPECT_EQ(JSON_TYPE_WNOVAL, encode(types, values, "Mail", hash)); +} diff --git a/unittests/firmware/ethereum.cpp b/unittests/firmware/ethereum.cpp index f8329a416..83e67af23 100644 --- a/unittests/firmware/ethereum.cpp +++ b/unittests/firmware/ethereum.cpp @@ -1,11 +1,19 @@ extern "C" { +#include "keepkey/firmware/ethereum.h" +#include "keepkey/firmware/ethereum_contracts/zxappliquid.h" +#include "keepkey/firmware/ethereum_contracts/zxliquidtx.h" +#include "keepkey/firmware/ethereum_tokens.h" #include "trezor/crypto/address.h" } #include "gtest/gtest.h" +#include #include +bool kkconfirm_preload(int nYes, int nNo); +int kkconfirm_drain(void); + static uint8_t bin_from_ascii(char c) { if ('a' <= c && c <= 'f') return c - 'a' + 0xa; @@ -16,7 +24,7 @@ static uint8_t bin_from_ascii(char c) { __builtin_unreachable(); } -static void test_checksum(const std::string &addr) { +static void test_checksum(const std::string& addr) { uint8_t addr_bin[20]; for (size_t i = 0; i < addr.size(); i += 2) { addr_bin[i / 2] = bin_from_ascii(addr[i + 1]) | bin_from_ascii(addr[i]) @@ -38,3 +46,209 @@ TEST(Ethereum, AddressChecksum) { test_checksum("dbF03B407c01E7cD3CBea99509d93f8DDDC8C6FB"); test_checksum("D1220A0cf47c7B9Be7A2E6BA89F429762e7b9aDb"); } + +TEST(Ethereum, TypedHashSigningRequiresAdvancedMode) { + EXPECT_FALSE(ethereum_typed_hash_policy_allows(false)); + EXPECT_TRUE(ethereum_typed_hash_policy_allows(true)); +} + +TEST(Ethereum, DomainOnlyPrimaryTypeRequiresExactMatch) { + EXPECT_TRUE(ethereum_eip712_is_domain_primary_type("EIP712Domain")); + EXPECT_FALSE(ethereum_eip712_is_domain_primary_type("EIP")); + EXPECT_FALSE(ethereum_eip712_is_domain_primary_type("EIP712Domain[]")); + EXPECT_FALSE(ethereum_eip712_is_domain_primary_type("")); + EXPECT_FALSE(ethereum_eip712_is_domain_primary_type(nullptr)); +} + +static const uint8_t DAI_MAINNET_ADDRESS[20] = { + 0x6b, 0x17, 0x54, 0x74, 0xe8, 0x90, 0x94, 0xc4, 0x4d, 0xa9, + 0x8b, 0x95, 0x4e, 0xed, 0xea, 0xc4, 0x95, 0x27, 0x1d, 0x0f}; +static const uint8_t USDC_MAINNET_ADDRESS[20] = { + 0xa0, 0xb8, 0x69, 0x91, 0xc6, 0x21, 0x8b, 0x36, 0xc1, 0xd1, + 0x9d, 0x4a, 0x2e, 0x9e, 0xb0, 0xce, 0x36, 0x06, 0xeb, 0x48}; + +static EthereumSignTx liquidity_tx( + bool known_token, bool add = true, + const uint8_t* token_address = DAI_MAINNET_ADDRESS) { + EthereumSignTx msg; + memset(&msg, 0, sizeof(msg)); + msg.has_chain_id = true; + msg.chain_id = 1; + msg.has_to = true; + msg.to.size = 20; + memcpy(msg.to.bytes, UNISWAP_ROUTER_ADDRESS, 20); + msg.has_data_initial_chunk = true; + msg.data_initial_chunk.size = 4 + 6 * 32; + memcpy(msg.data_initial_chunk.bytes, + add ? "\xf3\x05\xd7\x19" : "\x02\x75\x1c\xec", 4); + + const TokenType* token = tokenByChainAddress(1, token_address); + EXPECT_NE(UnknownToken, token); + if (token == UnknownToken) return msg; + uint8_t unknown[20]; + memset(unknown, 0xa5, sizeof(unknown)); + memcpy( + msg.data_initial_chunk.bytes + 4 + 32 - 20, + known_token ? reinterpret_cast(token->address) : unknown, + 20); + + // Token desired/minimum and native minimum. + msg.data_initial_chunk.bytes[4 + 2 * 32 - 1] = 1; + msg.data_initial_chunk.bytes[4 + 3 * 32 - 1] = 1; + msg.data_initial_chunk.bytes[4 + 4 * 32 - 1] = 1; + // Recipient and deadline. + memset(msg.data_initial_chunk.bytes + 4 + 5 * 32 - 20, 0x11, 20); + msg.data_initial_chunk.bytes[4 + 6 * 32 - 1] = 1; + msg.has_value = true; + if (add) { + msg.value.size = 1; + msg.value.bytes[0] = 1; + } + return msg; +} + +static void set_word_u64(EthereumSignTx& msg, size_t word, uint64_t value) { + uint8_t* out = msg.data_initial_chunk.bytes + 4 + word * 32; + memset(out, 0, 32); + for (size_t i = 0; i < 8; i++) { + out[31 - i] = static_cast(value); + value >>= 8; + } +} + +static EthereumSignTx approve_liquidity_tx() { + EthereumSignTx msg; + memset(&msg, 0, sizeof(msg)); + msg.has_chain_id = true; + msg.chain_id = 1; + msg.has_to = true; + msg.to.size = 20; + // Canonical mainnet DAI/WETH Uniswap V2 pair. + const uint8_t pair[20] = {0xa4, 0x78, 0xc2, 0x97, 0x5a, 0xb1, 0xea, + 0x89, 0xe8, 0x19, 0x68, 0x11, 0xf5, 0x1a, + 0x7b, 0x7a, 0xde, 0x33, 0xeb, 0x11}; + memcpy(msg.to.bytes, pair, sizeof(pair)); + msg.has_data_initial_chunk = true; + msg.data_initial_chunk.size = 4 + 2 * 32; + memcpy(msg.data_initial_chunk.bytes, "\x09\x5e\xa7\xb3", 4); + memcpy(msg.data_initial_chunk.bytes + 4 + 12, UNISWAP_ROUTER_ADDRESS, 20); + msg.data_initial_chunk.bytes[4 + 2 * 32 - 1] = 1; + msg.has_value = true; + return msg; +} + +TEST(Ethereum, LiquiditySelectorChecksDeclaredCalldataLength) { + EthereumSignTx msg; + memset(&msg, 0, sizeof(msg)); + msg.has_to = true; + msg.to.size = 20; + memcpy(msg.to.bytes, UNISWAP_ROUTER_ADDRESS, 20); + msg.has_data_initial_chunk = true; + msg.data_initial_chunk.size = 3; + memcpy(msg.data_initial_chunk.bytes, "\xf3\x05\xd7", 3); + EXPECT_FALSE(zx_isZxLiquidTx(&msg)); + + msg.data_initial_chunk.size = 4; + memcpy(msg.data_initial_chunk.bytes, "\x09\x5e\xa7\xb3", 4); + EXPECT_FALSE(zx_isZxApproveLiquid(&msg)); + + msg.data_initial_chunk.size = 4 + 2 * 32 + 1; + memcpy(msg.data_initial_chunk.bytes, "\x09\x5e\xa7\xb3", 4); + memcpy(msg.data_initial_chunk.bytes + 4 + 32 - 20, UNISWAP_ROUTER_ADDRESS, + 20); + EXPECT_FALSE(zx_isZxApproveLiquid(&msg)); + + msg.data_initial_chunk.size = 4 + 6 * 32 + 1; + memcpy(msg.data_initial_chunk.bytes, "\xf3\x05\xd7\x19", 4); + EXPECT_FALSE(zx_isZxLiquidTx(&msg)); +} + +TEST(Ethereum, LiquidityCancellationFailsClosed) { + EthereumSignTx msg = liquidity_tx(true); + ASSERT_TRUE(kkconfirm_preload(0, 1)); + EXPECT_FALSE(zx_confirmZxLiquidTx(msg.data_initial_chunk.size, &msg)); + EXPECT_EQ(0, kkconfirm_drain()); +} + +TEST(Ethereum, LiquidityRejectsUnknownTokenBeforeConfirmation) { + EthereumSignTx msg = liquidity_tx(false); + EXPECT_FALSE(zx_confirmZxLiquidTx(msg.data_initial_chunk.size, &msg)); +} + +TEST(Ethereum, LiquidityClearSigningIsMainnetOnly) { + EthereumSignTx msg = liquidity_tx(true); + EXPECT_TRUE(zx_isZxLiquidTx(&msg)); + + msg.chain_id = 137; + EXPECT_FALSE(zx_isZxLiquidTx(&msg)); + msg.chain_id = 1; + msg.has_chain_id = false; + EXPECT_FALSE(zx_isZxLiquidTx(&msg)); +} + +TEST(Ethereum, LiquidityRejectsTruncatedDeadlineAndNoncanonicalAddresses) { + EthereumSignTx msg = liquidity_tx(true); + msg.data_initial_chunk.bytes[4 + 5 * 32] = 1; + EXPECT_FALSE(zx_isZxLiquidTx(&msg)); + EXPECT_FALSE(zx_confirmZxLiquidTx(msg.data_initial_chunk.size, &msg)); + + msg = liquidity_tx(true); + msg.data_initial_chunk.bytes[4] = 1; + EXPECT_FALSE(zx_isZxLiquidTx(&msg)); + + msg = liquidity_tx(true); + msg.data_initial_chunk.bytes[4 + 4 * 32] = 1; + EXPECT_FALSE(zx_isZxLiquidTx(&msg)); +} + +TEST(Ethereum, RemoveLiquidityRejectsNativeValue) { + EthereumSignTx msg = liquidity_tx(true, false); + EXPECT_TRUE(zx_isZxLiquidTx(&msg)); + msg.value.size = 1; + msg.value.bytes[0] = 1; + EXPECT_FALSE(zx_isZxLiquidTx(&msg)); +} + +TEST(Ethereum, RemoveLiquidityFormatsPrimaryAmountAsLpTokens) { + EthereumSignTx add = liquidity_tx(true, true, USDC_MAINNET_ADDRESS); + set_word_u64(add, 1, UINT64_C(1000000000000000000)); + char formatted[96]; + ASSERT_TRUE( + zx_formatZxLiquidityPrimaryAmount(&add, formatted, sizeof(formatted))); + EXPECT_STREQ("1000000000000 USDC", formatted); + + EthereumSignTx remove = liquidity_tx(true, false, USDC_MAINNET_ADDRESS); + set_word_u64(remove, 1, UINT64_C(1000000000000000000)); + ASSERT_TRUE( + zx_formatZxLiquidityPrimaryAmount(&remove, formatted, sizeof(formatted))); + EXPECT_STREQ("1 LP", formatted); +} + +TEST(Ethereum, LiquidityFormatsFullUint256WithoutBlankConfirmation) { + EthereumSignTx msg = liquidity_tx(true); + memset(msg.data_initial_chunk.bytes + 4 + 32, 0xff, 32); + char formatted[96]; + ASSERT_TRUE( + zx_formatZxLiquidityPrimaryAmount(&msg, formatted, sizeof(formatted))); + EXPECT_GT(strlen(formatted), 32u); + + ASSERT_TRUE(kkconfirm_preload(0, 1)); + EXPECT_FALSE(zx_confirmZxLiquidTx(msg.data_initial_chunk.size, &msg)); + EXPECT_EQ(0, kkconfirm_drain()); +} + +TEST(Ethereum, LpApprovalRequiresMainnetDerivedPairAndCanonicalSpender) { + EthereumSignTx msg = approve_liquidity_tx(); + EXPECT_TRUE(zx_isZxApproveLiquid(&msg)); + + msg.to.bytes[0] ^= 1; + EXPECT_FALSE(zx_isZxApproveLiquid(&msg)); + + msg = approve_liquidity_tx(); + msg.chain_id = 137; + EXPECT_FALSE(zx_isZxApproveLiquid(&msg)); + + msg = approve_liquidity_tx(); + msg.data_initial_chunk.bytes[4] = 1; + EXPECT_FALSE(zx_isZxApproveLiquid(&msg)); +}