Merge pull request #608 from LedgerHQ/fix/apa/eip712_filters_counter

EIP-712 filters counter fix + reactivation

Author: apaillier-ledger

src/mem.c

@@ -11,7 +11,7 @@
 #include <stdint.h>
 #include "mem.h"
 
-#define SIZE_MEM_BUFFER 8192
+#define SIZE_MEM_BUFFER 10240
 
 static uint8_t mem_buffer[SIZE_MEM_BUFFER];
 static size_t mem_idx;

src_bagl/ui_flow_signMessage712.c

@@ -12,10 +12,9 @@ static void dummy_cb(void) {
             if (ui_pos == UI_712_POS_REVIEW) {
                 ux_flow_next();
                 ui_pos = UI_712_POS_END;
-            } else  // UI_712_POS_END
-            {
-                ux_flow_prev();
-                ui_pos = UI_712_POS_REVIEW;
+            } else {
+                // Keep user at the end of the flow
+                ux_flow_next();
             }
             break;
         case EIP712_FIELD_INCOMING:

src_features/signMessageEIP712/commands_712.c

@@ -195,6 +195,14 @@ bool handle_eip712_filtering(const uint8_t *const apdu_buf) {
             apdu_response_code = APDU_RESPONSE_INVALID_P1_P2;
             ret = false;
     }
+    if ((apdu_buf[OFFSET_P2] > P2_FILT_MESSAGE_INFO) && ret) {
+        if (ui_712_push_new_filter_path()) {
+            if (!ui_712_filters_counter_incr()) {
+                ret = false;
+                apdu_response_code = APDU_RESPONSE_INVALID_DATA;
+            }
+        }
+    }
     if (reply_apdu) {
         handle_eip712_return_code(ret);
     }
@@ -221,6 +229,10 @@ bool handle_eip712_sign(const uint8_t *const apdu_buf) {
                        sizeof(tmpCtx.messageSigningContext712.messageHash)) ||
              (path_get_field() != NULL)) {
         apdu_response_code = APDU_RESPONSE_CONDITION_NOT_SATISFIED;
+    } else if ((ui_712_get_filtering_mode() == EIP712_FILTERING_FULL) &&
+               (ui_712_remaining_filters() != 0)) {
+        PRINTF("%d EIP712 filters are missing\n", ui_712_remaining_filters());
+        apdu_response_code = APDU_RESPONSE_REF_DATA_NOT_FOUND;
     } else if (parseBip32(&apdu_buf[OFFSET_CDATA], &length, &tmpCtx.messageSigningContext.bip32) !=
                NULL) {
         if (!N_storage.verbose_eip712 && (ui_712_get_filtering_mode() == EIP712_FILTERING_BASIC)) {

src_features/signMessageEIP712/filtering.c

@@ -11,6 +11,7 @@
 #include "typed_data.h"
 #include "path.h"
 #include "ui_logic.h"
+#include "filtering.h"
 
 #define FILT_MAGIC_MESSAGE_INFO      183
 #define FILT_MAGIC_AMOUNT_JOIN_TOKEN 11
@@ -188,6 +189,10 @@ bool filtering_message_info(const uint8_t *payload, uint8_t length) {
         return false;
     }
     filters_count = payload[offset++];
+    if (filters_count > MAX_FILTERS) {
+        PRINTF("%u filters planned but can only store up to %u.\n", filters_count, MAX_FILTERS);
+        return false;
+    }
     if ((offset + sizeof(sig_len)) > length) {
         return false;
     }

src_features/signMessageEIP712/filtering.h

@@ -6,6 +6,8 @@
 #include <stdbool.h>
 #include <stdint.h>
 
+#define MAX_FILTERS 50
+
 bool filtering_message_info(const uint8_t *payload, uint8_t length);
 bool filtering_date_time(const uint8_t *payload, uint8_t length);
 bool filtering_amount_join_token(const uint8_t *payload, uint8_t length);

src_features/signMessageEIP712/path.c

@@ -9,7 +9,6 @@
 #include "type_hash.h"
 #include "shared_context.h"
 #include "mem_utils.h"
-#include "ui_logic.h"
 #include "apdu_constants.h"  // APDU response codes
 #include "typed_data.h"
 
@@ -541,7 +540,6 @@ static bool path_advance_in_struct(void) {
     }
     if (path_struct->depth_count > 0) {
         *depth += 1;
-        ui_712_notify_filter_change();
         end_reached = (*depth == fields_count);
     }
     if (end_reached) {
@@ -634,6 +632,35 @@ uint8_t path_get_depth_count(void) {
     return path_struct->depth_count;
 }
 
+/**
+ * Generate a unique checksum out of the current path
+ *
+ * Goes over the fields of the \ref path_struct with a few exceptions : we skip the root_type since
+ * we already go over root_struct, and in array_depths we only go over path_index since it would
+ * otherwise generate a different CRC for different fields which are targeted by the same filtering
+ * path.
+ *
+ * @return CRC-32 checksum
+ */
+uint32_t get_path_crc(void) {
+    uint32_t value = CX_CRC32_INIT;
+
+    value = cx_crc32_update(value, &path_struct->root_struct, sizeof(path_struct->root_struct));
+    value = cx_crc32_update(value, &path_struct->depth_count, sizeof(path_struct->depth_count));
+    value = cx_crc32_update(value,
+                            path_struct->depths,
+                            sizeof(path_struct->depths[0]) * path_struct->depth_count);
+    value = cx_crc32_update(value,
+                            &path_struct->array_depth_count,
+                            sizeof(path_struct->array_depth_count));
+    for (int i = 0; i < path_struct->array_depth_count; ++i) {
+        value = cx_crc32_update(value,
+                                &path_struct->array_depths[i].path_index,
+                                sizeof(path_struct->array_depths[i].path_index));
+    }
+    return value;
+}
+
 /**
  * Initialize the path context with its indexes in memory and sets it with a depth of 0.
  *

src_features/signMessageEIP712/path.h

@@ -37,6 +37,7 @@ const void *path_get_root(void);
 const void *path_get_nth_field(uint8_t n);
 const void *path_get_nth_field_to_last(uint8_t n);
 uint8_t path_get_depth_count(void);
+uint32_t get_path_crc(void);
 
 #endif  // HAVE_EIP712_FULL_SUPPORT
 

src_features/signMessageEIP712/ui_logic.c

@@ -18,6 +18,7 @@
 #include "commands_712.h"
 #include "common_ui.h"
 #include "uint_common.h"
+#include "filtering.h"
 
 #define AMOUNT_JOIN_FLAG_TOKEN (1 << 0)
 #define AMOUNT_JOIN_FLAG_VALUE (1 << 1)
@@ -56,6 +57,8 @@ typedef struct {
     uint8_t field_flags;
     uint8_t structs_to_review;
     s_amount_context amount;
+    uint8_t filters_received;
+    uint32_t filters_crc[MAX_FILTERS];
 #ifdef SCREEN_SIZE_WALLET
     char ui_pairs_buffer[(SHARED_CTX_FIELD_1_SIZE + SHARED_CTX_FIELD_2_SIZE) * 2];
 #endif
@@ -617,12 +620,8 @@ void ui_712_end_sign(void) {
  */
 bool ui_712_init(void) {
     if ((ui_ctx = MEM_ALLOC_AND_ALIGN_TYPE(*ui_ctx))) {
-        ui_ctx->shown = false;
-        ui_ctx->end_reached = false;
+        explicit_bzero(ui_ctx, sizeof(*ui_ctx));
         ui_ctx->filtering_mode = EIP712_FILTERING_BASIC;
-        explicit_bzero(&ui_ctx->amount, sizeof(ui_ctx->amount));
-        explicit_bzero(strings.tmp.tmp, sizeof(strings.tmp.tmp));
-        explicit_bzero(strings.tmp.tmp2, sizeof(strings.tmp.tmp2));
     } else {
         apdu_response_code = APDU_RESPONSE_INSUFFICIENT_MEMORY;
     }
@@ -717,7 +716,7 @@ void ui_712_set_filters_count(uint8_t count) {
  * @return number of filters
  */
 uint8_t ui_712_remaining_filters(void) {
-    return ui_ctx->filters_to_process;
+    return ui_ctx->filters_to_process - ui_ctx->filters_received;
 }
 
 /**
@@ -739,20 +738,16 @@ void ui_712_queue_struct_to_review(void) {
 }
 
 /**
- * Notify of a filter change from a path advance
+ * Increment the filters counter
  *
- * This function figures out by itself if there is anything to do
- */
-void ui_712_notify_filter_change(void) {
-    if (path_get_root_type() == ROOT_MESSAGE) {
-        if (ui_ctx->filtering_mode == EIP712_FILTERING_FULL) {
-            if (ui_ctx->filters_to_process > 0) {
-                if (ui_ctx->field_flags & UI_712_FIELD_SHOWN) {
-                    ui_ctx->filters_to_process -= 1;
-                }
-            }
-        }
+ * @return if the counter could be incremented
+ */
+bool ui_712_filters_counter_incr(void) {
+    if (ui_ctx->filters_received > ui_ctx->filters_to_process) {
+        return false;
     }
+    ui_ctx->filters_received += 1;
+    return true;
 }
 
 void ui_712_token_join_prepare_addr_check(uint8_t index) {
@@ -789,6 +784,26 @@ bool ui_712_show_raw_key(const void *field_ptr) {
     return true;
 }
 
+/**
+ * Push a new filter path
+ *
+ * @return if the path was pushed or not (in case it was already present)
+ */
+bool ui_712_push_new_filter_path(void) {
+    uint32_t path_crc = get_path_crc();
+
+    // check if already present
+    for (int i = 0; i < ui_ctx->filters_received; ++i) {
+        if (ui_ctx->filters_crc[i] == path_crc) {
+            PRINTF("EIP-712 path CRC (%x) already found at index %u!\n", path_crc, i);
+            return false;
+        }
+    }
+    PRINTF("Pushing new EIP-712 path CRC (%x) at index %u\n", path_crc, ui_ctx->filters_received);
+    ui_ctx->filters_crc[ui_ctx->filters_received] = path_crc;
+    return true;
+}
+
 #ifdef SCREEN_SIZE_WALLET
 /*
  * Get UI pairs buffer

src_features/signMessageEIP712/ui_logic.h

@@ -38,11 +38,12 @@ e_eip712_filtering_mode ui_712_get_filtering_mode(void);
 void ui_712_set_filters_count(uint8_t count);
 uint8_t ui_712_remaining_filters(void);
 void ui_712_queue_struct_to_review(void);
-void ui_712_notify_filter_change(void);
+bool ui_712_filters_counter_incr(void);
 void ui_712_token_join_prepare_addr_check(uint8_t index);
 void ui_712_token_join_prepare_amount(uint8_t index, const char *name, uint8_t name_length);
 void amount_join_set_token_received(void);
 bool ui_712_show_raw_key(const void *field_ptr);
+bool ui_712_push_new_filter_path(void);
 #ifdef SCREEN_SIZE_WALLET
 char *get_ui_pairs_buffer(size_t *size);
 #endif