Merge pull request #656 from LedgerHQ/fix/apa/eip712_discarded_filters

Fix EIP-712 discarded filters

src_features/signMessageEIP712/commands_712.c

@@ -184,6 +184,7 @@ uint16_t handle_eip712_filtering(uint8_t p1,
                                  uint32_t *flags) {
     bool ret = true;
     bool reply_apdu = true;
+    uint32_t path_crc = 0;
 
     if (eip712_context == NULL) {
         apdu_reply(false);
@@ -211,28 +212,28 @@ uint16_t handle_eip712_filtering(uint8_t p1,
             break;
 #ifdef HAVE_TRUSTED_NAME
         case P2_FILT_CONTRACT_NAME:
-            ret = filtering_trusted_name(cdata, length, p1 == 1);
+            ret = filtering_trusted_name(cdata, length, p1 == 1, &path_crc);
             break;
 #endif
         case P2_FILT_DATE_TIME:
-            ret = filtering_date_time(cdata, length, p1 == 1);
+            ret = filtering_date_time(cdata, length, p1 == 1, &path_crc);
             break;
         case P2_FILT_AMOUNT_JOIN_TOKEN:
-            ret = filtering_amount_join_token(cdata, length, p1 == 1);
+            ret = filtering_amount_join_token(cdata, length, p1 == 1, &path_crc);
             break;
         case P2_FILT_AMOUNT_JOIN_VALUE:
-            ret = filtering_amount_join_value(cdata, length, p1 == 1);
+            ret = filtering_amount_join_value(cdata, length, p1 == 1, &path_crc);
             break;
         case P2_FILT_RAW_FIELD:
-            ret = filtering_raw_field(cdata, length, p1 == 1);
+            ret = filtering_raw_field(cdata, length, p1 == 1, &path_crc);
             break;
         default:
             PRINTF("Unknown P2 0x%x\n", p2);
             apdu_response_code = APDU_RESPONSE_INVALID_P1_P2;
             ret = false;
     }
     if ((p2 > P2_FILT_MESSAGE_INFO) && ret) {
-        if (ui_712_push_new_filter_path()) {
+        if (ui_712_push_new_filter_path(path_crc)) {
             if (!ui_712_filters_counter_incr()) {
                 ret = false;
                 apdu_response_code = APDU_RESPONSE_INVALID_DATA;

src_features/signMessageEIP712/filtering.c

@@ -27,28 +27,32 @@
 #define TOKEN_IDX_ADDR_IN_DOMAIN 0xff
 
 /**
- * Reconstruct the field path and hash it
+ * Reconstruct the field path and hash it for the signature and the CRC
  *
  * @param[in] hash_ctx the hashing context
  * @param[in] discarded if the filter targets a field that does not exist (within an empty array)
+ * @param[out] path_crc pointer to the CRC of the filter path
  */
-static void hash_filtering_path(cx_hash_t *hash_ctx, bool discarded) {
+static void hash_filtering_path(cx_hash_t *hash_ctx, bool discarded, uint32_t *path_crc) {
     const void *field_ptr;
     const char *key;
     uint8_t key_len;
 
     if (discarded) {
         key = ui_712_get_discarded_path(&key_len);
         hash_nbytes((uint8_t *) key, key_len, hash_ctx);
+        *path_crc = cx_crc32_update(*path_crc, key, key_len);
     } else {
         for (uint8_t i = 0; i < path_get_depth_count(); ++i) {
             if (i > 0) {
                 hash_byte('.', hash_ctx);
+                *path_crc = cx_crc32_update(*path_crc, ".", 1);
             }
             if ((field_ptr = path_get_nth_field(i + 1)) != NULL) {
                 if ((key = get_struct_field_keyname(field_ptr, &key_len)) != NULL) {
                     // field name
                     hash_nbytes((uint8_t *) key, key_len, hash_ctx);
+                    *path_crc = cx_crc32_update(*path_crc, key, key_len);
 
                     // array levels
                     if (struct_field_is_array(field_ptr)) {
@@ -57,6 +61,7 @@ static void hash_filtering_path(cx_hash_t *hash_ctx, bool discarded) {
                         get_struct_field_array_lvls_array(field_ptr, &lvl_count);
                         for (int j = 0; j < lvl_count; ++j) {
                             hash_nbytes((uint8_t *) ".[]", 3, hash_ctx);
+                            *path_crc = cx_crc32_update(*path_crc, ".[]", 3);
                         }
                     }
                 }
@@ -328,9 +333,13 @@ bool filtering_discarded_path(const uint8_t *payload, uint8_t length) {
  * @param[in] payload the payload to parse
  * @param[in] length the payload length
  * @param[in] discarded if the filter targets a field that is does not exist (within an empty array)
+ * @param[out] path_crc pointer to the CRC of the filter path
  * @return whether it was successful or not
  */
-bool filtering_trusted_name(const uint8_t *payload, uint8_t length, bool discarded) {
+bool filtering_trusted_name(const uint8_t *payload,
+                            uint8_t length,
+                            bool discarded,
+                            uint32_t *path_crc) {
     uint8_t name_len;
     const char *name;
     uint8_t types_count;
@@ -413,7 +422,7 @@ bool filtering_trusted_name(const uint8_t *payload, uint8_t length, bool discard
     if (!sig_verif_start(&hash_ctx, FILT_MAGIC_TRUSTED_NAME)) {
         return false;
     }
-    hash_filtering_path((cx_hash_t *) &hash_ctx, discarded);
+    hash_filtering_path((cx_hash_t *) &hash_ctx, discarded, path_crc);
     hash_nbytes((uint8_t *) name, sizeof(char) * name_len, (cx_hash_t *) &hash_ctx);
     hash_nbytes(types, types_count, (cx_hash_t *) &hash_ctx);
     hash_nbytes(sources, sources_count, (cx_hash_t *) &hash_ctx);
@@ -440,9 +449,13 @@ bool filtering_trusted_name(const uint8_t *payload, uint8_t length, bool discard
  * @param[in] payload the payload to parse
  * @param[in] length the payload length
  * @param[in] discarded if the filter targets a field that is does not exist (within an empty array)
+ * @param[out] path_crc pointer to the CRC of the filter path
  * @return whether it was successful or not
  */
-bool filtering_date_time(const uint8_t *payload, uint8_t length, bool discarded) {
+bool filtering_date_time(const uint8_t *payload,
+                         uint8_t length,
+                         bool discarded,
+                         uint32_t *path_crc) {
     uint8_t name_len;
     const char *name;
     uint8_t sig_len;
@@ -478,7 +491,7 @@ bool filtering_date_time(const uint8_t *payload, uint8_t length, bool discarded)
     if (!sig_verif_start(&hash_ctx, FILT_MAGIC_DATETIME)) {
         return false;
     }
-    hash_filtering_path((cx_hash_t *) &hash_ctx, discarded);
+    hash_filtering_path((cx_hash_t *) &hash_ctx, discarded, path_crc);
     hash_nbytes((uint8_t *) name, sizeof(char) * name_len, (cx_hash_t *) &hash_ctx);
     if (!sig_verif_end(&hash_ctx, sig, sig_len)) {
         return false;
@@ -501,9 +514,13 @@ bool filtering_date_time(const uint8_t *payload, uint8_t length, bool discarded)
  * @param[in] payload the payload to parse
  * @param[in] length the payload length
  * @param[in] discarded if the filter targets a field that is does not exist (within an empty array)
+ * @param[out] path_crc pointer to the CRC of the filter path
  * @return whether it was successful or not
  */
-bool filtering_amount_join_token(const uint8_t *payload, uint8_t length, bool discarded) {
+bool filtering_amount_join_token(const uint8_t *payload,
+                                 uint8_t length,
+                                 bool discarded,
+                                 uint32_t *path_crc) {
     uint8_t token_idx;
     uint8_t sig_len;
     const uint8_t *sig;
@@ -533,7 +550,7 @@ bool filtering_amount_join_token(const uint8_t *payload, uint8_t length, bool di
     if (!sig_verif_start(&hash_ctx, FILT_MAGIC_AMOUNT_JOIN_TOKEN)) {
         return false;
     }
-    hash_filtering_path((cx_hash_t *) &hash_ctx, discarded);
+    hash_filtering_path((cx_hash_t *) &hash_ctx, discarded, path_crc);
     hash_byte(token_idx, (cx_hash_t *) &hash_ctx);
     if (!sig_verif_end(&hash_ctx, sig, sig_len)) {
         return false;
@@ -554,9 +571,13 @@ bool filtering_amount_join_token(const uint8_t *payload, uint8_t length, bool di
  * @param[in] payload the payload to parse
  * @param[in] length the payload length
  * @param[in] discarded if the filter targets a field that is does not exist (within an empty array)
+ * @param[out] path_crc pointer to the CRC of the filter path
  * @return whether it was successful or not
  */
-bool filtering_amount_join_value(const uint8_t *payload, uint8_t length, bool discarded) {
+bool filtering_amount_join_value(const uint8_t *payload,
+                                 uint8_t length,
+                                 bool discarded,
+                                 uint32_t *path_crc) {
     uint8_t name_len;
     const char *name;
     uint8_t token_idx;
@@ -600,7 +621,7 @@ bool filtering_amount_join_value(const uint8_t *payload, uint8_t length, bool di
     if (!sig_verif_start(&hash_ctx, FILT_MAGIC_AMOUNT_JOIN_VALUE)) {
         return false;
     }
-    hash_filtering_path((cx_hash_t *) &hash_ctx, discarded);
+    hash_filtering_path((cx_hash_t *) &hash_ctx, discarded, path_crc);
     hash_nbytes((uint8_t *) name, sizeof(char) * name_len, (cx_hash_t *) &hash_ctx);
     hash_byte(token_idx, (cx_hash_t *) &hash_ctx);
     if (!sig_verif_end(&hash_ctx, sig, sig_len)) {
@@ -635,9 +656,13 @@ bool filtering_amount_join_value(const uint8_t *payload, uint8_t length, bool di
  * @param[in] payload the payload to parse
  * @param[in] length the payload length
  * @param[in] discarded if the filter targets a field that is does not exist (within an empty array)
+ * @param[out] path_crc pointer to the CRC of the filter path
  * @return whether it was successful or not
  */
-bool filtering_raw_field(const uint8_t *payload, uint8_t length, bool discarded) {
+bool filtering_raw_field(const uint8_t *payload,
+                         uint8_t length,
+                         bool discarded,
+                         uint32_t *path_crc) {
     uint8_t name_len;
     const char *name;
     uint8_t sig_len;
@@ -673,7 +698,7 @@ bool filtering_raw_field(const uint8_t *payload, uint8_t length, bool discarded)
     if (!sig_verif_start(&hash_ctx, FILT_MAGIC_RAW_FIELD)) {
         return false;
     }
-    hash_filtering_path((cx_hash_t *) &hash_ctx, discarded);
+    hash_filtering_path((cx_hash_t *) &hash_ctx, discarded, path_crc);
     hash_nbytes((uint8_t *) name, sizeof(char) * name_len, (cx_hash_t *) &hash_ctx);
     if (!sig_verif_end(&hash_ctx, sig, sig_len)) {
         return false;

src_features/signMessageEIP712/filtering.h

@@ -9,11 +9,26 @@
 #define MAX_FILTERS 50
 
 bool filtering_message_info(const uint8_t *payload, uint8_t length);
-bool filtering_trusted_name(const uint8_t *payload, uint8_t length, bool discarded);
-bool filtering_date_time(const uint8_t *payload, uint8_t length, bool discarded);
-bool filtering_amount_join_token(const uint8_t *payload, uint8_t length, bool discarded);
-bool filtering_amount_join_value(const uint8_t *payload, uint8_t length, bool discarded);
-bool filtering_raw_field(const uint8_t *payload, uint8_t length, bool discarded);
+bool filtering_trusted_name(const uint8_t *payload,
+                            uint8_t length,
+                            bool discarded,
+                            uint32_t *path_crc);
+bool filtering_date_time(const uint8_t *payload,
+                         uint8_t length,
+                         bool discarded,
+                         uint32_t *path_crc);
+bool filtering_amount_join_token(const uint8_t *payload,
+                                 uint8_t length,
+                                 bool discarded,
+                                 uint32_t *path_crc);
+bool filtering_amount_join_value(const uint8_t *payload,
+                                 uint8_t length,
+                                 bool discarded,
+                                 uint32_t *path_crc);
+bool filtering_raw_field(const uint8_t *payload,
+                         uint8_t length,
+                         bool discarded,
+                         uint32_t *path_crc);
 bool filtering_discarded_path(const uint8_t *payload, uint8_t length);
 
 #endif  // HAVE_EIP712_FULL_SUPPORT

src_features/signMessageEIP712/path.c

@@ -708,16 +708,14 @@ bool path_exists_in_backup(const char *path, size_t length) {
     size_t offset = 0;
     size_t i;
     const void *field_ptr;
-    s_path tmp_path;
     const char *typename;
     uint8_t typename_len;
     const void *struct_ptr;
     uint8_t fields_count;
     const char *key;
     uint8_t key_len;
 
-    memcpy(&tmp_path, path_backup, sizeof(tmp_path));
-    field_ptr = get_nth_field_from(&tmp_path, NULL, tmp_path.depth_count);
+    field_ptr = get_nth_field_from(path_backup, NULL, path_backup->depth_count);
     while (offset < length) {
         if (((offset + 1) > length) || (memcmp(path + offset, ".", 1) != 0)) {
             return false;
@@ -755,35 +753,6 @@ bool path_exists_in_backup(const char *path, size_t length) {
     return true;
 }
 
-/**
- * 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 = 0;
-
-    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

@@ -40,7 +40,6 @@ bool path_exists_in_backup(const char *path, size_t length);
 const void *path_get_nth_field_to_last(uint8_t n);
 uint8_t path_get_depth_count(void);
 uint8_t path_backup_get_depth_count(void);
-uint32_t get_path_crc(void);
 
 #endif  // HAVE_EIP712_FULL_SUPPORT
 

src_features/signMessageEIP712/ui_logic.c

@@ -882,11 +882,10 @@ bool ui_712_show_raw_key(const void *field_ptr) {
 /**
  * Push a new filter path
  *
+ * @param[in] path_crc CRC of the 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();
-
+bool ui_712_push_new_filter_path(uint32_t path_crc) {
     // check if already present
     for (int i = 0; i < ui_ctx->filters_received; ++i) {
         if (ui_ctx->filters_crc[i] == path_crc) {

src_features/signMessageEIP712/ui_logic.h

@@ -48,7 +48,7 @@ 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);
+bool ui_712_push_new_filter_path(uint32_t path_crc);
 void ui_712_set_discarded_path(const char *path, uint8_t length);
 const char *ui_712_get_discarded_path(uint8_t *length);
 #ifdef HAVE_TRUSTED_NAME