From 8e3fd4233ae2118333af2da58436d48f3b0cecc4 Mon Sep 17 00:00:00 2001 From: Francois Beutin Date: Fri, 1 Dec 2023 12:35:17 +0100 Subject: [PATCH 1/3] Add STACK_CANARY for Ethereum and clones not using standard path --- makefile_conf/chain/ethereum.mk | 3 ++- makefile_conf/chain/goerli.mk | 3 ++- makefile_conf/chain/ropsten.mk | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/makefile_conf/chain/ethereum.mk b/makefile_conf/chain/ethereum.mk index 6d52a8b8b..16e4b8cae 100644 --- a/makefile_conf/chain/ethereum.mk +++ b/makefile_conf/chain/ethereum.mk @@ -11,4 +11,5 @@ APP_LOAD_PARAMS += --path "12381/3600" --curve bls12381g1 DEFINES += HAVE_ETH2 APPNAME = "Ethereum" DEFINES_LIB= -APP_LOAD_FLAGS=--appFlags 0xa40 \ No newline at end of file +DEFINES += HAVE_BOLOS_APP_STACK_CANARY +APP_LOAD_FLAGS=--appFlags 0xa40 diff --git a/makefile_conf/chain/goerli.mk b/makefile_conf/chain/goerli.mk index 1de903909..1f8e37d53 100644 --- a/makefile_conf/chain/goerli.mk +++ b/makefile_conf/chain/goerli.mk @@ -12,4 +12,5 @@ APP_LOAD_PARAMS += --path "12381/3600" --curve bls12381g1 DEFINES += HAVE_ETH2 APPNAME = "Eth Goerli" DEFINES_LIB= -APP_LOAD_FLAGS=--appFlags 0xa40 \ No newline at end of file +DEFINES += HAVE_BOLOS_APP_STACK_CANARY +APP_LOAD_FLAGS=--appFlags 0xa40 diff --git a/makefile_conf/chain/ropsten.mk b/makefile_conf/chain/ropsten.mk index d3415ad9e..793a31417 100644 --- a/makefile_conf/chain/ropsten.mk +++ b/makefile_conf/chain/ropsten.mk @@ -12,4 +12,5 @@ APP_LOAD_PARAMS += --path "12381/3600" --curve bls12381g1 DEFINES += HAVE_ETH2 APPNAME = "Eth Ropsten" DEFINES_LIB= -APP_LOAD_FLAGS=--appFlags 0xa40 \ No newline at end of file +DEFINES += HAVE_BOLOS_APP_STACK_CANARY +APP_LOAD_FLAGS=--appFlags 0xa40 From f64addc0d2165833fbcf38d6beab24846c60bdde Mon Sep 17 00:00:00 2001 From: Francois Beutin Date: Fri, 1 Dec 2023 12:38:48 +0100 Subject: [PATCH 2/3] Remove THROW from lib mode handlers and remove associated TRY CATCH context --- src/handle_check_address.c | 61 ++++++++++++++++++++---------- src/handle_check_address.h | 6 +-- src/handle_get_printable_amount.c | 11 +++--- src/handle_get_printable_amount.h | 6 +-- src/handle_swap_sign_transaction.c | 12 +++--- src/handle_swap_sign_transaction.h | 2 +- src/main.c | 48 +++++++---------------- 7 files changed, 72 insertions(+), 74 deletions(-) diff --git a/src/handle_check_address.c b/src/handle_check_address.c index ef6d5edc4..3fdb1f12e 100644 --- a/src/handle_check_address.c +++ b/src/handle_check_address.c @@ -4,18 +4,18 @@ #include "ethUtils.h" #include "string.h" -#define ZERO(x) memset(&x, 0, sizeof(x)) +#define ZERO(x) explicit_bzero(&x, sizeof(x)) -int handle_check_address(check_address_parameters_t* params, chain_config_t* chain_config) { +void handle_check_address(check_address_parameters_t* params, chain_config_t* chain_config) { + params->result = 0; PRINTF("Params on the address %d\n", (unsigned int) params); PRINTF("Address to check %s\n", params->address_to_check); PRINTF("Inside handle_check_address\n"); if (params->address_to_check == 0) { PRINTF("Address to check == 0\n"); - return 0; + return; } - uint8_t i; const uint8_t* bip32_path_ptr = params->address_parameters; uint8_t bip32PathLength = *(bip32_path_ptr++); cx_sha3_t local_sha3; @@ -27,37 +27,55 @@ int handle_check_address(check_address_parameters_t* params, chain_config_t* cha char address[51]; } locals_union1; union group2 { - uint8_t privateKeyData[32]; + uint8_t privateKeyData[64]; cx_ecfp_public_key_t publicKey; } locals_union2; if ((bip32PathLength < 0x01) || (bip32PathLength > MAX_BIP32_PATH) || (bip32PathLength * 4 != params->address_parameters_length - 1)) { PRINTF("Invalid path\n"); - return 0; + return; } - for (i = 0; i < bip32PathLength; i++) { + for (uint8_t i = 0; i < bip32PathLength; i++) { locals_union1.bip32Path[i] = U4BE(bip32_path_ptr, 0); bip32_path_ptr += 4; } - os_perso_derive_node_bip32(CX_CURVE_256K1, - locals_union1.bip32Path, - bip32PathLength, - locals_union2.privateKeyData, - NULL); + if (os_derive_bip32_no_throw(CX_CURVE_256K1, + locals_union1.bip32Path, + bip32PathLength, + locals_union2.privateKeyData, + NULL) != CX_OK) { + ZERO(locals_union1); + ZERO(locals_union2); + return; + } + ZERO(locals_union1); - cx_ecfp_init_private_key(CX_CURVE_256K1, - locals_union2.privateKeyData, - 32, - &locals_union1.privateKey); + if (cx_ecfp_init_private_key_no_throw(CX_CURVE_256K1, + locals_union2.privateKeyData, + 32, + &locals_union1.privateKey) != CX_OK) { + ZERO(locals_union1); + ZERO(locals_union2); + return; + } ZERO(locals_union2); - cx_ecfp_generate_pair(CX_CURVE_256K1, &locals_union2.publicKey, &locals_union1.privateKey, 1); + if (cx_ecfp_generate_pair_no_throw(CX_CURVE_256K1, + &locals_union2.publicKey, + &locals_union1.privateKey, + 1) != CX_OK) { + ZERO(locals_union1); + ZERO(locals_union2); + return; + } ZERO(locals_union1); if (!getEthAddressStringFromKey(&locals_union2.publicKey, locals_union1.address, &local_sha3, chain_config->chainId)) { - THROW(CX_INVALID_PARAMETER); + ZERO(locals_union1); + ZERO(locals_union2); + return; } ZERO(locals_union2); @@ -68,8 +86,9 @@ int handle_check_address(check_address_parameters_t* params, chain_config_t* cha if (strcmp(locals_union1.address, params->address_to_check + offset_0x) != 0) { PRINTF("Addresses don't match\n"); - return 0; + } else { + PRINTF("Addresses match\n"); + params->result = 1; } - PRINTF("Addresses match\n"); - return 1; + ZERO(locals_union1); } diff --git a/src/handle_check_address.h b/src/handle_check_address.h index 4267a3035..92db829c5 100644 --- a/src/handle_check_address.h +++ b/src/handle_check_address.h @@ -4,7 +4,7 @@ #include "swap_lib_calls.h" #include "chainConfig.h" -int handle_check_address(check_address_parameters_t* check_address_params, - chain_config_t* chain_config); +void handle_check_address(check_address_parameters_t* check_address_params, + chain_config_t* chain_config); -#endif // _HANDLE_CHECK_ADDRESS_H_ \ No newline at end of file +#endif // _HANDLE_CHECK_ADDRESS_H_ diff --git a/src/handle_get_printable_amount.c b/src/handle_get_printable_amount.c index a890ff5fe..ced188a31 100644 --- a/src/handle_get_printable_amount.c +++ b/src/handle_get_printable_amount.c @@ -7,13 +7,14 @@ #include #include -int handle_get_printable_amount(get_printable_amount_parameters_t* params, chain_config_t* config) { +void handle_get_printable_amount(get_printable_amount_parameters_t* params, + chain_config_t* config) { uint8_t decimals; char ticker[MAX_TICKER_LEN]; memset(params->printable_amount, 0, sizeof(params->printable_amount)); if (params->amount_length > 32) { PRINTF("Amount is too big, 32 bytes max but buffer has %u bytes", params->amount_length); - return 0; + return; } // If the amount is a fee, its value is nominated in ETH even if we're doing an ERC20 swap @@ -29,7 +30,7 @@ int handle_get_printable_amount(get_printable_amount_parameters_t* params, chain ticker, &decimals)) { PRINTF("Error while parsing config\n"); - return 0; + return; } } @@ -39,7 +40,7 @@ int handle_get_printable_amount(get_printable_amount_parameters_t* params, chain ticker, params->printable_amount, sizeof(params->printable_amount))) { - THROW(EXCEPTION_OVERFLOW); + memset(params->printable_amount, 0, sizeof(params->printable_amount)); } - return 1; + return; } diff --git a/src/handle_get_printable_amount.h b/src/handle_get_printable_amount.h index 0bf20153b..b81729404 100644 --- a/src/handle_get_printable_amount.h +++ b/src/handle_get_printable_amount.h @@ -4,7 +4,7 @@ #include "swap_lib_calls.h" #include "chainConfig.h" -int handle_get_printable_amount(get_printable_amount_parameters_t* get_printable_amount_params, - chain_config_t* config); +void handle_get_printable_amount(get_printable_amount_parameters_t* get_printable_amount_params, + chain_config_t* config); -#endif // _HANDLE_GET_PRINTABLE_AMOUNT_H_ \ No newline at end of file +#endif // _HANDLE_GET_PRINTABLE_AMOUNT_H_ diff --git a/src/handle_swap_sign_transaction.c b/src/handle_swap_sign_transaction.c index 495586a3b..7161833c1 100644 --- a/src/handle_swap_sign_transaction.c +++ b/src/handle_swap_sign_transaction.c @@ -41,7 +41,7 @@ bool copy_transaction_parameters(create_transaction_parameters_t* sign_transacti ticker, stack_data.fullAmount, sizeof(stack_data.fullAmount))) { - THROW(EXCEPTION_OVERFLOW); + return false; } // If the amount is a fee, its value is nominated in ETH even if we're doing an ERC20 swap @@ -53,7 +53,7 @@ bool copy_transaction_parameters(create_transaction_parameters_t* sign_transacti ticker, stack_data.maxFee, sizeof(stack_data.maxFee))) { - THROW(EXCEPTION_OVERFLOW); + return false; } // Full reset the global variables @@ -71,9 +71,10 @@ void __attribute__((noreturn)) finalize_exchange_sign_transaction(bool is_succes os_lib_end(); } -void handle_swap_sign_transaction(chain_config_t* config) { - UX_INIT(); +void __attribute__((noreturn)) handle_swap_sign_transaction(chain_config_t* config) { #ifdef HAVE_NBGL + // On Stax, display a spinner at startup + UX_INIT(); nbgl_useCaseSpinner("Signing"); #endif // HAVE_NBGL @@ -93,10 +94,9 @@ void handle_swap_sign_transaction(chain_config_t* config) { nvm_write((void*) &N_storage, (void*) &storage, sizeof(internalStorage_t)); } + PRINTF("USB power ON/OFF\n"); USB_power(0); USB_power(1); - // ui_idle(); - PRINTF("USB power ON/OFF\n"); #ifdef HAVE_BLE // grab the current plane mode setting G_io_app.plane_mode = os_setting_get(OS_SETTING_PLANEMODE, NULL, 0); diff --git a/src/handle_swap_sign_transaction.h b/src/handle_swap_sign_transaction.h index d34a6d860..2c20f1eb5 100644 --- a/src/handle_swap_sign_transaction.h +++ b/src/handle_swap_sign_transaction.h @@ -6,6 +6,6 @@ bool copy_transaction_parameters(create_transaction_parameters_t* sign_transaction_params, chain_config_t* config); -void handle_swap_sign_transaction(chain_config_t* config); +void __attribute__((noreturn)) handle_swap_sign_transaction(chain_config_t* config); void __attribute__((noreturn)) finalize_exchange_sign_transaction(bool is_success); diff --git a/src/main.c b/src/main.c index f8a623211..7a4c2e165 100644 --- a/src/main.c +++ b/src/main.c @@ -731,57 +731,35 @@ void coin_main(libargs_t *args) { app_exit(); } -static void library_main_helper(libargs_t *args) { - check_api_level(CX_COMPAT_APILEVEL); +void library_main(libargs_t *args) { + chain_config_t coin_config; + if (args->chain_config == NULL) { + // We have been started directly by Exchange, not by a Clone. Init default chain + init_coin_config(&coin_config); + args->chain_config = &coin_config; + } + PRINTF("Inside a library \n"); switch (args->command) { case CHECK_ADDRESS: - // ensure result is zero if an exception is thrown - args->check_address->result = 0; - args->check_address->result = - handle_check_address(args->check_address, args->chain_config); + handle_check_address(args->check_address, args->chain_config); break; case SIGN_TRANSACTION: if (copy_transaction_parameters(args->create_transaction, args->chain_config)) { // never returns handle_swap_sign_transaction(args->chain_config); + } else { + // Failed to copy, non recoverable + os_sched_exit(-1); } break; case GET_PRINTABLE_AMOUNT: - // ensure result is zero if an exception is thrown (compatibility breaking, disabled - // until LL is ready) - // args->get_printable_amount->result = 0; - // args->get_printable_amount->result = handle_get_printable_amount(args->get_printable_amount, args->chain_config); break; default: break; } -} - -void library_main(libargs_t *args) { - chain_config_t coin_config; - if (args->chain_config == NULL) { - init_coin_config(&coin_config); - args->chain_config = &coin_config; - } - volatile bool end = false; - /* This loop ensures that library_main_helper and os_lib_end are called - * within a try context, even if an exception is thrown */ - while (1) { - BEGIN_TRY { - TRY { - if (!end) { - library_main_helper(args); - } - os_lib_end(); - } - FINALLY { - end = true; - } - } - END_TRY; - } + os_lib_end(); } __attribute__((section(".boot"))) int main(int arg0) { From 172338698533accdf5b81f4da0a1332ecd856547 Mon Sep 17 00:00:00 2001 From: Francois Beutin Date: Mon, 4 Dec 2023 14:41:46 +0100 Subject: [PATCH 3/3] Split clone main and ethereum main, add comments and remove unnecessary lines --- src/main.c | 81 ++++++++++++++++++++++++++++++++---------------------- 1 file changed, 48 insertions(+), 33 deletions(-) diff --git a/src/main.c b/src/main.c index 7a4c2e165..f5ca89c5f 100644 --- a/src/main.c +++ b/src/main.c @@ -762,69 +762,77 @@ void library_main(libargs_t *args) { os_lib_end(); } -__attribute__((section(".boot"))) int main(int arg0) { -#ifdef USE_LIB_ETHEREUM +/* Eth clones do not actually contain any logic, they delegate everything to the ETH application. + * Start Eth in lib mode with the correct chain config + */ +__attribute__((noreturn)) void clone_main(libargs_t *args) { + PRINTF("Starting in clone_main\n"); BEGIN_TRY { TRY { unsigned int libcall_params[5]; chain_config_t local_chainConfig; init_coin_config(&local_chainConfig); - PRINTF("Hello from Eth-clone\n"); - check_api_level(CX_COMPAT_APILEVEL); - // delegate to Ethereum app/lib libcall_params[0] = (unsigned int) "Ethereum"; libcall_params[1] = 0x100; - libcall_params[2] = RUN_APPLICATION; libcall_params[3] = (unsigned int) &local_chainConfig; -#ifdef HAVE_NBGL - const char app_name[] = APPNAME; - caller_app_t capp; - nbgl_icon_details_t icon_details; - uint8_t bitmap[sizeof(ICONBITMAP)]; - - memcpy(&icon_details, &ICONGLYPH, sizeof(ICONGLYPH)); - memcpy(&bitmap, &ICONBITMAP, sizeof(bitmap)); - icon_details.bitmap = (const uint8_t *) bitmap; - capp.name = app_name; - capp.icon = &icon_details; - libcall_params[4] = (unsigned int) &capp; -#else - libcall_params[4] = NULL; -#endif // HAVE_NBGL - if (arg0) { - // call as a library - libcall_params[2] = ((unsigned int *) arg0)[1]; - libcall_params[4] = ((unsigned int *) arg0)[3]; // library arguments + // Clone called by Exchange, forward the request to Ethereum + if (args != NULL) { + if (args->id != 0x100) { + os_sched_exit(0); + } + libcall_params[2] = args->command; + libcall_params[4] = (unsigned int) args->get_printable_amount; os_lib_call((unsigned int *) &libcall_params); - ((unsigned int *) arg0)[0] = libcall_params[1]; + // Ethereum fulfilled the request and returned to us. We return to Exchange. os_lib_end(); } else { - // launch coin application - libcall_params[1] = 0x100; // use the Init call, as we won't exit + // Clone called from Dashboard, start Ethereum + libcall_params[2] = RUN_APPLICATION; +// On Stax, forward our icon to Ethereum +#ifdef HAVE_NBGL + const char app_name[] = APPNAME; + caller_app_t capp; + nbgl_icon_details_t icon_details; + uint8_t bitmap[sizeof(ICONBITMAP)]; + + memcpy(&icon_details, &ICONGLYPH, sizeof(ICONGLYPH)); + memcpy(&bitmap, &ICONBITMAP, sizeof(bitmap)); + icon_details.bitmap = (const uint8_t *) bitmap; + capp.name = app_name; + capp.icon = &icon_details; + libcall_params[4] = (unsigned int) &capp; +#else + libcall_params[4] = 0; +#endif // HAVE_NBGL os_lib_call((unsigned int *) &libcall_params); + // Ethereum should not return to us + os_sched_exit(-1); } } FINALLY { } } END_TRY; - // no return -#else + + // os_lib_call will raise if Ethereum application is not installed. Do not try to recover. + os_sched_exit(-1); +} + +int ethereum_main(libargs_t *args) { // exit critical section __asm volatile("cpsie i"); // ensure exception will work as planned os_boot(); - if (!arg0) { + if (args == NULL) { // called from dashboard as standalone eth app coin_main(NULL); return 0; } - libargs_t *args = (libargs_t *) arg0; if (args->id != 0x100) { app_exit(); return 0; @@ -838,6 +846,13 @@ __attribute__((section(".boot"))) int main(int arg0) { // called as ethereum or altcoin library library_main(args); } -#endif return 0; } + +__attribute__((section(".boot"))) int main(int arg0) { +#ifdef USE_LIB_ETHEREUM + clone_main((libargs_t *) arg0); +#else + return ethereum_main((libargs_t *) arg0); +#endif +}