From 48562ccfc7e88372bbe3669424a8d36f76999d59 Mon Sep 17 00:00:00 2001 From: pscott Date: Thu, 15 Apr 2021 15:25:46 +0200 Subject: [PATCH 01/23] Add TransactionType and adapt parser to parse them --- src_common/ethUstream.c | 7 +++++++ src_common/ethUstream.h | 7 +++++++ 2 files changed, 14 insertions(+) diff --git a/src_common/ethUstream.c b/src_common/ethUstream.c index 9ddf672..a97a418 100644 --- a/src_common/ethUstream.c +++ b/src_common/ethUstream.c @@ -263,7 +263,14 @@ static parserStatus_e processTxInternal(txContext_t *context) { if (context->commandLength == 0) { return USTREAM_PROCESSING; } + // EIP 2718: TransactionType might be present before the TransactionPayload. + if (*context->workBuffer > 0x00 && *context->workBuffer < 0x7f) { + context->txType = *context->workBuffer; + context->workBuffer++; + PRINTF("TX TYPE: %u\n", context->txType); + } if (!context->processingField) { + PRINTF("PROCESSING FIELD\n"); bool canDecode = false; uint32_t offset; while (context->commandLength != 0) { diff --git a/src_common/ethUstream.h b/src_common/ethUstream.h index ff5c28e..24cbaa0 100644 --- a/src_common/ethUstream.h +++ b/src_common/ethUstream.h @@ -53,6 +53,12 @@ typedef enum rlpTxField_e { TX_RLP_DONE } rlpTxField_e; +// EIP 2718 TransactionType +typedef enum txType_e { + LegacyTransaction = 1, + BerlinTransaction = 2 +} txType_e; + typedef enum parserStatus_e { USTREAM_PROCESSING, USTREAM_SUSPENDED, @@ -93,6 +99,7 @@ typedef struct txContext_t { ustreamProcess_t customProcessor; txContent_t *content; void *extra; + uint8_t txType; } txContext_t; void initTx(txContext_t *context, From 3faa172ec2748f655c64444c624a0b08739045ad Mon Sep 17 00:00:00 2001 From: pscott Date: Thu, 15 Apr 2021 15:36:21 +0200 Subject: [PATCH 02/23] Add MAX and MIN tx types --- src_common/ethUstream.c | 6 +++++- src_common/ethUstream.h | 6 ++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src_common/ethUstream.c b/src_common/ethUstream.c index a97a418..18cd896 100644 --- a/src_common/ethUstream.c +++ b/src_common/ethUstream.c @@ -265,9 +265,13 @@ static parserStatus_e processTxInternal(txContext_t *context) { } // EIP 2718: TransactionType might be present before the TransactionPayload. if (*context->workBuffer > 0x00 && *context->workBuffer < 0x7f) { + PRINTF("TX TYPE: %u\n", context->txType); + if (*context->workBuffer < MIN_TX_TYPE || *context->workBuffer > MAX_TX_TYPE ) { + PRINTF("Transaction type not supported\n"); + return USTREAM_FAULT; + } context->txType = *context->workBuffer; context->workBuffer++; - PRINTF("TX TYPE: %u\n", context->txType); } if (!context->processingField) { PRINTF("PROCESSING FIELD\n"); diff --git a/src_common/ethUstream.h b/src_common/ethUstream.h index 24cbaa0..cd30835 100644 --- a/src_common/ethUstream.h +++ b/src_common/ethUstream.h @@ -55,8 +55,10 @@ typedef enum rlpTxField_e { // EIP 2718 TransactionType typedef enum txType_e { - LegacyTransaction = 1, - BerlinTransaction = 2 + MIN_TX_TYPE, + LEGACY_TX, + BERLIN_TX, + MAX_TX_TYPE, } txType_e; typedef enum parserStatus_e { From 9c20c54013ffe34477cfa6d31c171b2b1b200aba Mon Sep 17 00:00:00 2001 From: pscott Date: Thu, 15 Apr 2021 15:37:26 +0200 Subject: [PATCH 03/23] Apply clang format --- src_common/ethUstream.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src_common/ethUstream.c b/src_common/ethUstream.c index 18cd896..fd6cc2e 100644 --- a/src_common/ethUstream.c +++ b/src_common/ethUstream.c @@ -266,7 +266,7 @@ static parserStatus_e processTxInternal(txContext_t *context) { // EIP 2718: TransactionType might be present before the TransactionPayload. if (*context->workBuffer > 0x00 && *context->workBuffer < 0x7f) { PRINTF("TX TYPE: %u\n", context->txType); - if (*context->workBuffer < MIN_TX_TYPE || *context->workBuffer > MAX_TX_TYPE ) { + if (*context->workBuffer < MIN_TX_TYPE || *context->workBuffer > MAX_TX_TYPE) { PRINTF("Transaction type not supported\n"); return USTREAM_FAULT; } From 65ac116e911722160b3e1632cde52b1318fd0420 Mon Sep 17 00:00:00 2001 From: pscott Date: Thu, 15 Apr 2021 15:38:38 +0200 Subject: [PATCH 04/23] Restrict txType to be within enum bounds --- src_common/ethUstream.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src_common/ethUstream.c b/src_common/ethUstream.c index fd6cc2e..20d96d8 100644 --- a/src_common/ethUstream.c +++ b/src_common/ethUstream.c @@ -266,7 +266,7 @@ static parserStatus_e processTxInternal(txContext_t *context) { // EIP 2718: TransactionType might be present before the TransactionPayload. if (*context->workBuffer > 0x00 && *context->workBuffer < 0x7f) { PRINTF("TX TYPE: %u\n", context->txType); - if (*context->workBuffer < MIN_TX_TYPE || *context->workBuffer > MAX_TX_TYPE) { + if (*context->workBuffer <= MIN_TX_TYPE || *context->workBuffer >= MAX_TX_TYPE) { PRINTF("Transaction type not supported\n"); return USTREAM_FAULT; } From 9e6610c56a2c7bc18c01668f6c9f84c7e569e362 Mon Sep 17 00:00:00 2001 From: pscott Date: Thu, 15 Apr 2021 15:42:46 +0200 Subject: [PATCH 05/23] Fix debug messages --- src_common/ethUstream.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src_common/ethUstream.c b/src_common/ethUstream.c index 20d96d8..023dc45 100644 --- a/src_common/ethUstream.c +++ b/src_common/ethUstream.c @@ -265,7 +265,7 @@ static parserStatus_e processTxInternal(txContext_t *context) { } // EIP 2718: TransactionType might be present before the TransactionPayload. if (*context->workBuffer > 0x00 && *context->workBuffer < 0x7f) { - PRINTF("TX TYPE: %u\n", context->txType); + PRINTF("TX TYPE: %u\n", *context->workBuffer); if (*context->workBuffer <= MIN_TX_TYPE || *context->workBuffer >= MAX_TX_TYPE) { PRINTF("Transaction type not supported\n"); return USTREAM_FAULT; @@ -274,7 +274,6 @@ static parserStatus_e processTxInternal(txContext_t *context) { context->workBuffer++; } if (!context->processingField) { - PRINTF("PROCESSING FIELD\n"); bool canDecode = false; uint32_t offset; while (context->commandLength != 0) { From 9876999b8d49174c08e086093c6e8a3800f45766 Mon Sep 17 00:00:00 2001 From: pscott Date: Thu, 15 Apr 2021 16:13:42 +0200 Subject: [PATCH 06/23] Enumerate through variants --- src_common/ethUstream.c | 12 ++++++++---- src_common/ethUstream.h | 3 --- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src_common/ethUstream.c b/src_common/ethUstream.c index 023dc45..517691e 100644 --- a/src_common/ethUstream.c +++ b/src_common/ethUstream.c @@ -265,13 +265,17 @@ static parserStatus_e processTxInternal(txContext_t *context) { } // EIP 2718: TransactionType might be present before the TransactionPayload. if (*context->workBuffer > 0x00 && *context->workBuffer < 0x7f) { - PRINTF("TX TYPE: %u\n", *context->workBuffer); - if (*context->workBuffer <= MIN_TX_TYPE || *context->workBuffer >= MAX_TX_TYPE) { + uint8_t maybeType = *context->workBuffer; + PRINTF("TX TYPE: %u\n", maybeType); + + // Enumerate through all supported txTypes here... + if (maybeType == LEGACY_TX) { + context->txType = *context->workBuffer; + context->workBuffer++; + } else { PRINTF("Transaction type not supported\n"); return USTREAM_FAULT; } - context->txType = *context->workBuffer; - context->workBuffer++; } if (!context->processingField) { bool canDecode = false; diff --git a/src_common/ethUstream.h b/src_common/ethUstream.h index cd30835..1b9bf2e 100644 --- a/src_common/ethUstream.h +++ b/src_common/ethUstream.h @@ -55,10 +55,7 @@ typedef enum rlpTxField_e { // EIP 2718 TransactionType typedef enum txType_e { - MIN_TX_TYPE, LEGACY_TX, - BERLIN_TX, - MAX_TX_TYPE, } txType_e; typedef enum parserStatus_e { From 99950a396c37be4f314cc7dfcf3293b4f8a01066 Mon Sep 17 00:00:00 2001 From: pscott Date: Thu, 15 Apr 2021 16:19:12 +0200 Subject: [PATCH 07/23] Add max and min values and restrict bounds --- src_common/ethUstream.c | 2 +- src_common/ethUstream.h | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src_common/ethUstream.c b/src_common/ethUstream.c index 517691e..5d404fe 100644 --- a/src_common/ethUstream.c +++ b/src_common/ethUstream.c @@ -264,7 +264,7 @@ static parserStatus_e processTxInternal(txContext_t *context) { return USTREAM_PROCESSING; } // EIP 2718: TransactionType might be present before the TransactionPayload. - if (*context->workBuffer > 0x00 && *context->workBuffer < 0x7f) { + if (*context->workBuffer >= MIN_TX_TYPE && *context->workBuffer <= MAX_TX_TYPE) { uint8_t maybeType = *context->workBuffer; PRINTF("TX TYPE: %u\n", maybeType); diff --git a/src_common/ethUstream.h b/src_common/ethUstream.h index 1b9bf2e..05921bb 100644 --- a/src_common/ethUstream.h +++ b/src_common/ethUstream.h @@ -53,6 +53,9 @@ typedef enum rlpTxField_e { TX_RLP_DONE } rlpTxField_e; +#define MIN_TX_TYPE 0x00 +#define MAX_TX_TYPE 0x7f + // EIP 2718 TransactionType typedef enum txType_e { LEGACY_TX, From 01fd39ea96ced88c7122313574fbe234527b7119 Mon Sep 17 00:00:00 2001 From: pscott Date: Thu, 15 Apr 2021 16:20:39 +0200 Subject: [PATCH 08/23] Add LEGACY_TX --- src_common/ethUstream.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src_common/ethUstream.h b/src_common/ethUstream.h index 05921bb..34bd413 100644 --- a/src_common/ethUstream.h +++ b/src_common/ethUstream.h @@ -58,7 +58,7 @@ typedef enum rlpTxField_e { // EIP 2718 TransactionType typedef enum txType_e { - LEGACY_TX, + LEGACY_TX = 1, } txType_e; typedef enum parserStatus_e { From 6ccc36fa39f81e95a4c2d7f76934f559de366002 Mon Sep 17 00:00:00 2001 From: pscott Date: Thu, 15 Apr 2021 17:08:09 +0200 Subject: [PATCH 09/23] Move txType parsing outside of forloop --- src_common/ethUstream.c | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/src_common/ethUstream.c b/src_common/ethUstream.c index 5d404fe..2bd36a6 100644 --- a/src_common/ethUstream.c +++ b/src_common/ethUstream.c @@ -249,6 +249,22 @@ static void processV(txContext_t *context) { } static parserStatus_e processTxInternal(txContext_t *context) { + // EIP 2718: TransactionType might be present before the TransactionPayload. + uint8_t txType = *context->workBuffer; + if (txType >= MIN_TX_TYPE && txType <= MAX_TX_TYPE) { + PRINTF("TX TYPE: %u\n", txType); + + // Enumerate through all supported txTypes here... + if (txType == LEGACY_TX) { + context->txType = txType; + context->workBuffer++; + } else { + PRINTF("Transaction type not supported\n"); + return USTREAM_FAULT; + } + } + + // Parse the TransactionPayload. for (;;) { customStatus_e customStatus = CUSTOM_NOT_HANDLED; // EIP 155 style transasction @@ -263,20 +279,7 @@ static parserStatus_e processTxInternal(txContext_t *context) { if (context->commandLength == 0) { return USTREAM_PROCESSING; } - // EIP 2718: TransactionType might be present before the TransactionPayload. - if (*context->workBuffer >= MIN_TX_TYPE && *context->workBuffer <= MAX_TX_TYPE) { - uint8_t maybeType = *context->workBuffer; - PRINTF("TX TYPE: %u\n", maybeType); - // Enumerate through all supported txTypes here... - if (maybeType == LEGACY_TX) { - context->txType = *context->workBuffer; - context->workBuffer++; - } else { - PRINTF("Transaction type not supported\n"); - return USTREAM_FAULT; - } - } if (!context->processingField) { bool canDecode = false; uint32_t offset; From 5dd99c3d48b252e719dcbdf7126d0721cd52070a Mon Sep 17 00:00:00 2001 From: pscott Date: Thu, 15 Apr 2021 17:59:41 +0200 Subject: [PATCH 10/23] Move TxType parsing in cmd_signTx and add 0x6501 error --- doc/ethapp.asc | 1 + src_common/ethUstream.c | 18 +----------------- src_features/signTx/cmd_signTx.c | 16 ++++++++++++++++ 3 files changed, 18 insertions(+), 17 deletions(-) diff --git a/doc/ethapp.asc b/doc/ethapp.asc index d57e13c..35ef221 100644 --- a/doc/ethapp.asc +++ b/doc/ethapp.asc @@ -473,6 +473,7 @@ The following standard Status Words are returned for all APDUs - some specific S [width="80%"] |=============================================================================================== | *SW* | *Description* +| 6501 | TransactionType not supported | 6700 | Incorrect length | 6982 | Security status not satisfied (Canceled by user) | 6A80 | Invalid data diff --git a/src_common/ethUstream.c b/src_common/ethUstream.c index 2bd36a6..6cf92d9 100644 --- a/src_common/ethUstream.c +++ b/src_common/ethUstream.c @@ -249,25 +249,9 @@ static void processV(txContext_t *context) { } static parserStatus_e processTxInternal(txContext_t *context) { - // EIP 2718: TransactionType might be present before the TransactionPayload. - uint8_t txType = *context->workBuffer; - if (txType >= MIN_TX_TYPE && txType <= MAX_TX_TYPE) { - PRINTF("TX TYPE: %u\n", txType); - - // Enumerate through all supported txTypes here... - if (txType == LEGACY_TX) { - context->txType = txType; - context->workBuffer++; - } else { - PRINTF("Transaction type not supported\n"); - return USTREAM_FAULT; - } - } - - // Parse the TransactionPayload. for (;;) { customStatus_e customStatus = CUSTOM_NOT_HANDLED; - // EIP 155 style transasction + // EIP 155 style transaction if (context->currentField == TX_RLP_DONE) { return USTREAM_FINISHED; } diff --git a/src_features/signTx/cmd_signTx.c b/src_features/signTx/cmd_signTx.c index e721d06..d08c528 100644 --- a/src_features/signTx/cmd_signTx.c +++ b/src_features/signTx/cmd_signTx.c @@ -40,6 +40,22 @@ void handleSign(uint8_t p1, } dataPresent = false; dataContext.tokenContext.pluginAvailable = 0; + + // EIP 2718: TransactionType might be present before the TransactionPayload. + uint8_t txType = *workBuffer; + if (txType >= MIN_TX_TYPE && txType <= MAX_TX_TYPE) { + PRINTF("Transaction type: %u\n", txType); + + // Enumerate through all supported txTypes here... + if (txType == LEGACY_TX) { + txContext.txType = txType; + workBuffer++; + dataLength--; + } else { + PRINTF("Transaction type not supported\n"); + THROW(0x6501); + } + } initTx(&txContext, &global_sha3, &tmpContent.txContent, customProcessor, NULL); } else if (p1 != P1_MORE) { THROW(0x6B00); From 970f0355dd4adc976e7342d16f33e6ed9fc0b795 Mon Sep 17 00:00:00 2001 From: pscott Date: Wed, 21 Apr 2021 16:56:17 +0200 Subject: [PATCH 11/23] Add support for EIP2718 (enveloppe) and EIP2930 (acess list tx); Display chain ID when different from 1 (ethereum mainnet) --- Makefile | 7 + doc/ethapp.asc | 1 + src/chainConfig.h | 2 + src/eth_plugin_handler.c | 1 + src/main.c | 4 + src/shared_context.h | 4 +- src/utils.c | 66 ++++-- src/utils.h | 6 +- src_common/ethUstream.c | 292 ++++++++++++++++++------- src_common/ethUstream.h | 71 ++++-- src_features/signTx/cmd_signTx.c | 8 +- src_features/signTx/logic_signTx.c | 45 +++- src_features/signTx/ui_common_signTx.c | 2 +- src_features/signTx/ui_flow_signTx.c | 56 +++-- 14 files changed, 419 insertions(+), 146 deletions(-) diff --git a/Makefile b/Makefile index df9c25a..203dcb9 100755 --- a/Makefile +++ b/Makefile @@ -251,6 +251,13 @@ else DEFINES += IO_SEPROXYHAL_BUFFER_SIZE_B=72 endif +# Enables direct data signing without having to specify it in the settings. Useful when testing with speculos. +ALLOW_DATA:=0 +ifneq ($(ALLOW_DATA),0) +DEFINES += HAVE_ALLOW_DATA +endif + + # Enabling debug PRINTF DEBUG:=0 ifneq ($(DEBUG),0) diff --git a/doc/ethapp.asc b/doc/ethapp.asc index 35ef221..ac716dd 100644 --- a/doc/ethapp.asc +++ b/doc/ethapp.asc @@ -474,6 +474,7 @@ The following standard Status Words are returned for all APDUs - some specific S |=============================================================================================== | *SW* | *Description* | 6501 | TransactionType not supported +| 6502 | Not enough space for conversion from uint32_t to string | 6700 | Incorrect length | 6982 | Security status not satisfied (Canceled by user) | 6A80 | Invalid data diff --git a/src/chainConfig.h b/src/chainConfig.h index db38a3e..2172b0b 100644 --- a/src/chainConfig.h +++ b/src/chainConfig.h @@ -61,4 +61,6 @@ typedef struct chain_config_s { chain_kind_t kind; } chain_config_t; +#define ETHEREUM_MAINNET_CHAINID 1 + #endif /* _CHAIN_CONFIG_H_ */ diff --git a/src/eth_plugin_handler.c b/src/eth_plugin_handler.c index ccaa86e..d9f3d5d 100644 --- a/src/eth_plugin_handler.c +++ b/src/eth_plugin_handler.c @@ -59,6 +59,7 @@ void eth_plugin_prepare_query_contract_UI(ethQueryContractUI_t *queryContractUI, int eth_plugin_perform_init(uint8_t *contractAddress, ethPluginInitContract_t *init) { uint8_t i; const uint8_t **selectors; + return 0; dataContext.tokenContext.pluginAvailable = 0; // Handle hardcoded plugin list PRINTF("Selector %.*H\n", 4, init->selector); diff --git a/src/main.c b/src/main.c index c6e5898..c7f699e 100644 --- a/src/main.c +++ b/src/main.c @@ -714,7 +714,11 @@ void coin_main(chain_config_t *coin_config) { if (N_storage.initialized != 0x01) { internalStorage_t storage; +#ifdef HAVE_ALLOW_DATA + storage.dataAllowed = 0x01; +#else storage.dataAllowed = 0x00; +#endif storage.contractDetails = 0x00; storage.displayNonce = 0x00; storage.initialized = 0x01; diff --git a/src/shared_context.h b/src/shared_context.h index 63db781..7da81b4 100644 --- a/src/shared_context.h +++ b/src/shared_context.h @@ -158,7 +158,9 @@ typedef struct txStringProperties_t { char fullAddress[43]; char fullAmount[50]; char maxFee[50]; - char nonce[8]; // 10M tx per account ought to be enough for everybody + char nonce[8]; // 10M tx per account ought to be enough for everybody + char chainID[8]; // 10M different chainID ought to be enough for people to find a unique + // chainID for their token / chain. } txStringProperties_t; typedef struct strDataTmp_t { diff --git a/src/utils.c b/src/utils.c index 43ccc61..0b7d583 100644 --- a/src/utils.c +++ b/src/utils.c @@ -53,22 +53,58 @@ int local_strchr(char *string, char ch) { return -1; } -uint32_t getV(txContent_t *txContent) { - uint32_t v = 0; - if (txContent->vLength == 1) { - v = txContent->v[0]; - } else if (txContent->vLength == 2) { - v = (txContent->v[0] << 8) | txContent->v[1]; - } else if (txContent->vLength == 3) { - v = (txContent->v[0] << 16) | (txContent->v[1] << 8) | txContent->v[2]; - } else if (txContent->vLength == 4) { - v = (txContent->v[0] << 24) | (txContent->v[1] << 16) | (txContent->v[2] << 8) | - txContent->v[3]; - } else if (txContent->vLength != 0) { - PRINTF("Unexpected v format\n"); - THROW(EXCEPTION); +// Almost like U4BE except that it takes `size` as a parameter. +uint32_t u32_from_BE(uint8_t *in, uint8_t size) { + uint32_t res = 0; + if (size == 1) { + res = in[0]; + } else if (size == 2) { + res = (in[0] << 8) | in[1]; + } else if (size == 3) { + res = (in[0] << 16) | (in[1] << 8) | in[2]; + } else { + res = (in[0] << 24) | (in[1] << 16) | (in[2] << 8) | in[3]; + } + return res; +} + +// Converts a uint32_t to a string. +void u32_to_str(char *dest, uint32_t in, uint8_t dest_size) { + uint8_t i = 0; + + // Get the first digit (in case it's 0). + dest[i] = in % 10 + '0'; + in /= 10; + i++; + + // Get every digit. + while (in != 0) { + if (i >= dest_size) { + THROW(6502); + } + dest[i] = in % 10 + '0'; + in /= 10; + i++; + } + + // Null terminate the string. + dest[i] = '\0'; + i--; + + // Reverse the string + uint8_t end = i; + char tmp; + i = 0; + while (i < end) { + // Swap the first and last elements. + tmp = dest[i]; + dest[i] = dest[end]; + dest[end] = tmp; + + // Decrease the interval size. + i++; + end--; } - return v; } void amountToString(uint8_t *amount, diff --git a/src/utils.h b/src/utils.h index 46dfa5d..582fb62 100644 --- a/src/utils.h +++ b/src/utils.h @@ -28,7 +28,11 @@ void convertUint256BE(uint8_t* data, uint32_t length, uint256_t* target); int local_strchr(char* string, char ch); -uint32_t getV(txContent_t* txContent); +// `itoa` for uint32_t. +void u32_to_str(char* dest, uint8_t dest_size, uint32_t in); + +// Converts a list of bytes (in BE) of length `size` to a uint32_t. +uint32_t u32_from_BE(uint8_t* in, uint8_t size); void amountToString(uint8_t* amount, uint8_t amount_len, diff --git a/src_common/ethUstream.c b/src_common/ethUstream.c index 6cf92d9..22afedc 100644 --- a/src_common/ethUstream.c +++ b/src_common/ethUstream.c @@ -20,6 +20,7 @@ #include "ethUstream.h" #include "ethUtils.h" +#include "utils.h" #define MAX_INT256 32 #define MAX_ADDRESS 20 @@ -30,12 +31,15 @@ void initTx(txContext_t *context, txContent_t *content, ustreamProcess_t customProcessor, void *extra) { + uint8_t save = context->txType; + memset(context, 0, sizeof(txContext_t)); + context->txType = save; context->sha3 = sha3; context->content = content; context->customProcessor = customProcessor; context->extra = extra; - context->currentField = TX_RLP_CONTENT; + context->currentField = RLP_NONE + 1; cx_keccak_init(context->sha3, 256); } @@ -86,6 +90,22 @@ static void processContent(txContext_t *context) { context->processingField = false; } +static void processAccessList(txContext_t *context) { + if (!context->currentFieldIsList) { + PRINTF("Invalid type for RLP_DATA\n"); + THROW(EXCEPTION); + } + if (context->currentFieldPos < context->currentFieldLength) { + uint32_t copySize = + MIN(context->commandLength, context->currentFieldLength - context->currentFieldPos); + copyTxData(context, NULL, copySize); + } + if (context->currentFieldPos == context->currentFieldLength) { + context->currentField++; + context->processingField = false; + } +} + static void processType(txContext_t *context) { if (context->currentFieldIsList) { PRINTF("Invalid type for RLP_TYPE\n"); @@ -106,6 +126,28 @@ static void processType(txContext_t *context) { } } +static void processChainID(txContext_t *context) { + if (context->currentFieldIsList) { + PRINTF("Invalid type for RLP_CHAINID\n"); + THROW(EXCEPTION); + } + if (context->currentFieldLength > MAX_INT256) { + PRINTF("Invalid length for RLP_CHAINID\n"); + THROW(EXCEPTION); + } + if (context->currentFieldPos < context->currentFieldLength) { + uint32_t copySize = + MIN(context->commandLength, context->currentFieldLength - context->currentFieldPos); + copyTxData(context, context->content->chainID.value, copySize); + } + if (context->currentFieldPos == context->currentFieldLength) { + context->content->chainID.length = context->currentFieldLength; + context->currentField++; + context->processingField = false; + } + PRINTF("chainID: %.*H\n", context->content->chainID.length, context->content->chainID.value); +} + static void processNonce(txContext_t *context) { if (context->currentFieldIsList) { PRINTF("Invalid type for RLP_NONCE\n"); @@ -148,6 +190,11 @@ static void processStartGas(txContext_t *context) { } } +// Alias over `processStartGas()`. +static void processGasLimit(txContext_t *context) { + processStartGas(context); +} + static void processGasprice(txContext_t *context) { if (context->currentFieldIsList) { PRINTF("Invalid type for RLP_GASPRICE\n"); @@ -248,68 +295,167 @@ static void processV(txContext_t *context) { } } +static bool processEIP2930Tx(txContext_t *context) { + switch (context->currentField) { + case EIP2930_RLP_CONTENT: + processContent(context); + if ((context->processingFlags & TX_FLAG_TYPE) == 0) { + context->currentField++; + } + break; + case EIP2930_RLP_TYPE: + processType(context); + break; + case EIP2930_RLP_CHAINID: + processChainID(context); + break; + case EIP2930_RLP_NONCE: + processNonce(context); + break; + case EIP2930_RLP_GASPRICE: + processGasprice(context); + break; + case EIP2930_RLP_GASLIMIT: + processGasLimit(context); + break; + case EIP2930_RLP_TO: + processTo(context); + break; + case EIP2930_RLP_VALUE: + processValue(context); + break; + case EIP2930_RLP_YPARITY: + processV(context); + break; + case EIP2930_RLP_ACCESS_LIST: + processAccessList(context); + break; + case EIP2930_RLP_DATA: + case EIP2930_RLP_SENDER_R: + case EIP2930_RLP_SENDER_S: + processData(context); + break; + default: + PRINTF("Invalid RLP decoder context\n"); + return true; + } + return false; +} + +static bool processLegacyTx(txContext_t *context) { + PRINTF("Processing legacy\n"); + switch (context->currentField) { + case LEGACY_RLP_CONTENT: + processContent(context); + if ((context->processingFlags & TX_FLAG_TYPE) == 0) { + context->currentField++; + } + break; + case LEGACY_RLP_TYPE: + processType(context); + break; + case LEGACY_RLP_NONCE: + processNonce(context); + break; + case LEGACY_RLP_GASPRICE: + processGasprice(context); + break; + case LEGACY_RLP_STARTGAS: + processStartGas(context); + break; + case LEGACY_RLP_TO: + processTo(context); + break; + case LEGACY_RLP_VALUE: + processValue(context); + break; + case LEGACY_RLP_DATA: + case LEGACY_RLP_R: + case LEGACY_RLP_S: + processData(context); + break; + case LEGACY_RLP_V: + processV(context); + break; + default: + PRINTF("Invalid RLP decoder context\n"); + return true; + } + return false; +} + +static parserStatus_e parseRLP(txContext_t *context) { + bool canDecode = false; + uint32_t offset; + while (context->commandLength != 0) { + bool valid; + // Feed the RLP buffer until the length can be decoded + context->rlpBuffer[context->rlpBufferPos++] = readTxByte(context); + if (rlpCanDecode(context->rlpBuffer, context->rlpBufferPos, &valid)) { + // Can decode now, if valid + if (!valid) { + PRINTF("RLP pre-decode error\n"); + return USTREAM_FAULT; + } + canDecode = true; + break; + } + // Cannot decode yet + // Sanity check + if (context->rlpBufferPos == sizeof(context->rlpBuffer)) { + PRINTF("RLP pre-decode logic error\n"); + return USTREAM_FAULT; + } + } + if (!canDecode) { + PRINTF("Can't decode\n"); + return USTREAM_PROCESSING; + } + // Ready to process this field + if (!rlpDecodeLength(context->rlpBuffer, + context->rlpBufferPos, + &context->currentFieldLength, + &offset, + &context->currentFieldIsList)) { + PRINTF("RLP decode error\n"); + return USTREAM_FAULT; + } + if (offset == 0) { + // Hack for single byte, self encoded + context->workBuffer--; + context->commandLength++; + context->fieldSingleByte = true; + } else { + context->fieldSingleByte = false; + } + context->currentFieldPos = 0; + context->rlpBufferPos = 0; + context->processingField = true; + return USTREAM_CONTINUE; +} + static parserStatus_e processTxInternal(txContext_t *context) { for (;;) { customStatus_e customStatus = CUSTOM_NOT_HANDLED; // EIP 155 style transaction - if (context->currentField == TX_RLP_DONE) { + if (IS_PARSING_DONE(context)) { return USTREAM_FINISHED; } // Old style transaction - if ((context->currentField == TX_RLP_V) && (context->commandLength == 0)) { + if ((context->currentField == LEGACY_RLP_V || + context->currentField == EIP2930_RLP_YPARITY) && + (context->commandLength == 0)) { context->content->vLength = 0; return USTREAM_FINISHED; } if (context->commandLength == 0) { return USTREAM_PROCESSING; } - if (!context->processingField) { - bool canDecode = false; - uint32_t offset; - while (context->commandLength != 0) { - bool valid; - // Feed the RLP buffer until the length can be decoded - context->rlpBuffer[context->rlpBufferPos++] = readTxByte(context); - if (rlpCanDecode(context->rlpBuffer, context->rlpBufferPos, &valid)) { - // Can decode now, if valid - if (!valid) { - PRINTF("RLP pre-decode error\n"); - return USTREAM_FAULT; - } - canDecode = true; - break; - } - // Cannot decode yet - // Sanity check - if (context->rlpBufferPos == sizeof(context->rlpBuffer)) { - PRINTF("RLP pre-decode logic error\n"); - return USTREAM_FAULT; - } + parserStatus_e status = parseRLP(context); + if (status != USTREAM_CONTINUE) { + return status; } - if (!canDecode) { - return USTREAM_PROCESSING; - } - // Ready to process this field - if (!rlpDecodeLength(context->rlpBuffer, - context->rlpBufferPos, - &context->currentFieldLength, - &offset, - &context->currentFieldIsList)) { - PRINTF("RLP decode error\n"); - return USTREAM_FAULT; - } - if (offset == 0) { - // Hack for single byte, self encoded - context->workBuffer--; - context->commandLength++; - context->fieldSingleByte = true; - } else { - context->fieldSingleByte = false; - } - context->currentFieldPos = 0; - context->rlpBufferPos = 0; - context->processingField = true; } if (context->customProcessor != NULL) { customStatus = context->customProcessor(context); @@ -328,41 +474,25 @@ static parserStatus_e processTxInternal(txContext_t *context) { } } if (customStatus == CUSTOM_NOT_HANDLED) { - switch (context->currentField) { - case TX_RLP_CONTENT: - processContent(context); - if ((context->processingFlags & TX_FLAG_TYPE) == 0) { - context->currentField++; + PRINTF("Current field: %u\n", context->currentField); + switch (context->txType) { + bool fault; + case LEGACY: + fault = processLegacyTx(context); + if (fault) { + return USTREAM_FAULT; + } else { + break; + } + case EIP2930: + fault = processEIP2930Tx(context); + if (fault) { + return USTREAM_FAULT; + } else { + break; } - break; - case TX_RLP_TYPE: - processType(context); - break; - case TX_RLP_NONCE: - processNonce(context); - break; - case TX_RLP_GASPRICE: - processGasprice(context); - break; - case TX_RLP_STARTGAS: - processStartGas(context); - break; - case TX_RLP_VALUE: - processValue(context); - break; - case TX_RLP_TO: - processTo(context); - break; - case TX_RLP_DATA: - case TX_RLP_R: - case TX_RLP_S: - processData(context); - break; - case TX_RLP_V: - processV(context); - break; default: - PRINTF("Invalid RLP decoder context\n"); + PRINTF("Transaction type %u is not supported\n", context->txType); return USTREAM_FAULT; } } diff --git a/src_common/ethUstream.h b/src_common/ethUstream.h index 34bd413..531750b 100644 --- a/src_common/ethUstream.h +++ b/src_common/ethUstream.h @@ -37,35 +37,63 @@ typedef customStatus_e (*ustreamProcess_t)(struct txContext_t *context); #define TX_FLAG_TYPE 0x01 -typedef enum rlpTxField_e { - TX_RLP_NONE = 0, - TX_RLP_CONTENT, - TX_RLP_TYPE, - TX_RLP_NONCE, - TX_RLP_GASPRICE, - TX_RLP_STARTGAS, - TX_RLP_TO, - TX_RLP_VALUE, - TX_RLP_DATA, - TX_RLP_V, - TX_RLP_R, - TX_RLP_S, - TX_RLP_DONE -} rlpTxField_e; +// First variant of every Tx enum. +#define RLP_NONE 0 + +#define IS_PARSING_DONE(ctx) \ + ((ctx->txType == LEGACY && ctx->currentField == LEGACY_RLP_DONE) || \ + (ctx->txType == EIP2930 && ctx->currentField == EIP2930_RLP_DONE)) + +typedef enum rlpLegacyTxField_e { + LEGACY_RLP_NONE = RLP_NONE, + LEGACY_RLP_CONTENT, + LEGACY_RLP_TYPE, + LEGACY_RLP_NONCE, + LEGACY_RLP_GASPRICE, + LEGACY_RLP_STARTGAS, + LEGACY_RLP_TO, + LEGACY_RLP_VALUE, + LEGACY_RLP_DATA, + LEGACY_RLP_V, + LEGACY_RLP_R, + LEGACY_RLP_S, + LEGACY_RLP_DONE +} rlpLegacyTxField_e; + +typedef enum rlpEIP2930TxField_e { + EIP2930_RLP_NONE = RLP_NONE, + EIP2930_RLP_CONTENT, + EIP2930_RLP_TYPE, + EIP2930_RLP_CHAINID, + EIP2930_RLP_NONCE, + EIP2930_RLP_GASPRICE, + EIP2930_RLP_GASLIMIT, + EIP2930_RLP_TO, + EIP2930_RLP_VALUE, + EIP2930_RLP_DATA, + EIP2930_RLP_ACCESS_LIST, + EIP2930_RLP_YPARITY, + EIP2930_RLP_SENDER_R, + EIP2930_RLP_SENDER_S, + EIP2930_RLP_DONE +} rlpEIP2930TxField_e; #define MIN_TX_TYPE 0x00 #define MAX_TX_TYPE 0x7f // EIP 2718 TransactionType +// Valid transaction types should be in [0x00, 0x7f] typedef enum txType_e { - LEGACY_TX = 1, + EIP2930 = 0x01, + LEGACY = 0xc0 // Legacy tx are greater than or equal to 0xc0. } txType_e; typedef enum parserStatus_e { - USTREAM_PROCESSING, - USTREAM_SUSPENDED, - USTREAM_FINISHED, - USTREAM_FAULT + USTREAM_PROCESSING, // Parsing is in progress + USTREAM_SUSPENDED, // Parsing has been suspended + USTREAM_FINISHED, // Parsing is done + USTREAM_FAULT, // An error was encountered while parsing + USTREAM_CONTINUE // Used internally to signify we can keep on parsing } parserStatus_e; typedef struct txInt256_t { @@ -78,6 +106,7 @@ typedef struct txContent_t { txInt256_t startgas; txInt256_t value; txInt256_t nonce; + txInt256_t chainID; uint8_t destination[20]; uint8_t destinationLength; uint8_t v[4]; @@ -85,7 +114,7 @@ typedef struct txContent_t { } txContent_t; typedef struct txContext_t { - rlpTxField_e currentField; + uint8_t currentField; cx_sha3_t *sha3; uint32_t currentFieldLength; uint32_t currentFieldPos; diff --git a/src_features/signTx/cmd_signTx.c b/src_features/signTx/cmd_signTx.c index d08c528..eabbbcf 100644 --- a/src_features/signTx/cmd_signTx.c +++ b/src_features/signTx/cmd_signTx.c @@ -44,10 +44,8 @@ void handleSign(uint8_t p1, // EIP 2718: TransactionType might be present before the TransactionPayload. uint8_t txType = *workBuffer; if (txType >= MIN_TX_TYPE && txType <= MAX_TX_TYPE) { - PRINTF("Transaction type: %u\n", txType); - // Enumerate through all supported txTypes here... - if (txType == LEGACY_TX) { + if (txType == EIP2930) { txContext.txType = txType; workBuffer++; dataLength--; @@ -55,6 +53,8 @@ void handleSign(uint8_t p1, PRINTF("Transaction type not supported\n"); THROW(0x6501); } + } else { + txContext.txType = LEGACY; } initTx(&txContext, &global_sha3, &tmpContent.txContent, customProcessor, NULL); } else if (p1 != P1_MORE) { @@ -67,7 +67,7 @@ void handleSign(uint8_t p1, PRINTF("Signature not initialized\n"); THROW(0x6985); } - if (txContext.currentField == TX_RLP_NONE) { + if (txContext.currentField == RLP_NONE) { PRINTF("Parser not initialized\n"); THROW(0x6985); } diff --git a/src_features/signTx/logic_signTx.c b/src_features/signTx/logic_signTx.c index cfbfa8a..8be1d51 100644 --- a/src_features/signTx/logic_signTx.c +++ b/src_features/signTx/logic_signTx.c @@ -28,12 +28,18 @@ uint32_t splitBinaryParameterPart(char *result, uint8_t *parameter) { } customStatus_e customProcessor(txContext_t *context) { - if ((context->currentField == TX_RLP_DATA) && (context->currentFieldLength != 0)) { + if (((context->txType == LEGACY && context->currentField == LEGACY_RLP_DATA) || + (context->txType == EIP2930 && context->currentField == EIP2930_RLP_DATA)) && + (context->currentFieldLength != 0)) { dataPresent = true; // If handling a new contract rather than a function call, abort immediately if (tmpContent.txContent.destinationLength == 0) { return CUSTOM_NOT_HANDLED; } + // If data field is less than 4 bytes long, do not try to use a plugin. + if (context->currentFieldLength < 4) { + return CUSTOM_NOT_HANDLED; + } if (context->currentFieldPos == 0) { ethPluginInitContract_t pluginInit; // If handling the beginning of the data field, assume that the function selector is @@ -51,6 +57,7 @@ customStatus_e customProcessor(txContext_t *context) { context->currentFieldLength); dataContext.tokenContext.pluginAvailable = eth_plugin_perform_init(tmpContent.txContent.destination, &pluginInit); + PRINTF("a\n"); } PRINTF("pluginAvailable %d\n", dataContext.tokenContext.pluginAvailable); if (dataContext.tokenContext.pluginAvailable) { @@ -238,10 +245,21 @@ void finalizeParsing(bool direct) { // Verify the chain if (chainConfig->chainId != 0) { - uint32_t v = getV(&tmpContent.txContent); - if (chainConfig->chainId != v) { + uint32_t id = 0; + + if (txContext.txType == LEGACY) { + id = u32_from_BE(txContext.content->v, txContext.content->vLength); + } else if (txContext.txType == EIP2930) { + id = u32_from_BE(txContext.content->chainID.value, txContext.content->chainID.length); + } else { + PRINTF("TxType `%u` not supported while checking for chainID\n", txContext.txType); + return; + } + + PRINTF("OFFICIAL: %u, RECEIVED: %u\n", chainConfig->chainId, id); + if (chainConfig->chainId != id) { + PRINTF("Invalid chainID %u expected %u\n", id, chainConfig->chainId); reset_app_context(); - PRINTF("Invalid chainId %d expected %d\n", v, chainConfig->chainId); reportFinalizeError(direct); if (!direct) { return; @@ -323,7 +341,6 @@ void finalizeParsing(bool direct) { } if (dataPresent && !N_storage.dataAllowed) { - PRINTF("Data field forbidden\n"); reportFinalizeError(direct); if (!direct) { return; @@ -368,6 +385,24 @@ void finalizeParsing(bool direct) { compareOrCopy(strings.common.maxFee, displayBuffer, called_from_swap); } + // Prepare chainID field + if (genericUI) { + if (txContext.txType == LEGACY) { + uint32_t id = u32_from_BE(txContext.content->v, txContext.content->vLength); + u32_to_str((char *) strings.common.chainID, sizeof(strings.common.chainID), id); + } else if (txContext.txType == EIP2930) { + uint256_t chainID; + convertUint256BE(tmpContent.txContent.chainID.value, + tmpContent.txContent.chainID.length, + &chainID); + tostring256(&chainID, 10, displayBuffer, sizeof(displayBuffer)); + strncpy(strings.common.chainID, displayBuffer, sizeof(strings.common.chainID)); + } else { + PRINTF("Txtype `%u` not supported while generating chainID\n", txContext.txType); + return; + } + } + bool no_consent = false; no_consent = called_from_swap; diff --git a/src_features/signTx/ui_common_signTx.c b/src_features/signTx/ui_common_signTx.c index 23cc80f..54ce048 100644 --- a/src_features/signTx/ui_common_signTx.c +++ b/src_features/signTx/ui_common_signTx.c @@ -8,7 +8,7 @@ unsigned int io_seproxyhal_touch_tx_ok(const bagl_element_t *e) { uint8_t signatureLength; cx_ecfp_private_key_t privateKey; uint32_t tx = 0; - uint32_t v = getV(&tmpContent.txContent); + uint32_t v = u32_from_BE(tmpContent.txContent.v, tmpContent.txContent.vLength); io_seproxyhal_io_heartbeat(); os_perso_derive_node_bip32(CX_CURVE_256K1, tmpCtx.transactionContext.bip32Path, diff --git a/src_features/signTx/ui_flow_signTx.c b/src_features/signTx/ui_flow_signTx.c index 477a25a..3edf610 100644 --- a/src_features/signTx/ui_flow_signTx.c +++ b/src_features/signTx/ui_flow_signTx.c @@ -1,5 +1,7 @@ #include "shared_context.h" #include "ui_callbacks.h" +#include "chainConfig.h" +#include "utils.h" // clang-format off UX_STEP_NOCB( @@ -85,7 +87,7 @@ UX_FLOW(ux_confirm_parameter_flow, ////////////////////////////////////////////////////////////////////// // clang-format off -UX_STEP_NOCB(ux_approval_tx_1_step, +UX_STEP_NOCB(ux_approval_review_step, pnn, { &C_icon_eye, @@ -93,28 +95,35 @@ UX_STEP_NOCB(ux_approval_tx_1_step, "transaction", }); UX_STEP_NOCB( - ux_approval_tx_2_step, + ux_approval_amount_step, bnnn_paging, { .title = "Amount", .text = strings.common.fullAmount }); UX_STEP_NOCB( - ux_approval_tx_3_step, + ux_approval_address_step, bnnn_paging, { .title = "Address", .text = strings.common.fullAddress, }); UX_STEP_NOCB( - ux_approval_tx_4_step, + ux_approval_fees_step, bnnn_paging, { .title = "Max Fees", .text = strings.common.maxFee, }); +UX_STEP_NOCB( + ux_approval_chainid_step, + bnnn_paging, + { + .title = "Chain ID", + .text = strings.common.chainID, + }); UX_STEP_CB( - ux_approval_tx_5_step, + ux_approval_accept_step, pbb, io_seproxyhal_touch_tx_ok(NULL), { @@ -123,7 +132,7 @@ UX_STEP_CB( "and send", }); UX_STEP_CB( - ux_approval_tx_6_step, + ux_approval_reject_step, pb, io_seproxyhal_touch_tx_cancel(NULL), { @@ -132,14 +141,14 @@ UX_STEP_CB( }); UX_STEP_NOCB( - ux_approval_tx_display_nonce_step, + ux_approval_nonce_step, bnnn_paging, { .title = "Nonce", .text = strings.common.nonce, }); -UX_STEP_NOCB(ux_approval_tx_data_warning_step, +UX_STEP_NOCB(ux_approval_data_warning_step, pbb, { &C_icon_warning, @@ -148,22 +157,35 @@ UX_STEP_NOCB(ux_approval_tx_data_warning_step, }); // clang-format on -const ux_flow_step_t *ux_approval_tx_flow_[9]; +const ux_flow_step_t *ux_approval_tx_flow_[10]; void ux_approve_tx(bool dataPresent) { int step = 0; - ux_approval_tx_flow_[step++] = &ux_approval_tx_1_step; + ux_approval_tx_flow_[step++] = &ux_approval_review_step; if (dataPresent && !N_storage.contractDetails) { - ux_approval_tx_flow_[step++] = &ux_approval_tx_data_warning_step; + ux_approval_tx_flow_[step++] = &ux_approval_data_warning_step; } - ux_approval_tx_flow_[step++] = &ux_approval_tx_2_step; - ux_approval_tx_flow_[step++] = &ux_approval_tx_3_step; + ux_approval_tx_flow_[step++] = &ux_approval_amount_step; + ux_approval_tx_flow_[step++] = &ux_approval_address_step; if (N_storage.displayNonce) { - ux_approval_tx_flow_[step++] = &ux_approval_tx_display_nonce_step; + ux_approval_tx_flow_[step++] = &ux_approval_nonce_step; } - ux_approval_tx_flow_[step++] = &ux_approval_tx_4_step; - ux_approval_tx_flow_[step++] = &ux_approval_tx_5_step; - ux_approval_tx_flow_[step++] = &ux_approval_tx_6_step; + + uint32_t id; + if (txContext.txType == LEGACY) { + id = u32_from_BE(txContext.content->v, txContext.content->vLength); + } else if (txContext.txType == EIP2930) { + id = u32_from_BE(txContext.content->chainID.value, txContext.content->chainID.length); + } else { + PRINTF("TxType `%u` not supported while preparing to approve tx\n", txContext.txType); + THROW(0x6501); + } + if (id != ETHEREUM_MAINNET_CHAINID) { + ux_approval_tx_flow_[step++] = &ux_approval_chainid_step; + } + ux_approval_tx_flow_[step++] = &ux_approval_fees_step; + ux_approval_tx_flow_[step++] = &ux_approval_accept_step; + ux_approval_tx_flow_[step++] = &ux_approval_reject_step; ux_approval_tx_flow_[step++] = FLOW_END_STEP; ux_flow_init(0, ux_approval_tx_flow_, NULL); From 81ec019242d87086b7396766334c733cec8a1f34 Mon Sep 17 00:00:00 2001 From: pscott Date: Wed, 21 Apr 2021 17:04:09 +0200 Subject: [PATCH 12/23] Re-enable plugins --- src/eth_plugin_handler.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/eth_plugin_handler.c b/src/eth_plugin_handler.c index d9f3d5d..ccaa86e 100644 --- a/src/eth_plugin_handler.c +++ b/src/eth_plugin_handler.c @@ -59,7 +59,6 @@ void eth_plugin_prepare_query_contract_UI(ethQueryContractUI_t *queryContractUI, int eth_plugin_perform_init(uint8_t *contractAddress, ethPluginInitContract_t *init) { uint8_t i; const uint8_t **selectors; - return 0; dataContext.tokenContext.pluginAvailable = 0; // Handle hardcoded plugin list PRINTF("Selector %.*H\n", 4, init->selector); From dafdc404ac1e14a9b0787ee2e91f2344b3f145bd Mon Sep 17 00:00:00 2001 From: pscott Date: Wed, 21 Apr 2021 17:07:16 +0200 Subject: [PATCH 13/23] Add strict parameter to u32_from_BE --- src/utils.c | 8 ++++++-- src/utils.h | 2 +- src_features/signTx/logic_signTx.c | 6 +++--- src_features/signTx/ui_common_signTx.c | 2 +- src_features/signTx/ui_flow_signTx.c | 4 ++-- 5 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/utils.c b/src/utils.c index 0b7d583..9c16b6f 100644 --- a/src/utils.c +++ b/src/utils.c @@ -54,7 +54,8 @@ int local_strchr(char *string, char ch) { } // Almost like U4BE except that it takes `size` as a parameter. -uint32_t u32_from_BE(uint8_t *in, uint8_t size) { +// The `strict` parameter defines whether we should throw in case of a length > 4. +uint32_t u32_from_BE(uint8_t *in, uint8_t size, bool strict) { uint32_t res = 0; if (size == 1) { res = in[0]; @@ -62,8 +63,11 @@ uint32_t u32_from_BE(uint8_t *in, uint8_t size) { res = (in[0] << 8) | in[1]; } else if (size == 3) { res = (in[0] << 16) | (in[1] << 8) | in[2]; - } else { + } else if (size == 4) { res = (in[0] << 24) | (in[1] << 16) | (in[2] << 8) | in[3]; + } else if (strict && size != 0) { + PRINTF("Unexpected format\n"); + THROW(EXCEPTION); } return res; } diff --git a/src/utils.h b/src/utils.h index 582fb62..b13c5e2 100644 --- a/src/utils.h +++ b/src/utils.h @@ -32,7 +32,7 @@ int local_strchr(char* string, char ch); void u32_to_str(char* dest, uint8_t dest_size, uint32_t in); // Converts a list of bytes (in BE) of length `size` to a uint32_t. -uint32_t u32_from_BE(uint8_t* in, uint8_t size); +uint32_t u32_from_BE(uint8_t* in, uint8_t size, bool strict); void amountToString(uint8_t* amount, uint8_t amount_len, diff --git a/src_features/signTx/logic_signTx.c b/src_features/signTx/logic_signTx.c index 8be1d51..989331b 100644 --- a/src_features/signTx/logic_signTx.c +++ b/src_features/signTx/logic_signTx.c @@ -248,9 +248,9 @@ void finalizeParsing(bool direct) { uint32_t id = 0; if (txContext.txType == LEGACY) { - id = u32_from_BE(txContext.content->v, txContext.content->vLength); + id = u32_from_BE(txContext.content->v, txContext.content->vLength, true); } else if (txContext.txType == EIP2930) { - id = u32_from_BE(txContext.content->chainID.value, txContext.content->chainID.length); + id = u32_from_BE(txContext.content->chainID.value, txContext.content->chainID.length, false); } else { PRINTF("TxType `%u` not supported while checking for chainID\n", txContext.txType); return; @@ -388,7 +388,7 @@ void finalizeParsing(bool direct) { // Prepare chainID field if (genericUI) { if (txContext.txType == LEGACY) { - uint32_t id = u32_from_BE(txContext.content->v, txContext.content->vLength); + uint32_t id = u32_from_BE(txContext.content->v, txContext.content->vLength, true); u32_to_str((char *) strings.common.chainID, sizeof(strings.common.chainID), id); } else if (txContext.txType == EIP2930) { uint256_t chainID; diff --git a/src_features/signTx/ui_common_signTx.c b/src_features/signTx/ui_common_signTx.c index 54ce048..ac3023a 100644 --- a/src_features/signTx/ui_common_signTx.c +++ b/src_features/signTx/ui_common_signTx.c @@ -8,7 +8,7 @@ unsigned int io_seproxyhal_touch_tx_ok(const bagl_element_t *e) { uint8_t signatureLength; cx_ecfp_private_key_t privateKey; uint32_t tx = 0; - uint32_t v = u32_from_BE(tmpContent.txContent.v, tmpContent.txContent.vLength); + uint32_t v = u32_from_BE(tmpContent.txContent.v, tmpContent.txContent.vLength, true); io_seproxyhal_io_heartbeat(); os_perso_derive_node_bip32(CX_CURVE_256K1, tmpCtx.transactionContext.bip32Path, diff --git a/src_features/signTx/ui_flow_signTx.c b/src_features/signTx/ui_flow_signTx.c index 3edf610..345fef8 100644 --- a/src_features/signTx/ui_flow_signTx.c +++ b/src_features/signTx/ui_flow_signTx.c @@ -173,9 +173,9 @@ void ux_approve_tx(bool dataPresent) { uint32_t id; if (txContext.txType == LEGACY) { - id = u32_from_BE(txContext.content->v, txContext.content->vLength); + id = u32_from_BE(txContext.content->v, txContext.content->vLength, true); } else if (txContext.txType == EIP2930) { - id = u32_from_BE(txContext.content->chainID.value, txContext.content->chainID.length); + id = u32_from_BE(txContext.content->chainID.value, txContext.content->chainID.length, false); } else { PRINTF("TxType `%u` not supported while preparing to approve tx\n", txContext.txType); THROW(0x6501); From d701f75197e6c414bf376f06067858ba04369466 Mon Sep 17 00:00:00 2001 From: pscott Date: Wed, 21 Apr 2021 17:09:45 +0200 Subject: [PATCH 14/23] Add comments to u32_from_BE in header file --- src/utils.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/utils.h b/src/utils.h index b13c5e2..b7d3118 100644 --- a/src/utils.h +++ b/src/utils.h @@ -31,7 +31,8 @@ int local_strchr(char* string, char ch); // `itoa` for uint32_t. void u32_to_str(char* dest, uint8_t dest_size, uint32_t in); -// Converts a list of bytes (in BE) of length `size` to a uint32_t. +// Converts a list of bytes (in BE) of length `size` to a uint32_t. `strict` will make the function +// throw if the size is > 4. uint32_t u32_from_BE(uint8_t* in, uint8_t size, bool strict); void amountToString(uint8_t* amount, From fa0dacb44731d7b90b245c464688d7afaba9843c Mon Sep 17 00:00:00 2001 From: pscott Date: Wed, 21 Apr 2021 17:10:58 +0200 Subject: [PATCH 15/23] Remove printf when parsing chainID --- src_common/ethUstream.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src_common/ethUstream.c b/src_common/ethUstream.c index 22afedc..4cd8f22 100644 --- a/src_common/ethUstream.c +++ b/src_common/ethUstream.c @@ -145,7 +145,6 @@ static void processChainID(txContext_t *context) { context->currentField++; context->processingField = false; } - PRINTF("chainID: %.*H\n", context->content->chainID.length, context->content->chainID.value); } static void processNonce(txContext_t *context) { From cac2b9513604b88cb0b4216c941ab086bd40f62a Mon Sep 17 00:00:00 2001 From: pscott Date: Wed, 21 Apr 2021 17:11:51 +0200 Subject: [PATCH 16/23] Remove printf when parsing legacy tx --- src_common/ethUstream.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src_common/ethUstream.c b/src_common/ethUstream.c index 4cd8f22..06a79d7 100644 --- a/src_common/ethUstream.c +++ b/src_common/ethUstream.c @@ -342,7 +342,6 @@ static bool processEIP2930Tx(txContext_t *context) { } static bool processLegacyTx(txContext_t *context) { - PRINTF("Processing legacy\n"); switch (context->currentField) { case LEGACY_RLP_CONTENT: processContent(context); From 704c34a5d5162d6723f9573324b25448a0005317 Mon Sep 17 00:00:00 2001 From: pscott Date: Wed, 21 Apr 2021 17:15:43 +0200 Subject: [PATCH 17/23] Rename to PARSING_IS_DONE --- src_common/ethUstream.c | 2 +- src_common/ethUstream.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src_common/ethUstream.c b/src_common/ethUstream.c index 06a79d7..ba1223f 100644 --- a/src_common/ethUstream.c +++ b/src_common/ethUstream.c @@ -436,7 +436,7 @@ static parserStatus_e processTxInternal(txContext_t *context) { for (;;) { customStatus_e customStatus = CUSTOM_NOT_HANDLED; // EIP 155 style transaction - if (IS_PARSING_DONE(context)) { + if (PARSING_IS_DONE(context)) { return USTREAM_FINISHED; } // Old style transaction diff --git a/src_common/ethUstream.h b/src_common/ethUstream.h index 531750b..78b10cb 100644 --- a/src_common/ethUstream.h +++ b/src_common/ethUstream.h @@ -40,7 +40,7 @@ typedef customStatus_e (*ustreamProcess_t)(struct txContext_t *context); // First variant of every Tx enum. #define RLP_NONE 0 -#define IS_PARSING_DONE(ctx) \ +#define PARSING_IS_DONE(ctx) \ ((ctx->txType == LEGACY && ctx->currentField == LEGACY_RLP_DONE) || \ (ctx->txType == EIP2930 && ctx->currentField == EIP2930_RLP_DONE)) From 1a1a3198f9643d8edb6dd1bee3adea92d826a6be Mon Sep 17 00:00:00 2001 From: pscott Date: Wed, 21 Apr 2021 17:17:21 +0200 Subject: [PATCH 18/23] Remove debugggin printf --- src_features/signTx/logic_signTx.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src_features/signTx/logic_signTx.c b/src_features/signTx/logic_signTx.c index 989331b..38dd199 100644 --- a/src_features/signTx/logic_signTx.c +++ b/src_features/signTx/logic_signTx.c @@ -57,7 +57,6 @@ customStatus_e customProcessor(txContext_t *context) { context->currentFieldLength); dataContext.tokenContext.pluginAvailable = eth_plugin_perform_init(tmpContent.txContent.destination, &pluginInit); - PRINTF("a\n"); } PRINTF("pluginAvailable %d\n", dataContext.tokenContext.pluginAvailable); if (dataContext.tokenContext.pluginAvailable) { From 11701b6fa2e4f8024b4d558d94fbf50defe0ad7a Mon Sep 17 00:00:00 2001 From: pscott Date: Wed, 21 Apr 2021 17:19:02 +0200 Subject: [PATCH 19/23] Remove debugging printf --- src_features/signTx/logic_signTx.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src_features/signTx/logic_signTx.c b/src_features/signTx/logic_signTx.c index 38dd199..e7b10cf 100644 --- a/src_features/signTx/logic_signTx.c +++ b/src_features/signTx/logic_signTx.c @@ -255,7 +255,6 @@ void finalizeParsing(bool direct) { return; } - PRINTF("OFFICIAL: %u, RECEIVED: %u\n", chainConfig->chainId, id); if (chainConfig->chainId != id) { PRINTF("Invalid chainID %u expected %u\n", id, chainConfig->chainId); reset_app_context(); From d5b32af95e12220070e6329e4fa8a8813ea155da Mon Sep 17 00:00:00 2001 From: pscott Date: Wed, 21 Apr 2021 17:24:54 +0200 Subject: [PATCH 20/23] Clang format --- src_features/signTx/logic_signTx.c | 4 +++- src_features/signTx/ui_flow_signTx.c | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src_features/signTx/logic_signTx.c b/src_features/signTx/logic_signTx.c index e7b10cf..002f21e 100644 --- a/src_features/signTx/logic_signTx.c +++ b/src_features/signTx/logic_signTx.c @@ -249,7 +249,9 @@ void finalizeParsing(bool direct) { if (txContext.txType == LEGACY) { id = u32_from_BE(txContext.content->v, txContext.content->vLength, true); } else if (txContext.txType == EIP2930) { - id = u32_from_BE(txContext.content->chainID.value, txContext.content->chainID.length, false); + id = u32_from_BE(txContext.content->chainID.value, + txContext.content->chainID.length, + false); } else { PRINTF("TxType `%u` not supported while checking for chainID\n", txContext.txType); return; diff --git a/src_features/signTx/ui_flow_signTx.c b/src_features/signTx/ui_flow_signTx.c index 345fef8..31de33d 100644 --- a/src_features/signTx/ui_flow_signTx.c +++ b/src_features/signTx/ui_flow_signTx.c @@ -175,7 +175,8 @@ void ux_approve_tx(bool dataPresent) { if (txContext.txType == LEGACY) { id = u32_from_BE(txContext.content->v, txContext.content->vLength, true); } else if (txContext.txType == EIP2930) { - id = u32_from_BE(txContext.content->chainID.value, txContext.content->chainID.length, false); + id = + u32_from_BE(txContext.content->chainID.value, txContext.content->chainID.length, false); } else { PRINTF("TxType `%u` not supported while preparing to approve tx\n", txContext.txType); THROW(0x6501); From ebffb48cd3a1e063f9353e75e44b893d892484d8 Mon Sep 17 00:00:00 2001 From: pscott Date: Wed, 28 Apr 2021 10:48:18 +0200 Subject: [PATCH 21/23] Use snprintf instead of u32_to_str --- src/utils.c | 39 ------------------------------ src/utils.h | 3 --- src_features/signTx/logic_signTx.c | 8 +++++- 3 files changed, 7 insertions(+), 43 deletions(-) diff --git a/src/utils.c b/src/utils.c index 9c16b6f..b319736 100644 --- a/src/utils.c +++ b/src/utils.c @@ -72,45 +72,6 @@ uint32_t u32_from_BE(uint8_t *in, uint8_t size, bool strict) { return res; } -// Converts a uint32_t to a string. -void u32_to_str(char *dest, uint32_t in, uint8_t dest_size) { - uint8_t i = 0; - - // Get the first digit (in case it's 0). - dest[i] = in % 10 + '0'; - in /= 10; - i++; - - // Get every digit. - while (in != 0) { - if (i >= dest_size) { - THROW(6502); - } - dest[i] = in % 10 + '0'; - in /= 10; - i++; - } - - // Null terminate the string. - dest[i] = '\0'; - i--; - - // Reverse the string - uint8_t end = i; - char tmp; - i = 0; - while (i < end) { - // Swap the first and last elements. - tmp = dest[i]; - dest[i] = dest[end]; - dest[end] = tmp; - - // Decrease the interval size. - i++; - end--; - } -} - void amountToString(uint8_t *amount, uint8_t amount_size, uint8_t decimals, diff --git a/src/utils.h b/src/utils.h index b7d3118..ba878e1 100644 --- a/src/utils.h +++ b/src/utils.h @@ -28,9 +28,6 @@ void convertUint256BE(uint8_t* data, uint32_t length, uint256_t* target); int local_strchr(char* string, char ch); -// `itoa` for uint32_t. -void u32_to_str(char* dest, uint8_t dest_size, uint32_t in); - // Converts a list of bytes (in BE) of length `size` to a uint32_t. `strict` will make the function // throw if the size is > 4. uint32_t u32_from_BE(uint8_t* in, uint8_t size, bool strict); diff --git a/src_features/signTx/logic_signTx.c b/src_features/signTx/logic_signTx.c index 002f21e..9a5dc42 100644 --- a/src_features/signTx/logic_signTx.c +++ b/src_features/signTx/logic_signTx.c @@ -389,7 +389,13 @@ void finalizeParsing(bool direct) { if (genericUI) { if (txContext.txType == LEGACY) { uint32_t id = u32_from_BE(txContext.content->v, txContext.content->vLength, true); - u32_to_str((char *) strings.common.chainID, sizeof(strings.common.chainID), id); + uint8_t res = + snprintf(strings.common.chainID, sizeof(strings.common.chainID), "%u", id); + if (res >= sizeof(strings.common.chainID)) { + // If the return value is higher or equal to the size passed in as parameter, then + // the output was truncated. Return the appropriate error code. + THROW(0x6502); + } } else if (txContext.txType == EIP2930) { uint256_t chainID; convertUint256BE(tmpContent.txContent.chainID.value, From 7f0afc764a3fa837a8f1128bf851f2552f5c385f Mon Sep 17 00:00:00 2001 From: pscott Date: Wed, 28 Apr 2021 10:48:37 +0200 Subject: [PATCH 22/23] Change error code description --- doc/ethapp.asc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/ethapp.asc b/doc/ethapp.asc index ac716dd..d35664c 100644 --- a/doc/ethapp.asc +++ b/doc/ethapp.asc @@ -474,7 +474,7 @@ The following standard Status Words are returned for all APDUs - some specific S |=============================================================================================== | *SW* | *Description* | 6501 | TransactionType not supported -| 6502 | Not enough space for conversion from uint32_t to string +| 6502 | Output buffer too small for snprintf input | 6700 | Incorrect length | 6982 | Security status not satisfied (Canceled by user) | 6A80 | Invalid data From c7ec0c7dfaa88d3060bbf526d529aec74af157f7 Mon Sep 17 00:00:00 2001 From: pscott Date: Wed, 28 Apr 2021 11:39:47 +0200 Subject: [PATCH 23/23] Use %d flag instead of %u --- src_features/signTx/logic_signTx.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src_features/signTx/logic_signTx.c b/src_features/signTx/logic_signTx.c index 9a5dc42..e392a54 100644 --- a/src_features/signTx/logic_signTx.c +++ b/src_features/signTx/logic_signTx.c @@ -389,8 +389,9 @@ void finalizeParsing(bool direct) { if (genericUI) { if (txContext.txType == LEGACY) { uint32_t id = u32_from_BE(txContext.content->v, txContext.content->vLength, true); + PRINTF("Chain ID: %u\n", id); uint8_t res = - snprintf(strings.common.chainID, sizeof(strings.common.chainID), "%u", id); + snprintf(strings.common.chainID, sizeof(strings.common.chainID), "%d", id); if (res >= sizeof(strings.common.chainID)) { // If the return value is higher or equal to the size passed in as parameter, then // the output was truncated. Return the appropriate error code.