Stellar: use strings instead of bytes for addresses (#372)

* Use the new protobuf messages that send accounts as strings (fixes #367)
 * `stellar_signingAbort()` now takes an error message
 * Operations now check if a transaction is being signed (fixes #368)
 * Operations now return false on error or if the user cancels
 * Stellar `fsm_*` methods now check operation result and return early if the operation fails

This PR also re-enables the stellar code in the firmware.

firmware/Makefile

@@ -28,7 +28,7 @@ OBJS += ethereum.o
 OBJS += ethereum_tokens.o
 OBJS += nem2.o
 OBJS += nem_mosaics.o
-# OBJS += stellar.o
+OBJS += stellar.o
 
 OBJS += debug.o
 

firmware/fsm.c

@@ -247,5 +247,5 @@ static bool fsm_layoutAddress(const char *address, const char *desc, bool ignore
 #include "fsm_msg_ethereum.h"
 #include "fsm_msg_crypto.h"
 #include "fsm_msg_nem.h"
-// #include "fsm_msg_stellar.h"
+#include "fsm_msg_stellar.h"
 #include "fsm_msg_debug.h"

firmware/fsm_msg_stellar.h

@@ -96,7 +96,7 @@ void fsm_msgStellarSignTx(StellarSignTx *msg)
 
 void fsm_msgStellarCreateAccountOp(StellarCreateAccountOp *msg)
 {
-    stellar_confirmCreateAccountOp(msg);
+    if (!stellar_confirmCreateAccountOp(msg)) return;
 
     if (stellar_allOperationsConfirmed()) {
         RESP_INIT(StellarSignedTx);
@@ -116,7 +116,7 @@ void fsm_msgStellarCreateAccountOp(StellarCreateAccountOp *msg)
 void fsm_msgStellarPaymentOp(StellarPaymentOp *msg)
 {
     // This will display additional dialogs to the user
-    stellar_confirmPaymentOp(msg);
+    if (!stellar_confirmPaymentOp(msg)) return;
 
     // Last operation was confirmed, send a StellarSignedTx
     if (stellar_allOperationsConfirmed()) {
@@ -136,7 +136,7 @@ void fsm_msgStellarPaymentOp(StellarPaymentOp *msg)
 
 void fsm_msgStellarPathPaymentOp(StellarPathPaymentOp *msg)
 {
-    stellar_confirmPathPaymentOp(msg);
+    if (!stellar_confirmPathPaymentOp(msg)) return;
 
     if (stellar_allOperationsConfirmed()) {
         RESP_INIT(StellarSignedTx);
@@ -155,7 +155,7 @@ void fsm_msgStellarPathPaymentOp(StellarPathPaymentOp *msg)
 
 void fsm_msgStellarManageOfferOp(StellarManageOfferOp *msg)
 {
-    stellar_confirmManageOfferOp(msg);
+    if (!stellar_confirmManageOfferOp(msg)) return;
 
     if (stellar_allOperationsConfirmed()) {
         RESP_INIT(StellarSignedTx);
@@ -174,7 +174,7 @@ void fsm_msgStellarManageOfferOp(StellarManageOfferOp *msg)
 
 void fsm_msgStellarCreatePassiveOfferOp(StellarCreatePassiveOfferOp *msg)
 {
-    stellar_confirmCreatePassiveOfferOp(msg);
+    if (!stellar_confirmCreatePassiveOfferOp(msg)) return;
 
     if (stellar_allOperationsConfirmed()) {
         RESP_INIT(StellarSignedTx);
@@ -193,7 +193,7 @@ void fsm_msgStellarCreatePassiveOfferOp(StellarCreatePassiveOfferOp *msg)
 
 void fsm_msgStellarSetOptionsOp(StellarSetOptionsOp *msg)
 {
-    stellar_confirmSetOptionsOp(msg);
+    if (!stellar_confirmSetOptionsOp(msg)) return;
 
     if (stellar_allOperationsConfirmed()) {
         RESP_INIT(StellarSignedTx);
@@ -212,7 +212,7 @@ void fsm_msgStellarSetOptionsOp(StellarSetOptionsOp *msg)
 
 void fsm_msgStellarChangeTrustOp(StellarChangeTrustOp *msg)
 {
-    stellar_confirmChangeTrustOp(msg);
+    if (!stellar_confirmChangeTrustOp(msg)) return;
 
     if (stellar_allOperationsConfirmed()) {
         RESP_INIT(StellarSignedTx);
@@ -231,7 +231,7 @@ void fsm_msgStellarChangeTrustOp(StellarChangeTrustOp *msg)
 
 void fsm_msgStellarAllowTrustOp(StellarAllowTrustOp *msg)
 {
-    stellar_confirmAllowTrustOp(msg);
+    if (!stellar_confirmAllowTrustOp(msg)) return;
 
     if (stellar_allOperationsConfirmed()) {
         RESP_INIT(StellarSignedTx);
@@ -250,7 +250,7 @@ void fsm_msgStellarAllowTrustOp(StellarAllowTrustOp *msg)
 
 void fsm_msgStellarAccountMergeOp(StellarAccountMergeOp *msg)
 {
-    stellar_confirmAccountMergeOp(msg);
+    if (!stellar_confirmAccountMergeOp(msg)) return;
 
     if (stellar_allOperationsConfirmed()) {
         RESP_INIT(StellarSignedTx);
@@ -269,7 +269,7 @@ void fsm_msgStellarAccountMergeOp(StellarAccountMergeOp *msg)
 
 void fsm_msgStellarManageDataOp(StellarManageDataOp *msg)
 {
-    stellar_confirmManageDataOp(msg);
+    if (!stellar_confirmManageDataOp(msg)) return;
 
     if (stellar_allOperationsConfirmed()) {
         RESP_INIT(StellarSignedTx);
@@ -288,7 +288,7 @@ void fsm_msgStellarManageDataOp(StellarManageDataOp *msg)
 
 void fsm_msgStellarBumpSequenceOp(StellarBumpSequenceOp *msg)
 {
-    stellar_confirmBumpSequenceOp(msg);
+    if (!stellar_confirmBumpSequenceOp(msg)) return;
 
     if (stellar_allOperationsConfirmed()) {
         RESP_INIT(StellarSignedTx);

firmware/protob/Makefile

@@ -12,7 +12,7 @@ PYTHON ?= python
 	protoc -I/usr/include -I. $< --python_out=.
 
 messages_map.h: messages_map.py messages_pb2.py types_pb2.py
-	$(PYTHON) $< | grep -v -e MessageType_Lisk -e MessageType_Stellar > $@
+	$(PYTHON) $< | grep -v -e MessageType_Lisk > $@
 
 clean:
 	rm -f *.pb *.o *.d *.pb.c *.pb.h *_pb2.py messages_map.h

firmware/protob/types.options

@@ -70,7 +70,7 @@ NEMCosignatoryModification.public_key	max_size:32
 NEMImportanceTransfer.public_key	max_size:32
 
 StellarAssetType.code			max_size:13
-StellarAssetType.issuer			max_size:32
+StellarAssetType.issuer			max_size:57
 
 # Lisk will be supported later
 

firmware/stellar.c

@@ -14,13 +14,13 @@
  * You should have received a copy of the GNU Lesser General Public License
  * along with this library.  If not, see <http://www.gnu.org/licenses/>.
  *
- * Stellar signing has the following workflow:
- *  1. Client sends first 1024 bytes of the transaction
- *  2. Trezor parses the transaction header and confirms the details with the user
- *  3. Trezor responds to the client with an offset for where to send the next chunk of bytes
- *  4. Client sends next 1024 bytes starting at <offset>
- *  5. Trezor parses and confirms the next operation
- *  6. Trezor responds with either an offset for the next operation or a signature
+ * Stellar signing workflow:
+ *  1. Client sends a StellarSignTx method to the device with transaction header information
+ *  2. Device confirms transaction details with the user and requests first operation
+ *  3. Client sends protobuf message with details about the operation to sign
+ *  4. Device confirms operation with user
+ *  5a. If there are more operations in the transaction, device responds with StellarTxOpRequest. Go to 3
+ *  5b. If the operation is the last one, device responds with StellarSignedTx
  */
 
 #include <stdbool.h>
@@ -41,6 +41,7 @@
 #include "util.h"
 #include "layout2.h"
 #include "fonts.h"
+#include "memzero.h"
 
 static bool stellar_signing = false;
 static StellarTransaction stellar_activeTx;
@@ -151,11 +152,17 @@ void stellar_signingInit(StellarSignTx *msg)
     }
 }
 
-void stellar_confirmSourceAccount(bool has_source_account, uint8_t *bytes)
+bool stellar_confirmSourceAccount(bool has_source_account, char *str_account)
 {
     if (!has_source_account) {
         stellar_hashupdate_bool(false);
-        return;
+        return true;
+    }
+
+    // Convert account string to public key bytes
+    uint8_t bytes[32];
+    if (!stellar_getAddressBytes(str_account, bytes)) {
+        return false;
     }
 
     const char **str_addr_rows = stellar_lineBreakAddress(bytes);
@@ -168,21 +175,36 @@ void stellar_confirmSourceAccount(bool has_source_account, uint8_t *bytes)
         str_addr_rows[2]
     );
     if (!protectButton(ButtonRequestType_ButtonRequest_ProtectCall, false)) {
-        stellar_signingAbort();
-        return;
+        stellar_signingAbort(_("User canceled"));
+        return false;
     }
 
     // Hash: source account
     stellar_hashupdate_address(bytes);
+
+    return true;
 }
 
-void stellar_confirmCreateAccountOp(StellarCreateAccountOp *msg)
+bool stellar_confirmCreateAccountOp(StellarCreateAccountOp *msg)
 {
-    stellar_confirmSourceAccount(msg->has_source_account, msg->source_account.bytes);
+    if (!stellar_signing) return false;
+
+    if (!stellar_confirmSourceAccount(msg->has_source_account, msg->source_account)) {
+        stellar_signingAbort(_("Source account error"));
+        return false;
+    }
+
     // Hash: operation type
     stellar_hashupdate_uint32(0);
 
-    const char **str_addr_rows = stellar_lineBreakAddress(msg->new_account.bytes);
+    // Validate new account and convert to bytes
+    uint8_t new_account_bytes[STELLAR_KEY_SIZE];
+    if (!stellar_getAddressBytes(msg->new_account, new_account_bytes)) {
+        stellar_signingAbort(_("Invalid destination account"));
+        return false;
+    }
+
+    const char **str_addr_rows = stellar_lineBreakAddress(new_account_bytes);
 
     // Amount being funded
     char str_amount_line[32];
@@ -201,25 +223,39 @@ void stellar_confirmCreateAccountOp(StellarCreateAccountOp *msg)
         str_amount_line
     );
     if (!protectButton(ButtonRequestType_ButtonRequest_ProtectCall, false)) {
-        stellar_signingAbort();
-        return;
+        stellar_signingAbort(_("User canceled"));
+        return false;
     }
 
     // Hash: address
-    stellar_hashupdate_address(msg->new_account.bytes);
+    stellar_hashupdate_address(new_account_bytes);
     // Hash: starting amount
     stellar_hashupdate_uint64(msg->starting_balance);
 
     stellar_activeTx.confirmed_operations++;
+    return true;
 }
 
-void stellar_confirmPaymentOp(StellarPaymentOp *msg)
+bool stellar_confirmPaymentOp(StellarPaymentOp *msg)
 {
-    stellar_confirmSourceAccount(msg->has_source_account, msg->source_account.bytes);
+    if (!stellar_signing) return false;
+
+    if (!stellar_confirmSourceAccount(msg->has_source_account, msg->source_account)) {
+        stellar_signingAbort(_("Source account error"));
+        return false;
+    }
+
     // Hash: operation type
     stellar_hashupdate_uint32(1);
 
-    const char **str_addr_rows = stellar_lineBreakAddress(msg->destination_account.bytes);
+    // Validate destination account and convert to bytes
+    uint8_t destination_account_bytes[STELLAR_KEY_SIZE];
+    if (!stellar_getAddressBytes(msg->destination_account, destination_account_bytes)) {
+        stellar_signingAbort(_("Invalid destination account"));
+        return false;
+    }
+
+    const char **str_addr_rows = stellar_lineBreakAddress(destination_account_bytes);
 
     // To: G...
     char str_to[32];
@@ -245,12 +281,12 @@ void stellar_confirmPaymentOp(StellarPaymentOp *msg)
         str_addr_rows[2]
     );
     if (!protectButton(ButtonRequestType_ButtonRequest_ProtectCall, false)) {
-        stellar_signingAbort();
-        return;
+        stellar_signingAbort(_("User canceled"));
+        return false;
     }
 
     // Hash destination
-    stellar_hashupdate_address(msg->destination_account.bytes);
+    stellar_hashupdate_address(destination_account_bytes);
     // asset
     stellar_hashupdate_asset(&(msg->asset));
     // amount (even though amount is signed it doesn't matter for hashing)
@@ -258,15 +294,28 @@ void stellar_confirmPaymentOp(StellarPaymentOp *msg)
 
     // At this point, the operation is confirmed
     stellar_activeTx.confirmed_operations++;
+    return true;
 }
 
-void stellar_confirmPathPaymentOp(StellarPathPaymentOp *msg)
+bool stellar_confirmPathPaymentOp(StellarPathPaymentOp *msg)
 {
-    stellar_confirmSourceAccount(msg->has_source_account, msg->source_account.bytes);
+    if (!stellar_signing) return false;
+
+    if (!stellar_confirmSourceAccount(msg->has_source_account, msg->source_account)) {
+        stellar_signingAbort(_("Source account error"));
+        return false;
+    }
+
     // Hash: operation type
     stellar_hashupdate_uint32(2);
 
-    const char **str_dest_rows = stellar_lineBreakAddress(msg->destination_account.bytes);
+    // Validate destination account and convert to bytes
+    uint8_t destination_account_bytes[STELLAR_KEY_SIZE];
+    if (!stellar_getAddressBytes(msg->destination_account, destination_account_bytes)) {
+        stellar_signingAbort(_("Invalid destination account"));
+        return false;
+    }
+    const char **str_dest_rows = stellar_lineBreakAddress(destination_account_bytes);
 
     // To: G...
     char str_to[32];
@@ -301,8 +350,8 @@ void stellar_confirmPathPaymentOp(StellarPathPaymentOp *msg)
         str_dest_rows[2]
     );
     if (!protectButton(ButtonRequestType_ButtonRequest_ProtectCall, false)) {
-        stellar_signingAbort();
-        return;
+        stellar_signingAbort(_("User canceled"));
+        return false;
     }
 
     // Confirm what the sender is using to pay
@@ -321,8 +370,8 @@ void stellar_confirmPathPaymentOp(StellarPathPaymentOp *msg)
         _("from your account.")
     );
     if (!protectButton(ButtonRequestType_ButtonRequest_ProtectCall, false)) {
-        stellar_signingAbort();
-        return;
+        stellar_signingAbort(_("User canceled"));
+        return false;
     }
     // Note: no confirmation for intermediate steps since they don't impact the user
 
@@ -331,7 +380,7 @@ void stellar_confirmPathPaymentOp(StellarPathPaymentOp *msg)
     // send max (signed vs. unsigned doesn't matter wrt hashing)
     stellar_hashupdate_uint64(msg->send_max);
     // destination account
-    stellar_hashupdate_address(msg->destination_account.bytes);
+    stellar_hashupdate_address(destination_account_bytes);
     // destination asset
     stellar_hashupdate_asset(&(msg->destination_asset));
     // destination amount
@@ -345,11 +394,18 @@ void stellar_confirmPathPaymentOp(StellarPathPaymentOp *msg)
 
     // At this point, the operation is confirmed
     stellar_activeTx.confirmed_operations++;
+    return true;
 }
 
-void stellar_confirmManageOfferOp(StellarManageOfferOp *msg)
+bool stellar_confirmManageOfferOp(StellarManageOfferOp *msg)
 {
-    stellar_confirmSourceAccount(msg->has_source_account, msg->source_account.bytes);
+    if (!stellar_signing) return false;
+
+    if (!stellar_confirmSourceAccount(msg->has_source_account, msg->source_account)) {
+        stellar_signingAbort(_("Source account error"));
+        return false;
+    }
+
     // Hash: operation type
     stellar_hashupdate_uint32(3);
 
@@ -409,8 +465,8 @@ void stellar_confirmManageOfferOp(StellarManageOfferOp *msg)
         str_buying_asset
     );
     if (!protectButton(ButtonRequestType_ButtonRequest_ProtectCall, false)) {
-        stellar_signingAbort();
-        return;
+        stellar_signingAbort(_("User canceled"));
+        return false;
     }
 
     // Hash selling asset
@@ -428,11 +484,18 @@ void stellar_confirmManageOfferOp(StellarManageOfferOp *msg)
 
     // At this point, the operation is confirmed
     stellar_activeTx.confirmed_operations++;
+    return true;
 }
 
-void stellar_confirmCreatePassiveOfferOp(StellarCreatePassiveOfferOp *msg)
+bool stellar_confirmCreatePassiveOfferOp(StellarCreatePassiveOfferOp *msg)
 {
-    stellar_confirmSourceAccount(msg->has_source_account, msg->source_account.bytes);
+    if (!stellar_signing) return false;
+
+    if (!stellar_confirmSourceAccount(msg->has_source_account, msg->source_account)) {
+        stellar_signingAbort(_("Source account error"));
+        return false;
+    }
+
     // Hash: operation type
     stellar_hashupdate_uint32(4);
 
@@ -482,8 +545,8 @@ void stellar_confirmCreatePassiveOfferOp(StellarCreatePassiveOfferOp *msg)
         str_buying_asset
     );
     if (!protectButton(ButtonRequestType_ButtonRequest_ProtectCall, false)) {
-        stellar_signingAbort();
-        return;
+        stellar_signingAbort(_("User canceled"));
+        return false;
     }
 
     // Hash selling asset
@@ -499,11 +562,18 @@ void stellar_confirmCreatePassiveOfferOp(StellarCreatePassiveOfferOp *msg)
 
     // At this point, the operation is confirmed
     stellar_activeTx.confirmed_operations++;
+    return true;
 }
 
-void stellar_confirmSetOptionsOp(StellarSetOptionsOp *msg)
+bool stellar_confirmSetOptionsOp(StellarSetOptionsOp *msg)
 {
-    stellar_confirmSourceAccount(msg->has_source_account, msg->source_account.bytes);
+    if (!stellar_signing) return false;
+
+    if (!stellar_confirmSourceAccount(msg->has_source_account, msg->source_account)) {
+        stellar_signingAbort(_("Source account error"));
+        return false;
+    }
+
     // Hash: operation type
     stellar_hashupdate_uint32(5);
 
@@ -517,7 +587,14 @@ void stellar_confirmSetOptionsOp(StellarSetOptionsOp *msg)
     stellar_hashupdate_bool(msg->has_inflation_destination_account);
     if (msg->has_inflation_destination_account) {
         strlcpy(str_title, _("Set Inflation Destination"), sizeof(str_title));
-        const char **str_addr_rows = stellar_lineBreakAddress(msg->inflation_destination_account.bytes);
+
+        // Validate account and convert to bytes
+        uint8_t inflation_destination_account_bytes[STELLAR_KEY_SIZE];
+        if (!stellar_getAddressBytes(msg->inflation_destination_account, inflation_destination_account_bytes)) {
+            stellar_signingAbort(_("Invalid inflation destination account"));
+            return false;
+        }
+        const char **str_addr_rows = stellar_lineBreakAddress(inflation_destination_account_bytes);
 
         stellar_layoutTransactionDialog(
             str_title,
@@ -527,12 +604,12 @@ void stellar_confirmSetOptionsOp(StellarSetOptionsOp *msg)
             str_addr_rows[2]
         );
         if (!protectButton(ButtonRequestType_ButtonRequest_ProtectCall, false)) {
-            stellar_signingAbort();
-            return;
+            stellar_signingAbort(_("User canceled"));
+            return false;
         }
 
         // address
-        stellar_hashupdate_address(msg->inflation_destination_account.bytes);
+        stellar_hashupdate_address(inflation_destination_account_bytes);
     }
 
     // Clear flags
@@ -559,8 +636,8 @@ void stellar_confirmSetOptionsOp(StellarSetOptionsOp *msg)
             rows[3]
         );
         if (!protectButton(ButtonRequestType_ButtonRequest_ProtectCall, false)) {
-            stellar_signingAbort();
-            return;
+            stellar_signingAbort(_("User canceled"));
+            return false;
         }
         memset(rows, 0, sizeof(rows));
         row_idx = 0;
@@ -593,8 +670,8 @@ void stellar_confirmSetOptionsOp(StellarSetOptionsOp *msg)
             rows[3]
         );
         if (!protectButton(ButtonRequestType_ButtonRequest_ProtectCall, false)) {
-            stellar_signingAbort();
-            return;
+            stellar_signingAbort(_("User canceled"));
+            return false;
         }
         memset(rows, 0, sizeof(rows));
         row_idx = 0;
@@ -666,8 +743,8 @@ void stellar_confirmSetOptionsOp(StellarSetOptionsOp *msg)
             rows[3]
         );
         if (!protectButton(ButtonRequestType_ButtonRequest_ProtectCall, false)) {
-            stellar_signingAbort();
-            return;
+            stellar_signingAbort(_("User canceled"));
+            return false;
         }
         memset(rows, 0, sizeof(rows));
         row_idx = 0;
@@ -696,8 +773,8 @@ void stellar_confirmSetOptionsOp(StellarSetOptionsOp *msg)
             NULL
         );
         if (!protectButton(ButtonRequestType_ButtonRequest_ProtectCall, false)) {
-            stellar_signingAbort();
-            return;
+            stellar_signingAbort(_("User canceled"));
+            return false;
         }
         memset(rows, 0, sizeof(rows));
         row_idx = 0;
@@ -738,8 +815,8 @@ void stellar_confirmSetOptionsOp(StellarSetOptionsOp *msg)
                 str_addr_rows[2]
             );
             if (!protectButton(ButtonRequestType_ButtonRequest_ProtectCall, false)) {
-                stellar_signingAbort();
-                return;
+                stellar_signingAbort(_("User canceled"));
+                return false;
             }
         }
         if (msg->signer_type == 1) {
@@ -755,8 +832,8 @@ void stellar_confirmSetOptionsOp(StellarSetOptionsOp *msg)
                 _("screen)")
             );
             if (!protectButton(ButtonRequestType_ButtonRequest_ProtectCall, false)) {
-                stellar_signingAbort();
-                return;
+                stellar_signingAbort(_("User canceled"));
+                return false;
             }
         }
         if (msg->signer_type == 2) {
@@ -772,8 +849,8 @@ void stellar_confirmSetOptionsOp(StellarSetOptionsOp *msg)
                 _("screen)")
             );
             if (!protectButton(ButtonRequestType_ButtonRequest_ProtectCall, false)) {
-                stellar_signingAbort();
-                return;
+                stellar_signingAbort(_("User canceled"));
+                return false;
             }
         }
 
@@ -792,8 +869,8 @@ void stellar_confirmSetOptionsOp(StellarSetOptionsOp *msg)
                 rows[3]
             );
             if (!protectButton(ButtonRequestType_ButtonRequest_ProtectCall, false)) {
-                stellar_signingAbort();
-                return;
+                stellar_signingAbort(_("User canceled"));
+                return false;
             }
             memset(rows, 0, sizeof(rows));
             row_idx = 0;
@@ -809,11 +886,18 @@ void stellar_confirmSetOptionsOp(StellarSetOptionsOp *msg)
 
     // At this point, the operation is confirmed
     stellar_activeTx.confirmed_operations++;
+    return true;
 }
 
-void stellar_confirmChangeTrustOp(StellarChangeTrustOp *msg)
+bool stellar_confirmChangeTrustOp(StellarChangeTrustOp *msg)
 {
-    stellar_confirmSourceAccount(msg->has_source_account, msg->source_account.bytes);
+    if (!stellar_signing) return false;
+
+    if (!stellar_confirmSourceAccount(msg->has_source_account, msg->source_account)) {
+        stellar_signingAbort(_("Source account error"));
+        return false;
+    }
+
     // Hash: operation type
     stellar_hashupdate_uint32(6);
 
@@ -840,8 +924,16 @@ void stellar_confirmChangeTrustOp(StellarChangeTrustOp *msg)
         strlcat(str_amount_row, str_amount, sizeof(str_amount_row));
     }
 
+    // Validate destination account and convert to bytes
+    uint8_t asset_issuer_bytes[STELLAR_KEY_SIZE];
+    if (!stellar_getAddressBytes(msg->asset.issuer, asset_issuer_bytes)) {
+        stellar_signingAbort(_("User canceled"));
+        fsm_sendFailure(FailureType_Failure_ProcessError, _("Invalid asset issuer"));
+        return false;
+    }
+
     // Display full issuer address
-    const char **str_addr_rows = stellar_lineBreakAddress(msg->asset.issuer.bytes);
+    const char **str_addr_rows = stellar_lineBreakAddress(asset_issuer_bytes);
 
     stellar_layoutTransactionDialog(
         str_title,
@@ -851,8 +943,8 @@ void stellar_confirmChangeTrustOp(StellarChangeTrustOp *msg)
         str_addr_rows[2]
     );
     if (!protectButton(ButtonRequestType_ButtonRequest_ProtectCall, false)) {
-        stellar_signingAbort();
-        return;
+        stellar_signingAbort(_("User canceled"));
+        return false;
     }
 
     // Hash: asset
@@ -862,11 +954,18 @@ void stellar_confirmChangeTrustOp(StellarChangeTrustOp *msg)
 
     // At this point, the operation is confirmed
     stellar_activeTx.confirmed_operations++;
+    return true;
 }
 
-void stellar_confirmAllowTrustOp(StellarAllowTrustOp *msg)
+bool stellar_confirmAllowTrustOp(StellarAllowTrustOp *msg)
 {
-    stellar_confirmSourceAccount(msg->has_source_account, msg->source_account.bytes);
+    if (!stellar_signing) return false;
+
+    if (!stellar_confirmSourceAccount(msg->has_source_account, msg->source_account)) {
+        stellar_signingAbort(_("Source account error"));
+        return false;
+    }
+
     // Hash: operation type
     stellar_hashupdate_uint32(7);
 
@@ -883,7 +982,14 @@ void stellar_confirmAllowTrustOp(StellarAllowTrustOp *msg)
     char str_asset_row[32];
     strlcpy(str_asset_row, msg->asset_code, sizeof(str_asset_row));
 
-    const char **str_trustor_rows = stellar_lineBreakAddress(msg->trusted_account.bytes);
+    // Validate account and convert to bytes
+    uint8_t trusted_account_bytes[STELLAR_KEY_SIZE];
+    if (!stellar_getAddressBytes(msg->trusted_account, trusted_account_bytes)) {
+        stellar_signingAbort(_("Invalid trusted account"));
+        return false;
+    }
+
+    const char **str_trustor_rows = stellar_lineBreakAddress(trusted_account_bytes);
 
     // By: G...
     char str_by[32];
@@ -898,12 +1004,12 @@ void stellar_confirmAllowTrustOp(StellarAllowTrustOp *msg)
         str_trustor_rows[2]
     );
     if (!protectButton(ButtonRequestType_ButtonRequest_ProtectCall, false)) {
-        stellar_signingAbort();
-        return;
+        stellar_signingAbort(_("User canceled"));
+        return false;
     }
 
     // Hash: trustor account (the account being allowed to access the asset)
-    stellar_hashupdate_address(msg->trusted_account.bytes);
+    stellar_hashupdate_address(trusted_account_bytes);
     // asset type
     stellar_hashupdate_uint32(msg->asset_type);
     // asset code
@@ -924,15 +1030,29 @@ void stellar_confirmAllowTrustOp(StellarAllowTrustOp *msg)
 
     // At this point, the operation is confirmed
     stellar_activeTx.confirmed_operations++;
+    return true;
 }
 
-void stellar_confirmAccountMergeOp(StellarAccountMergeOp *msg)
+bool stellar_confirmAccountMergeOp(StellarAccountMergeOp *msg)
 {
-    stellar_confirmSourceAccount(msg->has_source_account, msg->source_account.bytes);
+    if (!stellar_signing) return false;
+
+    if (!stellar_confirmSourceAccount(msg->has_source_account, msg->source_account)) {
+        stellar_signingAbort(_("Source account error"));
+        return false;
+    }
+
     // Hash: operation type
     stellar_hashupdate_uint32(8);
 
-    const char **str_destination_rows = stellar_lineBreakAddress(msg->destination_account.bytes);
+    // Validate account and convert to bytes
+    uint8_t destination_account_bytes[STELLAR_KEY_SIZE];
+    if (!stellar_getAddressBytes(msg->destination_account, destination_account_bytes)) {
+        stellar_signingAbort(_("Invalid destination account"));
+        return false;
+    }
+
+    const char **str_destination_rows = stellar_lineBreakAddress(destination_account_bytes);
 
     stellar_layoutTransactionDialog(
         _("Merge Account"),
@@ -942,20 +1062,27 @@ void stellar_confirmAccountMergeOp(StellarAccountMergeOp *msg)
         str_destination_rows[2]
     );
     if (!protectButton(ButtonRequestType_ButtonRequest_ProtectCall, false)) {
-        stellar_signingAbort();
-        return;
+        stellar_signingAbort(_("User canceled"));
+        return false;
     }
 
     // Hash: destination account
-    stellar_hashupdate_address(msg->destination_account.bytes);
+    stellar_hashupdate_address(destination_account_bytes);
 
     // At this point, the operation is confirmed
     stellar_activeTx.confirmed_operations++;
+    return true;
 }
 
-void stellar_confirmManageDataOp(StellarManageDataOp *msg)
+bool stellar_confirmManageDataOp(StellarManageDataOp *msg)
 {
-    stellar_confirmSourceAccount(msg->has_source_account, msg->source_account.bytes);
+    if (!stellar_signing) return false;
+
+    if (!stellar_confirmSourceAccount(msg->has_source_account, msg->source_account)) {
+        stellar_signingAbort(_("Source account error"));
+        return false;
+    }
+
     // Hash: operation type
     stellar_hashupdate_uint32(10);
 
@@ -978,8 +1105,8 @@ void stellar_confirmManageDataOp(StellarManageDataOp *msg)
         str_key_lines[3]
     );
     if (!protectButton(ButtonRequestType_ButtonRequest_ProtectCall, false)) {
-        stellar_signingAbort();
-        return;
+        stellar_signingAbort(_("User canceled"));
+        return false;
     }
 
     // Confirm value by displaying sha256 hash since this can contain non-printable characters
@@ -998,8 +1125,8 @@ void stellar_confirmManageDataOp(StellarManageDataOp *msg)
             str_hash_lines[3]
         );
         if (!protectButton(ButtonRequestType_ButtonRequest_ProtectCall, false)) {
-            stellar_signingAbort();
-            return;
+            stellar_signingAbort(_("User canceled"));
+            return false;
         }
     }
 
@@ -1018,11 +1145,18 @@ void stellar_confirmManageDataOp(StellarManageDataOp *msg)
 
     // At this point, the operation is confirmed
     stellar_activeTx.confirmed_operations++;
+    return true;
 }
 
-void stellar_confirmBumpSequenceOp(StellarBumpSequenceOp *msg)
+bool stellar_confirmBumpSequenceOp(StellarBumpSequenceOp *msg)
 {
-    stellar_confirmSourceAccount(msg->has_source_account, msg->source_account.bytes);
+    if (!stellar_signing) return false;
+
+    if (!stellar_confirmSourceAccount(msg->has_source_account, msg->source_account)) {
+        stellar_signingAbort(_("Source account error"));
+        return false;
+    }
+
     // Hash: operation type
     stellar_hashupdate_uint32(11);
 
@@ -1037,8 +1171,8 @@ void stellar_confirmBumpSequenceOp(StellarBumpSequenceOp *msg)
         NULL
     );
     if (!protectButton(ButtonRequestType_ButtonRequest_ProtectCall, false)) {
-        stellar_signingAbort();
-        return;
+        stellar_signingAbort(_("User canceled"));
+        return false;
     }
 
     // Hash: bump to
@@ -1046,12 +1180,17 @@ void stellar_confirmBumpSequenceOp(StellarBumpSequenceOp *msg)
 
     // At this point, the operation is confirmed
     stellar_activeTx.confirmed_operations++;
+    return true;
 }
 
-void stellar_signingAbort()
+void stellar_signingAbort(const char *reason)
 {
+    if (!reason) {
+        reason = _("Unknown error");
+    }
+
     stellar_signing = false;
-    fsm_sendFailure(FailureType_Failure_ActionCancelled, NULL);
+    fsm_sendFailure(FailureType_Failure_ProcessError, reason);
     layoutHome();
 }
 
@@ -1207,8 +1346,6 @@ const char **stellar_lineBreakAddress(uint8_t *addrbytes)
 void stellar_format_asset(StellarAssetType *asset, char *str_formatted, size_t len)
 {
     char str_asset_code[12 + 1];
-    // Full asset issuer string
-    char str_asset_issuer[56+1];
     // truncated asset issuer, final length depends on length of asset code
     char str_asset_issuer_trunc[13 + 1];
 
@@ -1216,8 +1353,11 @@ void stellar_format_asset(StellarAssetType *asset, char *str_formatted, size_t l
     memset(str_asset_code, 0, sizeof(str_asset_code));
     memset(str_asset_issuer_trunc, 0, sizeof(str_asset_issuer_trunc));
 
-    // Get string representation of address
-    stellar_publicAddressAsStr(asset->issuer.bytes, str_asset_issuer, sizeof(str_asset_issuer));
+    // Validate issuer account for non-native assets
+    if (asset->type != 0 && !stellar_validateAddress(asset->issuer)) {
+        stellar_signingAbort(_("Invalid asset issuer"));
+        return;
+    }
 
     // Native asset
     if (asset->type == 0) {
@@ -1229,7 +1369,7 @@ void stellar_format_asset(StellarAssetType *asset, char *str_formatted, size_t l
         strlcpy(str_formatted, str_asset_code, len);
 
         // Truncate issuer to 13 chars
-        memcpy(str_asset_issuer_trunc, str_asset_issuer, 13);
+        memcpy(str_asset_issuer_trunc, asset->issuer, 13);
     }
     // 12-character custom
     if (asset->type == 2) {
@@ -1237,7 +1377,7 @@ void stellar_format_asset(StellarAssetType *asset, char *str_formatted, size_t l
         strlcpy(str_formatted, str_asset_code, len);
 
         // Truncate issuer to 5 characters
-        memcpy(str_asset_issuer_trunc, str_asset_issuer, 5);
+        memcpy(str_asset_issuer_trunc, asset->issuer, 5);
     }
     // Issuer is read the same way for both types of custom assets
     if (asset->type == 1 || asset->type == 2) {
@@ -1267,6 +1407,65 @@ size_t stellar_publicAddressAsStr(uint8_t *bytes, char *out, size_t outlen)
     return 56;
 }
 
+/**
+ * Stellar account string is a base32-encoded string that starts with "G"
+ *
+ * It decodes to the following format:
+ *  Byte 0 - always 0x30 ("G" when base32 encoded), version byte indicating a public key
+ *  Bytes 1-33 - 32-byte public key bytes
+ *  Bytes 34-35 - 2-byte CRC16 checksum of the version byte + public key bytes (first 33 bytes)
+ *
+ * Note that the stellar "seed" (private key) also uses this format except the version byte
+ * is 0xC0 which encodes to "S" in base32
+ */
+bool stellar_validateAddress(const char *str_address)
+{
+    bool valid = false;
+    uint8_t decoded[STELLAR_ADDRESS_SIZE_RAW];
+
+    if (strlen(str_address) != STELLAR_ADDRESS_SIZE) {
+        return false;
+    }
+
+    // Check that it decodes correctly
+    uint8_t *ret = base32_decode(str_address, STELLAR_ADDRESS_SIZE, decoded, sizeof(decoded), BASE32_ALPHABET_RFC4648);
+    valid = (ret != NULL);
+
+    // ... and that version byte is 0x30
+    if (valid && decoded[0] != 0x30) {
+        valid = false;
+    }
+
+    // ... and that checksums match
+    uint16_t checksum_expected = stellar_crc16(decoded, 33);
+    uint16_t checksum_actual = (decoded[34] << 8) | decoded[33]; // unsigned short (little endian)
+    if (valid && checksum_expected != checksum_actual) {
+        valid = false;
+    }
+
+    memzero(decoded, sizeof(decoded));
+    return valid;
+}
+
+/**
+ * Converts a string address (G...) to the 32-byte raw address
+ */
+bool stellar_getAddressBytes(char* str_address, uint8_t *out_bytes)
+{
+    uint8_t decoded[STELLAR_ADDRESS_SIZE_RAW];
+
+    // Ensure address is valid
+    if (!stellar_validateAddress(str_address)) return false;
+
+    base32_decode(str_address, STELLAR_ADDRESS_SIZE, decoded, sizeof(decoded), BASE32_ALPHABET_RFC4648);
+
+    // The 32 bytes with offset 1-33 represent the public key
+    memcpy(out_bytes, &decoded[1], 32);
+
+    memzero(decoded, sizeof(decoded));
+    return true;
+}
+
 /*
  * CRC16 implementation compatible with the Stellar version
  * Ported from this implementation: http://introcs.cs.princeton.edu/java/61data/CRC16CCITT.java.html
@@ -1406,6 +1605,13 @@ void stellar_hashupdate_asset(StellarAssetType *asset)
 {
     stellar_hashupdate_uint32(asset->type);
 
+    // For non-native assets, validate issuer account and convert to bytes
+    uint8_t issuer_bytes[STELLAR_KEY_SIZE];
+    if (asset->type != 0 && !stellar_getAddressBytes(asset->issuer, issuer_bytes)) {
+        stellar_signingAbort(_("Invalid asset issuer"));
+        return;
+    }
+
     // 4-character asset code
     if (asset->type == 1) {
         char code4[4+1];
@@ -1413,7 +1619,7 @@ void stellar_hashupdate_asset(StellarAssetType *asset)
         strlcpy(code4, asset->code, sizeof(code4));
 
         stellar_hashupdate_bytes((uint8_t *)code4, 4);
-        stellar_hashupdate_address(asset->issuer.bytes);
+        stellar_hashupdate_address(issuer_bytes);
     }
 
     // 12-character asset code
@@ -1423,7 +1629,7 @@ void stellar_hashupdate_asset(StellarAssetType *asset)
         strlcpy(code12, asset->code, sizeof(code12));
 
         stellar_hashupdate_bytes((uint8_t *)code12, 12);
-        stellar_hashupdate_address(asset->issuer.bytes);
+        stellar_hashupdate_address(issuer_bytes);
     }
 }
 
@@ -1475,7 +1681,7 @@ void stellar_layoutTransactionSummary(StellarSignTx *msg)
         str_addr_rows[2]
     );
     if (!protectButton(ButtonRequestType_ButtonRequest_ProtectCall, false)) {
-        stellar_signingAbort();
+        stellar_signingAbort(_("User canceled"));
         return;
     }
 
@@ -1529,7 +1735,7 @@ void stellar_layoutTransactionSummary(StellarSignTx *msg)
         str_lines[4]
     );
     if (!protectButton(ButtonRequestType_ButtonRequest_ProtectCall, false)) {
-        stellar_signingAbort();
+        stellar_signingAbort(_("User canceled"));
         return;
     }
 
@@ -1577,7 +1783,7 @@ void stellar_layoutTransactionSummary(StellarSignTx *msg)
             str_lines[3]
         );
         if (!protectButton(ButtonRequestType_ButtonRequest_ProtectCall, false)) {
-            stellar_signingAbort();
+            stellar_signingAbort(_("User canceled"));
             return;
         }
     }

firmware/stellar.h

@@ -23,6 +23,14 @@
 #include "crypto.h"
 #include "messages.pb.h"
 #include "fsm.h"
+#include "base32.h"
+
+// 56 character base-32 encoded string
+#define STELLAR_ADDRESS_SIZE 56
+// Decodes to 35 bytes
+#define STELLAR_ADDRESS_SIZE_RAW 35
+// Raw key size is 32 bytes
+#define STELLAR_KEY_SIZE 32
 
 typedef struct {
     // BIP32 path to the address being used for signing
@@ -44,18 +52,19 @@ typedef struct {
 
 // Signing process
 void stellar_signingInit(StellarSignTx *tx);
-void stellar_signingAbort(void);
-void stellar_confirmCreateAccountOp(StellarCreateAccountOp *msg);
-void stellar_confirmPaymentOp(StellarPaymentOp *msg);
-void stellar_confirmPathPaymentOp(StellarPathPaymentOp *msg);
-void stellar_confirmManageOfferOp(StellarManageOfferOp *msg);
-void stellar_confirmCreatePassiveOfferOp(StellarCreatePassiveOfferOp *msg);
-void stellar_confirmSetOptionsOp(StellarSetOptionsOp *msg);
-void stellar_confirmChangeTrustOp(StellarChangeTrustOp *msg);
-void stellar_confirmAllowTrustOp(StellarAllowTrustOp *msg);
-void stellar_confirmAccountMergeOp(StellarAccountMergeOp *msg);
-void stellar_confirmManageDataOp(StellarManageDataOp *msg);
-void stellar_confirmBumpSequenceOp(StellarBumpSequenceOp *msg);
+void stellar_signingAbort(const char *reason);
+bool stellar_confirmSourceAccount(bool has_source_account, char *str_account);
+bool stellar_confirmCreateAccountOp(StellarCreateAccountOp *msg);
+bool stellar_confirmPaymentOp(StellarPaymentOp *msg);
+bool stellar_confirmPathPaymentOp(StellarPathPaymentOp *msg);
+bool stellar_confirmManageOfferOp(StellarManageOfferOp *msg);
+bool stellar_confirmCreatePassiveOfferOp(StellarCreatePassiveOfferOp *msg);
+bool stellar_confirmSetOptionsOp(StellarSetOptionsOp *msg);
+bool stellar_confirmChangeTrustOp(StellarChangeTrustOp *msg);
+bool stellar_confirmAllowTrustOp(StellarAllowTrustOp *msg);
+bool stellar_confirmAccountMergeOp(StellarAccountMergeOp *msg);
+bool stellar_confirmManageDataOp(StellarManageDataOp *msg);
+bool stellar_confirmBumpSequenceOp(StellarBumpSequenceOp *msg);
 
 // Layout
 void stellar_layoutTransactionDialog(const char *line1, const char *line2, const char *line3, const char *line4, const char *line5);
@@ -87,6 +96,8 @@ void stellar_format_stroops(uint64_t number, char *out, size_t outlen);
 void stellar_format_asset(StellarAssetType *asset, char *str_formatted, size_t len);
 void stellar_format_price(uint32_t numerator, uint32_t denominator, char *out, size_t outlen);
 
+bool stellar_validateAddress(const char *str_address);
+bool stellar_getAddressBytes(char* str_address, uint8_t *out_bytes);
 uint16_t stellar_crc16(uint8_t *bytes, uint32_t length);
 
 #endif