chore: sync with LedgerHQ/app-cosmos v2.39.3

.gitmodules

@@ -1,9 +1,6 @@
 [submodule "deps/ledger-zxlib"]
 	path = deps/ledger-zxlib
 	url = https://github.com/Zondax/ledger-zxlib.git
-[submodule "deps/ledger-secure-sdk"]
-	path = deps/ledger-secure-sdk
-	url = https://github.com/LedgerHQ/ledger-secure-sdk
 [submodule "js"]
 	path = js
 	url = https://github.com/Zondax/ledger-cosmos-js

app/Makefile

@@ -65,7 +65,7 @@ CURVE_APP_LOAD_PARAMS = secp256k1
 # If your app needs it, you can specify multiple path by using:
 # `PATH_APP_LOAD_PARAMS = "44'/1'" "45'/1'"`
 
-APPPATH =  "44'/118'" "44'/60'"
+APPPATH =  "44'/118'" "44'/60'" "44'/1200'"
 $(info PATHS LIST = $(APPPATH))
 PATH_APP_LOAD_PARAMS = $(APPPATH)
 

app/Makefile.version

@@ -3,4 +3,4 @@ APPVERSION_M=2
 # This is the `spec_version` field of `Runtime`
 APPVERSION_N=39
 # This is the patch version of this release
-APPVERSION_P=1
+APPVERSION_P=3

app/src/addr.c

@@ -16,6 +16,7 @@
 
 #include "app_mode.h"
 #include "coin.h"
+#include "common/actions.h"
 #include "crypto.h"
 #include "zxerror.h"
 #include "zxformat.h"
@@ -51,8 +52,10 @@ zxerr_t addr_getItem(int8_t displayIdx, char *outKey, uint16_t outKeyLen,
   switch (displayIdx) {
   case 0:
     snprintf(outKey, outKeyLen, "Address");
+    // From app RAM, not G_io_apdu_buffer: the buffer holds whatever APDU
+    // arrived last, and one can arrive while this review is on screen.
     pageString(outVal, outValLen,
-               (char *)(G_io_apdu_buffer + VIEW_ADDRESS_OFFSET_SECP256K1),
+               (char *)(action_addrResponse + VIEW_ADDRESS_OFFSET_SECP256K1),
                pageIdx, pageCount);
     ZEMU_LOGF(200, "[addr_getItem] pageCount %d\n", *pageCount)
     return zxerr_ok;

app/src/apdu_handler.c

@@ -108,9 +108,7 @@ __Z_INLINE void extractHDPath(uint32_t rx, uint32_t offset) {
          sizeof(uint32_t) * HDPATH_LEN_DEFAULT);
 
   // Check values
-  if (hdPath[0] != HDPATH_0_DEFAULT ||
-      ((hdPath[1] != HDPATH_1_DEFAULT) &&
-       (hdPath[1] != HDPATH_ETH_1_DEFAULT)) ||
+  if (hdPath[0] != HDPATH_0_DEFAULT || !isSupportedCoinType(hdPath[1]) ||
       hdPath[3] != HDPATH_3_DEFAULT) {
     THROW(APDU_CODE_INVALID_HD_PATH_COIN_VALUE);
   }
@@ -139,7 +137,11 @@ static void extractHDPath_HRP(uint32_t rx, uint32_t offset) {
       ZEMU_LOGF(50, "Chain config not supported for: %s\n", bech32_hrp)
       THROW(APDU_CODE_CHAIN_CONFIG_NOT_SUPPORTED);
     }
-  } else if (hdPath[1] == HDPATH_ETH_1_DEFAULT) {
+  } else if (hdPath[1] != HDPATH_1_DEFAULT) {
+    // Only the generic Cosmos path (118) has a meaningful default HRP.
+    // Falling through on any other coin type would derive a key on that
+    // chain's path while defaulting the review screen to a cosmos1...
+    // address -- a silent mismatch on the one screen the user checks.
     THROW(APDU_CODE_INVALID_HD_PATH_COIN_VALUE);
   } else {
     // No HRP was provided on the default Cosmos path. Restore the default
@@ -230,6 +232,7 @@ __Z_INLINE void handleGetAddrSecp256K1(volatile uint32_t *flags,
     return;
   }
 
+  app_load_address_response();
   *tx = action_addrResponseLen;
   THROW(APDU_CODE_OK);
 }
@@ -262,8 +265,9 @@ __Z_INLINE void handleSign(volatile uint32_t *flags, volatile uint32_t *tx,
     THROW(APDU_CODE_DATA_INVALID);
   }
 
+  // Held for the whole review, so it has to outlive the APDU buffer.
   parser_tx_obj.tx_json.own_addr =
-      (const char *)(G_io_apdu_buffer + VIEW_ADDRESS_OFFSET_SECP256K1);
+      (const char *)(action_addrResponse + VIEW_ADDRESS_OFFSET_SECP256K1);
   parser_tx_obj.tx_json.own_addr_len =
       action_addrResponseLen - PK_LEN_SECP256K1;
   const char *error_msg = tx_parse(sign_type);
@@ -300,6 +304,21 @@ void handleApdu(volatile uint32_t *flags, volatile uint32_t *tx, uint32_t rx) {
 
   BEGIN_TRY {
     TRY {
+      // A screen owns the device: a review the user is reading, or an error
+      // modal that still owes the host a reply. io_exchange hands over the
+      // next APDU as soon as it arrives, which for a handler that returned
+      // IO_ASYNCH_REPLY is while the user is still deciding -- so a request
+      // answered here would replace the derivation path, the address encoding
+      // or the parsed transaction behind what is on screen, or draw a second
+      // review over the first and take the press meant to dismiss one as
+      // approval of the other. Refuse until the screen has been answered.
+      //
+      // COMMAND_NOT_ALLOWED on purpose: the CATCH_OTHER below leaves the
+      // in-flight request's state alone for exactly this status word.
+      if (view_review_is_pending()) {
+        THROW(APDU_CODE_COMMAND_NOT_ALLOWED);
+      }
+
       if (G_io_apdu_buffer[OFFSET_CLA] != CLA) {
         THROW(APDU_CODE_CLA_NOT_SUPPORTED);
       }

app/src/cbor/cbor_parser_helper.c

@@ -34,19 +34,32 @@ parser_error_t parser_mapCborError(CborError err) {
   }
 }
 
-static parser_error_t cbor_check_optFields(CborValue *data,
-                                           Cbor_container *container) {
+// prevKey is the last key already consumed from this screen's map, or 0 when
+// the walk starts at the first key. Callers differ: the parse pass walks the
+// whole map from the beginning, the display pass resumes after the content key.
+static parser_error_t
+cbor_check_optFields(CborValue *data, Cbor_container *container, int prevKey) {
   if (data == NULL || container == NULL) {
     return parser_unexpected_value;
   }
   int key;
+  // Every key has to be greater than the one before it. Requiring the increase
+  // is what makes a key single-valued: without it,
+  // { 1: "Safe", 2: "Safe", 1: "Evil" } was accepted, the review took the title
+  // and content from the first two pairs and stepped over the repeat, and the
+  // screen read "Safe" while the signature also covered "Evil". A map whose
+  // keys ascend cannot say two things about one field. ADR-050 requires that
+  // ordering anyway, so a document that breaks it is refused rather than
+  // resolved by whichever value the walk happened to reach first.
   for (size_t i = 0; i < container->n_field; i++) {
 
     PARSER_ASSERT_OR_ERROR(cbor_value_is_integer(data), parser_unexpected_type)
     // Checked variant: the plain getter truncates anything wider than an int
     // instead of failing, so an oversized encoded value could land on one of
     // the key ids below and be dispatched as that field.
     CHECK_CBOR_MAP_ERR(cbor_value_get_int_checked(data, &key))
+    PARSER_ASSERT_OR_ERROR(key > prevKey, parser_unexpected_field)
+    prevKey = key;
     CHECK_CBOR_MAP_ERR(cbor_value_advance(data))
 
     switch (key) {
@@ -154,7 +167,7 @@ parser_error_t cbor_get_containerInfo(CborValue *data,
 
   if (container->n_field > 2) {
     container->n_field -= 2;
-    CHECK_PARSER_ERR(cbor_check_optFields(data, container))
+    CHECK_PARSER_ERR(cbor_check_optFields(data, container, CONTENT_KEY_ID))
   } else {
     container->screen.indent = 0;
     container->screen.expert = false;
@@ -169,7 +182,7 @@ parser_error_t cbor_check_expert(CborValue *data, Cbor_container *container) {
   }
 
   if (container->n_field > 1) {
-    CHECK_PARSER_ERR(cbor_check_optFields(data, container))
+    CHECK_PARSER_ERR(cbor_check_optFields(data, container, 0))
   } else {
     container->screen.indent = 0;
     container->screen.expert = false;

app/src/chain_config.c

@@ -24,7 +24,8 @@ typedef struct {
 } chain_config_t;
 
 // To enable custom config for a new chain, just add a new entry in this array
-// with path, hrp and encoding
+// with path, hrp and encoding. Declaring a coin type here also makes it
+// derivable -- see isSupportedCoinType.
 static const chain_config_t chainConfig[] = {
     {118, "cosmos", BECH32_COSMOS},   {60, "inj", BECH32_ETH},
     {60, "evmos", BECH32_ETH},        {60, "xpla", BECH32_ETH},
@@ -33,11 +34,25 @@ static const chain_config_t chainConfig[] = {
     {118, "osmo", BECH32_COSMOS},     {118, "dydx", BECH32_COSMOS},
     {118, "mantra", BECH32_COSMOS},   {118, "xion", BECH32_COSMOS},
     {118, "celestia", BECH32_COSMOS}, {118, "core", BECH32_COSMOS},
-    {118, "neutron", BECH32_COSMOS}};
+    {118, "neutron", BECH32_COSMOS},  {1200, "gonka", BECH32_COSMOS}};
 
 static const uint32_t chainConfigLen =
     sizeof(chainConfig) / sizeof(chainConfig[0]);
 
+bool isSupportedCoinType(uint32_t path) {
+  if (path == HDPATH_1_DEFAULT || path == HDPATH_ETH_1_DEFAULT) {
+    return true;
+  }
+
+  for (uint32_t i = 0; i < chainConfigLen; i++) {
+    if (path == (0x80000000u | chainConfig[i].path)) {
+      return true;
+    }
+  }
+
+  return false;
+}
+
 address_encoding_e checkChainConfig(uint32_t path, const char *hrp,
                                     uint8_t hrpLen) {
   if (hrp == NULL || hrpLen == 0) {

app/src/chain_config.h

@@ -26,6 +26,10 @@ extern "C" {
 address_encoding_e checkChainConfig(uint32_t path, const char *hrp,
                                     uint8_t hrpLen);
 
+// `path` is the hardened coin type element of the derivation path (hdPath[1]),
+// as taken by checkChainConfig.
+bool isSupportedCoinType(uint32_t path);
+
 #ifdef __cplusplus
 }
 #endif

app/src/common/actions.c

@@ -16,4 +16,5 @@
 
 #include "actions.h"
 
+uint8_t action_addrResponse[ACTION_ADDR_RESPONSE_SIZE];
 uint16_t action_addrResponseLen;

app/src/common/actions.h

@@ -30,6 +30,24 @@ typedef enum {
 } tx_state_e;
 
 extern tx_state_e g_tx_state;
+
+// The answer to a GetAddr request, kept in app RAM rather than in
+// G_io_apdu_buffer.
+//
+// G_io_apdu_buffer is where the next incoming APDU lands, and app_main's
+// io_exchange hands one over as soon as it arrives -- which, once a review is
+// on screen and the handler has returned IO_ASYNCH_REPLY, is while the user is
+// still reading. An address paged out of that buffer at draw time, or replied
+// out of it at approve time, is then whatever the host sent last instead of
+// what the user was shown and agreed to.
+//
+// Sized for the longest response crypto_fillAddress can produce: the
+// compressed public key, then the address text -- bech32 with the longest HRP
+// the app accepts, or the hex form the ETH encoding uses -- and a terminator.
+#define ACTION_ADDR_RESPONSE_SIZE                                              \
+  (PK_LEN_SECP256K1 + MAX_BECH32_HRP_LEN + MIN_ADDRESS_BUFFER_SPACE)
+
+extern uint8_t action_addrResponse[ACTION_ADDR_RESPONSE_SIZE];
 extern uint16_t action_addrResponseLen;
 
 __Z_INLINE void app_sign() {
@@ -51,20 +69,31 @@ __Z_INLINE void app_sign() {
 }
 
 __Z_INLINE zxerr_t app_fill_address() {
-  // Put data directly in the apdu buffer
-  MEMZERO(G_io_apdu_buffer, IO_APDU_BUFFER_SIZE);
+  // Into app RAM, so the address the review draws and the address the reply
+  // carries are the ones this request derived (see action_addrResponse).
+  MEMZERO(action_addrResponse, sizeof(action_addrResponse));
 
   action_addrResponseLen = 0;
-  const zxerr_t err = crypto_fillAddress(
-      G_io_apdu_buffer, IO_APDU_BUFFER_SIZE - 2, &action_addrResponseLen);
+  const zxerr_t err =
+      crypto_fillAddress(action_addrResponse, sizeof(action_addrResponse),
+                         &action_addrResponseLen);
 
   if (err != zxerr_ok || action_addrResponseLen == 0) {
+    MEMZERO(action_addrResponse, sizeof(action_addrResponse));
+    action_addrResponseLen = 0;
     THROW(APDU_CODE_EXECUTION_ERROR);
   }
 
   return zxerr_ok;
 }
 
+// Copies the held answer into the APDU buffer, which by now may hold an
+// unrelated request the host sent while the review was up.
+__Z_INLINE void app_load_address_response() {
+  MEMZERO(G_io_apdu_buffer, IO_APDU_BUFFER_SIZE);
+  MEMCPY(G_io_apdu_buffer, action_addrResponse, action_addrResponseLen);
+}
+
 __Z_INLINE void app_reject() {
   g_tx_state = TX_STATE_IDLE;
   MEMZERO(G_io_apdu_buffer, IO_APDU_BUFFER_SIZE);
@@ -74,6 +103,7 @@ __Z_INLINE void app_reject() {
 
 __Z_INLINE void app_reply_address() {
   g_tx_state = TX_STATE_IDLE;
+  app_load_address_response();
   set_code(G_io_apdu_buffer, action_addrResponseLen, APDU_CODE_OK);
   io_exchange(CHANNEL_APDU | IO_RETURN_AFTER_TX, action_addrResponseLen + 2);
 }

app/src/parser.c

@@ -184,6 +184,56 @@ __Z_INLINE bool parser_isAmount(char *key) {
   return false;
 }
 
+// An sdk.Int in its JSON form: decimal digits only. No sign, no separator, no
+// exponent, no leading or trailing space. A single leading zero is only valid
+// when the amount is exactly "0".
+__Z_INLINE parser_error_t coin_amount_is_valid(const char *amount,
+                                               uint16_t amountLen) {
+  if (amount == NULL || amountLen == 0) {
+    return parser_unexpected_value;
+  }
+
+  for (uint16_t i = 0; i < amountLen; i++) {
+    if (amount[i] < '0' || amount[i] > '9') {
+      return parser_unexpected_characters;
+    }
+  }
+
+  if (amountLen > 1 && amount[0] == '0') {
+    return parser_unexpected_value;
+  }
+
+  return parser_ok;
+}
+
+// The Cosmos SDK denom regex: [a-zA-Z][a-zA-Z0-9/:._-]{2,127}. Enforcing it
+// here keeps a denom that the chain would reject from reaching the screen as if
+// it were spendable, and refuses the near-misses of the unit the app shifts by
+// six decimals -- "uatom " with a trailing space, or a padded look-alike.
+__Z_INLINE parser_error_t coin_denom_is_valid(const char *denom,
+                                              uint16_t denomLen) {
+  if (denom == NULL || denomLen < 3 || denomLen > 128) {
+    return parser_unexpected_value;
+  }
+
+  const char first = denom[0];
+  if (!((first >= 'a' && first <= 'z') || (first >= 'A' && first <= 'Z'))) {
+    return parser_unexpected_characters;
+  }
+
+  for (uint16_t i = 1; i < denomLen; i++) {
+    const char c = denom[i];
+    const bool allowed = (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') ||
+                         (c >= '0' && c <= '9') || c == '/' || c == ':' ||
+                         c == '.' || c == '_' || c == '-';
+    if (!allowed) {
+      return parser_unexpected_characters;
+    }
+  }
+
+  return parser_ok;
+}
+
 __Z_INLINE parser_error_t is_default_denom_base(const char *denom,
                                                 uint8_t denom_len,
                                                 bool *is_default) {
@@ -298,6 +348,15 @@ __Z_INLINE parser_error_t parser_formatAmountItem(uint16_t amountToken,
     return parser_unexpected_error;
   }
 
+  // Length was the only thing checked here, so anything that fit was rendered
+  // as if it were a coin. A Coin's Amount is an sdk.Int in decimal digits and
+  // its Denom matches [a-zA-Z][a-zA-Z0-9/:._-]{2,127}; a value outside either
+  // is not a coin the chain will accept, so showing it as one only invents a
+  // reading the signer can act on. "1.5", "-1", "0x10" and a trailing space all
+  // reached the screen looking like amounts.
+  CHECK_PARSER_ERR(coin_amount_is_valid(amountPtr, (uint16_t)amountLen))
+  CHECK_PARSER_ERR(coin_denom_is_valid(denomPtr, (uint16_t)denomLen))
+
   const size_t totalLen = amountLen + denomLen + 2;
   if (sizeof(bufferUI) < totalLen) {
     return parser_unexpected_buffer_end;

app/src/parser_impl.c

@@ -37,6 +37,16 @@ const char *parser_getErrorDescription(parser_error_t err) {
     return "Unexpected characters";
   case parser_unexpected_field:
     return "Unexpected field";
+  case parser_unexpected_error:
+    return "Unexpected error";
+  case parser_unexpected_method:
+    return "Unexpected method";
+  case parser_unexpected_value:
+    return "Unexpected value";
+  case parser_unexpected_number_items:
+    return "Unexpected number of items";
+  case parser_invalid_address:
+    return "Invalid address";
   case parser_duplicated_field:
     return "Unexpected duplicated field";
   case parser_value_out_of_range:
@@ -93,6 +103,8 @@ const char *parser_getErrorDescription(parser_error_t err) {
     return "context unexpected size";
   case parser_context_invalid_chars:
     return "context invalid chars";
+  case parser_context_unknown_prefix:
+    return "context unknown prefix";
   case parser_transaction_too_big:
     return "Transaction is too big";
   // swap