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.
This commit is contained in:
Jorge Martins
2022-11-02 13:34:26 +01:00
parent 912c8afca6
commit ead85a0aaa
4 changed files with 35 additions and 1 deletions

View File

@@ -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]

View File

@@ -1,5 +1,6 @@
#include <string.h>
#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

View File

@@ -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 {

View File

@@ -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)