From ead85a0aaa45755e2068b1ea33363a517020c345 Mon Sep 17 00:00:00 2001 From: Jorge Martins Date: Wed, 2 Nov 2022 13:34:26 +0100 Subject: [PATCH] Add funcs to avoid tricking user when using plugin Usually the length of an array is sent in a parameter. Most of the times the developer simply uses U2BE/U4BE to get this length. It is possible to forge a tx with a `length > sizeof(uint16_t/uint32_t)` and trick the user into signing something different from what is shown. For instance consider the following parameter: 00 ... 01 00 00 00 01 if the developer uses U2BE/U4BE, it is possible that this length is shown to the user and if it is, the user will see the length as 1. --- doc/ethapp_plugins.adoc | 11 +++++++++++ src/eth_plugin_internal.c | 19 +++++++++++++++++++ src/eth_plugin_internal.h | 4 ++++ tools/build_sdk.py | 2 +- 4 files changed, 35 insertions(+), 1 deletion(-) diff --git a/doc/ethapp_plugins.adoc b/doc/ethapp_plugins.adoc index af89087..61f6e0d 100644 --- a/doc/ethapp_plugins.adoc +++ b/doc/ethapp_plugins.adoc @@ -134,6 +134,17 @@ The following return codes are expected, any other will abort the signing proces * ETH_PLUGIN_RESULT_OK : if the plugin can be successfully initialized * ETH_PLUGIN_RESULT_FALLBACK : if the signing logic should fallback to the generic one +There are already defined functions to extract data from a parameter: +[source,C] +---- +void copy_address(uint8_t* dst, const uint8_t* parameter, uint8_t dst_size); +void copy_parameter(uint8_t* dst, const uint8_t* parameter, uint8_t dst_size); + +// Get the value from the beginning of the parameter (right to left) and check if the rest of it is zero +bool U2BE_from_parameter(uint8_t* parameter, uint16_t* value); +bool U4BE_from_parameter(uint8_t* parameter, uint32_t* value); +---- + ### ETH_PLUGIN_FINALIZE [source,C] diff --git a/src/eth_plugin_internal.c b/src/eth_plugin_internal.c index b266969..b608177 100644 --- a/src/eth_plugin_internal.c +++ b/src/eth_plugin_internal.c @@ -1,5 +1,6 @@ #include #include "eth_plugin_internal.h" +#include "ethUtils.h" // allzeroes bool erc20_plugin_available_check(void); @@ -15,6 +16,24 @@ void copy_parameter(uint8_t* dst, const uint8_t* parameter, uint8_t dst_size) { memmove(dst, parameter, copy_size); } +bool U2BE_from_parameter(uint8_t* parameter, uint16_t* value) { + if (allzeroes(parameter, PARAMETER_LENGTH - sizeof(uint16_t))) { + *value = U2BE(parameter, PARAMETER_LENGTH - sizeof(uint16_t)); + return true; + } + + return false; +} + +bool U4BE_from_parameter(uint8_t* parameter, uint32_t* value) { + if (allzeroes(parameter, PARAMETER_LENGTH - sizeof(uint32_t))) { + *value = U4BE(parameter, PARAMETER_LENGTH - sizeof(uint32_t)); + return true; + } + + return false; +} + #ifdef HAVE_STARKWARE void starkware_plugin_call(int message, void* parameters); #endif diff --git a/src/eth_plugin_internal.h b/src/eth_plugin_internal.h index 1ad8382..3d7ee55 100644 --- a/src/eth_plugin_internal.h +++ b/src/eth_plugin_internal.h @@ -16,6 +16,10 @@ void copy_parameter(uint8_t* dst, const uint8_t* parameter, uint8_t dst_size); void erc721_plugin_call(int message, void* parameters); void erc1155_plugin_call(int message, void* parameters); +// Get the value from the beginning of the parameter (right to left) and check if the rest of it is zero +bool U2BE_from_parameter(uint8_t* parameter, uint16_t* value); +bool U4BE_from_parameter(uint8_t* parameter, uint32_t* value); + typedef bool (*PluginAvailableCheck)(void); typedef struct internalEthPlugin_t { diff --git a/tools/build_sdk.py b/tools/build_sdk.py index aab3a6e..b68b368 100755 --- a/tools/build_sdk.py +++ b/tools/build_sdk.py @@ -164,7 +164,7 @@ if __name__ == "__main__": "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"] + "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", "U4BE_from_parameter"] } merge_headers(headers_to_merge, nodes_to_extract)