Merge pull request #773 from LedgerHQ/fix/apa/potential_memory_leak_tlv_parsing

Fix potential memory leak with incorrect TLV structs

Author: apaillier-ledger

src_features/generic_tx_parser/cmd_field.c

@@ -13,11 +13,14 @@
 static bool handle_tlv_payload(const uint8_t *payload, uint16_t size, bool to_free) {
     s_field field = {0};
     s_field_ctx ctx = {0};
+    bool parsing_ret;
+    bool hashing_ret;
 
     ctx.field = &field;
-    if (!tlv_parse(payload, size, (f_tlv_data_handler) &handle_field_struct, &ctx)) return false;
-    if (cx_hash_no_throw(get_fields_hash_ctx(), 0, payload, size, NULL, 0) != CX_OK) return false;
+    parsing_ret = tlv_parse(payload, size, (f_tlv_data_handler) &handle_field_struct, &ctx);
+    hashing_ret = cx_hash_no_throw(get_fields_hash_ctx(), 0, payload, size, NULL, 0) == CX_OK;
     if (to_free) mem_dealloc(size);
+    if (!parsing_ret || !hashing_ret) return false;
     if (!verify_field_struct(&ctx)) {
         PRINTF("Error: could not verify the field struct!\n");
         return false;

src_features/generic_tx_parser/cmd_tx_info.c

@@ -17,13 +17,14 @@ static uint8_t g_tx_info_alignment;
 
 static bool handle_tlv_payload(const uint8_t *payload, uint16_t size, bool to_free) {
     s_tx_info_ctx ctx = {0};
+    bool parsing_ret;
 
     ctx.tx_info = g_tx_info;
     explicit_bzero(ctx.tx_info, sizeof(*ctx.tx_info));
     cx_sha256_init((cx_sha256_t *) &ctx.struct_hash);
-    if (!tlv_parse(payload, size, (f_tlv_data_handler) &handle_tx_info_struct, &ctx)) return false;
+    parsing_ret = tlv_parse(payload, size, (f_tlv_data_handler) &handle_tx_info_struct, &ctx);
     if (to_free) mem_dealloc(sizeof(size));
-    if (!verify_tx_info_struct(&ctx)) {
+    if (!parsing_ret || !verify_tx_info_struct(&ctx)) {
         return false;
     }
     if (cx_sha3_init_no_throw(&hash_ctx, 256) != CX_OK) {

src_features/provide_proxy_info/cmd_proxy_info.c

@@ -8,13 +8,12 @@
 
 static bool handle_tlv_payload(const uint8_t *payload, uint16_t size, bool to_free) {
     s_proxy_info_ctx ctx = {0};
+    bool parsing_ret;
 
     cx_sha256_init(&ctx.struct_hash);
-    if (!tlv_parse(payload, size, (f_tlv_data_handler) &handle_proxy_info_struct, &ctx)) {
-        return false;
-    }
+    parsing_ret = tlv_parse(payload, size, (f_tlv_data_handler) &handle_proxy_info_struct, &ctx);
     if (to_free) mem_dealloc(sizeof(size));
-    if (!verify_proxy_info_struct(&ctx)) {
+    if (!parsing_ret || !verify_proxy_info_struct(&ctx)) {
         return false;
     }
     return true;

src_features/provide_trusted_name/cmd_trusted_name.c

@@ -10,14 +10,13 @@
 
 static bool handle_tlv_payload(const uint8_t *payload, uint16_t size, bool to_free) {
     s_trusted_name_ctx ctx = {0};
-    bool parsing_success;
+    bool parsing_ret;
 
     ctx.trusted_name.name = g_trusted_name;
     cx_sha256_init(&ctx.hash_ctx);
-    parsing_success =
-        tlv_parse(payload, size, (f_tlv_data_handler) &handle_trusted_name_struct, &ctx);
+    parsing_ret = tlv_parse(payload, size, (f_tlv_data_handler) &handle_trusted_name_struct, &ctx);
     if (to_free) mem_dealloc(size);
-    if (!parsing_success || !verify_trusted_name_struct(&ctx)) {
+    if (!parsing_ret || !verify_trusted_name_struct(&ctx)) {
         roll_challenge();  // prevent brute-force guesses
         return false;
     }