Merge pull request #88 from Zondax/dev

Fixes
Zondax · Dec 18, 2023 · 1b3c046 · 1b3c046
2 parents 75a5d21 + 7e45056
commit 1b3c046
Show file tree

Hide file tree

Showing 25 changed files with 140 additions and 263 deletions.
diff --git a/app/Makefile.version b/app/Makefile.version
@@ -3,4 +3,4 @@ APPVERSION_M=3
 # This is the minor version of this release
 APPVERSION_N=3
 # This is the patch version of this release
-APPVERSION_P=1
+APPVERSION_P=2
diff --git a/app/src/apdu_handler.c b/app/src/apdu_handler.c
@@ -82,12 +82,6 @@ __Z_INLINE void extractHDPath(uint32_t rx, uint32_t offset) {
 __Z_INLINE bool process_chunk(__Z_UNUSED volatile uint32_t *tx, uint32_t rx) {
   const uint8_t payloadType = G_io_apdu_buffer[OFFSET_PAYLOAD_TYPE];
 
-  /*
-  if (G_io_apdu_buffer[OFFSET_P2] != 0 && G_io_apdu_buffer[OFFSET_P2] != 4 &&
-  G_io_apdu_buffer[OFFSET_P2] != 5 ) { THROW(APDU_CODE_INVALIDP1P2);
-  }
-   */
-
   if (rx < OFFSET_DATA) {
     THROW(APDU_CODE_WRONG_LENGTH);
   }
@@ -616,16 +610,14 @@ __Z_INLINE void handleGetAddrSaplingDiv(volatile uint32_t *flags,
   parser_addr_div_t parser_addr;
   MEMZERO(&parser_addr, sizeof(parser_addr_div_t));
 
-  parser_error_t prserr = parser_sapling_path_with_div(
-      G_io_apdu_buffer + OFFSET_DATA, DATA_LENGTH_GET_ADDR_DIV, &parser_addr);
+  parser_error_t parseErr = parser_sapling_path_with_div(G_io_apdu_buffer + OFFSET_DATA, DATA_LENGTH_GET_ADDR_DIV, &parser_addr);
   MEMZERO(G_io_apdu_buffer, IO_APDU_BUFFER_SIZE);
-  if (prserr != parser_ok) {
+  if (parseErr != parser_ok) {
     *tx = 0;
     THROW(APDU_CODE_DATA_INVALID);
   }
-  zxerr_t err = crypto_fillAddress_with_diversifier_sapling(
-      G_io_apdu_buffer, IO_APDU_BUFFER_SIZE - 3, parser_addr.path,
-      parser_addr.div, &replyLen);
+  zxerr_t err = crypto_fillAddress_with_diversifier_sapling(G_io_apdu_buffer, IO_APDU_BUFFER_SIZE - 3, parser_addr.path,
+                                                            parser_addr.div, &replyLen);
   if (err != zxerr_ok) {
     *tx = 0;
     THROW(APDU_CODE_DATA_INVALID);
@@ -708,8 +700,7 @@ __Z_INLINE void handleGetAddrSapling(volatile uint32_t *flags,
   uint8_t requireConfirmation = G_io_apdu_buffer[OFFSET_P1];
 
   uint32_t zip32path = 0;
-  parser_error_t prserr = parser_sapling_path(
-      G_io_apdu_buffer + OFFSET_DATA, DATA_LENGTH_GET_ADDR_SAPLING, &zip32path);
+  parser_error_t prserr = parser_sapling_path(G_io_apdu_buffer + OFFSET_DATA, DATA_LENGTH_GET_ADDR_SAPLING, &zip32path);
   MEMZERO(G_io_apdu_buffer, IO_APDU_BUFFER_SIZE);
   if (prserr != parser_ok) {
     *tx = 0;
@@ -738,49 +729,6 @@ __Z_INLINE void handleGetAddrSapling(volatile uint32_t *flags,
 
 __Z_INLINE void handleSignSapling() { THROW(APDU_CODE_COMMAND_NOT_ALLOWED); }
 
-#if defined(APP_TESTING)
-#include "cx.h"
-#include "jubjub.h"
-#include "rslib.h"
-#include <zxmacros.h>
-
-void handleTest(volatile uint32_t *tx) {
-
-  uint8_t point[32] = {48,  181, 242, 170, 173, 50,  86,  48, 188, 221, 219,
-                       206, 77,  103, 101, 109, 5,   253, 28, 194, 208, 55,
-                       187, 83,  117, 182, 233, 109, 158, 1,  161, 215};
-
-  uint8_t scalar[32] = {
-      0x66, 0x5e, 0xd6, 0xf7, 0xb7, 0x93, 0xaf, 0xa1, 0x82, 0x21, 0xe1,
-      0x57, 0xba, 0xd5, 0x43, 0x3c, 0x54, 0x23, 0xf4, 0xfe, 0xc9, 0x46,
-      0xe0, 0x8e, 0xd6, 0x30, 0xa0, 0xc6, 0x0a, 0x1f, 0xac, 0x02,
-  };
-
-  jubjub_extendedpoint p;
-  jubjub_fq scal;
-  if (jubjub_field_frombytes(scal, scalar) != zxerr_ok) {
-    *tx = 0;
-    MEMZERO(point, sizeof(point));
-    THROW(APDU_CODE_OK);
-  }
-
-  jubjub_extendedpoint_tobytes(point, JUBJUB_GEN);
-  const zxerr_t err = jubjub_extendedpoint_frombytes(&p, point);
-  if (err != zxerr_ok) {
-    *tx = 0;
-    MEMZERO(point, sizeof(point));
-    THROW(APDU_CODE_OK);
-  }
-  // MEMCPY(&p, &JUBJUB_GEN, 32);
-  // jubjub_extendedpoint_scalarmult(&p, scal);
-  jubjub_extendedpoint_tobytes(point, p);
-
-  MEMCPY(G_io_apdu_buffer, point, 32);
-  *tx = 32;
-  THROW(APDU_CODE_OK);
-}
-#endif
-
 void handleApdu(volatile uint32_t *flags, volatile uint32_t *tx, uint32_t rx) {
   uint16_t sw = 0;
 
@@ -891,18 +839,6 @@ void handleApdu(volatile uint32_t *flags, volatile uint32_t *tx, uint32_t rx) {
         break;
       }
 
-#if defined(APP_TESTING)
-      case INS_TEST: {
-        handleTest(tx);
-        /*
-        G_io_apdu_buffer[0] = 0xCA;
-        G_io_apdu_buffer[1] = 0xFE;
-        *tx = 3;
-        */
-        THROW(APDU_CODE_OK);
-        break;
-      }
-#endif
       default:
         THROW(APDU_CODE_INS_NOT_SUPPORTED);
       }

diff --git a/app/src/c_api/rust.c b/app/src/c_api/rust.c
@@ -174,97 +174,3 @@ void c_jubjub_spending_base_scalarmult(uint8_t *point, const uint8_t *scalar) {
     MEMZERO(point, JUBJUB_FIELD_BYTES);
   }
 }
-
-// Replace functions affected by non-constant time opcodes
-// Overriding requires -z muldefs
-// FIXME: add a python script to ensure that the correct version is used by
-// inspecting app.asm
-#if ENABLE_SDK_MULT
-#define SWAP_BYTES(x, y, tmp)                                                  \
-  {                                                                            \
-    tmp = x;                                                                   \
-    x = y;                                                                     \
-    y = tmp;                                                                   \
-  }
-
-#define SWAP_ENDIAN(x, tmp)                                                    \
-  {                                                                            \
-    SWAP_BYTES(*x, *(x + 7), tmp);                                             \
-    SWAP_BYTES(*(x + 1), *(x + 6), tmp);                                       \
-    SWAP_BYTES(*(x + 2), *(x + 5), tmp);                                       \
-    SWAP_BYTES(*(x + 3), *(x + 4), tmp);                                       \
-  }
-
-#define LMULSIZE sizeof(long long)
-
-long long __attribute__((noinline)) __aeabi_lmul(long long a, long long b) {
-  uint8_t tmp = 0;
-  uint8_t *ptr = (uint8_t *)&a;
-
-  SWAP_ENDIAN(ptr, tmp);
-
-  ptr = (uint8_t *)&b;
-  SWAP_ENDIAN(ptr, tmp);
-
-  char result[2 * LMULSIZE];
-  MEMZERO(result, 2 * LMULSIZE);
-  cx_math_mult((unsigned char *)&result, (unsigned char *)&a,
-               (unsigned char *)&b, LMULSIZE);
-
-  SWAP_ENDIAN(ptr, tmp);
-  ptr = (uint8_t *)&a;
-  SWAP_ENDIAN(ptr, tmp);
-
-  ptr = (uint8_t *)&result[LMULSIZE];
-  SWAP_ENDIAN(ptr, tmp);
-  return *((long long *)ptr);
-}
-#endif
-/*
-long long __multi3(long long a, long long b) {
-    long long a_be = ((a & 0xff) << 56)
-                     | ((a >> 8 & 0xff) << 48)
-                     | ((a >> 16 & 0xff) << 40)
-                     | ((a >> 24 & 0xff) << 32)
-                     | ((a >> 32 & 0xff) << 24)
-                     | ((a >> 40 & 0xff) << 16)
-                     | ((a >> 48 & 0xff) << 8)
-                     | ((a >> 56 & 0xff) << 0);
-
-    long long b_be = ((b & 0xff) << 56)
-                     | ((b >> 8 & 0xff) << 48)
-                     | ((b >> 16 & 0xff) << 40)
-                     | ((b >> 24 & 0xff) << 32)
-                     | ((b >> 32 & 0xff) << 24)
-                     | ((b >> 40 & 0xff) << 16)
-                     | ((b >> 48 & 0xff) << 8)
-                     | ((b >> 56 & 0xff) << 0);
-
-
-    char result[2 * LMULSIZE];
-    MEMZERO(result, 2 * LMULSIZE);
-    cx_math_mult((unsigned char *) &result, (unsigned char *) &a_be, (unsigned
-char *) &b_be, LMULSIZE);
-
-    long long r_le = 0;
-    r_le |= result[LMULSIZE]; r_le <<= 8;
-    r_le |= result[LMULSIZE + 1]; r_le <<= 8;
-    r_le |= result[LMULSIZE + 2]; r_le <<= 8;
-    r_le |= result[LMULSIZE + 3]; r_le <<= 8;
-    r_le |= result[LMULSIZE + 4]; r_le <<= 8;
-    r_le |= result[LMULSIZE + 5]; r_le <<= 8;
-    r_le |= result[LMULSIZE + 6]; r_le <<= 8;
-    r_le |= result[LMULSIZE + 7];
-    return r_le;
-}
- */
-
-// typedef struct {
-//     unsigned quot;
-//     unsigned rem;
-// } __aeabi_uidivmod_result_t;
-//
-//// FIXME: Complete implementation, redirect and enable
-////__aeabi_uidivmod_result_t __aeabi_uidivmod(unsigned numerator, unsigned
-///denominator) { /    __aeabi_uidivmod_result_t r; /    return r;
-////}
diff --git a/app/src/coin.h b/app/src/coin.h
@@ -95,7 +95,7 @@ typedef enum { key_ivk = 0, key_ovk = 1, key_fvk = 2, nf = 3 } key_type_e;
 #define MENU_MAIN_APP_LINE2_SECRET "?"
 #define COIN_SECRET_REQUIRED_CLICKS 0
 
-#define COIN_AMOUNT_DECIMAL_PLACES 18 // FIXME: Check this
+#define COIN_AMOUNT_DECIMAL_PLACES 18
 #define CRYPTO_BLOB_SKIP_BYTES 0
 
 #ifdef __cplusplus

diff --git a/app/src/common/tx.c b/app/src/common/tx.c
@@ -90,7 +90,7 @@ zxerr_t tx_getItem(int8_t displayIdx, char *outKey, uint16_t outKeyLen,
 
   CHECK_ZXERR(tx_getNumItems(&numItems))
 
-  if (displayIdx < 0 || displayIdx > numItems) {
+  if (displayIdx < 0 || displayIdx >= numItems) {
     return zxerr_no_data;
   }
 
@@ -99,8 +99,9 @@ zxerr_t tx_getItem(int8_t displayIdx, char *outKey, uint16_t outKeyLen,
 
   // Convert error codes
   if (err == parser_no_data || err == parser_display_idx_out_of_range ||
-      err == parser_display_page_out_of_range)
+      err == parser_display_page_out_of_range) {
     return zxerr_no_data;
+  }
 
   if (err != parser_ok)
     return zxerr_unknown;

diff --git a/app/src/crypto.c b/app/src/crypto.c
@@ -122,25 +122,12 @@ static zxerr_t crypto_extractPublicKey(uint8_t *pubKey, uint16_t pubKeyLen) {
   uint8_t privateKeyData[64] = {0};
 
   zxerr_t error = zxerr_unknown;
-  CATCH_CXERROR(os_derive_bip32_no_throw(
-      CX_CURVE_256K1, hdPath, HDPATH_LEN_DEFAULT, privateKeyData, NULL))
+  CATCH_CXERROR(os_derive_bip32_no_throw(CX_CURVE_256K1, hdPath, HDPATH_LEN_DEFAULT, privateKeyData, NULL));
+  CATCH_CXERROR(cx_ecfp_init_private_key_no_throw(CX_CURVE_256K1, privateKeyData, SK_SECP256K1_SIZE, &cx_privateKey));
+  CATCH_CXERROR(cx_ecfp_init_public_key_no_throw(CX_CURVE_256K1, NULL, 0, &cx_publicKey));
+  CATCH_CXERROR(cx_ecfp_generate_pair_no_throw(CX_CURVE_256K1, &cx_publicKey, &cx_privateKey, 1));
 
-  CATCH_CXERROR(cx_ecfp_init_private_key_no_throw(
-      CX_CURVE_256K1, privateKeyData, SK_SECP256K1_SIZE, &cx_privateKey))
-  CATCH_CXERROR(
-      cx_ecfp_init_public_key_no_throw(CX_CURVE_256K1, NULL, 0, &cx_publicKey))
-  CATCH_CXERROR(cx_ecfp_generate_pair_no_throw(CX_CURVE_256K1, &cx_publicKey,
-                                               &cx_privateKey, 1))
-
-  // Format pubkey
-  for (int i = 0; i < PUB_KEY_SIZE; i++) {
-    pubKey[i] = cx_publicKey.W[64 - i];
-  }
-  cx_publicKey.W[0] =
-      cx_publicKey.W[64] & 1 ? 0x03 : 0x02; // "Compress" public key in place
-  if ((cx_publicKey.W[PUB_KEY_SIZE] & 1) != 0) {
-    pubKey[PUB_KEY_SIZE - 1] |= 0x80;
-  }
+  cx_publicKey.W[0] = cx_publicKey.W[64] & 1 ? 0x03 : 0x02; // "Compress" public key in place
   memcpy(pubKey, cx_publicKey.W, PK_LEN_SECP256K1);
   error = zxerr_ok;
 
@@ -1232,7 +1219,7 @@ zxerr_t crypto_sign_and_check_transparent(uint8_t *buffer, uint16_t bufferLen,
     size_t signatureLen = DER_MAX_SIZE;
     CATCH_CXERROR(cx_ecdsa_sign_no_throw(
         &cx_privateKey, CX_RND_RFC6979 | CX_LAST, CX_SHA256, message_digest,
-        CX_SHA256_SIZE, signature->step1.der_signature, &signatureLen, &info))
+        CX_SHA256_SIZE, signature->step1.der_signature, &signatureLen, &info));
 
     if (convertDERtoRSV(signature->step1.der_signature, info,
                         signature->step1.r, signature->step1.s,
@@ -1556,6 +1543,7 @@ zxerr_t crypto_fillAddress_with_diversifier_sapling(uint8_t *buffer,
 
   uint8_t zip32_seed[ZIP32_SEED_SIZE] = {0};
 
+  // Initialize diversifier
   MEMCPY(out->diversifier, div, DIV_SIZE);
   if (!is_valid_diversifier(out->diversifier)) {
     return zxerr_unknown;
@@ -1569,11 +1557,15 @@ zxerr_t crypto_fillAddress_with_diversifier_sapling(uint8_t *buffer,
   }
   CHECK_APP_CANARY()
 
+  // Initialize pkd
   get_pkd(zip32_seed, p, out->diversifier, out->pkd);
   CHECK_APP_CANARY()
 
   MEMZERO(zip32_seed, sizeof(zip32_seed));
 
+  // To simplify the code and avoid making copies, read the 'address_raw' variable.
+  // This variable completely overlaps with the 'diversifier' and 'pkd' fields.
+  // Therefore, using 'address_raw' is equivalent to have [diversifier(11) | pkd(32)]
   if (bech32EncodeFromBytes(out->address_bech32,
                             sizeof_field(tmp_buf_addr_s, address_bech32),
                             BECH32_HRP, out->address_raw,

diff --git a/app/src/jubjub.c b/app/src/jubjub.c
@@ -1,5 +1,5 @@
 /*******************************************************************************
- *   (c) 2021 Zondax AG
+ *   (c) 2018 - 2023 Zondax AG
  *
  *  Licensed under the Apache License, Version 2.0 (the "License");
  *  you may not use this file except in compliance with the License.
@@ -184,7 +184,7 @@ zxerr_t jubjub_field_sqrt(jubjub_fq r, const jubjub_fq a) {
   CHECK_ZXERR(jubjub_field_mult(b, x, w));
   jubjub_field_copy(z, JUBJUB_FQ_ROOT_OF_UNITY);
   jubjub_fq tmp;
-  // uint8_t index = 0;
+
   for (uint8_t max_v = 32; max_v >= 1; max_v--) {
     uint8_t k = 1;
     CHECK_ZXERR(jubjub_field_square(tmp, b));
@@ -238,9 +238,7 @@ static zxerr_t jubjub_extendedpoint_add(jubjub_extendedpoint *r, const jubjub_ex
   if (r == NULL || p == NULL) {
     return zxerr_no_data;
   }
-  // jubjub_extendedpoint np;
-  // jubjub_extendedpoint_normalize(&np, p);
-  // extendednielspoint
+
   jubjub_fq v_minus_u, v_plus_u, t2d;
 
   CHECK_ZXERR(jubjub_field_add(v_plus_u, p->V, p->U));

diff --git a/app/src/jubjub.h b/app/src/jubjub.h
@@ -14,6 +14,7 @@
  *  limitations under the License.
  ********************************************************************************/
 #include <zxerror.h>
+#include <stdint.h>
 
 #define JUBJUB_SCALAR_BYTES 32
 #define JUBJUB_FIELD_BYTES 32

diff --git a/app/src/nvdata.c b/app/src/nvdata.c
@@ -130,12 +130,11 @@ zxerr_t transparent_signatures_append(uint8_t *signature) {
 }
 
 zxerr_t get_next_transparent_signature(uint8_t *result) {
-  uint8_t index = transaction_header.t_in_len - transaction_header.t_sign_index;
-  if (transaction_header.t_in_len <= index || index < 0) {
+  const uint8_t index = transaction_header.t_in_len - transaction_header.t_sign_index;
+  if (index >= transaction_header.t_in_len) {
     return zxerr_unknown;
   }
-  MEMCPY(result, (void *)&N_transactioninfo.transparent_signatures[index],
-         SIGNATURE_SIZE);
+  MEMCPY(result, (void *)&N_transactioninfo.transparent_signatures[index], SIGNATURE_SIZE);
   transaction_header.t_sign_index--;
   if (!transparent_signatures_more_extract() &&
       !spend_signatures_more_extract()) {
@@ -163,13 +162,11 @@ zxerr_t spend_signatures_append(uint8_t *signature) {
 }
 
 zxerr_t get_next_spend_signature(uint8_t *result) {
-  uint8_t index =
-      transaction_header.spendlist_len - transaction_header.spends_sign_index;
-  if (transaction_header.spendlist_len <= index || index < 0) {
+  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);
+  MEMCPY(result, (void *)&N_transactioninfo.spend_signatures[index], SIGNATURE_SIZE);
   transaction_header.spends_sign_index--;
   if (!transparent_signatures_more_extract() &&
       !spend_signatures_more_extract()) {