From 2f13f4c352f5dc2060b23be4806439d8d032e1bd Mon Sep 17 00:00:00 2001 From: Alexandre Paillier Date: Thu, 22 Aug 2024 11:59:48 +0200 Subject: [PATCH] Fix EIP-712 end of empty arrays traversal If the app was skipping an empty array that was followed by a non-empty array of structs, the app would not stop at the struct array field but instead at its first non-array child, so its typehash was not computed and the resulting message hash was wrong. --- src_features/signMessageEIP712/path.c | 15 +++++++-------- src_features/signMessageEIP712/path.h | 2 +- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src_features/signMessageEIP712/path.c b/src_features/signMessageEIP712/path.c index 732cbcb46..c403880ac 100644 --- a/src_features/signMessageEIP712/path.c +++ b/src_features/signMessageEIP712/path.c @@ -287,9 +287,10 @@ static bool array_depth_list_pop(void) { * * @param[in] skip_if_array skip if path is already pointing at an array * @param[in] stop_at_array stop at the first downstream array + * @param[in] do_typehash if a typehash needs to be done when a new struct is encountered * @return whether the path update worked or not */ -static bool path_update(bool skip_if_array, bool stop_at_array) { +static bool path_update(bool skip_if_array, bool stop_at_array, bool do_typehash) { uint8_t fields_count; const void *struct_ptr; const void *starting_field_ptr; @@ -327,9 +328,7 @@ static bool path_update(bool skip_if_array, bool stop_at_array) { return false; } - // The only times they are both at false is if we are traversing an empty array, - // don't do a typehash in that case - if ((skip_if_array != false) || (stop_at_array != false)) { + if (do_typehash) { // get the struct typehash if (type_hash(typename, typename_len, hash) == false) { return false; @@ -399,7 +398,7 @@ bool path_set_root(const char *const struct_name, uint8_t name_length) { struct_state = DEFINED; // because the first field could be a struct type - path_update(true, true); + path_update(true, true, true); return true; } @@ -467,7 +466,7 @@ bool path_new_array_depth(const uint8_t *const data, uint8_t length) { } array_size = *data; - if (!path_update(false, array_size > 0)) { + if (!path_update(false, array_size > 0, array_size > 0)) { return false; } array_depth_count_bak = path_struct->array_depth_count; @@ -583,7 +582,7 @@ static bool path_advance_in_array(void) { * * @return whether the advancement was successful or not */ -bool path_advance(bool array_check) { +bool path_advance(bool do_typehash) { bool end_reached; do { @@ -593,7 +592,7 @@ bool path_advance(bool array_check) { end_reached = false; } } while (end_reached); - return path_update(array_check, array_check); + return path_update(true, true, do_typehash); } /** diff --git a/src_features/signMessageEIP712/path.h b/src_features/signMessageEIP712/path.h index fbb655eec..336659e85 100644 --- a/src_features/signMessageEIP712/path.h +++ b/src_features/signMessageEIP712/path.h @@ -28,7 +28,7 @@ typedef struct { bool path_set_root(const char *const struct_name, uint8_t length); const void *path_get_field(void); -bool path_advance(bool array_check); +bool path_advance(bool do_typehash); bool path_init(void); void path_deinit(void); bool path_new_array_depth(const uint8_t *const data, uint8_t length);