Fix swap checks in token context

Makefile

@@ -38,7 +38,7 @@ APPNAME = "Solana"
 APPVERSION_M = 1
 APPVERSION_N = 7
 APPVERSION_P = 0
-APPVERSION = "$(APPVERSION_M).$(APPVERSION_N).$(APPVERSION_P)-splswapv2"
+APPVERSION = "$(APPVERSION_M).$(APPVERSION_N).$(APPVERSION_P)-splswapv3"
 
 # Application source files
 APP_SOURCE_PATH += src

libsol/ed25519_helpers.c

@@ -34,7 +34,7 @@ static bool is_on_curve_internal(const uint8_t compressed_point[PUBKEY_LENGTH])
     return is_on_curve;
 }
 
-bool is_on_curve(const uint8_t compressed_point[PUBKEY_LENGTH]) {
+static bool is_on_curve(const uint8_t compressed_point[PUBKEY_LENGTH]) {
     CX_ASSERT(cx_bn_lock(PUBKEY_LENGTH, 0));
     bool is_on_curve = is_on_curve_internal(compressed_point);
     if (cx_bn_is_locked()) {

libsol/ed25519_helpers.h

@@ -2,8 +2,6 @@
 
 #include "globals.h"
 
-bool is_on_curve(const uint8_t compressed_point[PUBKEY_LENGTH]);
-
 bool validate_associated_token_address(const uint8_t owner_account[PUBKEY_LENGTH],
                                        const uint8_t mint_account[PUBKEY_LENGTH],
                                        const uint8_t provided_ata[PUBKEY_LENGTH]);

libsol/spl_token_instruction.c

@@ -5,7 +5,6 @@
 #include "sol/transaction_summary.h"
 #include "token_info.h"
 #include "util.h"
-#include "base58.h"
 #include "globals.h"
 #include "ed25519_helpers.h"
 #include "sol/trusted_info.h"

src/handle_sign_message.c

@@ -7,6 +7,8 @@
 #include "sol/print_config.h"
 #include "sol/message.h"
 #include "sol/transaction_summary.h"
+#include "sol/trusted_info.h"
+#include "ed25519_helpers.h"
 
 #include "handle_sign_message.h"
 #include "ui_api.h"
@@ -91,17 +93,72 @@ void handle_sign_message_parse_message(volatile unsigned int *tx) {
     }
 }
 
-static bool check_swap_validity(const SummaryItemKind_t kinds[MAX_TRANSACTION_SUMMARY_ITEMS],
-                                size_t num_summary_steps) {
-    bool is_token_swap = is_token_transaction();
+// Accept amount + recipient (+ fees)
+static bool check_swap_validity_native(const SummaryItemKind_t kinds[MAX_TRANSACTION_SUMMARY_ITEMS],
+                                       size_t num_summary_steps) {
     bool amount_ok = false;
     bool recipient_ok = false;
-    uint8_t expected_steps;
-    if (is_token_swap) {
-        expected_steps = 4;
-    } else {
-        expected_steps = 2;
+    uint8_t expected_steps = 2;
+
+    // Accept base step number + optional fee step
+    if (num_summary_steps != expected_steps && num_summary_steps != expected_steps + 1) {
+        PRINTF("%d steps expected for transaction in swap context, not %u\n",
+               expected_steps,
+               num_summary_steps);
+        return false;
     }
+
+    for (size_t i = 0; i < num_summary_steps; ++i) {
+        transaction_summary_display_item(i, DisplayFlagNone | DisplayFlagLongPubkeys);
+        PRINTF("Item (%d) '%s', '%s'\n",
+               kinds[i],
+               G_transaction_summary_title,
+               G_transaction_summary_text);
+        switch (kinds[i]) {
+            case SummaryItemAmount:
+                if (strcmp(G_transaction_summary_title, "Max fees") == 0) {
+                    break;
+                }
+                if (strcmp(G_transaction_summary_title, "Transfer") != 0) {
+                    PRINTF("Refused title '%s', expecting '%s'\n", G_transaction_summary_title, "Transfer");
+                    return false;
+                }
+                if (!check_swap_amount(G_transaction_summary_text)) {
+                    PRINTF("check_swap_amount failed\n");
+                    return false;
+                }
+                amount_ok = true;
+                break;
+
+            case SummaryItemPubkey:
+                if (strcmp(G_transaction_summary_title, "Recipient") != 0) {
+                    PRINTF("Refused title '%s', expecting '%s'\n", G_transaction_summary_title, "Recipient");
+                    return false;
+                }
+                if (!check_swap_recipient(G_transaction_summary_text)) {
+                    PRINTF("check_swap_recipient failed\n");
+                    return false;
+                }
+                recipient_ok = true;
+                break;
+
+            default:
+                PRINTF("Refused kind '%u'\n", kinds[i]);
+                return false;
+        }
+    }
+    return amount_ok && recipient_ok;
+}
+
+// Accept token amount + SOL recipient + mint + from + ATA recipient (+ fees)
+static bool check_swap_validity_token(const SummaryItemKind_t kinds[MAX_TRANSACTION_SUMMARY_ITEMS],
+                                      size_t num_summary_steps) {
+    bool amount_ok = false;
+    bool mint_ok = false;
+    bool dest_ata_ok = false;
+    bool dest_sol_address_ok = false;
+    uint8_t expected_steps = 5;
+
     // Accept base step number + optional fee step
     if (num_summary_steps != expected_steps && num_summary_steps != expected_steps + 1) {
         PRINTF("%d steps expected for token transaction in swap context, not %u\n",
@@ -110,6 +167,19 @@ static bool check_swap_validity(const SummaryItemKind_t kinds[MAX_TRANSACTION_SU
         return false;
     }
 
+    if (!g_trusted_info.received) {
+        // This case should never happen because this is already checked at TX parsing
+        PRINTF("Descriptor info is required for a SPL transfer\n");
+        return -1;
+    }
+    if (!validate_associated_token_address(g_trusted_info.owner_address,
+                                           g_trusted_info.mint_address,
+                                           g_trusted_info.token_address)) {
+        // This case should never happen because this is already checked at TX parsing
+        PRINTF("Failed to validate ATA\n");
+        return -1;
+    }
+
     for (size_t i = 0; i < num_summary_steps; ++i) {
         transaction_summary_display_item(i, DisplayFlagNone | DisplayFlagLongPubkeys);
         PRINTF("Item (%d) '%s', '%s'\n",
@@ -118,35 +188,81 @@ static bool check_swap_validity(const SummaryItemKind_t kinds[MAX_TRANSACTION_SU
                G_transaction_summary_text);
         switch (kinds[i]) {
             case SummaryItemTokenAmount:
-                amount_ok =
-                    check_swap_amount(G_transaction_summary_title, G_transaction_summary_text);
+                if (strcmp(G_transaction_summary_title, "Transfer tokens") != 0) {
+                    PRINTF("Refused title '%s', expecting '%s'\n", G_transaction_summary_title, "Transfer tokens");
+                    return false;
+                }
+                if (!check_swap_amount(G_transaction_summary_text)) {
+                    PRINTF("check_swap_amount failed\n");
+                    return false;
+                }
+                amount_ok = true;
                 break;
+
             case SummaryItemAmount:
-                if (strcmp(G_transaction_summary_title, "Max fees") == 0) {
-                    break;
+                if (strcmp(G_transaction_summary_title, "Max fees") != 0) {
+                    PRINTF("Refuse non fee amount in token swap context\n");
+                    return false;
                 }
-                amount_ok =
-                    check_swap_amount(G_transaction_summary_title, G_transaction_summary_text);
                 break;
+
             case SummaryItemPubkey:
-                if (is_token_swap && strcmp(G_transaction_summary_title, "Token address") == 0) {
-                    PRINTF("Skip %s field\n", G_transaction_summary_title);
+                if (strcmp(G_transaction_summary_title, "Token address") == 0) {
+                    // MINT
+                    if (strcmp(g_trusted_info.encoded_mint_address, G_transaction_summary_text) != 0) {
+                        // This case should never happen because this is already checked at TX parsing
+                        PRINTF("Mint address does not match with mint address in descriptor\n");
+                        return false;
+                    }
+                    mint_ok = true;
+                } else if (strcmp(G_transaction_summary_title, "From (token account)") == 0) {
+                    // SRC ACCOUNT
                     break;
+                } else if (strcmp(G_transaction_summary_title, "To (token account)") == 0) {
+                    // Destination ATA
+                    if (strcmp(g_trusted_info.encoded_token_address, G_transaction_summary_text) != 0) {
+                        // This case should never happen because this is already checked at TX parsing
+                        PRINTF("Dest ATA address does not match with ATA in descriptor\n");
+                        return false;
+                    }
+                    dest_ata_ok = true;
                 }
-                if (is_token_swap &&
-                    strcmp(G_transaction_summary_title, "From (token account)") == 0) {
-                    PRINTF("Skip %s field\n", G_transaction_summary_title);
-                    break;
+                break;
+
+            case SummaryItemString:
+                if (strcmp(G_transaction_summary_title, "To") != 0) {
+                    PRINTF("Refuse string item != 'To'\n");
+                    return false;
                 }
-                recipient_ok =
-                    check_swap_recipient(G_transaction_summary_title, G_transaction_summary_text);
+                if (strcmp(g_trusted_info.encoded_owner_address, G_transaction_summary_text) != 0) {
+                    // This case should never happen because this is already checked at TX parsing
+                    PRINTF("Dest SOL address does not match with SOL address in descriptor\n");
+                    return false;
+                }
+                if (!check_swap_recipient(G_transaction_summary_text)) {
+                    PRINTF("check_swap_recipient failed\n");
+                    return false;
+                }
+                dest_sol_address_ok = true;
                 break;
+
             default:
                 PRINTF("Refused kind '%u'\n", kinds[i]);
                 return false;
         }
     }
-    return amount_ok && recipient_ok;
+
+    // All expected elements should have been received and validated
+    return amount_ok && mint_ok && dest_ata_ok && dest_sol_address_ok;
+}
+
+static bool check_swap_validity(const SummaryItemKind_t kinds[MAX_TRANSACTION_SUMMARY_ITEMS],
+                                size_t num_summary_steps) {
+    if (is_token_transaction()) {
+        return check_swap_validity_token(kinds, num_summary_steps);
+    } else {
+        return check_swap_validity_native(kinds, num_summary_steps);
+    }
 }
 
 void handle_sign_message_ui(volatile unsigned int *flags) {

src/main.c

@@ -112,6 +112,9 @@ void app_main(void) {
     reset_getpubkey_globals();
     reset_main_globals();
 
+    // to prevent it from having a fixed value at boot
+    roll_challenge();
+
     // DESIGN NOTE: the bootloader ignores the way APDU are fetched. The only
     // goal is to retrieve APDU.
     // When APDU are to be fetched from multiple IOs, like NFC+USB+BLE, make
@@ -223,9 +226,6 @@ void coin_main(void) {
                 BLE_power(1, NULL);
 #endif  // HAVE_BLE
 
-                // to prevent it from having a fixed value at boot
-                roll_challenge();
-
                 app_main();
             }
             CATCH(ApduReplySdkExceptionIoReset) {

src/swap/handle_swap_sign_transaction.c

@@ -82,22 +82,11 @@ bool copy_transaction_parameters(create_transaction_parameters_t *params) {
 }
 
 // Check that the amount in parameter is the same as the previously saved amount
-bool check_swap_amount(const char *title, const char *text) {
+bool check_swap_amount(const char *text) {
     if (!G_swap_validated.initialized) {
         return false;
     }
 
-    char expected_title[MAX(sizeof("Transfer tokens"), sizeof("Transfer"))] = {'\0'};
-    if (is_token_transaction()) {
-        strcpy(expected_title, "Transfer tokens");
-    } else {
-        strcpy(expected_title, "Transfer");
-    }
-    if (strcmp(title, expected_title) != 0) {
-        PRINTF("Refused title '%s', expecting '%s'\n", title, expected_title);
-        return false;
-    }
-
     char validated_amount[MAX_PRINTABLE_AMOUNT_SIZE];
     if (print_token_amount(G_swap_validated.amount,
                            G_swap_validated.ticker,
@@ -118,22 +107,11 @@ bool check_swap_amount(const char *title, const char *text) {
 }
 
 // Check that the recipient in parameter is the same as the previously saved recipient
-bool check_swap_recipient(const char *title, const char *text) {
+bool check_swap_recipient(const char *text) {
     if (!G_swap_validated.initialized) {
         return false;
     }
 
-    char expected_title[MAX(sizeof("To (token account)"), sizeof("Recipient"))] = {'\0'};
-    if (is_token_transaction()) {
-        strcpy(expected_title, "To (token account)");
-    } else {
-        strcpy(expected_title, "Recipient");
-    }
-    if (strcmp(title, expected_title) != 0) {
-        PRINTF("Refused title '%s', expecting '%s'\n", title, expected_title);
-        return false;
-    }
-
     if (strcmp(G_swap_validated.recipient, text) == 0) {
         return true;
     } else {

src/swap/handle_swap_sign_transaction.h

@@ -4,9 +4,9 @@
 
 bool copy_transaction_parameters(create_transaction_parameters_t *sign_transaction_params);
 
-bool check_swap_amount(const char *title, const char *text);
+bool check_swap_amount(const char *text);
 
-bool check_swap_recipient(const char *title, const char *text);
+bool check_swap_recipient(const char *text);
 
 bool is_token_transaction();