fix: solve swap implementation issues (#233)

* Fix swap implementation * Better handle of TIP banning in swap mode
Zondax · Oct 15, 2024 · 0f71207 · 0f71207
1 parent f4705ed
commit 0f71207
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 8 deletions.
diff --git a/app/src/apdu_handler.c b/app/src/apdu_handler.c
@@ -179,6 +179,17 @@ __Z_INLINE void handleSign(volatile uint32_t *flags, volatile uint32_t *tx, uint
         THROW(APDU_CODE_DATA_INVALID);
     }
 
+#ifdef HAVE_SWAP
+    if (G_swap_state.called_from_swap && G_swap_state.should_exit && error_msg == NULL) {
+        // Call app_sign_ed25519 without going through UI display, the UI validation was done in
+        // Exchange app already
+        app_sign_ed25519();
+        // Go back to Exchange and report our success to display the modal
+        finalize_exchange_sign_transaction(true);
+        // Unreachable
+    }
+#endif
+
     view_review_init(tx_getItem, tx_getNumItems, app_sign_ed25519);
     view_review_show(REVIEW_TXN);
     *flags |= IO_ASYNCH_REPLY;

diff --git a/app/src/swap/handle_sign_transaction.c b/app/src/swap/handle_sign_transaction.c
@@ -75,15 +75,17 @@ bool copy_transaction_parameters(create_transaction_parameters_t *sign_transacti
     return true;
 }
 
+// Ensure the received transaction matches what was validated in the Exchange app UI
 parser_error_t check_swap_conditions(parser_tx_t *txObj) {
     parser_error_t err = parser_unexpected_error;
     if (txObj == NULL) {
         return err;
     }
     // Check transaction method arguments number. Balance transfer Should be 3 (for tx v26).
     // [dest(address type) |  dest(address) | value(amount)]
-    if (txObj->blob.totalMethodItems != SWAP_EXPECTED_ITEMS) {
-        zemu_log("Wrong swap tx method arguments count.\n");
+    // We will check that index 5 does not have the TIP
+    if (txObj->blob.totalMethodItems != SWAP_EXPECTED_ITEMS && txObj->blob.totalMethodItems != SWAP_EXPECTED_ITEMS + 1) {
+        ZEMU_LOGF(100, "Wrong swap tx method arguments count %d.\n", txObj->blob.totalMethodItems);
         return parser_swap_tx_wrong_method_args_num;
     }
     // Check network.
@@ -101,8 +103,9 @@ parser_error_t check_swap_conditions(parser_tx_t *txObj) {
                            .pageCount = &pageCount};
 
     CHECK_ERROR(parser_getItem(txObj, &uiFields));
-    if (strncmp(valid_network, tmpValue, strlen(valid_network)) != 0) {
+    if (strncmp(valid_network, tmpValue, strlen(valid_network) + 1) != 0) {
         ZEMU_LOGF(200, "Swap not enable on %s network.\n", tmpValue);
+        return parser_swap_tx_wrong_method;
     }
 
     // Check method.
@@ -113,13 +116,24 @@ parser_error_t check_swap_conditions(parser_tx_t *txObj) {
     const char *valid_tx_call = "transfer_allow_death";
 
     CHECK_ERROR(parser_getItem(txObj, &uiFields));
-    if (strncmp(valid_tx_pallet, tmpKey, strlen(valid_tx_pallet)) != 0 ||
-        strncmp(valid_tx_call, tmpValue, strlen(valid_tx_call)) != 0) {
+    if (strncmp(valid_tx_pallet, tmpKey, strlen(valid_tx_pallet) + 1) != 0 ||
+        strncmp(valid_tx_call, tmpValue, strlen(valid_tx_call) + 1) != 0) {
         ZEMU_LOGF(200, "Wrong swap tx method (%s %s, should be : %s %s).\n", tmpKey, tmpValue, valid_tx_pallet,
                   valid_tx_call);
         return parser_swap_tx_wrong_method;
     }
 
+    // Check destination id
+    uiFields.displayIdx = 2;
+    MEMZERO(tmpKey, sizeof(tmpKey));
+    MEMZERO(tmpValue, sizeof(tmpValue));
+    const char *valid_field = "dest";
+    CHECK_ERROR(parser_getItem(txObj, &uiFields));
+    if (strncmp(valid_field, tmpKey, strlen(valid_tx_pallet) + 1) != 0) {
+        ZEMU_LOGF(200, "Wrong field (%s, should be : %s).\n", tmpKey, valid_field);
+        return parser_swap_tx_wrong_method;
+    }
+
     // // Check destination address.
     uiFields.displayIdx = 3;
     MEMZERO(tmpKey, sizeof(tmpKey));
@@ -141,7 +155,7 @@ parser_error_t check_swap_conditions(parser_tx_t *txObj) {
     MEMZERO(tmpValue, sizeof(tmpValue));
     if (parser_getItem(txObj, &uiFields) != parser_ok) {
         ZEMU_LOGF(100, "Could not parse swap tx amount.\n");
-        return parser_swap_tx_wrong_dest_addr;
+        return parser_swap_tx_wrong_amount;
     }
     char tmpAmount[100] = {0};
     const zxerr_t zxerr =
@@ -150,12 +164,21 @@ parser_error_t check_swap_conditions(parser_tx_t *txObj) {
     const size_t strLen = strlen(tmpValue);
     const size_t amountLen = strlen(tmpAmount);
     if (zxerr != zxerr_ok || strLen != amountLen || strncmp(tmpValue, tmpAmount, strLen) != 0) {
-        ZEMU_LOGF(200, "Wrong swap tx amount (%s, should be : %s).\n", tmp_str, tmpAmount);
+        ZEMU_LOGF(200, "Wrong swap tx amount (%s, should be : %s).\n", tmpValue, tmpAmount);
         return parser_swap_tx_wrong_amount;
     }
 
+    // No item nb 5
+    uiFields.displayIdx = 5;
+    MEMZERO(tmpKey, sizeof(tmpKey));
+    MEMZERO(tmpValue, sizeof(tmpValue));
+    if (parser_getItem(txObj, &uiFields) == parser_ok) {
+        ZEMU_LOGF(100, "Refusing item number 5 %s.\n", tmpKey);
+        return parser_swap_tx_wrong_method_args_num;
+    }
+
     ZEMU_LOGF(50, "Swap parameters verified by current tx\n");
-    return err;
+    return parser_ok;
 }
 
 void __attribute__((noreturn)) finalize_exchange_sign_transaction(bool is_success) {