From eeb52344df1e48463f5d1de4b69e9e8d52eebae9 Mon Sep 17 00:00:00 2001 From: tdejoigny-ledger Date: Tue, 11 Jul 2023 17:23:49 +0200 Subject: [PATCH 1/7] ETH plugin SDK : Move some parts from plugin boilerplate to Eth plugin SDK to ease the plugins development, remove throw and fix the CI issues --- src/utils.c | 7 +-- src/utils.h | 2 +- src_common/ethUtils.c | 86 ++++++++++++++++++++++-------- src_common/ethUtils.h | 10 ++-- src_features/signTx/logic_signTx.c | 16 +++--- src_plugins_sdk/plugin_main.c | 68 +++++++++++++++++++++++ src_plugins_sdk/plugin_main.h | 19 +++++++ tools/build_sdk.py | 24 +++++++-- 8 files changed, 192 insertions(+), 40 deletions(-) create mode 100644 src_plugins_sdk/plugin_main.c create mode 100644 src_plugins_sdk/plugin_main.h diff --git a/src/utils.c b/src/utils.c index 2c553b4a8..3926bf25e 100644 --- a/src/utils.c +++ b/src/utils.c @@ -116,7 +116,7 @@ bool uint256_to_decimal(const uint8_t *value, size_t value_len, char *out, size_ return true; } -void amountToString(const uint8_t *amount, +bool amountToString(const uint8_t *amount, uint8_t amount_size, uint8_t decimals, const char *ticker, @@ -125,7 +125,7 @@ void amountToString(const uint8_t *amount, char tmp_buffer[100] = {0}; if (uint256_to_decimal(amount, amount_size, tmp_buffer, sizeof(tmp_buffer)) == false) { - THROW(EXCEPTION_OVERFLOW); + return false; } uint8_t amount_len = strnlen(tmp_buffer, sizeof(tmp_buffer)); @@ -141,10 +141,11 @@ void amountToString(const uint8_t *amount, out_buffer + ticker_len, out_buffer_size - ticker_len - 1, decimals) == false) { - THROW(EXCEPTION_OVERFLOW); + return false; } out_buffer[out_buffer_size - 1] = '\0'; + return true; } bool parse_swap_config(const uint8_t *config, uint8_t config_len, char *ticker, uint8_t *decimals) { diff --git a/src/utils.h b/src/utils.h index 504c1b139..c276b6dd2 100644 --- a/src/utils.h +++ b/src/utils.h @@ -34,7 +34,7 @@ uint64_t u64_from_BE(const uint8_t* in, uint8_t size); bool uint256_to_decimal(const uint8_t* value, size_t value_len, char* out, size_t out_len); -void amountToString(const uint8_t* amount, +bool amountToString(const uint8_t* amount, uint8_t amount_len, uint8_t decimals, const char* ticker, diff --git a/src_common/ethUtils.c b/src_common/ethUtils.c index e66e0d2e5..0eb8ca372 100644 --- a/src_common/ethUtils.c +++ b/src_common/ethUtils.c @@ -117,30 +117,59 @@ bool rlpDecodeLength(uint8_t *buffer, uint32_t *fieldLength, uint32_t *offset, b return true; } -void getEthAddressFromKey(cx_ecfp_public_key_t *publicKey, uint8_t *out, cx_sha3_t *sha3Context) { +bool getEthAddressFromKey(cx_ecfp_public_key_t *publicKey, uint8_t *out, cx_sha3_t *sha3Context) { uint8_t hashAddress[INT256_LENGTH]; - cx_keccak_init(sha3Context, 256); - cx_hash((cx_hash_t *) sha3Context, CX_LAST, publicKey->W + 1, 64, hashAddress, 32); + + if (cx_keccak_init_no_throw(sha3Context, 256) != CX_OK) { + return false; + } + + if (cx_hash_no_throw((cx_hash_t *) sha3Context, + CX_LAST, + publicKey->W + 1, + 64, + hashAddress, + 32) != CX_OK) { + return false; + } + memmove(out, hashAddress + 12, 20); + return true; } -void getEthAddressStringFromKey(cx_ecfp_public_key_t *publicKey, +bool getEthAddressStringFromKey(cx_ecfp_public_key_t *publicKey, char *out, cx_sha3_t *sha3Context, uint64_t chainId) { uint8_t hashAddress[INT256_LENGTH]; - cx_keccak_init(sha3Context, 256); - cx_hash((cx_hash_t *) sha3Context, CX_LAST, publicKey->W + 1, 64, hashAddress, 32); - getEthAddressStringFromBinary(hashAddress + 12, out, sha3Context, chainId); + + if (cx_keccak_init_no_throw(sha3Context, 256) != CX_OK) { + return false; + } + + if (cx_hash_no_throw((cx_hash_t *) sha3Context, + CX_LAST, + publicKey->W + 1, + 64, + hashAddress, + 32) != CX_OK) { + return false; + } + + if (!getEthAddressStringFromBinary(hashAddress + 12, out, sha3Context, chainId)) { + return false; + } + + return true; } -void u64_to_string(uint64_t src, char *dst, uint8_t dst_size) { +bool u64_to_string(uint64_t src, char *dst, uint8_t dst_size) { // Copy the numbers in ASCII format. uint8_t i = 0; do { // Checking `i + 1` to make sure we have enough space for '\0'. if (i + 1 >= dst_size) { - THROW(0x6502); + return false; } dst[i] = src % 10 + '0'; src /= 10; @@ -160,9 +189,10 @@ void u64_to_string(uint64_t src, char *dst, uint8_t dst_size) { i--; j++; } + return true; } -void getEthAddressStringFromBinary(uint8_t *address, +bool getEthAddressStringFromBinary(uint8_t *address, char *out, cx_sha3_t *sha3Context, uint64_t chainId) { @@ -182,7 +212,9 @@ void getEthAddressStringFromBinary(uint8_t *address, break; } if (eip1191) { - u64_to_string(chainId, (char *) locals_union.tmp, sizeof(locals_union.tmp)); + if (!u64_to_string(chainId, (char *) locals_union.tmp, sizeof(locals_union.tmp))) { + return false; + } offset = strnlen((char *) locals_union.tmp, sizeof(locals_union.tmp)); strlcat((char *) locals_union.tmp + offset, "0x", sizeof(locals_union.tmp) - offset); offset = strnlen((char *) locals_union.tmp, sizeof(locals_union.tmp)); @@ -192,13 +224,18 @@ void getEthAddressStringFromBinary(uint8_t *address, locals_union.tmp[offset + 2 * i] = HEXDIGITS[(digit >> 4) & 0x0f]; locals_union.tmp[offset + 2 * i + 1] = HEXDIGITS[digit & 0x0f]; } - cx_keccak_init(sha3Context, 256); - cx_hash((cx_hash_t *) sha3Context, - CX_LAST, - locals_union.tmp, - offset + 40, - locals_union.hashChecksum, - 32); + if (cx_keccak_init_no_throw(sha3Context, 256) != CX_OK) { + return false; + } + + if (cx_hash_no_throw((cx_hash_t *) sha3Context, + CX_LAST, + locals_union.tmp, + offset + 40, + locals_union.hashChecksum, + 32) != CX_OK) { + return false; + } for (i = 0; i < 40; i++) { uint8_t digit = address[i / 2]; if ((i % 2) == 0) { @@ -218,24 +255,31 @@ void getEthAddressStringFromBinary(uint8_t *address, } } out[40] = '\0'; + + return true; } /* Fills the `out` buffer with the lowercase string representation of the pubkey passed in as binary format by `in`. (eg: uint8_t*:0xb47e3cd837dDF8e4c57F05d70Ab865de6e193BBB -> char*:"0xb47e3cd837dDF8e4c57F05d70Ab865de6e193BBB\0" ) `sha3` context doesn't have have to be initialized prior to call.*/ -void getEthDisplayableAddress(uint8_t *in, +bool getEthDisplayableAddress(uint8_t *in, char *out, size_t out_len, cx_sha3_t *sha3, uint64_t chainId) { if (out_len < 43) { strlcpy(out, "ERROR", out_len); - return; + return false; } out[0] = '0'; out[1] = 'x'; - getEthAddressStringFromBinary(in, out + 2, sha3, chainId); + if (!getEthAddressStringFromBinary(in, out + 2, sha3, chainId)) { + strlcpy(out, "ERROR", out_len); + return false; + } + + return true; } bool adjustDecimals(const char *src, diff --git a/src_common/ethUtils.h b/src_common/ethUtils.h index 870253351..294224301 100644 --- a/src_common/ethUtils.h +++ b/src_common/ethUtils.h @@ -40,21 +40,21 @@ bool rlpDecodeLength(uint8_t *buffer, uint32_t *fieldLength, uint32_t *offset, b bool rlpCanDecode(uint8_t *buffer, uint32_t bufferLength, bool *valid); -void getEthAddressFromKey(cx_ecfp_public_key_t *publicKey, uint8_t *out, cx_sha3_t *sha3Context); +bool getEthAddressFromKey(cx_ecfp_public_key_t *publicKey, uint8_t *out, cx_sha3_t *sha3Context); -void getEthAddressStringFromKey(cx_ecfp_public_key_t *publicKey, +bool getEthAddressStringFromKey(cx_ecfp_public_key_t *publicKey, char *out, cx_sha3_t *sha3Context, uint64_t chainId); -void u64_to_string(uint64_t src, char *dst, uint8_t dst_size); +bool u64_to_string(uint64_t src, char *dst, uint8_t dst_size); -void getEthAddressStringFromBinary(uint8_t *address, +bool getEthAddressStringFromBinary(uint8_t *address, char *out, cx_sha3_t *sha3Context, uint64_t chainId); -void getEthDisplayableAddress(uint8_t *in, +bool getEthDisplayableAddress(uint8_t *in, char *out, size_t out_len, cx_sha3_t *sha3, diff --git a/src_features/signTx/logic_signTx.c b/src_features/signTx/logic_signTx.c index c8143a0c1..c84e9d108 100644 --- a/src_features/signTx/logic_signTx.c +++ b/src_features/signTx/logic_signTx.c @@ -463,12 +463,16 @@ void finalizeParsing(bool direct) { // Format the amount in a temporary buffer, if in swap case compare it with validated // amount, else commit it - amountToString(tmpContent.txContent.value.value, - tmpContent.txContent.value.length, - decimals, - ticker, - displayBuffer, - sizeof(displayBuffer)); + if (!amountToString(tmpContent.txContent.value.value, + tmpContent.txContent.value.length, + decimals, + ticker, + displayBuffer, + sizeof(displayBuffer))) { + PRINTF("OVERFLOW, amount to string failed\n"); + THROW(EXCEPTION_OVERFLOW); + } + if (G_called_from_swap) { // Ensure the values are the same that the ones that have been previously validated if (strcmp(strings.common.fullAmount, displayBuffer) != 0) { diff --git a/src_plugins_sdk/plugin_main.c b/src_plugins_sdk/plugin_main.c new file mode 100644 index 000000000..06bd3a49f --- /dev/null +++ b/src_plugins_sdk/plugin_main.c @@ -0,0 +1,68 @@ +/***************************************************************************** + * Ledger Plugins SDK. + * (c) 2023 Ledger SAS. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + *****************************************************************************/ + +// Calls the ethereum app. +void call_app_ethereum() { + unsigned int libcall_params[5]; + libcall_params[0] = (unsigned int) "Ethereum"; + libcall_params[1] = 0x100; + libcall_params[2] = RUN_APPLICATION; + libcall_params[3] = (unsigned int) NULL; +#ifdef HAVE_NBGL + caller_app_t capp; + const char name[] = APPNAME; + 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 = (const char *) name; + capp.icon = &icon_details; + libcall_params[4] = (unsigned int) &capp; +#else + libcall_params[4] = (unsigned int) NULL; +#endif + os_lib_call((unsigned int *) &libcall_params); +} + +// Low-level main for plugins. +__attribute__((section(".boot"))) int main(int arg0) { + // Exit critical section + __asm volatile("cpsie i"); + + os_boot(); + + check_api_level(CX_COMPAT_APILEVEL); + + // Check if plugin is called from the dashboard. + if (!arg0) { + // Called from dashboard, launch Ethereum app + call_app_ethereum(); + return 0; + } else { + // Not called from dashboard: called from the ethereum app! + // launch plugin main + plugin_main(arg0); + } + + // Call `os_lib_end`, go back to the ethereum app. + os_lib_end(); + + // Will not get reached. + return 0; +} \ No newline at end of file diff --git a/src_plugins_sdk/plugin_main.h b/src_plugins_sdk/plugin_main.h new file mode 100644 index 000000000..5801aa9bf --- /dev/null +++ b/src_plugins_sdk/plugin_main.h @@ -0,0 +1,19 @@ +/***************************************************************************** + * Ledger Plugins SDK. + * (c) 2023 Ledger SAS. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + *****************************************************************************/ + +// applicative main for plugins +void plugin_main(int arg0); \ No newline at end of file diff --git a/tools/build_sdk.py b/tools/build_sdk.py index 552bb25cf..6cc1e83b4 100755 --- a/tools/build_sdk.py +++ b/tools/build_sdk.py @@ -5,7 +5,7 @@ required to exchange information with ethereum external plugins. It should always be launched from app-ethereum: -python3 ethereum-plugin-sdk/build_sdk.py +python3 tools/build_sdk.py ''' @@ -88,6 +88,7 @@ def merge_headers(sources, nodes_to_extract): '#include "cx.h"', '#ifdef HAVE_NBGL', '#include "nbgl_types.h"', + '#include "glyphs.h"', '#endif' ] @@ -159,7 +160,8 @@ def merge_c_files(sources, nodes_to_extract): "src/shared_context.h", "src/eth_plugin_internal.h", "src/nft.h", - "src/swap_lib_calls.h" + "src/swap_lib_calls.h", + "src_plugins_sdk/plugin_main.h" ] nodes_to_extract = { "#define": ["MAX_TICKER_LEN", "ADDRESS_LENGTH", "INT256_LENGTH", "WEI_TO_ETHER", "SELECTOR_SIZE", "PARAMETER_LENGTH", "RUN_APPLICATION", "COLLECTION_NAME_MAX_LEN"], @@ -168,7 +170,20 @@ def merge_c_files(sources, nodes_to_extract): "typedef union": ["extraInfo_t"], "__attribute__((no_instrument_function)) inline": ["int allzeroes"], "const": ["HEXDIGITS"], - "fn": ["void getEthAddressStringFromBinary", "void getEthAddressFromKey", "void getEthDisplayableAddress", "bool adjustDecimals", "bool uint256_to_decimal", "void amountToString", "void u64_to_string", "void copy_address", "void copy_parameter", "bool U2BE_from_parameter", "bool U4BE_from_parameter"] + "fn": ["bool getEthAddressStringFromBinary", + "bool getEthAddressFromKey", + "bool getEthDisplayableAddress", + "bool adjustDecimals", + "bool uint256_to_decimal", + "bool amountToString", + "bool u64_to_string", + "void copy_address", + "void copy_parameter", + "bool U2BE_from_parameter", + "bool U4BE_from_parameter", + "void plugin_main", + "void call_app_ethereum", + "int main"] } merge_headers(headers_to_merge, nodes_to_extract) @@ -179,6 +194,7 @@ def merge_c_files(sources, nodes_to_extract): c_files_to_merge = [ "src/utils.c", "src_common/ethUtils.c", - "src/eth_plugin_internal.c" + "src/eth_plugin_internal.c", + "src_plugins_sdk/plugin_main.c" ] merge_c_files(c_files_to_merge, nodes_to_extract["fn"]) From 5d1d16c2dee970a24f60b7a419f2d3b34b6038d9 Mon Sep 17 00:00:00 2001 From: tdejoigny-ledger Date: Fri, 4 Aug 2023 13:43:03 +0200 Subject: [PATCH 2/7] Update the ragger app client to support "set external plugin" APDU and take into account PR review remarks --- .../src/ledger_app_clients/ethereum/client.py | 29 ++++++------ .../ethereum/command_builder.py | 17 +++++++ src_plugins_sdk/plugin_main.c | 46 +++++++++++++------ src_plugins_sdk/plugin_main.h | 2 +- 4 files changed, 65 insertions(+), 29 deletions(-) diff --git a/client/src/ledger_app_clients/ethereum/client.py b/client/src/ledger_app_clients/ethereum/client.py index f8b98e2fa..150cfb9f3 100644 --- a/client/src/ledger_app_clients/ethereum/client.py +++ b/client/src/ledger_app_clients/ethereum/client.py @@ -2,7 +2,7 @@ from enum import IntEnum from ragger.backend import BackendInterface from ragger.utils import RAPDU -from typing import List, Optional, Union +from typing import List, Optional from .command_builder import CommandBuilder from .eip712 import EIP712FieldType @@ -30,8 +30,7 @@ class StatusWord(IntEnum): CONDITION_NOT_SATISFIED = 0x6985 REF_DATA_NOT_FOUND = 0x6a88 - -class DOMAIN_NAME_TAG(IntEnum): +class DomainNameTag(IntEnum): STRUCTURE_TYPE = 0x01 STRUCTURE_VERSION = 0x02 CHALLENGE = 0x12 @@ -52,7 +51,7 @@ def _send(self, payload: bytes): return self._client.exchange_async_raw(payload) def response(self) -> Optional[RAPDU]: - return self._client._last_async_response + return self._client.last_async_response def eip712_send_struct_def_struct_name(self, name: str): return self._send(self._cmd_builder.eip712_send_struct_def_struct_name(name)) @@ -190,15 +189,15 @@ def get_public_addr(self, chain_id)) def provide_domain_name(self, challenge: int, name: str, addr: bytes): - payload = format_tlv(DOMAIN_NAME_TAG.STRUCTURE_TYPE, 3) # TrustedDomainName - payload += format_tlv(DOMAIN_NAME_TAG.STRUCTURE_VERSION, 1) - payload += format_tlv(DOMAIN_NAME_TAG.SIGNER_KEY_ID, 0) # test key - payload += format_tlv(DOMAIN_NAME_TAG.SIGNER_ALGO, 1) # secp256k1 - payload += format_tlv(DOMAIN_NAME_TAG.CHALLENGE, challenge) - payload += format_tlv(DOMAIN_NAME_TAG.COIN_TYPE, 0x3c) # ETH in slip-44 - payload += format_tlv(DOMAIN_NAME_TAG.DOMAIN_NAME, name) - payload += format_tlv(DOMAIN_NAME_TAG.ADDRESS, addr) - payload += format_tlv(DOMAIN_NAME_TAG.SIGNATURE, + payload = format_tlv(DomainNameTag.STRUCTURE_TYPE, 3) # TrustedDomainName + payload += format_tlv(DomainNameTag.STRUCTURE_VERSION, 1) + payload += format_tlv(DomainNameTag.SIGNER_KEY_ID, 0) # test key + payload += format_tlv(DomainNameTag.SIGNER_ALGO, 1) # secp256k1 + payload += format_tlv(DomainNameTag.CHALLENGE, challenge) + payload += format_tlv(DomainNameTag.COIN_TYPE, 0x3c) # ETH in slip-44 + payload += format_tlv(DomainNameTag.DOMAIN_NAME, name) + payload += format_tlv(DomainNameTag.ADDRESS, addr) + payload += format_tlv(DomainNameTag.SIGNATURE, sign_data(Key.DOMAIN_NAME, payload)) chunks = self._cmd_builder.provide_domain_name(payload) @@ -271,3 +270,7 @@ def provide_nft_metadata(self, key_id, algo_id, sig)) + + + def external_plugin_setup(self, plugin_name: str, contract_address: bytes, method_selelector: bytes, sig: bytes): + return self._send(self._cmd_builder.external_plugin_setup(plugin_name, contract_address, method_selelector, sig)) diff --git a/client/src/ledger_app_clients/ethereum/command_builder.py b/client/src/ledger_app_clients/ethereum/command_builder.py index e63f061a9..caed1d9f7 100644 --- a/client/src/ledger_app_clients/ethereum/command_builder.py +++ b/client/src/ledger_app_clients/ethereum/command_builder.py @@ -1,3 +1,6 @@ +# documentation about APDU format is available here: +# https://github.com/LedgerHQ/app-ethereum/blob/develop/doc/ethapp.adoc + import struct from enum import IntEnum from typing import Optional @@ -18,6 +21,7 @@ class InsType(IntEnum): EIP712_SIGN = 0x0c GET_CHALLENGE = 0x20 PROVIDE_DOMAIN_NAME = 0x22 + EXTERNAL_PLUGIN_SETUP = 0x12 class P1Type(IntEnum): @@ -178,6 +182,19 @@ def eip712_filtering_show_field(self, name: str, sig: bytes) -> bytes: P2Type.FILTERING_FIELD_NAME, self._eip712_filtering_send_name(name, sig)) + def external_plugin_setup(self, plugin_name: str, contract_address: bytes, method_selelector: bytes, sig: bytes) -> bytes: + data = bytearray() + data.append(len(plugin_name)) + data += self._string_to_bytes(plugin_name) + data += contract_address + data += method_selelector + data += sig + + return self._serialize(InsType.EXTERNAL_PLUGIN_SETUP, + P1Type.COMPLETE_SEND, + 0x00, + data) + def sign(self, bip32_path: str, rlp_data: bytes) -> list[bytes]: apdus = list() payload = pack_derivation_path(bip32_path) diff --git a/src_plugins_sdk/plugin_main.c b/src_plugins_sdk/plugin_main.c index 06bd3a49f..881ad709b 100644 --- a/src_plugins_sdk/plugin_main.c +++ b/src_plugins_sdk/plugin_main.c @@ -49,20 +49,36 @@ __attribute__((section(".boot"))) int main(int arg0) { check_api_level(CX_COMPAT_APILEVEL); - // Check if plugin is called from the dashboard. - if (!arg0) { - // Called from dashboard, launch Ethereum app - call_app_ethereum(); - return 0; - } else { - // Not called from dashboard: called from the ethereum app! - // launch plugin main - plugin_main(arg0); - } + BEGIN_TRY { + TRY { + // Check if plugin is called from the dashboard. + if (!arg0) { + // Called from dashboard, launch Ethereum app + call_app_ethereum(); + + // Will not get reached. + __builtin_unreachable(); + + os_sched_exit(-1); - // Call `os_lib_end`, go back to the ethereum app. - os_lib_end(); + } else { + // Not called from dashboard: called from the ethereum app! + // launch plugin main + plugin_main(arg0); + } - // Will not get reached. - return 0; -} \ No newline at end of file + // Call `os_lib_end`, go back to the ethereum app. + os_lib_end(); + + // Will not get reached. + __builtin_unreachable(); + } + CATCH_OTHER(e) { + PRINTF("Exiting following exception: %d\n", e); + } + FINALLY { + os_lib_end(); + } + } + END_TRY; +} diff --git a/src_plugins_sdk/plugin_main.h b/src_plugins_sdk/plugin_main.h index 5801aa9bf..a120c5a7b 100644 --- a/src_plugins_sdk/plugin_main.h +++ b/src_plugins_sdk/plugin_main.h @@ -16,4 +16,4 @@ *****************************************************************************/ // applicative main for plugins -void plugin_main(int arg0); \ No newline at end of file +void plugin_main(int arg0); From d7bf69b56cdb71069c621ff9364ea6c69e96152d Mon Sep 17 00:00:00 2001 From: tdejoigny-ledger Date: Mon, 18 Sep 2023 11:45:49 +0200 Subject: [PATCH 3/7] take into account PR remarks on Ethereum python client --- client/src/ledger_app_clients/ethereum/client.py | 14 ++++++++++++-- .../ledger_app_clients/ethereum/command_builder.py | 2 +- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/client/src/ledger_app_clients/ethereum/client.py b/client/src/ledger_app_clients/ethereum/client.py index 150cfb9f3..d2015a1aa 100644 --- a/client/src/ledger_app_clients/ethereum/client.py +++ b/client/src/ledger_app_clients/ethereum/client.py @@ -271,6 +271,16 @@ def provide_nft_metadata(self, algo_id, sig)) + def set_external_plugin(self, + plugin_name: str, + contract_address: bytes, + method_selelector: bytes, + sig: Optional[bytes] = None): + if sig is None: + # Temporarily get a command with an empty signature to extract the payload and + # compute the signature on it + tmp = self._cmd_builder.set_external_plugin(plugin_name, contract_address, method_selelector, bytes()) - def external_plugin_setup(self, plugin_name: str, contract_address: bytes, method_selelector: bytes, sig: bytes): - return self._send(self._cmd_builder.external_plugin_setup(plugin_name, contract_address, method_selelector, sig)) + # skip APDU header & empty sig + sig = sign_data(Key.SET_PLUGIN, tmp[5:-1]) + return self._send(self._cmd_builder.set_external_plugin(plugin_name, contract_address, method_selelector, sig)) diff --git a/client/src/ledger_app_clients/ethereum/command_builder.py b/client/src/ledger_app_clients/ethereum/command_builder.py index caed1d9f7..4170a3086 100644 --- a/client/src/ledger_app_clients/ethereum/command_builder.py +++ b/client/src/ledger_app_clients/ethereum/command_builder.py @@ -182,7 +182,7 @@ def eip712_filtering_show_field(self, name: str, sig: bytes) -> bytes: P2Type.FILTERING_FIELD_NAME, self._eip712_filtering_send_name(name, sig)) - def external_plugin_setup(self, plugin_name: str, contract_address: bytes, method_selelector: bytes, sig: bytes) -> bytes: + def set_external_plugin(self, plugin_name: str, contract_address: bytes, method_selelector: bytes, sig: bytes) -> bytes: data = bytearray() data.append(len(plugin_name)) data += self._string_to_bytes(plugin_name) From 9a9e946b50e39e58a4966531ddff5b557139fd3b Mon Sep 17 00:00:00 2001 From: tdejoigny-ledger Date: Tue, 19 Sep 2023 16:02:47 +0200 Subject: [PATCH 4/7] remove obsolete function check_api_level --- src_plugins_sdk/plugin_main.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src_plugins_sdk/plugin_main.c b/src_plugins_sdk/plugin_main.c index 881ad709b..c793cf8b4 100644 --- a/src_plugins_sdk/plugin_main.c +++ b/src_plugins_sdk/plugin_main.c @@ -47,8 +47,6 @@ __attribute__((section(".boot"))) int main(int arg0) { os_boot(); - check_api_level(CX_COMPAT_APILEVEL); - BEGIN_TRY { TRY { // Check if plugin is called from the dashboard. From 49da32af8e1eeb975dca0c5f226a1f5981d80f25 Mon Sep 17 00:00:00 2001 From: Alexandre Paillier Date: Thu, 5 Oct 2023 15:01:58 +0200 Subject: [PATCH 5/7] Added missing return value checks following recent changes --- src/handle_check_address.c | 10 +- src/handle_get_printable_amount.c | 14 +-- src/handle_swap_sign_transaction.c | 28 +++--- src_bagl/ui_flow_stark_sign.c | 17 +++- src_features/getPublicKey/cmd_getPublicKey.c | 10 +- .../cmd_performPrivacyOperation.c | 10 +- src_features/signMessageEIP712/ui_logic.c | 12 ++- src_features/signTx/logic_signTx.c | 15 ++- src_nbgl/ui_stark_transfer.c | 12 ++- src_plugins/erc1155/erc1155_ui.c | 91 ++++++++++-------- src_plugins/erc20/erc20_plugin.c | 26 +++--- src_plugins/erc721/erc721_ui.c | 92 +++++++++++-------- src_plugins/eth2/eth2_plugin.c | 27 +++--- src_plugins/starkware/starkware_plugin.c | 56 ++++++----- 14 files changed, 254 insertions(+), 166 deletions(-) diff --git a/src/handle_check_address.c b/src/handle_check_address.c index 7950f7ea9..ef6d5edc4 100644 --- a/src/handle_check_address.c +++ b/src/handle_check_address.c @@ -53,10 +53,12 @@ int handle_check_address(check_address_parameters_t* params, chain_config_t* cha ZERO(locals_union2); cx_ecfp_generate_pair(CX_CURVE_256K1, &locals_union2.publicKey, &locals_union1.privateKey, 1); ZERO(locals_union1); - getEthAddressStringFromKey(&locals_union2.publicKey, - locals_union1.address, - &local_sha3, - chain_config->chainId); + if (!getEthAddressStringFromKey(&locals_union2.publicKey, + locals_union1.address, + &local_sha3, + chain_config->chainId)) { + THROW(CX_INVALID_PARAMETER); + } ZERO(locals_union2); uint8_t offset_0x = 0; diff --git a/src/handle_get_printable_amount.c b/src/handle_get_printable_amount.c index 715c85d10..a890ff5fe 100644 --- a/src/handle_get_printable_amount.c +++ b/src/handle_get_printable_amount.c @@ -33,11 +33,13 @@ int handle_get_printable_amount(get_printable_amount_parameters_t* params, chain } } - amountToString(params->amount, - params->amount_length, - decimals, - ticker, - params->printable_amount, - sizeof(params->printable_amount)); + if (!amountToString(params->amount, + params->amount_length, + decimals, + ticker, + params->printable_amount, + sizeof(params->printable_amount))) { + THROW(EXCEPTION_OVERFLOW); + } return 1; } diff --git a/src/handle_swap_sign_transaction.c b/src/handle_swap_sign_transaction.c index 604b8a26c..789cbdd14 100644 --- a/src/handle_swap_sign_transaction.c +++ b/src/handle_swap_sign_transaction.c @@ -35,22 +35,26 @@ bool copy_transaction_parameters(create_transaction_parameters_t* sign_transacti PRINTF("Error while parsing config\n"); return false; } - amountToString(sign_transaction_params->amount, - sign_transaction_params->amount_length, - decimals, - ticker, - stack_data.fullAmount, - sizeof(stack_data.fullAmount)); + if (!amountToString(sign_transaction_params->amount, + sign_transaction_params->amount_length, + decimals, + ticker, + stack_data.fullAmount, + sizeof(stack_data.fullAmount))) { + THROW(EXCEPTION_OVERFLOW); + } // If the amount is a fee, its value is nominated in ETH even if we're doing an ERC20 swap strlcpy(ticker, config->coinName, MAX_TICKER_LEN); decimals = WEI_TO_ETHER; - amountToString(sign_transaction_params->fee_amount, - sign_transaction_params->fee_amount_length, - decimals, - ticker, - stack_data.maxFee, - sizeof(stack_data.maxFee)); + if (!amountToString(sign_transaction_params->fee_amount, + sign_transaction_params->fee_amount_length, + decimals, + ticker, + stack_data.maxFee, + sizeof(stack_data.maxFee))) { + THROW(EXCEPTION_OVERFLOW); + } // Full reset the global variables os_explicit_zero_BSS_segment(); diff --git a/src_bagl/ui_flow_stark_sign.c b/src_bagl/ui_flow_stark_sign.c index 548e8808f..03174c79b 100644 --- a/src_bagl/ui_flow_stark_sign.c +++ b/src_bagl/ui_flow_stark_sign.c @@ -4,6 +4,7 @@ #include "ui_callbacks.h" #include "ethUtils.h" #include "starkDisplayUtils.h" +#include "apdu_constants.h" // clang-format off UX_STEP_NOCB(ux_stark_limit_order_1_step, @@ -78,6 +79,16 @@ UX_FLOW(ux_stark_limit_order_flow, &ux_stark_limit_order_7_step, &ux_stark_limit_order_8_step); +static void stark_format_address(void) { + if (!getEthDisplayableAddress(dataContext.starkContext.conditionAddress, + strings.tmp.tmp, + sizeof(strings.tmp.tmp), + &global_sha3, + chainConfig->chainId)) { + THROW(APDU_RESPONSE_ERROR_NO_INFO); + } +} + ////////////////////////////////////////////////////////////////////// // clang-format off UX_STEP_NOCB(ux_stark_transfer_1_step, @@ -167,11 +178,7 @@ UX_STEP_NOCB_INIT( UX_STEP_NOCB_INIT( ux_stark_conditional_transfer_8_step, bnnn_paging, - getEthDisplayableAddress(dataContext.starkContext.conditionAddress, - strings.tmp.tmp, - sizeof(strings.tmp.tmp), - &global_sha3, - chainConfig->chainId), + stark_format_address(), { .title = "Cond. Address", .text = strings.tmp.tmp diff --git a/src_features/getPublicKey/cmd_getPublicKey.c b/src_features/getPublicKey/cmd_getPublicKey.c index bca0a88f1..eae16d5a4 100644 --- a/src_features/getPublicKey/cmd_getPublicKey.c +++ b/src_features/getPublicKey/cmd_getPublicKey.c @@ -49,10 +49,12 @@ void handleGetPublicKey(uint8_t p1, explicit_bzero(&privateKey, sizeof(privateKey)); explicit_bzero(privateKeyData, sizeof(privateKeyData)); io_seproxyhal_io_heartbeat(); - getEthAddressStringFromKey(&tmpCtx.publicKeyContext.publicKey, - tmpCtx.publicKeyContext.address, - &global_sha3, - chainConfig->chainId); + if (!getEthAddressStringFromKey(&tmpCtx.publicKeyContext.publicKey, + tmpCtx.publicKeyContext.address, + &global_sha3, + chainConfig->chainId)) { + THROW(CX_INVALID_PARAMETER); + } uint64_t chain_id = chainConfig->chainId; if (dataLength >= sizeof(chain_id)) { diff --git a/src_features/performPrivacyOperation/cmd_performPrivacyOperation.c b/src_features/performPrivacyOperation/cmd_performPrivacyOperation.c index 5cb95527c..aebb98774 100644 --- a/src_features/performPrivacyOperation/cmd_performPrivacyOperation.c +++ b/src_features/performPrivacyOperation/cmd_performPrivacyOperation.c @@ -62,10 +62,12 @@ void handlePerformPrivacyOperation(uint8_t p1, (tmpCtx.publicKeyContext.getChaincode ? tmpCtx.publicKeyContext.chainCode : NULL)); cx_ecfp_init_private_key(CX_CURVE_256K1, privateKeyData, 32, &privateKey); cx_ecfp_generate_pair(CX_CURVE_256K1, &tmpCtx.publicKeyContext.publicKey, &privateKey, 1); - getEthAddressStringFromKey(&tmpCtx.publicKeyContext.publicKey, - tmpCtx.publicKeyContext.address, - &global_sha3, - chainConfig->chainId); + if (!getEthAddressStringFromKey(&tmpCtx.publicKeyContext.publicKey, + tmpCtx.publicKeyContext.address, + &global_sha3, + chainConfig->chainId)) { + THROW(CX_INVALID_PARAMETER); + } if (p2 == P2_PUBLIC_ENCRYPTION_KEY) { decodeScalar(privateKeyData, privateKeyDataSwapped); cx_ecfp_init_private_key(CX_CURVE_Curve25519, privateKeyDataSwapped, 32, &privateKey); diff --git a/src_features/signMessageEIP712/ui_logic.c b/src_features/signMessageEIP712/ui_logic.c index e6f7b6a64..aa7dcd8cf 100644 --- a/src_features/signMessageEIP712/ui_logic.c +++ b/src_features/signMessageEIP712/ui_logic.c @@ -198,11 +198,13 @@ static bool ui_712_format_addr(const uint8_t *const data, uint8_t length) { return false; } if (ui_712_field_shown()) { - getEthDisplayableAddress((uint8_t *) data, - strings.tmp.tmp, - sizeof(strings.tmp.tmp), - &global_sha3, - chainConfig->chainId); + if (!getEthDisplayableAddress((uint8_t *) data, + strings.tmp.tmp, + sizeof(strings.tmp.tmp), + &global_sha3, + chainConfig->chainId)) { + THROW(APDU_RESPONSE_ERROR_NO_INFO); + } } return true; } diff --git a/src_features/signTx/logic_signTx.c b/src_features/signTx/logic_signTx.c index c84e9d108..5c47e643f 100644 --- a/src_features/signTx/logic_signTx.c +++ b/src_features/signTx/logic_signTx.c @@ -1,3 +1,4 @@ +#include #include "shared_context.h" #include "utils.h" #include "feature_signTx.h" @@ -9,7 +10,7 @@ #include "ethUtils.h" #include "common_ui.h" #include "ui_callbacks.h" -#include +#include "apdu_constants.h" #define ERR_SILENT_MODE_CHECK_FAILED 0x6001 @@ -189,7 +190,9 @@ static void address_to_string(uint8_t *in, cx_sha3_t *sha3, uint64_t chainId) { if (in_len != 0) { - getEthDisplayableAddress(in, out, out_len, sha3, chainId); + if (!getEthDisplayableAddress(in, out, out_len, sha3, chainId)) { + THROW(APDU_RESPONSE_ERROR_NO_INFO); + } } else { strlcpy(out, "Contract", out_len); } @@ -269,7 +272,9 @@ static void get_network_as_string(char *out, size_t out_size) { if (name == NULL) { // No network name found so simply copy the chain ID as the network name. - u64_to_string(chain_id, out, out_size); + if (!u64_to_string(chain_id, out, out_size)) { + THROW(0x6502); + } } else { // Network name found, simply copy it. strlcpy(out, name, out_size); @@ -294,7 +299,9 @@ static void get_public_key(uint8_t *out, uint8_t outLength) { cx_ecfp_generate_pair(CX_CURVE_256K1, &publicKey, &privateKey, 1); explicit_bzero(&privateKey, sizeof(privateKey)); explicit_bzero(privateKeyData, sizeof(privateKeyData)); - getEthAddressFromKey(&publicKey, out, &global_sha3); + if (!getEthAddressFromKey(&publicKey, out, &global_sha3)) { + THROW(CX_INVALID_PARAMETER); + } } /* Local implmentation of strncasecmp, workaround of the segfaulting base implem diff --git a/src_nbgl/ui_stark_transfer.c b/src_nbgl/ui_stark_transfer.c index d7decfbca..bb41e1eb9 100644 --- a/src_nbgl/ui_stark_transfer.c +++ b/src_nbgl/ui_stark_transfer.c @@ -65,11 +65,13 @@ static bool displayTransactionPage(uint8_t page, nbgl_pageContent_t *content) { } if (page == 1) { if (context.conditional) { - getEthDisplayableAddress(dataContext.starkContext.conditionAddress, - condAddressBuffer, - sizeof(condAddressBuffer), - &global_sha3, - chainConfig->chainId); + if (!getEthDisplayableAddress(dataContext.starkContext.conditionAddress, + condAddressBuffer, + sizeof(condAddressBuffer), + &global_sha3, + chainConfig->chainId)) { + return false; + } pairs[0].item = "Cond. Address"; pairs[0].value = condAddressBuffer; diff --git a/src_plugins/erc1155/erc1155_ui.c b/src_plugins/erc1155/erc1155_ui.c index 43067045f..72ea2230d 100644 --- a/src_plugins/erc1155/erc1155_ui.c +++ b/src_plugins/erc1155/erc1155_ui.c @@ -14,11 +14,13 @@ static void set_approval_for_all_ui(ethQueryContractUI_t *msg, erc1155_context_t } else { strlcpy(msg->title, "Revoke", msg->titleLength); } - getEthDisplayableAddress(context->address, - msg->msg, - msg->msgLength, - &global_sha3, - chainConfig->chainId); + if (!getEthDisplayableAddress(context->address, + msg->msg, + msg->msgLength, + &global_sha3, + chainConfig->chainId)) { + msg->result = ETH_PLUGIN_RESULT_ERROR; + } break; case 1: strlcpy(msg->title, "To Manage ALL", msg->titleLength); @@ -26,11 +28,13 @@ static void set_approval_for_all_ui(ethQueryContractUI_t *msg, erc1155_context_t break; case 2: strlcpy(msg->title, "NFT Address", msg->titleLength); - getEthDisplayableAddress(msg->item1->nft.contractAddress, - msg->msg, - msg->msgLength, - &global_sha3, - chainConfig->chainId); + if (!getEthDisplayableAddress(msg->item1->nft.contractAddress, + msg->msg, + msg->msgLength, + &global_sha3, + chainConfig->chainId)) { + msg->result = ETH_PLUGIN_RESULT_ERROR; + } break; default: PRINTF("Unsupported screen index %d\n", msg->screenIndex); @@ -43,11 +47,13 @@ static void set_transfer_ui(ethQueryContractUI_t *msg, erc1155_context_t *contex switch (msg->screenIndex) { case 0: strlcpy(msg->title, "To", msg->titleLength); - getEthDisplayableAddress(context->address, - msg->msg, - msg->msgLength, - &global_sha3, - chainConfig->chainId); + if (!getEthDisplayableAddress(context->address, + msg->msg, + msg->msgLength, + &global_sha3, + chainConfig->chainId)) { + msg->result = ETH_PLUGIN_RESULT_ERROR; + } break; case 1: strlcpy(msg->title, "Collection Name", msg->titleLength); @@ -55,22 +61,28 @@ static void set_transfer_ui(ethQueryContractUI_t *msg, erc1155_context_t *contex break; case 2: strlcpy(msg->title, "NFT Address", msg->titleLength); - getEthDisplayableAddress(msg->item1->nft.contractAddress, - msg->msg, - msg->msgLength, - &global_sha3, - chainConfig->chainId); + if (!getEthDisplayableAddress(msg->item1->nft.contractAddress, + msg->msg, + msg->msgLength, + &global_sha3, + chainConfig->chainId)) { + msg->result = ETH_PLUGIN_RESULT_ERROR; + } break; case 3: strlcpy(msg->title, "NFT ID", msg->titleLength); - uint256_to_decimal(context->tokenId, - sizeof(context->tokenId), - msg->msg, - msg->msgLength); + if (!uint256_to_decimal(context->tokenId, + sizeof(context->tokenId), + msg->msg, + msg->msgLength)) { + msg->result = ETH_PLUGIN_RESULT_ERROR; + } break; case 4: strlcpy(msg->title, "Quantity", msg->titleLength); - tostring256(&context->value, 10, msg->msg, msg->msgLength); + if (!tostring256(&context->value, 10, msg->msg, msg->msgLength)) { + msg->result = ETH_PLUGIN_RESULT_ERROR; + } break; default: PRINTF("Unsupported screen index %d\n", msg->screenIndex); @@ -85,11 +97,13 @@ static void set_batch_transfer_ui(ethQueryContractUI_t *msg, erc1155_context_t * switch (msg->screenIndex) { case 0: strlcpy(msg->title, "To", msg->titleLength); - getEthDisplayableAddress(context->address, - msg->msg, - msg->msgLength, - &global_sha3, - chainConfig->chainId); + if (!getEthDisplayableAddress(context->address, + msg->msg, + msg->msgLength, + &global_sha3, + chainConfig->chainId)) { + msg->result = ETH_PLUGIN_RESULT_ERROR; + } break; case 1: strlcpy(msg->title, "Collection Name", msg->titleLength); @@ -97,15 +111,20 @@ static void set_batch_transfer_ui(ethQueryContractUI_t *msg, erc1155_context_t * break; case 2: strlcpy(msg->title, "NFT Address", msg->titleLength); - getEthDisplayableAddress(msg->item1->nft.contractAddress, - msg->msg, - msg->msgLength, - &global_sha3, - chainConfig->chainId); + if (!getEthDisplayableAddress(msg->item1->nft.contractAddress, + msg->msg, + msg->msgLength, + &global_sha3, + chainConfig->chainId)) { + msg->result = ETH_PLUGIN_RESULT_ERROR; + } break; case 3: strlcpy(msg->title, "Total Quantity", msg->titleLength); - tostring256(&context->value, 10, &quantity_str[0], sizeof(quantity_str)); + if (!tostring256(&context->value, 10, &quantity_str[0], sizeof(quantity_str))) { + msg->result = ETH_PLUGIN_RESULT_ERROR; + break; + } snprintf(msg->msg, msg->msgLength, "%s from %d NFT IDs", diff --git a/src_plugins/erc20/erc20_plugin.c b/src_plugins/erc20/erc20_plugin.c index 220e05f9d..33380dbee 100644 --- a/src_plugins/erc20/erc20_plugin.c +++ b/src_plugins/erc20/erc20_plugin.c @@ -201,12 +201,14 @@ void erc20_plugin_call(int message, void *parameters) { strlcpy(msg->msg, "Unlimited ", msg->msgLength); strlcat(msg->msg, context->ticker, msg->msgLength); } else { - amountToString(context->amount, - sizeof(context->amount), - context->decimals, - context->ticker, - msg->msg, - 100); + if (!amountToString(context->amount, + sizeof(context->amount), + context->decimals, + context->ticker, + msg->msg, + 100)) { + THROW(EXCEPTION_OVERFLOW); + } } msg->result = ETH_PLUGIN_RESULT_OK; break; @@ -216,11 +218,13 @@ void erc20_plugin_call(int message, void *parameters) { strlcpy(msg->msg, context->contract_name, msg->msgLength); } else { strlcpy(msg->title, "Address", msg->titleLength); - getEthDisplayableAddress(context->destinationAddress, - msg->msg, - msg->msgLength, - msg->pluginSharedRW->sha3, - chainConfig->chainId); + if (!getEthDisplayableAddress(context->destinationAddress, + msg->msg, + msg->msgLength, + msg->pluginSharedRW->sha3, + chainConfig->chainId)) { + msg->result = ETH_PLUGIN_RESULT_ERROR; + } } msg->result = ETH_PLUGIN_RESULT_OK; diff --git a/src_plugins/erc721/erc721_ui.c b/src_plugins/erc721/erc721_ui.c index 8fc4c599e..caaf39578 100644 --- a/src_plugins/erc721/erc721_ui.c +++ b/src_plugins/erc721/erc721_ui.c @@ -10,11 +10,13 @@ static void set_approval_ui(ethQueryContractUI_t *msg, erc721_context_t *context switch (msg->screenIndex) { case 0: strlcpy(msg->title, "Allow", msg->titleLength); - getEthDisplayableAddress(context->address, - msg->msg, - msg->msgLength, - &global_sha3, - chainConfig->chainId); + if (!getEthDisplayableAddress(context->address, + msg->msg, + msg->msgLength, + &global_sha3, + chainConfig->chainId)) { + msg->result = ETH_PLUGIN_RESULT_ERROR; + } break; case 1: strlcpy(msg->title, "To Manage Your", msg->titleLength); @@ -22,18 +24,22 @@ static void set_approval_ui(ethQueryContractUI_t *msg, erc721_context_t *context break; case 2: strlcpy(msg->title, "NFT Address", msg->titleLength); - getEthDisplayableAddress(msg->item1->nft.contractAddress, - msg->msg, - msg->msgLength, - &global_sha3, - chainConfig->chainId); + if (!getEthDisplayableAddress(msg->item1->nft.contractAddress, + msg->msg, + msg->msgLength, + &global_sha3, + chainConfig->chainId)) { + msg->result = ETH_PLUGIN_RESULT_ERROR; + } break; case 3: strlcpy(msg->title, "NFT ID", msg->titleLength); - uint256_to_decimal(context->tokenId, - sizeof(context->tokenId), - msg->msg, - msg->msgLength); + if (!uint256_to_decimal(context->tokenId, + sizeof(context->tokenId), + msg->msg, + msg->msgLength)) { + msg->result = ETH_PLUGIN_RESULT_ERROR; + } break; default: PRINTF("Unsupported screen index %d\n", msg->screenIndex); @@ -50,11 +56,13 @@ static void set_approval_for_all_ui(ethQueryContractUI_t *msg, erc721_context_t } else { strlcpy(msg->title, "Revoke", msg->titleLength); } - getEthDisplayableAddress(context->address, - msg->msg, - msg->msgLength, - &global_sha3, - chainConfig->chainId); + if (!getEthDisplayableAddress(context->address, + msg->msg, + msg->msgLength, + &global_sha3, + chainConfig->chainId)) { + msg->result = ETH_PLUGIN_RESULT_ERROR; + } break; case 1: strlcpy(msg->title, "To Manage ALL", msg->titleLength); @@ -62,11 +70,13 @@ static void set_approval_for_all_ui(ethQueryContractUI_t *msg, erc721_context_t break; case 2: strlcpy(msg->title, "NFT Address", msg->titleLength); - getEthDisplayableAddress(msg->item1->nft.contractAddress, - msg->msg, - msg->msgLength, - &global_sha3, - chainConfig->chainId); + if (!getEthDisplayableAddress(msg->item1->nft.contractAddress, + msg->msg, + msg->msgLength, + &global_sha3, + chainConfig->chainId)) { + msg->result = ETH_PLUGIN_RESULT_ERROR; + } break; default: PRINTF("Unsupported screen index %d\n", msg->screenIndex); @@ -79,11 +89,13 @@ static void set_transfer_ui(ethQueryContractUI_t *msg, erc721_context_t *context switch (msg->screenIndex) { case 0: strlcpy(msg->title, "To", msg->titleLength); - getEthDisplayableAddress(context->address, - msg->msg, - msg->msgLength, - &global_sha3, - chainConfig->chainId); + if (!getEthDisplayableAddress(context->address, + msg->msg, + msg->msgLength, + &global_sha3, + chainConfig->chainId)) { + msg->result = ETH_PLUGIN_RESULT_ERROR; + } break; case 1: strlcpy(msg->title, "Collection Name", msg->titleLength); @@ -91,18 +103,22 @@ static void set_transfer_ui(ethQueryContractUI_t *msg, erc721_context_t *context break; case 2: strlcpy(msg->title, "NFT Address", msg->titleLength); - getEthDisplayableAddress(msg->item1->nft.contractAddress, - msg->msg, - msg->msgLength, - &global_sha3, - chainConfig->chainId); + if (!getEthDisplayableAddress(msg->item1->nft.contractAddress, + msg->msg, + msg->msgLength, + &global_sha3, + chainConfig->chainId)) { + msg->result = ETH_PLUGIN_RESULT_ERROR; + } break; case 3: strlcpy(msg->title, "NFT ID", msg->titleLength); - uint256_to_decimal(context->tokenId, - sizeof(context->tokenId), - msg->msg, - msg->msgLength); + if (!uint256_to_decimal(context->tokenId, + sizeof(context->tokenId), + msg->msg, + msg->msgLength)) { + msg->result = ETH_PLUGIN_RESULT_ERROR; + } break; default: PRINTF("Unsupported screen index %d\n", msg->screenIndex); diff --git a/src_plugins/eth2/eth2_plugin.c b/src_plugins/eth2/eth2_plugin.c index c93c82ec6..352a9f3cc 100644 --- a/src_plugins/eth2/eth2_plugin.c +++ b/src_plugins/eth2/eth2_plugin.c @@ -117,11 +117,14 @@ void eth2_plugin_call(int message, void *parameters) { // Use a temporary buffer to store the string representation. char tmp[ETH2_DEPOSIT_PUBKEY_LENGTH]; - getEthDisplayableAddress((uint8_t *) context->deposit_address, - tmp, - sizeof(tmp), - msg->pluginSharedRW->sha3, - chainConfig->chainId); + if (!getEthDisplayableAddress((uint8_t *) context->deposit_address, + tmp, + sizeof(tmp), + msg->pluginSharedRW->sha3, + chainConfig->chainId)) { + msg->result = ETH_PLUGIN_RESULT_ERROR; + return; + } // Copy back the string to the global variable. strlcpy(context->deposit_address, tmp, ETH2_DEPOSIT_PUBKEY_LENGTH); @@ -200,12 +203,14 @@ void eth2_plugin_call(int message, void *parameters) { uint8_t decimals = WEI_TO_ETHER; char *ticker = chainConfig->coinName; strlcpy(msg->title, "Amount", msg->titleLength); - amountToString(tmpContent.txContent.value.value, - tmpContent.txContent.value.length, - decimals, - ticker, - msg->msg, - 100); + if (!amountToString(tmpContent.txContent.value.value, + tmpContent.txContent.value.length, + decimals, + ticker, + msg->msg, + 100)) { + THROW(EXCEPTION_OVERFLOW); + } msg->result = ETH_PLUGIN_RESULT_OK; } break; case 1: { // Deposit pubkey screen diff --git a/src_plugins/starkware/starkware_plugin.c b/src_plugins/starkware/starkware_plugin.c index 634ea4e53..d0ee269fd 100644 --- a/src_plugins/starkware/starkware_plugin.c +++ b/src_plugins/starkware/starkware_plugin.c @@ -6,6 +6,7 @@ #include "stark_utils.h" #include "utils.h" #include "ethUtils.h" +#include "apdu_constants.h" #ifdef HAVE_STARKWARE @@ -352,11 +353,13 @@ void starkware_print_asset_contract(char *destination, size_t destinationLength) if (dataContext.tokenContext.quantumIndex != MAX_ITEMS) { tokenDefinition_t *token = &tmpCtx.transactionContext.extraInfo[dataContext.tokenContext.quantumIndex].token; - getEthDisplayableAddress(token->address, - destination, - destinationLength, - &global_sha3, - chainConfig->chainId); + if (!getEthDisplayableAddress(token->address, + destination, + destinationLength, + &global_sha3, + chainConfig->chainId)) { + THROW(APDU_RESPONSE_ERROR_NO_INFO); + } } else { strlcpy(destination, "UNKNOWN", destinationLength); } @@ -380,7 +383,12 @@ void starkware_get_source_address(char *destination) { io_seproxyhal_io_heartbeat(); destination[0] = '0'; destination[1] = 'x'; - getEthAddressStringFromKey(&publicKey, destination + 2, &global_sha3, chainConfig->chainId); + if (!getEthAddressStringFromKey(&publicKey, + destination + 2, + &global_sha3, + chainConfig->chainId)) { + THROW(CX_INVALID_PARAMETER); + } destination[42] = '\0'; } @@ -716,11 +724,13 @@ void starkware_plugin_call(int message, void *parameters) { if (is_deversify_contract(tmpContent.txContent.destination)) { strlcpy(msg->msg, "DeversiFi", msg->msgLength); } else { - getEthDisplayableAddress(tmpContent.txContent.destination, - msg->msg, - msg->msgLength, - &global_sha3, - chainConfig->chainId); + if (!getEthDisplayableAddress(tmpContent.txContent.destination, + msg->msg, + msg->msgLength, + &global_sha3, + chainConfig->chainId)) { + THROW(APDU_RESPONSE_ERROR_NO_INFO); + } } msg->result = ETH_PLUGIN_RESULT_OK; break; @@ -730,11 +740,13 @@ void starkware_plugin_call(int message, void *parameters) { case STARKWARE_REGISTER_AND_DEPOSIT_TOKEN: case STARKWARE_REGISTER_AND_DEPOSIT_ETH: strlcpy(msg->title, "From ETH Address", msg->titleLength); - getEthDisplayableAddress(context->amount, - msg->msg, - msg->msgLength, - &global_sha3, - chainConfig->chainId); + if (!getEthDisplayableAddress(context->amount, + msg->msg, + msg->msgLength, + &global_sha3, + chainConfig->chainId)) { + THROW(APDU_RESPONSE_ERROR_NO_INFO); + } break; case STARKWARE_ESCAPE: strlcpy(msg->title, "Amount", msg->titleLength); @@ -806,11 +818,13 @@ void starkware_plugin_call(int message, void *parameters) { case STARKWARE_WITHDRAW_TO: case STARKWARE_WITHDRAW_NFT_TO: strlcpy(msg->title, "To ETH Address", msg->titleLength); - getEthDisplayableAddress(context->amount, - msg->msg, - msg->msgLength, - &global_sha3, - chainConfig->chainId); + if (!getEthDisplayableAddress(context->amount, + msg->msg, + msg->msgLength, + &global_sha3, + chainConfig->chainId)) { + THROW(APDU_RESPONSE_ERROR_NO_INFO); + } break; case STARKWARE_WITHDRAW_AND_MINT: strlcpy(msg->title, "Asset Contract", msg->titleLength); From 3e09ee0cff94decb5a9285d9fa1016265a139756 Mon Sep 17 00:00:00 2001 From: Alexandre Paillier Date: Thu, 5 Oct 2023 15:02:16 +0200 Subject: [PATCH 6/7] Moved more of the plugins main.c code to the SDK + updated SDK script Also fixed flake8 warnings --- .../plugin_main.c => src_plugin_sdk/main.c | 51 +++++++++++++++++-- src_plugins_sdk/plugin_main.h | 19 ------- tools/build_sdk.py | 45 +++++++++++----- 3 files changed, 78 insertions(+), 37 deletions(-) rename src_plugins_sdk/plugin_main.c => src_plugin_sdk/main.c (57%) delete mode 100644 src_plugins_sdk/plugin_main.h diff --git a/src_plugins_sdk/plugin_main.c b/src_plugin_sdk/main.c similarity index 57% rename from src_plugins_sdk/plugin_main.c rename to src_plugin_sdk/main.c index c793cf8b4..76c3d4dac 100644 --- a/src_plugins_sdk/plugin_main.c +++ b/src_plugin_sdk/main.c @@ -1,6 +1,6 @@ /***************************************************************************** - * Ledger Plugins SDK. - * (c) 2023 Ledger SAS. + * Ledger Plugin SDK + * (c) 2023 Ledger SAS * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,6 +15,17 @@ * limitations under the License. *****************************************************************************/ +#include "eth_internals.h" +#include "eth_plugin_interface.h" + +// Functions implemented by the plugin +void handle_init_contract(ethPluginInitContract_t *parameters); +void handle_provide_parameter(ethPluginProvideParameter_t *parameters); +void handle_finalize(ethPluginFinalize_t *parameters); +void handle_provide_token(ethPluginProvideInfo_t *parameters); +void handle_query_contract_id(ethQueryContractID_t *parameters); +void handle_query_contract_ui(ethQueryContractUI_t *parameters); + // Calls the ethereum app. void call_app_ethereum() { unsigned int libcall_params[5]; @@ -40,6 +51,33 @@ void call_app_ethereum() { os_lib_call((unsigned int *) &libcall_params); } +// Function to dispatch calls from the ethereum app. +static void dispatch_call(int message, void *parameters) { + switch (message) { + case ETH_PLUGIN_INIT_CONTRACT: + handle_init_contract(parameters); + break; + case ETH_PLUGIN_PROVIDE_PARAMETER: + handle_provide_parameter(parameters); + break; + case ETH_PLUGIN_FINALIZE: + handle_finalize(parameters); + break; + case ETH_PLUGIN_PROVIDE_INFO: + handle_provide_token(parameters); + break; + case ETH_PLUGIN_QUERY_CONTRACT_ID: + handle_query_contract_id(parameters); + break; + case ETH_PLUGIN_QUERY_CONTRACT_UI: + handle_query_contract_ui(parameters); + break; + default: + PRINTF("Unhandled message %d\n", message); + break; + } +} + // Low-level main for plugins. __attribute__((section(".boot"))) int main(int arg0) { // Exit critical section @@ -61,8 +99,13 @@ __attribute__((section(".boot"))) int main(int arg0) { } else { // Not called from dashboard: called from the ethereum app! - // launch plugin main - plugin_main(arg0); + const unsigned int *args = (unsigned int *) arg0; + + // If `ETH_PLUGIN_CHECK_PRESENCE` is set, this means the caller is just trying to + // know whether this app exists or not. We can skip `paraswap_plugin_call`. + if (args[0] != ETH_PLUGIN_CHECK_PRESENCE) { + dispatch_call(args[0], (void *) args[1]); + } } // Call `os_lib_end`, go back to the ethereum app. diff --git a/src_plugins_sdk/plugin_main.h b/src_plugins_sdk/plugin_main.h deleted file mode 100644 index a120c5a7b..000000000 --- a/src_plugins_sdk/plugin_main.h +++ /dev/null @@ -1,19 +0,0 @@ -/***************************************************************************** - * Ledger Plugins SDK. - * (c) 2023 Ledger SAS. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - *****************************************************************************/ - -// applicative main for plugins -void plugin_main(int arg0); diff --git a/tools/build_sdk.py b/tools/build_sdk.py index 6cc1e83b4..bc3af58cc 100755 --- a/tools/build_sdk.py +++ b/tools/build_sdk.py @@ -10,6 +10,7 @@ ''' import os +import shutil def extract_from_headers(sources, nodes_to_extract): @@ -28,12 +29,12 @@ def extract_from_headers(sources, nodes_to_extract): if key in line and value in line: node += [line] unclosed_curvy_brackets = line.count('{') - line.count('}') - if unclosed_curvy_brackets == False: + if not unclosed_curvy_brackets: break elif (key == "fn" and value in line) or unclosed_parantheses: node += [line] unclosed_parantheses = line.find(")") == -1 - if unclosed_parantheses == False: + if not unclosed_parantheses: break elif unclosed_curvy_brackets: node += [line] @@ -150,7 +151,8 @@ def merge_c_files(sources, nodes_to_extract): if __name__ == "__main__": - # some nodes will be extracted from these headers and merged into a new one, copied to sdk + # some nodes will be extracted from these headers and merged into a new + # one, copied to sdk headers_to_merge = [ "src/tokens.h", "src/chainConfig.h", @@ -160,13 +162,23 @@ def merge_c_files(sources, nodes_to_extract): "src/shared_context.h", "src/eth_plugin_internal.h", "src/nft.h", - "src/swap_lib_calls.h", - "src_plugins_sdk/plugin_main.h" + "src/swap_lib_calls.h", ] nodes_to_extract = { - "#define": ["MAX_TICKER_LEN", "ADDRESS_LENGTH", "INT256_LENGTH", "WEI_TO_ETHER", "SELECTOR_SIZE", "PARAMETER_LENGTH", "RUN_APPLICATION", "COLLECTION_NAME_MAX_LEN"], + "#define": ["MAX_TICKER_LEN", + "ADDRESS_LENGTH", + "INT256_LENGTH", + "WEI_TO_ETHER", + "SELECTOR_SIZE", + "PARAMETER_LENGTH", + "RUN_APPLICATION", + "COLLECTION_NAME_MAX_LEN"], "typedef enum": [], - "typedef struct": ["tokenDefinition_t", "txInt256_t", "txContent_t", "nftInfo_t", "caller_app_t"], + "typedef struct": ["tokenDefinition_t", + "txInt256_t", + "txContent_t", + "nftInfo_t", + "caller_app_t"], "typedef union": ["extraInfo_t"], "__attribute__((no_instrument_function)) inline": ["int allzeroes"], "const": ["HEXDIGITS"], @@ -180,21 +192,26 @@ def merge_c_files(sources, nodes_to_extract): "void copy_address", "void copy_parameter", "bool U2BE_from_parameter", - "bool U4BE_from_parameter", - "void plugin_main", - "void call_app_ethereum", - "int main"] + "bool U4BE_from_parameter"] } merge_headers(headers_to_merge, nodes_to_extract) - # this header will be stripped from all #include related to previously merged headers, then copied to sdk + # this header will be stripped from all #include related to previously + # merged headers, then copied to sdk copy_header("src/eth_plugin_interface.h", headers_to_merge) # extract and merge function bodies c_files_to_merge = [ "src/utils.c", "src_common/ethUtils.c", - "src/eth_plugin_internal.c", - "src_plugins_sdk/plugin_main.c" + "src/eth_plugin_internal.c", ] merge_c_files(c_files_to_merge, nodes_to_extract["fn"]) + + files_to_copy = [ + "main.c", + ] + + for file in files_to_copy: + shutil.copyfile("src_plugin_sdk/" + file, + "ethereum-plugin-sdk/include/" + file) From 6126bb65139030a1c3e89342b7bab726d77af966 Mon Sep 17 00:00:00 2001 From: Alexandre Paillier Date: Mon, 9 Oct 2023 17:58:28 +0200 Subject: [PATCH 7/7] Added a find_selector function to the plugin SDK --- src_plugin_sdk/utils.c | 28 ++++++++++++++++++++++++++++ src_plugin_sdk/utils.h | 27 +++++++++++++++++++++++++++ tools/build_sdk.py | 2 ++ 3 files changed, 57 insertions(+) create mode 100644 src_plugin_sdk/utils.c create mode 100644 src_plugin_sdk/utils.h diff --git a/src_plugin_sdk/utils.c b/src_plugin_sdk/utils.c new file mode 100644 index 000000000..059430efd --- /dev/null +++ b/src_plugin_sdk/utils.c @@ -0,0 +1,28 @@ +/***************************************************************************** + * Ledger Plugin SDK + * (c) 2023 Ledger SAS + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + *****************************************************************************/ + +#include "utils.h" + +bool find_selector(uint32_t selector, const uint32_t *array, size_t size, size_t *idx) { + for (size_t i = 0; i < size; ++i) { + if (selector == array[i]) { + if (idx != NULL) *idx = i; + return true; + } + } + return false; +} diff --git a/src_plugin_sdk/utils.h b/src_plugin_sdk/utils.h new file mode 100644 index 000000000..e2816f91d --- /dev/null +++ b/src_plugin_sdk/utils.h @@ -0,0 +1,27 @@ +/***************************************************************************** + * Ledger Plugin SDK + * (c) 2023 Ledger SAS + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + *****************************************************************************/ + +#ifndef UTILS_H_ +#define UTILS_H_ + +#include +#include +#include + +bool find_selector(uint32_t selector, const uint32_t *array, size_t size, size_t *idx); + +#endif // UTILS_H_ diff --git a/tools/build_sdk.py b/tools/build_sdk.py index bc3af58cc..225ef5d39 100755 --- a/tools/build_sdk.py +++ b/tools/build_sdk.py @@ -210,6 +210,8 @@ def merge_c_files(sources, nodes_to_extract): files_to_copy = [ "main.c", + "utils.c", + "utils.h", ] for file in files_to_copy: