Masp Fixes & Improvements

.gitmodules

@@ -6,4 +6,7 @@
 	url = https://github.com/LedgerHQ/ledger-secure-sdk
 [submodule "deps/blake2"]
 	path = deps/blake2
-	url = https://github.com/Zondax/BLAKE2.git
+	url = https://github.com/Zondax/BLAKE2.git
+[submodule "js"]
+	path = js
+	url = https://github.com/Zondax/ledger-namada-js

app/Makefile.version

@@ -1,6 +1,6 @@
 # This is the `transaction_version` field of `Runtime`
-APPVERSION_M=0
+APPVERSION_M=1
 # This is the `spec_version` field of `Runtime`
 APPVERSION_N=0
 # This is the patch version of this release
-APPVERSION_P=28
+APPVERSION_P=0

app/src/apdu_handler.c

@@ -121,6 +121,7 @@ __Z_INLINE void handleSignTransaction(volatile uint32_t *flags, volatile uint32_
     CHECK_APP_CANARY()
     view_review_init(tx_getItem, tx_getNumItems, app_sign);
     view_review_show(REVIEW_TXN);
+    transaction_reset();
     *flags |= IO_ASYNCH_REPLY;
 }
 

app/src/common/actions.h

@@ -67,6 +67,10 @@ __Z_INLINE zxerr_t app_fill_randomness(masp_type_e type) {
     zemu_log("app_fill_randomness\n");
     MEMZERO(G_io_apdu_buffer, IO_APDU_BUFFER_SIZE);
 
+    if (get_state() != STATE_INITIAL && get_state() != STATE_PROCESSED_RANDOMNESS) {
+        return zxerr_unknown;
+    }
+
     cmdResponseLen = 0;
     zxerr_t err = crypto_computeRandomness(type, G_io_apdu_buffer, IO_APDU_BUFFER_SIZE - 3, &cmdResponseLen);
 
@@ -75,6 +79,7 @@ __Z_INLINE zxerr_t app_fill_randomness(masp_type_e type) {
         THROW(APDU_CODE_DATA_INVALID);
     }
 
+    set_state(STATE_PROCESSED_RANDOMNESS);
     return err;
 }
 

app/src/crypto_helper.c

@@ -625,6 +625,21 @@ parser_error_t computeValueCommitment(uint64_t value, uint8_t *rcv, uint8_t *ide
     return parser_ok;
 }
 
+parser_error_t computeConvertValueCommitment(uint64_t value, uint8_t *rcv, uint8_t *generator, uint8_t *cv) {
+    if(rcv == NULL || generator == NULL || cv == NULL) {
+        return parser_unexpected_error;
+    }
+
+    uint8_t value_bytes[32] = {0};
+    u64_to_bytes(value, value_bytes);
+
+    uint8_t scalar[32] = {0};
+    CHECK_ERROR(parser_scalar_multiplication(rcv, ValueCommitmentRandomnessGenerator, scalar));
+    CHECK_ERROR(add_points(generator, value_bytes, scalar, cv));
+
+    return parser_ok;
+}
+
 
 parser_error_t computeRk(keys_t *keys, uint8_t *alpha, uint8_t *rk) {
     if(keys == NULL || alpha == NULL || rk == NULL) {

app/src/crypto_helper.h

@@ -73,6 +73,7 @@ parser_error_t h_star(uint8_t *a, uint16_t a_len, uint8_t *b, uint16_t b_len, ui
 parser_error_t parser_scalar_multiplication(const uint8_t input[32], constant_key_t key, uint8_t output[32]);
 parser_error_t parser_compute_sbar(const uint8_t s[32], uint8_t r[32], uint8_t rsk[32], uint8_t sbar[32]);
 parser_error_t parser_randomized_secret_from_seed(const uint8_t ask[32], const uint8_t alpha[32], uint8_t output[32]);
+parser_error_t computeConvertValueCommitment(uint64_t value, uint8_t *rcv, uint8_t *generator, uint8_t *cv);
 #ifdef __cplusplus
 }
 #endif

app/src/keys_def.h

@@ -50,6 +50,13 @@ typedef uint8_t d_t[DIVERSIFIER_LENGTH];
 
 typedef uint8_t public_address_t[KEY_LENGTH];
 
+typedef struct {
+    char viewKey[2*KEY_LENGTH];
+    char ovk[KEY_LENGTH];
+    char ivk[KEY_LENGTH];
+    char dk[KEY_LENGTH];
+} KeyData;
+
 typedef struct {
     spending_key_t spendingKey;
     ask_t ask;

app/src/nvdata.c

@@ -120,42 +120,53 @@ uint8_t transaction_get_n_converts() {
 }
 
 bool spend_signatures_more_extract() {
-  return transaction_header.spends_sign_index > 0;
+  return transaction_header.spends_sign_index < transaction_header.spends_sign_len;
 }
 
 zxerr_t spend_signatures_append(uint8_t *signature) {
-  if (transaction_header.spends_sign_index >=
+  if (transaction_header.spends_sign_len >=
       transaction_header.spendlist_len) {
     return zxerr_unknown;
   }
 
   MEMCPY_NV((void *)&N_transactioninfo
-                .spend_signatures[transaction_header.spends_sign_index],
+                .spend_signatures[transaction_header.spends_sign_len],
             signature, SIGNATURE_SIZE);
-  transaction_header.spends_sign_index++;
+  transaction_header.spends_sign_len++;
   return zxerr_ok;
 }
 
 zxerr_t get_next_spend_signature(uint8_t *result) {
-  const uint8_t index = transaction_header.spendlist_len - transaction_header.spends_sign_index;
-  if (index >= transaction_header.spendlist_len) {
-    return zxerr_unknown;
-  }
-  MEMCPY(result, (void *)&N_transactioninfo.spend_signatures[index], SIGNATURE_SIZE);
-  transaction_header.spends_sign_index--;
   if (!spend_signatures_more_extract()) {
-    transaction_reset();
+      return zxerr_unknown;
   }
+  const uint8_t index = transaction_header.spends_sign_index;
+  MEMCPY(result, (void *)&N_transactioninfo.spend_signatures[index], SIGNATURE_SIZE);
+  transaction_header.spends_sign_index++;
+  set_state(STATE_EXTRACT_SPENDS);
   return zxerr_ok;
 }
 
+uint8_t get_state() {
+    return transaction_header.state;
+}
+
+void set_state(uint8_t state) {
+    transaction_header.state = state;
+}
+
+void state_reset() {
+    transaction_header.state = STATE_INITIAL;
+}
+
 void zeroize_signatures() {
   uint8_t sig[SIGNATURE_SIZE] = {0};
 
-  transaction_header.spends_sign_index = 0;
+  transaction_header.spends_sign_len = 0;
   for (int i = 0; i < SPEND_LIST_SIZE; i++) {
     spend_signatures_append(sig);
   }
+  transaction_header.spends_sign_len = 0;
   transaction_header.spends_sign_index = 0;
 }
 
@@ -197,5 +208,6 @@ void transaction_reset() {
     zeroize_outputs();
     zeroize_converts();
     zeroize_signatures();
+    set_state(STATE_INITIAL);
 }
 

app/src/nvdata.h

@@ -25,6 +25,12 @@
 #define SPEND_LIST_SIZE 15
 #define SIGNATURE_SIZE 64
 
+// Possible states
+#define STATE_INITIAL 0x00
+#define STATE_PROCESSED_RANDOMNESS 0x01
+#define STATE_SIGNED_SPENDS 0x02
+#define STATE_EXTRACT_SPENDS 0x03
+
 typedef struct {
   uint8_t rcv[RANDOM_LEN];
   uint8_t alpha[RANDOM_LEN];
@@ -53,7 +59,9 @@ typedef struct {
   uint8_t spendlist_len;
   uint8_t outputlist_len;
   uint8_t convertlist_len;
+  uint8_t spends_sign_len;
   uint8_t spends_sign_index;
+  uint8_t state;
 } transaction_header_t;
 
 typedef struct {
@@ -74,3 +82,7 @@ uint8_t transaction_get_n_converts();
 zxerr_t get_next_spend_signature(uint8_t *result);
 zxerr_t spend_signatures_append(uint8_t *signature);
 bool spend_signatures_more_extract();
+
+uint8_t get_state();
+void state_reset();
+void set_state(uint8_t state);

app/src/parser_impl.c

@@ -27,7 +27,9 @@ parser_error_t _read(parser_context_t *ctx, parser_tx_t *v) {
 
     CHECK_ERROR(validateTransactionParams(v))
 
-    CHECK_ERROR(verifyShieldedHash(ctx))
+    if(ctx->tx_obj->transaction.isMasp || ctx->tx_obj->ibc.is_ibc) {
+        CHECK_ERROR(verifyShieldedHash(ctx))
+    }
 
     if (ctx->offset != ctx->bufferLen) {
         return parser_unexpected_unparsed_bytes;

app/src/parser_impl_masp.c

@@ -170,9 +170,11 @@ static parser_error_t readSpendDescriptionInfo(parser_context_t *ctx, masp_sapli
 
     CHECK_ERROR(readUint32(ctx, &builder->n_spends))
 #if defined(LEDGER_SPECIFIC) && !defined(APP_TESTING)
-    uint32_t rnd_spends = (uint32_t)transaction_get_n_spends();
-    if (rnd_spends != builder->n_spends) {
-        return parser_invalid_number_of_spends;
+    if (G_io_apdu_buffer[OFFSET_INS] == INS_SIGN_MASP_SPENDS) {
+        uint32_t rnd_spends = (uint32_t)transaction_get_n_spends();
+        if (rnd_spends < builder->n_spends) {
+            return parser_invalid_number_of_spends;
+        }
     }
 #endif
 
@@ -240,7 +242,7 @@ parser_error_t getNextOutputDescription(parser_context_t *output, uint8_t index)
     for (int i = 0; i < index; i++) {
         uint8_t has_ovk = 0;
         CHECK_ERROR(readByte(output, &has_ovk));
-        CTX_CHECK_AND_ADVANCE(output, (has_ovk ? 32 : 0) + DIVERSIFIER_LEN + PAYMENT_ADDR_LEN + NOTE_LEN + MEMO_LEN);
+        CTX_CHECK_AND_ADVANCE(output, (has_ovk ? 32 : 0) + DIVERSIFIER_LEN + PAYMENT_ADDR_LEN + OUT_NOTE_LEN + MEMO_LEN);
     }
     return parser_ok;
 }
@@ -273,7 +275,7 @@ parser_error_t getNextConvertDescription(parser_context_t *convert, uint8_t inde
             allowed_size = (uint64_t)tag;
         }
         CTX_CHECK_AND_ADVANCE(convert, allowed_size * (ASSET_ID_LEN + INT_128_LEN) + sizeof(uint64_t));
-
+        CTX_CHECK_AND_ADVANCE(convert, 32);
         CHECK_ERROR(readByte(convert, &merkel_size));
         CTX_CHECK_AND_ADVANCE(convert, merkel_size * (32 + 1) + sizeof(uint64_t));
     }
@@ -288,9 +290,11 @@ static parser_error_t readConvertDescriptionInfo(parser_context_t *ctx, masp_sap
 
     CHECK_ERROR(readUint32(ctx, &builder->n_converts))
 #if defined(LEDGER_SPECIFIC) && !defined(APP_TESTING)
-    uint32_t rnd_converts = (uint32_t)transaction_get_n_converts();
-    if (rnd_converts < builder->n_converts) {
-        return parser_invalid_number_of_converts;
+    if (G_io_apdu_buffer[OFFSET_INS] == INS_SIGN_MASP_SPENDS) {
+        uint32_t rnd_converts = (uint32_t)transaction_get_n_converts();
+        if (rnd_converts < builder->n_converts) {
+            return parser_invalid_number_of_converts;
+        }
     }
 #endif
 
@@ -332,6 +336,14 @@ static parser_error_t readSaplingOutputDescriptionInfo(parser_context_t *ctx, ma
     }
 
     CHECK_ERROR(readUint32(ctx, &builder->n_outputs))
+#if defined(LEDGER_SPECIFIC) && !defined(APP_TESTING)
+    if (G_io_apdu_buffer[OFFSET_INS] == INS_SIGN_MASP_SPENDS) {
+        uint32_t rnd_outputs = (uint32_t)transaction_get_n_outputs();
+        if (rnd_outputs < builder->n_outputs) {
+            return parser_invalid_number_of_outputs;
+        }
+    }
+#endif
 
     // Get start pointer and offset to later calculate the size of the outputs
     builder->outputs.ptr = ctx->buffer + ctx->offset;

app/src/parser_impl_masp.h

@@ -26,6 +26,7 @@ extern "C" {
 #define MASPV5_TX_VERSION 0x02
 #define MASPV5_VERSION_GROUP_ID 0x26A7270A
 #define BRANCH_ID_IDENTIFIER 0xE9FF75A6
+#define DEFAULT_IDENTIFIER {156, 229, 191, 54, 209, 138, 169, 235, 234, 174, 120, 186, 142, 34, 183, 118, 64, 243, 100, 134, 234, 27, 248, 27, 36, 245, 9, 146, 30, 110, 203, 169}
 
 parser_error_t readMaspTx(parser_context_t *ctx, masp_tx_section_t *maspTx);
 parser_error_t readMaspBuilder(parser_context_t *ctx, masp_builder_section_t *maspBuilder);

app/src/parser_impl_txn.c

@@ -1432,6 +1432,14 @@ parser_error_t verifyShieldedHash(parser_context_t *ctx) {
             return parser_invalid_target_hash;
         }
     }
+
+    if (ctx->tx_obj->transfer.has_shielded_hash && memcmp(ctx->tx_obj->transfer.shielded_hash.ptr, tx_id_hash, HASH_LEN) != 0) {
+        return parser_invalid_target_hash;
+    }
+
+    if(ctx->tx_obj->ibc.transfer.has_shielded_hash && memcmp(ctx->tx_obj->ibc.transfer.shielded_hash.ptr, tx_id_hash, HASH_LEN) != 0) {
+        return parser_invalid_target_hash;
+    }
 #endif
 
     return parser_ok;

app/src/parser_txdef.h

@@ -27,7 +27,7 @@ extern "C" {
 
 #define MAX_EXTRA_DATA_SECS 4
 #define MAX_SIGNATURE_SECS 3
-
+#define OFFSET_INS 1
 #define ASSET_ID_LEN 32
 #define ANCHOR_LEN 32
 #define SHIELDED_OUTPUTS_LEN 788

app/src/review_keys.c

@@ -115,26 +115,23 @@ zxerr_t getItemViewKey(int8_t displayIdx, char *outKey, uint16_t outKeyLen, char
                      uint8_t *pageCount) {
     ZEMU_LOGF(50, "[addr_getItem] %d/%d\n", displayIdx, pageIdx)
 
+    const KeyData* keyData = (const KeyData*)G_io_apdu_buffer;
     switch (displayIdx) {
         case 0:
             snprintf(outKey, outKeyLen, "ViewKey");
-            const char* viewKey = (const char*)G_io_apdu_buffer;
-            pageStringHex(outVal, outValLen, viewKey, 2 * KEY_LENGTH, pageIdx, pageCount);
+            pageStringHex(outVal, outValLen, keyData->viewKey, 2 * KEY_LENGTH, pageIdx, pageCount);
             break;
         case 1:
             snprintf(outKey, outKeyLen, "IVK");
-            const char* ivk = (const char*)G_io_apdu_buffer + 3 * KEY_LENGTH;
-            pageStringHex(outVal, outValLen, ivk, KEY_LENGTH, pageIdx, pageCount);
+            pageStringHex(outVal, outValLen, keyData->ivk, KEY_LENGTH, pageIdx, pageCount);
             break;
         case 2:
             snprintf(outKey, outKeyLen, "OVK");
-            const char* ovk = (const char*)G_io_apdu_buffer + 2 * KEY_LENGTH;
-            pageStringHex(outVal, outValLen, ovk, KEY_LENGTH, pageIdx, pageCount);
+            pageStringHex(outVal, outValLen, keyData->ovk, KEY_LENGTH, pageIdx, pageCount);
             break;
         case 3:
             snprintf(outKey, outKeyLen, "DK");
-            const char* dk = (const char*)G_io_apdu_buffer + 4 * KEY_LENGTH;
-            pageStringHex(outVal, outValLen, dk, KEY_LENGTH, pageIdx, pageCount);
+            pageStringHex(outVal, outValLen, keyData->dk, KEY_LENGTH, pageIdx, pageCount);
             break;
         case 4: {
             snprintf(outKey, outKeyLen, "HD Path");