From 89968d703d81fe52ac2bfc25c09e869a2f14f46a Mon Sep 17 00:00:00 2001 From: Charles-Edouard de la Vergne Date: Mon, 5 May 2025 15:35:53 +0200 Subject: [PATCH 1/5] Move network related stuff in dedicated file --- src/network.c | 17 +++++++++++++++++ src/network.h | 1 + src_features/signTx/feature_signTx.h | 1 - src_features/signTx/logic_signTx.c | 16 ---------------- 4 files changed, 18 insertions(+), 17 deletions(-) diff --git a/src/network.c b/src/network.c index 7a614fb3f..c1095ab80 100644 --- a/src/network.c +++ b/src/network.c @@ -4,6 +4,7 @@ #include "network_info.h" #include "shared_context.h" #include "common_utils.h" +#include "apdu_constants.h" const char g_unknown_ticker[] = "???"; @@ -156,6 +157,22 @@ const char *get_network_name_from_chain_id(const uint64_t *chain_id) { return PIC(net->name); } +uint16_t get_network_as_string(char *out, size_t out_size) { + uint64_t chain_id = get_tx_chain_id(); + const char *name = get_network_name_from_chain_id(&chain_id); + + if (name == NULL) { + // No network name found so simply copy the chain ID as the network name. + if (!u64_to_string(chain_id, out, out_size)) { + return APDU_RESPONSE_CHAINID_OUT_BUF_SMALL; + } + } else { + // Network name found, simply copy it. + strlcpy(out, name, out_size); + } + return APDU_RESPONSE_OK; +} + const char *get_network_ticker_from_chain_id(const uint64_t *chain_id) { const network_info_t *net = get_network_from_chain_id(chain_id); diff --git a/src/network.h b/src/network.h index 5dfb62bfc..cef97db95 100644 --- a/src/network.h +++ b/src/network.h @@ -27,6 +27,7 @@ extern network_info_t DYNAMIC_NETWORK_INFO[]; extern const char g_unknown_ticker[]; const char *get_network_name_from_chain_id(const uint64_t *chain_id); +uint16_t get_network_as_string(char *out, size_t out_size); const char *get_network_ticker_from_chain_id(const uint64_t *chain_id); bool chain_is_ethereum_compatible(const uint64_t *chain_id); diff --git a/src_features/signTx/feature_signTx.h b/src_features/signTx/feature_signTx.h index 6a23e6d9b..90a602a2c 100644 --- a/src_features/signTx/feature_signTx.h +++ b/src_features/signTx/feature_signTx.h @@ -31,6 +31,5 @@ bool max_transaction_fee_to_string(const txInt256_t *BEGasPrice, const txInt256_t *BEGasLimit, char *displayBuffer, uint32_t displayBufferSize); -uint16_t get_network_as_string(char *out, size_t out_size); #endif // _SIGN_TX_H_ diff --git a/src_features/signTx/logic_signTx.c b/src_features/signTx/logic_signTx.c index bac7e92a7..ad4c65bbe 100644 --- a/src_features/signTx/logic_signTx.c +++ b/src_features/signTx/logic_signTx.c @@ -292,22 +292,6 @@ static void nonce_to_string(const txInt256_t *nonce, char *out, size_t out_size) tostring256(&nonce_uint256, 10, out, out_size); } -uint16_t get_network_as_string(char *out, size_t out_size) { - uint64_t chain_id = get_tx_chain_id(); - const char *name = get_network_name_from_chain_id(&chain_id); - - if (name == NULL) { - // No network name found so simply copy the chain ID as the network name. - if (!u64_to_string(chain_id, out, out_size)) { - return APDU_RESPONSE_CHAINID_OUT_BUF_SMALL; - } - } else { - // Network name found, simply copy it. - strlcpy(out, name, out_size); - } - return APDU_RESPONSE_OK; -} - uint16_t get_public_key(uint8_t *out, uint8_t outLength) { uint8_t raw_pubkey[65]; cx_err_t error = CX_INTERNAL_ERROR; From 5cc8d21f3b38c2272439d9c7eb4c27c4ad9dcdb3 Mon Sep 17 00:00:00 2001 From: Charles-Edouard de la Vergne Date: Mon, 5 May 2025 16:35:47 +0200 Subject: [PATCH 2/5] Factorize 'get_public_key' code --- .../gtp_param_trusted_name.c | 2 +- src_features/generic_tx_parser/gtp_value.c | 2 +- src_features/getPublicKey/cmd_getPublicKey.c | 2 +- src_features/getPublicKey/getPublicKey.c | 39 +++++++++++++++++++ ...{feature_getPublicKey.h => getPublicKey.h} | 1 + .../getPublicKey/logic_getPublicKey.c | 17 -------- .../getPublicKey/ui_common_getPublicKey.c | 2 +- .../cmd_get_tx_simulation.c | 2 +- src_features/signTx/feature_signTx.h | 1 - src_features/signTx/logic_signTx.c | 22 +---------- 10 files changed, 46 insertions(+), 44 deletions(-) create mode 100644 src_features/getPublicKey/getPublicKey.c rename src_features/getPublicKey/{feature_getPublicKey.h => getPublicKey.h} (71%) delete mode 100644 src_features/getPublicKey/logic_getPublicKey.c diff --git a/src_features/generic_tx_parser/gtp_param_trusted_name.c b/src_features/generic_tx_parser/gtp_param_trusted_name.c index 80a781d9d..26b330ff4 100644 --- a/src_features/generic_tx_parser/gtp_param_trusted_name.c +++ b/src_features/generic_tx_parser/gtp_param_trusted_name.c @@ -7,7 +7,7 @@ #include "gtp_field_table.h" #include "utils.h" #include "shared_context.h" -#include "feature_signTx.h" // get_public_key +#include "getPublicKey.h" #include "apdu_constants.h" enum { diff --git a/src_features/generic_tx_parser/gtp_value.c b/src_features/generic_tx_parser/gtp_value.c index d1437b90d..6ce3a2395 100644 --- a/src_features/generic_tx_parser/gtp_value.c +++ b/src_features/generic_tx_parser/gtp_value.c @@ -5,7 +5,7 @@ #include "gtp_data_path.h" #include "shared_context.h" // txContext #include "apdu_constants.h" // APDU_RESPONSE_OK -#include "feature_signTx.h" // get_public_key +#include "getPublicKey.h" #include "gtp_parsed_value.h" enum { diff --git a/src_features/getPublicKey/cmd_getPublicKey.c b/src_features/getPublicKey/cmd_getPublicKey.c index 39d6702cd..a6c975bfa 100644 --- a/src_features/getPublicKey/cmd_getPublicKey.c +++ b/src_features/getPublicKey/cmd_getPublicKey.c @@ -1,7 +1,7 @@ #include "shared_context.h" #include "apdu_constants.h" #include "common_utils.h" -#include "feature_getPublicKey.h" +#include "getPublicKey.h" #include "common_ui.h" #include "os_io_seproxyhal.h" #include "crypto_helpers.h" diff --git a/src_features/getPublicKey/getPublicKey.c b/src_features/getPublicKey/getPublicKey.c new file mode 100644 index 000000000..f31166097 --- /dev/null +++ b/src_features/getPublicKey/getPublicKey.c @@ -0,0 +1,39 @@ +#include +#include "crypto_helpers.h" +#include "shared_context.h" +#include "apdu_constants.h" + +uint16_t get_public_key(uint8_t *out, uint8_t outLength) { + uint8_t raw_pubkey[65]; + cx_err_t error = CX_INTERNAL_ERROR; + + if (outLength < ADDRESS_LENGTH) { + return APDU_RESPONSE_WRONG_DATA_LENGTH; + } + CX_CHECK(bip32_derive_get_pubkey_256(CX_CURVE_256K1, + tmpCtx.transactionContext.bip32.path, + tmpCtx.transactionContext.bip32.length, + raw_pubkey, + NULL, + CX_SHA512)); + + getEthAddressFromRawKey(raw_pubkey, out); + error = APDU_RESPONSE_OK; +end: + return error; +} + +uint32_t set_result_get_publicKey() { + uint32_t tx = 0; + G_io_apdu_buffer[tx++] = 65; + memmove(G_io_apdu_buffer + tx, tmpCtx.publicKeyContext.publicKey.W, 65); + tx += 65; + G_io_apdu_buffer[tx++] = 40; + memmove(G_io_apdu_buffer + tx, tmpCtx.publicKeyContext.address, 40); + tx += 40; + if (tmpCtx.publicKeyContext.getChaincode) { + memmove(G_io_apdu_buffer + tx, tmpCtx.publicKeyContext.chainCode, 32); + tx += 32; + } + return tx; +} diff --git a/src_features/getPublicKey/feature_getPublicKey.h b/src_features/getPublicKey/getPublicKey.h similarity index 71% rename from src_features/getPublicKey/feature_getPublicKey.h rename to src_features/getPublicKey/getPublicKey.h index 8f819ba36..b95203839 100644 --- a/src_features/getPublicKey/feature_getPublicKey.h +++ b/src_features/getPublicKey/getPublicKey.h @@ -3,6 +3,7 @@ #include "shared_context.h" +uint16_t get_public_key(uint8_t *out, uint8_t outLength); uint32_t set_result_get_publicKey(void); #endif // _GET_PUB_KEY_H_ diff --git a/src_features/getPublicKey/logic_getPublicKey.c b/src_features/getPublicKey/logic_getPublicKey.c deleted file mode 100644 index bc58e92fb..000000000 --- a/src_features/getPublicKey/logic_getPublicKey.c +++ /dev/null @@ -1,17 +0,0 @@ -#include -#include "shared_context.h" - -uint32_t set_result_get_publicKey() { - uint32_t tx = 0; - G_io_apdu_buffer[tx++] = 65; - memmove(G_io_apdu_buffer + tx, tmpCtx.publicKeyContext.publicKey.W, 65); - tx += 65; - G_io_apdu_buffer[tx++] = 40; - memmove(G_io_apdu_buffer + tx, tmpCtx.publicKeyContext.address, 40); - tx += 40; - if (tmpCtx.publicKeyContext.getChaincode) { - memmove(G_io_apdu_buffer + tx, tmpCtx.publicKeyContext.chainCode, 32); - tx += 32; - } - return tx; -} diff --git a/src_features/getPublicKey/ui_common_getPublicKey.c b/src_features/getPublicKey/ui_common_getPublicKey.c index abee55425..938eb0147 100644 --- a/src_features/getPublicKey/ui_common_getPublicKey.c +++ b/src_features/getPublicKey/ui_common_getPublicKey.c @@ -1,5 +1,5 @@ #include "apdu_constants.h" -#include "feature_getPublicKey.h" +#include "getPublicKey.h" #include "ui_callbacks.h" unsigned int io_seproxyhal_touch_address_ok(void) { diff --git a/src_features/provide_tx_simulation/cmd_get_tx_simulation.c b/src_features/provide_tx_simulation/cmd_get_tx_simulation.c index 0b7ae9fc9..3938f5664 100644 --- a/src_features/provide_tx_simulation/cmd_get_tx_simulation.c +++ b/src_features/provide_tx_simulation/cmd_get_tx_simulation.c @@ -4,7 +4,7 @@ #include "apdu_constants.h" #include "hash_bytes.h" #include "public_keys.h" -#include "feature_signTx.h" +#include "getPublicKey.h" #include "tlv.h" #include "tlv_apdu.h" #include "mem.h" diff --git a/src_features/signTx/feature_signTx.h b/src_features/signTx/feature_signTx.h index 90a602a2c..ab4cf6175 100644 --- a/src_features/signTx/feature_signTx.h +++ b/src_features/signTx/feature_signTx.h @@ -26,7 +26,6 @@ void start_signature_flow(void); uint16_t handle_parsing_status(parserStatus_e status); -uint16_t get_public_key(uint8_t *out, uint8_t outLength); bool max_transaction_fee_to_string(const txInt256_t *BEGasPrice, const txInt256_t *BEGasLimit, char *displayBuffer, diff --git a/src_features/signTx/logic_signTx.c b/src_features/signTx/logic_signTx.c index ad4c65bbe..9d065d98e 100644 --- a/src_features/signTx/logic_signTx.c +++ b/src_features/signTx/logic_signTx.c @@ -8,13 +8,13 @@ #include "common_ui.h" #include "ui_callbacks.h" #include "apdu_constants.h" -#include "crypto_helpers.h" #include "format.h" #include "manage_asset_info.h" #include "handle_swap_sign_transaction.h" #include "os_math.h" #include "calldata.h" #include "swap_error_code_helpers.h" +#include "getPublicKey.h" static bool g_use_standard_ui; @@ -292,26 +292,6 @@ static void nonce_to_string(const txInt256_t *nonce, char *out, size_t out_size) tostring256(&nonce_uint256, 10, out, out_size); } -uint16_t get_public_key(uint8_t *out, uint8_t outLength) { - uint8_t raw_pubkey[65]; - cx_err_t error = CX_INTERNAL_ERROR; - - if (outLength < ADDRESS_LENGTH) { - return APDU_RESPONSE_WRONG_DATA_LENGTH; - } - CX_CHECK(bip32_derive_get_pubkey_256(CX_CURVE_256K1, - tmpCtx.transactionContext.bip32.path, - tmpCtx.transactionContext.bip32.length, - raw_pubkey, - NULL, - CX_SHA512)); - - getEthAddressFromRawKey(raw_pubkey, out); - error = APDU_RESPONSE_OK; -end: - return error; -} - /* Local implementation of strncasecmp, workaround of the segfaulting base implem * Remove once strncasecmp is fixed */ From 9ae5edb55f400aafc95276bb462eb9afcf1bb8e5 Mon Sep 17 00:00:00 2001 From: Charles-Edouard de la Vergne Date: Mon, 5 May 2025 17:00:25 +0200 Subject: [PATCH 3/5] Factorize 'get_public_key_string' code --- src/handle_check_address.c | 10 ++-------- src_features/getPublicKey/cmd_getPublicKey.c | 12 ++++-------- src_features/getPublicKey/getPublicKey.c | 19 +++++++++++++++++++ src_features/getPublicKey/getPublicKey.h | 5 +++++ .../signAuthorizationEIP7702/commands_7702.c | 13 ++++++------- 5 files changed, 36 insertions(+), 23 deletions(-) diff --git a/src/handle_check_address.c b/src/handle_check_address.c index 0fc790524..a070135b8 100644 --- a/src/handle_check_address.c +++ b/src/handle_check_address.c @@ -1,6 +1,7 @@ #include "handle_check_address.h" #include "apdu_constants.h" #include "crypto_helpers.h" +#include "getPublicKey.h" #define ZERO(x) explicit_bzero(&x, sizeof(x)) @@ -26,14 +27,7 @@ uint16_t handle_check_address(check_address_parameters_t* params, chain_config_t PRINTF("Invalid path\n"); return APDU_RESPONSE_INVALID_DATA; } - CX_CHECK(bip32_derive_get_pubkey_256(CX_CURVE_256K1, - bip32.path, - bip32.length, - raw_pubkey, - NULL, - CX_SHA512)); - - getEthAddressStringFromRawKey((const uint8_t*) raw_pubkey, address, chain_config->chainId); + CX_CHECK(get_public_key_string(&bip32, raw_pubkey, address, NULL, chain_config->chainId)); uint8_t offset_0x = 0; if (memcmp(params->address_to_check, "0x", 2) == 0) { diff --git a/src_features/getPublicKey/cmd_getPublicKey.c b/src_features/getPublicKey/cmd_getPublicKey.c index a6c975bfa..0c070e6c0 100644 --- a/src_features/getPublicKey/cmd_getPublicKey.c +++ b/src_features/getPublicKey/cmd_getPublicKey.c @@ -34,16 +34,12 @@ uint16_t handleGetPublicKey(uint8_t p1, } tmpCtx.publicKeyContext.getChaincode = (p2 == P2_CHAINCODE); - CX_CHECK(bip32_derive_get_pubkey_256( - CX_CURVE_256K1, - bip32.path, - bip32.length, + CX_CHECK(get_public_key_string( + &bip32, tmpCtx.publicKeyContext.publicKey.W, + tmpCtx.publicKeyContext.address, (tmpCtx.publicKeyContext.getChaincode ? tmpCtx.publicKeyContext.chainCode : NULL), - CX_SHA512)); - getEthAddressStringFromRawKey(tmpCtx.publicKeyContext.publicKey.W, - tmpCtx.publicKeyContext.address, - chainConfig->chainId); + chainConfig->chainId)); uint64_t chain_id = chainConfig->chainId; if (dataLength >= sizeof(chain_id)) { diff --git a/src_features/getPublicKey/getPublicKey.c b/src_features/getPublicKey/getPublicKey.c index f31166097..52c497b42 100644 --- a/src_features/getPublicKey/getPublicKey.c +++ b/src_features/getPublicKey/getPublicKey.c @@ -3,6 +3,25 @@ #include "shared_context.h" #include "apdu_constants.h" +uint16_t get_public_key_string(bip32_path_t *bip32, + uint8_t *pubKey, + char *address, + uint8_t *chainCode, + uint64_t chainId) { + cx_err_t error = CX_INTERNAL_ERROR; + + CX_CHECK(bip32_derive_get_pubkey_256(CX_CURVE_256K1, + bip32->path, + bip32->length, + pubKey, + chainCode, + CX_SHA512)); + getEthAddressStringFromRawKey(pubKey, address, chainId); + error = CX_OK; +end: + return error; +} + uint16_t get_public_key(uint8_t *out, uint8_t outLength) { uint8_t raw_pubkey[65]; cx_err_t error = CX_INTERNAL_ERROR; diff --git a/src_features/getPublicKey/getPublicKey.h b/src_features/getPublicKey/getPublicKey.h index b95203839..7e155c1cd 100644 --- a/src_features/getPublicKey/getPublicKey.h +++ b/src_features/getPublicKey/getPublicKey.h @@ -4,6 +4,11 @@ #include "shared_context.h" uint16_t get_public_key(uint8_t *out, uint8_t outLength); +uint16_t get_public_key_string(bip32_path_t *bip32, + uint8_t *pubKey, + char *address, + uint8_t *chainCode, + uint64_t chainId); uint32_t set_result_get_publicKey(void); #endif // _GET_PUB_KEY_H_ diff --git a/src_features/signAuthorizationEIP7702/commands_7702.c b/src_features/signAuthorizationEIP7702/commands_7702.c index 71626843e..ce7823987 100644 --- a/src_features/signAuthorizationEIP7702/commands_7702.c +++ b/src_features/signAuthorizationEIP7702/commands_7702.c @@ -13,6 +13,7 @@ #include "rlp_encode.h" #include "whitelist_7702.h" #include "auth_7702.h" +#include "getPublicKey.h" // Avoid saving the full structure when parsing // Alternative option : add a callback to f_tlv_payload_handler @@ -126,15 +127,13 @@ static bool handleAuth7702TLV(const uint8_t *payload, uint16_t size, bool to_fre sizeof(tmpCtx.authSigningContext7702.authHash))); // Prepare information to be displayed // * Address to be delegated - CX_CHECK(bip32_derive_get_pubkey_256(CX_CURVE_256K1, - tmpCtx.authSigningContext7702.bip32.path, - tmpCtx.authSigningContext7702.bip32.length, - publicKey.W, - NULL, - CX_SHA512)); strings.common.fromAddress[0] = '0'; strings.common.fromAddress[1] = 'x'; - getEthAddressStringFromRawKey(publicKey.W, strings.common.fromAddress + 2, auth7702->chainId); + CX_CHECK(get_public_key_string(&tmpCtx.authSigningContext7702.bip32, + publicKey.W, + strings.common.fromAddress + 2, + NULL, + auth7702->chainId)); // * Delegate if (!allzeroes(auth7702->delegate, sizeof(auth7702->delegate))) { #ifdef HAVE_EIP7702_WHITELIST From 1061b7000ee8ee0dcada85209ad2ea5304217064 Mon Sep 17 00:00:00 2001 From: Charles-Edouard de la Vergne Date: Mon, 12 May 2025 12:48:38 +0200 Subject: [PATCH 4/5] update strcasecmp_workaround comment --- src_features/signTx/logic_signTx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src_features/signTx/logic_signTx.c b/src_features/signTx/logic_signTx.c index 9d065d98e..09eff1e76 100644 --- a/src_features/signTx/logic_signTx.c +++ b/src_features/signTx/logic_signTx.c @@ -292,7 +292,7 @@ static void nonce_to_string(const txInt256_t *nonce, char *out, size_t out_size) tostring256(&nonce_uint256, 10, out, out_size); } -/* Local implementation of strncasecmp, workaround of the segfaulting base implem +/* Local implementation of strncasecmp, workaround of the segfaulting base implem on return value * Remove once strncasecmp is fixed */ static int strcasecmp_workaround(const char *str1, const char *str2) { From f2608a919e70ab1fdb69c5656c83e5486464bdf0 Mon Sep 17 00:00:00 2001 From: Charles-Edouard de la Vergne Date: Mon, 5 May 2025 18:43:43 +0200 Subject: [PATCH 5/5] Adapt fuzzing tests accordingly --- tests/fuzzing/CMakeLists.txt | 3 ++- tests/fuzzing/src/mock.c | 13 +++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/tests/fuzzing/CMakeLists.txt b/tests/fuzzing/CMakeLists.txt index ef5190e03..c6e2fdd43 100644 --- a/tests/fuzzing/CMakeLists.txt +++ b/tests/fuzzing/CMakeLists.txt @@ -35,7 +35,7 @@ if (NOT DEFINED ENV{LIB_FUZZING_ENGINE}) elseif (SANITIZER MATCHES "memory") set(COMPILATION_FLAGS ${COMPILATION_FLAGS} -fsanitize=fuzzer,memory,undefined -fsanitize-memory-track-origins -fsanitize=fuzzer-no-link) else() - message(FATAL_ERROR "Unkown sanitizer type. It must be set to `address` or `memory`.") + message(FATAL_ERROR "Unknown sanitizer type. It must be set to `address` or `memory`.") endif() else() set(COMPILATION_FLAGS "$ENV{LIB_FUZZING_ENGINE} $ENV{CFLAGS} ${CUSTOM_C_FLAGS}") @@ -184,6 +184,7 @@ include_directories( ${CMAKE_SOURCE_DIR}/../../ethereum-plugin-sdk/src/ ${CMAKE_SOURCE_DIR}/../../src ${CMAKE_SOURCE_DIR}/../../src_features/generic_tx_parser/ + ${CMAKE_SOURCE_DIR}/../../src_features/getPublicKey/ ${CMAKE_SOURCE_DIR}/../../src_features/provide_enum_value/ ${CMAKE_SOURCE_DIR}/../../src_features/provide_network_info/ ${CMAKE_SOURCE_DIR}/../../src_features/signTx/ diff --git a/tests/fuzzing/src/mock.c b/tests/fuzzing/src/mock.c index ee8961098..f67fc05b1 100644 --- a/tests/fuzzing/src/mock.c +++ b/tests/fuzzing/src/mock.c @@ -162,6 +162,19 @@ uint16_t get_public_key(uint8_t *out, uint8_t outLength) { return 0; } +uint16_t get_public_key_string(bip32_path_t *bip32, + uint8_t *pubKey, + char *address, + uint8_t *chainCode, + uint64_t chainId) { + UNUSED(bip32); + UNUSED(pubKey); + UNUSED(chainCode); + UNUSED(chainId); + memset_s(address, 0, 10); + return 0; +} + void ui_gcs_cleanup(void) { }