nem2: Canonicalize mosaics in transfer transactions

NIS deserializes then serializes transactions in order to verify the
signature. This means that transactions must be serialized canonically,
otherwise the signature will not match. Due to [1], mosaics are sorted
and deduplicated in transfer transactions.

[1]: 4231550ddf

firmware/fsm.c

@@ -1206,6 +1206,10 @@ void fsm_msgNEMSignTx(NEMSignTx *msg) {
 	char address[NEM_ADDRESS_SIZE + 1];
 	hdnode_get_nem_address(node, common->network, address);
 
+	if (msg->has_transfer) {
+		msg->transfer.mosaics_count = nem_canonicalizeMosaics(msg->transfer.mosaics, msg->transfer.mosaics_count);
+	}
+
 	if (msg->has_transfer && !nem_askTransfer(common, &msg->transfer, network)) {
 		fsm_sendFailure(FailureType_Failure_ActionCancelled, _("Signing cancelled by user"));
 		layoutHome();

firmware/nem2.c

@@ -157,43 +157,21 @@ const char *nem_validate_aggregate_modification(const NEMAggregateModification *
 
 bool nem_askTransfer(const NEMTransactionCommon *common, const NEMTransfer *transfer, const char *desc) {
 	if (transfer->mosaics_count) {
-		struct {
+		const NEMMosaic *xem = NULL;
-			bool skip;
-			uint64_t quantity;
-			const NEMMosaicDefinition *definition;
-		} mosaics[transfer->mosaics_count], *xem = NULL;
-
-		memset(mosaics, 0, sizeof(mosaics));
-
 		bool unknownMosaic = false;
 
-		for (size_t i = 0; i < transfer->mosaics_count; i++) {
+		const NEMMosaicDefinition *definitions[transfer->mosaics_count];
-			// Skip duplicate mosaics
-			if (mosaics[i].skip) continue;
 
+		for (size_t i = 0; i < transfer->mosaics_count; i++) {
 			const NEMMosaic *mosaic = &transfer->mosaics[i];
 
-			if ((mosaics[i].definition = nem_mosaicByName(mosaic->namespace, mosaic->mosaic, common->network))) {
+			definitions[i] = nem_mosaicByName(mosaic->namespace, mosaic->mosaic, common->network);
-				// XEM is displayed separately
+
-				if (mosaics[i].definition == NEM_MOSAIC_DEFINITION_XEM) {
+			if (definitions[i] == NEM_MOSAIC_DEFINITION_XEM) {
-					// Do not display as a mosaic
+				xem = mosaic;
-					mosaics[i].skip = true;
+			} else if (definitions[i] == NULL) {
-					xem = &mosaics[i];
-				}
-			} else {
 				unknownMosaic = true;
 			}
-
-			mosaics[i].quantity = mosaic->quantity;
-			for (size_t j = i + 1; j < transfer->mosaics_count; j++) {
-				const NEMMosaic *new_mosaic = &transfer->mosaics[j];
-
-				if (nem_mosaicMatches(mosaics[i].definition, new_mosaic->namespace, new_mosaic->mosaic, common->network)) {
-					// Merge duplicate mosaics
-					mosaics[j].skip = true;
-					mosaics[i].quantity += new_mosaic->quantity;
-				}
-			}
 		}
 
 		bignum256 multiplier;
@@ -221,15 +199,16 @@ bool nem_askTransfer(const NEMTransactionCommon *common, const NEMTransfer *tran
 		}
 
 		for (size_t i = 0; i < transfer->mosaics_count; i++) {
-			// Skip duplicate mosaics or XEM
-			if (mosaics[i].skip) continue;
-
 			const NEMMosaic *mosaic = &transfer->mosaics[i];
 
-			if (mosaics[i].definition) {
+			if (mosaic == xem) {
-				layoutNEMTransferMosaic(mosaics[i].definition, mosaics[i].quantity, &multiplier, common->network);
+				continue;
+			}
+
+			if (definitions[i]) {
+				layoutNEMTransferMosaic(definitions[i], mosaic->quantity, &multiplier, common->network);
 			} else {
-				layoutNEMTransferUnknownMosaic(mosaic->namespace, mosaic->mosaic, mosaics[i].quantity, &multiplier);
+				layoutNEMTransferUnknownMosaic(mosaic->namespace, mosaic->mosaic, mosaic->quantity, &multiplier);
 			}
 
 			if (!protectButton(ButtonRequestType_ButtonRequest_ConfirmOutput, false)) {
@@ -707,6 +686,58 @@ static inline size_t format_amount(const NEMMosaicDefinition *definition, const
 		size);
 }
 
+size_t nem_canonicalizeMosaics(NEMMosaic *mosaics, size_t mosaics_count) {
+	if (mosaics_count <= 1) {
+		return mosaics_count;
+	}
+
+	size_t actual_count = 0;
+
+	bool skip[mosaics_count];
+	memset(skip, 0, sizeof(skip));
+
+	// Merge duplicates
+	for (size_t i = 0; i < mosaics_count; i++) {
+		if (skip[i]) continue;
+
+		NEMMosaic *mosaic = &mosaics[actual_count];
+
+		if (actual_count++ != i) {
+			memcpy(mosaic, &mosaics[i], sizeof(NEMMosaic));
+		}
+
+		for (size_t j = i + 1; j < mosaics_count; j++) {
+			if (skip[j]) continue;
+
+			const NEMMosaic *new_mosaic = &mosaics[j];
+
+			if (nem_mosaicCompare(mosaic, new_mosaic) == 0) {
+				skip[j] = true;
+				mosaic->quantity += new_mosaic->quantity;
+			}
+		}
+	}
+
+	NEMMosaic temp;
+
+	// Sort mosaics
+	for (size_t i = 0; i < actual_count - 1; i++) {
+		NEMMosaic *a = &mosaics[i];
+
+		for (size_t j = i + 1; j < actual_count; j++) {
+			NEMMosaic *b = &mosaics[j];
+
+			if (nem_mosaicCompare(a, b) > 0) {
+				memcpy(&temp, a, sizeof(NEMMosaic));
+				memcpy(a, b, sizeof(NEMMosaic));
+				memcpy(b, &temp, sizeof(NEMMosaic));
+			}
+		}
+	}
+
+	return actual_count;
+}
+
 void nem_mosaicFormatAmount(const NEMMosaicDefinition *definition, uint64_t quantity, const bignum256 *multiplier, char *str_out, size_t size) {
 	bignum256 amnt;
 	bn_read_uint64(quantity, &amnt);

firmware/nem2.h

@@ -55,6 +55,7 @@ bool nem_fsmMultisig(nem_transaction_ctx *context, const NEMTransactionCommon *c
 
 const NEMMosaicDefinition *nem_mosaicByName(const char *namespace, const char *mosaic, uint8_t network);
 
+size_t nem_canonicalizeMosaics(NEMMosaic *mosaics, size_t mosaics_count);
 void nem_mosaicFormatAmount(const NEMMosaicDefinition *definition, uint64_t quantity, const bignum256 *multiplier, char *str_out, size_t size);
 bool nem_mosaicFormatLevy(const NEMMosaicDefinition *definition, uint64_t quantity, const bignum256 *multiplier, uint8_t network, char *str_out, size_t size);
 
@@ -80,4 +81,27 @@ static inline bool nem_mosaicMatches(const NEMMosaicDefinition *definition, cons
 	return false;
 }
 
+static inline int nem_mosaicCompare(const NEMMosaic *a, const NEMMosaic *b) {
+	size_t namespace_length = strlen(a->namespace);
+
+	// Ensure that strlen(a->namespace) <= strlen(b->namespace)
+	if (namespace_length > strlen(b->namespace)) {
+		return -nem_mosaicCompare(b, a);
+	}
+
+	int r = strncmp(a->namespace, b->namespace, namespace_length);
+
+	if (r == 0 && b->namespace[namespace_length] != '\0') {
+		// The next character would be the separator
+		r = (':' - b->namespace[namespace_length]);
+	}
+
+	if (r == 0) {
+		// Finally compare the mosaic
+		r = strcmp(a->mosaic, b->mosaic);
+	}
+
+	return r;
+}
+
 #endif