From 0b288cd924f93da4637212abe29ef7d52b99ada2 Mon Sep 17 00:00:00 2001 From: Jorge Martins <106586648+jmartins-ledger@users.noreply.github.com> Date: Tue, 18 Oct 2022 11:09:00 +0200 Subject: [PATCH] Eip712 review (#355) * Possible security fix. It is possible to send a new structure definition after sending a structure implementation, which makes the app treat unrestricted data as if it was a well defined structure. This commit tries to fix that behaviour. Once a structure implementation is sent, we consider all structures to be defined and we do not allow new definitions. * Fix previous commit --- src_features/signMessageEIP712/commands_712.c | 5 +++++ src_features/signMessageEIP712/context_712.h | 2 +- src_features/signMessageEIP712/path.c | 2 ++ 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src_features/signMessageEIP712/commands_712.c b/src_features/signMessageEIP712/commands_712.c index dc29b3d..9945bd8 100644 --- a/src_features/signMessageEIP712/commands_712.c +++ b/src_features/signMessageEIP712/commands_712.c @@ -52,6 +52,11 @@ bool handle_eip712_struct_def(const uint8_t *const apdu_buf) { if (eip712_context == NULL) { ret = eip712_context_init(); } + + if (struct_state == DEFINED) { + ret = false; + } + if (ret) { switch (apdu_buf[OFFSET_P2]) { case P2_DEF_NAME: diff --git a/src_features/signMessageEIP712/context_712.h b/src_features/signMessageEIP712/context_712.h index d4cc6f5..2ae5c8d 100644 --- a/src_features/signMessageEIP712/context_712.h +++ b/src_features/signMessageEIP712/context_712.h @@ -17,7 +17,7 @@ extern s_eip712_context *eip712_context; bool eip712_context_init(void); void eip712_context_deinit(void); -typedef enum { NOT_INITIALIZED, INITIALIZED } e_struct_init; +typedef enum { NOT_INITIALIZED, INITIALIZED, DEFINED } e_struct_init; extern e_struct_init struct_state; #endif // HAVE_EIP712_FULL_SUPPORT diff --git a/src_features/signMessageEIP712/path.c b/src_features/signMessageEIP712/path.c index 998296c..53eddf3 100644 --- a/src_features/signMessageEIP712/path.c +++ b/src_features/signMessageEIP712/path.c @@ -363,6 +363,8 @@ bool path_set_root(const char *const struct_name, uint8_t name_length) { path_struct->root_type = ROOT_MESSAGE; } + struct_state = DEFINED; + // because the first field could be a struct type path_update(); return true;