Test/erc20 audit swap fix

.gitignore

@@ -19,7 +19,6 @@ test_tools/
 # Fuzzing artifacts and build outputs
 fuzzing/build/
 fuzzing/artifacts/
-fuzzing/corpus/proto_varlen_parser/
 tests/unit/test_proto_varlen_parser
 tests/unit/test_proto_varlen_edge_cases
 tests/unit/test_proto_varlen_security

Makefile

@@ -30,7 +30,7 @@ APPNAME = Hedera
 # Application version
 APPVERSION_M = 1
 APPVERSION_N = 8
-APPVERSION_P = 4
+APPVERSION_P = 6
 APPVERSION = "$(APPVERSION_M).$(APPVERSION_N).$(APPVERSION_P)"
 
 # Application source files

README.md

@@ -119,8 +119,8 @@ Or you can run your Ragger functional tests if you have implemented them :
 bash$ python -m virtualenv venv --system-site-package
 bash$ source ./venv/bin/activate
 # Install tests dependencies
-(venv) bash$ pip install -r tests/requirements.txt
+(venv) bash$ pip install -r tests/standalone/requirements.txt
 # Run ragger functional tests
-(venv) bash$ python -m pytest tests/ --tb=short -v --device nanos --display
+(venv) bash$ python -m pytest tests/standalone --tb=short -v --device nanos --display
 ```
 

fuzzing/corpus/proto_varlen_parser/deep_nested.bin

@@ -0,0 +1,5 @@
+
+ r
+r
+r
+Deeply Nested Value!

fuzzing/corpus/proto_varlen_parser/large_field.bin

@@ -0,0 +1,2 @@
+ÿÿÿÿ
+Hello

fuzzing/corpus/proto_varlen_parser/malformed_length.bin

@@ -0,0 +1,4 @@
+
+ÿr
+
+Test

fuzzing/corpus/proto_varlen_parser/minimal.bin

@@ -0,0 +1 @@
+

fuzzing/corpus/proto_varlen_parser/unknown_wire.bin

@@ -0,0 +1 @@
+

fuzzing/corpus/proto_varlen_parser/valid_nested.bin

@@ -0,0 +1,4 @@
+
+r
+
+Test Value

src/get_public_key.c

@@ -21,7 +21,7 @@ static bool get_pk() {
     memcpy(gpk_ctx.raw_pubkey, key_buffer, 32);
 
     // Populate Key Hex String
-    bin2hex(gpk_ctx.full_key, G_io_apdu_buffer, KEY_SIZE);
+    bin2hex(gpk_ctx.full_key, G_io_apdu_buffer, 32);
     gpk_ctx.full_key[KEY_SIZE] = '\0';
 
     return true;

src/main.c

@@ -114,6 +114,7 @@ void app_main() {
 
                     default:
                         sw = 0x6800 | (e & 0x7FF);
+                        MEMCLEAR(G_io_apdu_buffer);
                         break;
                 }
 

src/proto_varlen_parser.c

@@ -79,7 +79,7 @@ static bool extract_string_from_string_value(const uint8_t *sv_data,
                 return false;
             }
 
-            if (sv_data + string_length > sv_end) {
+            if (string_length > (uint64_t)(sv_end - sv_data)) {
                 return false;
             }
 
@@ -140,7 +140,7 @@ static bool parse_crypto_update_body(const uint8_t *crypto_data,
                 return false;
             }
 
-            if (crypto_data + string_value_length > crypto_end) {
+            if (string_value_length > (uint64_t)(crypto_end - crypto_data)) {
                 return false;
             }
 
@@ -195,7 +195,7 @@ bool extract_nested_string_field(const uint8_t *buffer, size_t buffer_size,
                 return false;
             }
 
-            if (data + crypto_update_length > end) {
+            if (crypto_update_length > (uint64_t)(end - data)) {
                 return false;
             }
 
@@ -243,7 +243,7 @@ static bool skip_field(const uint8_t **data, const uint8_t *end,
             if (!decode_varint(data, end, &length)) {
                 return false;
             }
-            if (*data + length > end) {
+            if (length > (uint64_t)(end - *data)) {
                 return false;
             }
             *data += length;

src/sign_transaction.c

@@ -140,7 +140,9 @@ void handle_transaction_body() {
     reformat_key_index();
 
     // All flows except Verify
-    if (!is_verify_account()) reformat_operator();
+    if (!is_verify_account()) {
+        reformat_operator();
+    }
 
     // Handle parsed protobuf message of transaction body
     switch (st_ctx.transaction.which_data) {
@@ -393,13 +395,6 @@ void handle_sign_transaction(uint8_t p1, uint8_t p2, uint8_t* buffer,
     // local and global buffers is impossible.
     memcpy(raw_transaction, (buffer + INDEX_SIZE), raw_transaction_length);
 
-    // Sign Transaction
-    if (!hedera_sign(st_ctx.key_index, raw_transaction, raw_transaction_length,
-                     G_io_apdu_buffer, &st_ctx.signature_length)) {
-        PRINTF("%s: signature failure\n", __func__);
-        THROW(EXCEPTION_MALFORMED_APDU);
-    }
-
     // Make in memory buffer into stream
     pb_istream_t stream =
         pb_istream_from_buffer(raw_transaction, raw_transaction_length);
@@ -409,6 +404,8 @@ void handle_sign_transaction(uint8_t p1, uint8_t p2, uint8_t* buffer,
                    &st_ctx.transaction)) {
         // Oh no couldn't ...
         PRINTF("%s: decoding failure\n", __func__);
+        MEMCLEAR(G_io_apdu_buffer);
+        MEMCLEAR(raw_transaction);
         THROW(EXCEPTION_MALFORMED_APDU);
     }
 
@@ -424,6 +421,17 @@ void handle_sign_transaction(uint8_t p1, uint8_t p2, uint8_t* buffer,
         }
     }
 
+    // Sign Transaction
+    if (!hedera_sign(st_ctx.key_index, raw_transaction, raw_transaction_length,
+                     G_io_apdu_buffer, &st_ctx.signature_length)) {
+        PRINTF("%s: signature failure\n", __func__);
+        MEMCLEAR(G_io_apdu_buffer);
+        MEMCLEAR(raw_transaction);
+        THROW(EXCEPTION_MALFORMED_APDU);
+                     }
+
+    MEMCLEAR(raw_transaction);
+
     handle_transaction_body();
 
     *flags |= IO_ASYNCH_REPLY;

src/swap/handle_get_printable_amount.c

@@ -2,7 +2,7 @@
 
 #include "handle_get_printable_amount.h"
 #include "hedera_format.h"
-#include "swap_utils.h"
+#include "swap_token_utils.h"
 #include <utils.h>
 
 int print_amount(uint64_t amount, char *out, size_t out_length) {

src/swap/handle_swap_sign_transaction.c

@@ -1,15 +1,16 @@
 #ifdef HAVE_SWAP
 
 #include "handle_swap_sign_transaction.h"
-#include "os.h"
-#include "string.h"
-#include "swap.h"
-#include "sign_transaction.h"
-#include "swap_utils.h"
 
 #include <inttypes.h>
 #include <sign_transaction.h>
 
+#include "os.h"
+#include "sign_transaction.h"
+#include "string.h"
+#include "swap.h"
+#include "swap_token_utils.h"
+
 typedef struct swap_validated_s {
     bool initialized;
     uint64_t amount;
@@ -21,6 +22,21 @@ static swap_validated_t G_swap_validated;
 
 static uint8_t *G_swap_sign_return_value_address;
 
+// Returns the positive amount entry from the given accountAmounts table to avoid using hardcoded order as it might differ.
+// If none is found, returns NULL.
+static const Hedera_AccountAmount *find_outbound_account_amount(const Hedera_AccountAmount *accountAmounts,
+                                                               size_t accountAmounts_count) {
+    if (accountAmounts == NULL) {
+        return NULL;
+    }
+    for (size_t i = 0; i < accountAmounts_count; i++) {
+        if (accountAmounts[i].amount > 0) {
+            return &accountAmounts[i];
+        }
+    }
+    return NULL;
+}
+
 bool copy_transaction_parameters(create_transaction_parameters_t *params) {
     if (params->coin_configuration != NULL || params->coin_configuration_length != 0) {
         PRINTF("No coin_configuration expected\n");
@@ -67,8 +83,12 @@ bool copy_transaction_parameters(create_transaction_parameters_t *params) {
 
 bool validate_swap_amount(uint64_t amount) {
 
+    PRINTF("validate_swap_amount %ld %d\n", amount, amount);
+
     if (amount != G_swap_validated.amount) {
         PRINTF("Amount not equal\n");
+        PRINTF("Amount requested in this transaction = %d\n", amount);
+        PRINTF("Amount validated in swap = %d\n", G_swap_validated.amount);
         return false;
     }
     char validated_amount_str[MAX_PRINTABLE_AMOUNT_SIZE];
@@ -96,9 +116,14 @@ bool swap_check_validity() {
         return false;
     }
 
-    if (!validate_swap_amount(st_ctx.transaction.data.cryptoTransfer.transfers
-                        .accountAmounts[0]
-                        .amount)) {
+    const Hedera_TransferList *transfer_list = &st_ctx.transaction.data.cryptoTransfer.transfers;
+    const Hedera_AccountAmount *swap_amount = find_outbound_account_amount(transfer_list->accountAmounts, transfer_list->accountAmounts_count);
+    if (swap_amount == NULL) {
+        PRINTF("No outbound (negative) transfer found in Transaction.\n");
+        return false;
+    }
+
+    if (!validate_swap_amount(swap_amount->amount)) {
         PRINTF("Amount on Transaction is different from validated package.\n");
         return false;
     }

src/swap/swap_utils.c → src/swap/swap_token_utils.c

@@ -1,9 +1,8 @@
-#include "swap_utils.h"
-
 #include <limits.h>
 #include <string.h>
 
 #include "../utils.h"
+#include "swap_token_utils.h"
 
 bool swap_str_to_u64(const uint8_t* src, size_t length, uint64_t* result) {
     if (length > sizeof(uint64_t)) {

src/swap/swap_utils.h → src/swap/swap_token_utils.h

@@ -1,8 +1,8 @@
 #pragma once
 
 #include "stdint.h"
-#include "stddef.h"
 #include "stdbool.h"
+#include "stddef.h"
 
 #define HEDERA_SIGN "HBAR"
 #define HEDERA_DECIMALS 8

src/ui/io.c

@@ -1,5 +1,5 @@
 #include "app_io.h"
-
+#include "utils.h"
 #include "ux.h"
 
 #ifdef HAVE_NBGL
@@ -100,4 +100,6 @@ void io_exchange_with_code(uint16_t code, uint16_t tx) {
     G_io_apdu_buffer[tx++] = code & 0xff;
 
     io_exchange(CHANNEL_APDU | IO_RETURN_AFTER_TX, tx);
+    // Clear io buffer after send
+    MEMCLEAR(G_io_apdu_buffer);
 }

tests/application_client/hedera_builder.py

@@ -249,6 +249,60 @@ def crypto_transfer_hbar_conf(
 
 
 def crypto_transfer_verify(
+        sender_shardNum: int,
+        sender_realmNum: int,
+        sender_accountNum: int,
+        receiver_shardNum: int,
+        receiver_realmNum: int,
+        receiver_accountNum: int,
+        amount: int = 0,
+        reverse_order: bool = False,
+) -> Dict:
+
+    # Build sender AccountID and AccountAmount (amount 0)
+    hedera_account_id_sender = basic_types_pb2.AccountID(
+        shardNum=sender_shardNum,
+        realmNum=sender_realmNum,
+        accountNum=sender_accountNum,
+    )
+
+    hedera_account_amount_sender = basic_types_pb2.AccountAmount(
+        accountID=hedera_account_id_sender,
+        amount=-amount,
+    )
+
+    # Build receiver AccountID and AccountAmount (amount provided)
+    hedera_account_id_receiver = basic_types_pb2.AccountID(
+        shardNum=receiver_shardNum,
+        realmNum=receiver_realmNum,
+        accountNum=receiver_accountNum,
+    )
+
+    hedera_account_amount_receiver = basic_types_pb2.AccountAmount(
+        accountID=hedera_account_id_receiver,
+        amount=amount,
+    )
+
+    if reverse_order:
+        account_amounts = [hedera_account_amount_receiver, hedera_account_amount_sender]
+    else:
+        account_amounts = [hedera_account_amount_sender, hedera_account_amount_receiver]
+
+    hedera_transfer_list = basic_types_pb2.TransferList(
+        accountAmounts=account_amounts,
+    )
+
+    crypto_transfer = crypto_transfer_pb2.CryptoTransferTransactionBody(
+        transfers=hedera_transfer_list,
+        tokenTransfers=[],
+    )
+
+    return {"cryptoTransfer": crypto_transfer}
+
+
+
+
+def crypto_transfer_simple_verify(
     sender_shardNum: int, sender_realmNum: int, sender_accountNum: int
 ) -> Dict: