Merge pull request #94 from cosmos/textual_pr

Preparing Textual PR

.github/workflows/codeql.yml

@@ -13,7 +13,7 @@ jobs:
     name: Analyse
     strategy:
       matrix:
-        sdk: ["$NANOS_SDK", "$NANOX_SDK", "$NANOSP_SDK"]
+        sdk: ["$NANOS_SDK", "$NANOX_SDK", "$NANOSP_SDK", "$STAX_SDK"]
     runs-on: ubuntu-latest
     container:
       image: ghcr.io/ledgerhq/ledger-app-builder/ledger-app-builder-legacy:latest

.github/workflows/guidelines_enforcer.yml

@@ -22,5 +22,4 @@ jobs:
     name: Call Ledger guidelines_enforcer
     uses: LedgerHQ/ledger-app-workflows/.github/workflows/reusable_guidelines_enforcer.yml@v1
     with:
-      relative_app_directory: app
-      run_for_devices: '["nanos", "nanosp", "nanox"]'
+      relative_app_directory: 'app'

.gitmodules

@@ -3,10 +3,13 @@
 	url = https://github.com/LedgerHQ/nanos-secure-sdk.git
 [submodule "deps/nanox-secure-sdk"]
 	path = deps/nanox-secure-sdk
-	url = https://github.com/LedgerHQ/nanox-secure-sdk.git
+	url = https://github.com/LedgerHQ/ledger-secure-sdk.git
 [submodule "deps/nanosplus-secure-sdk"]
 	path = deps/nanosplus-secure-sdk
-	url = https://github.com/LedgerHQ/nanosplus-secure-sdk
+	url = https://github.com/LedgerHQ/ledger-secure-sdk.git
 [submodule "deps/ledger-zxlib"]
 	path = deps/ledger-zxlib
 	url = https://github.com/Zondax/ledger-zxlib.git
+[submodule "deps/stax-secure-sdk"]
+	path = deps/stax-secure-sdk
+	url = https://github.com/LedgerHQ/ledger-secure-sdk

Makefile

@@ -1,5 +1,5 @@
 #*******************************************************************************
-#*   (c) 2019 Zondax GmbH
+#*   (c) 2019 - 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.
@@ -19,6 +19,7 @@
 # BOLOS_SDK NOT DEFINED		We use a containerized build approach
 
 ifeq ($(BOLOS_SDK),)
+ZXLIB_COMPILE_STAX ?= 1
 include $(CURDIR)/deps/ledger-zxlib/dockerized_build.mk
 else
 default:
@@ -30,5 +31,5 @@ endif
 
 test_all:
 	make zemu_install
-	make clean_build && make
+	make
 	make zemu_test

app/Makefile

@@ -1,6 +1,6 @@
 #*******************************************************************************
 #  Ledger App
-#  (c) 2018 - 2022 Zondax GmbH
+#  (c) 2018 - 2023 Zondax AG
 #  (c) 2017 Ledger
 #
 #  Licensed under the Apache License, Version 2.0 (the "License");
@@ -44,7 +44,6 @@ APPPATH = "44'/118'" --path "44'/60'"
 else
 define error_message
 
-
 COIN value not supported: [$(COIN)]
 
 endef

app/Makefile.version

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

app/src/apdu_handler.c

@@ -1,5 +1,5 @@
 /*******************************************************************************
-*   (c) 2018, 2019 Zondax GmbH
+*   (c) 2018 - 2023 Zondax AG
 *   (c) 2016 Ledger
 *
 *  Licensed under the Apache License, Version 2.0 (the "License");
@@ -49,7 +49,9 @@ __Z_INLINE void handle_getversion(__Z_UNUSED volatile uint32_t *flags, volatile
     G_io_apdu_buffer[1] = LEDGER_MAJOR_VERSION;
     G_io_apdu_buffer[2] = LEDGER_MINOR_VERSION;
     G_io_apdu_buffer[3] = LEDGER_PATCH_VERSION;
-    G_io_apdu_buffer[4] = !IS_UX_ALLOWED;
+    // SDK won't let the app reply an apdu message if screensaver is active
+    // device_locked field --> Always false
+    G_io_apdu_buffer[4] = 0;
 
     G_io_apdu_buffer[5] = (TARGET_ID >> 24) & 0xFF;
     G_io_apdu_buffer[6] = (TARGET_ID >> 16) & 0xFF;
@@ -60,7 +62,25 @@ __Z_INLINE void handle_getversion(__Z_UNUSED volatile uint32_t *flags, volatile
     THROW(APDU_CODE_OK);
 }
 
-static void extractHDPath(uint32_t rx, uint32_t offset) {
+__Z_INLINE uint8_t extractHRP(uint32_t rx, uint32_t offset) {
+    if (rx < offset + 1) {
+        THROW(APDU_CODE_DATA_INVALID);
+    }
+    MEMZERO(bech32_hrp, MAX_BECH32_HRP_LEN);
+
+    bech32_hrp_len = G_io_apdu_buffer[offset];
+
+    if (bech32_hrp_len == 0 || bech32_hrp_len > MAX_BECH32_HRP_LEN) {
+        THROW(APDU_CODE_DATA_INVALID);
+    }
+
+    memcpy(bech32_hrp, G_io_apdu_buffer + offset + 1, bech32_hrp_len);
+    bech32_hrp[bech32_hrp_len] = 0;     // zero terminate
+
+    return bech32_hrp_len;
+}
+
+__Z_INLINE void extractHDPath(uint32_t rx, uint32_t offset) {
     if ((rx - offset) < sizeof(uint32_t) * HDPATH_LEN_DEFAULT) {
         THROW(APDU_CODE_WRONG_LENGTH);
     }
@@ -74,12 +94,6 @@ static void extractHDPath(uint32_t rx, uint32_t offset) {
         THROW(APDU_CODE_DATA_INVALID);
     }
 
-    encoding = checkChainConfig(hdPath[1], bech32_hrp, bech32_hrp_len);
-    if (encoding == UNSUPPORTED) {
-        ZEMU_LOGF(50, "Chain config not supported for: %s\n", bech32_hrp)
-        THROW(APDU_CODE_COMMAND_NOT_ALLOWED);
-    }
-
     // Limit values unless the app is running in expert mode
     if (!app_mode_expert()) {
         for(int i=2; i < HDPATH_LEN_DEFAULT; i++) {
@@ -89,6 +103,24 @@ static void extractHDPath(uint32_t rx, uint32_t offset) {
     }
 }
 
+static void extractHDPath_HRP(uint32_t rx, uint32_t offset) {
+    extractHDPath(rx, offset);
+    // Set BECH32_COSMOS as default for backward compatibility
+    encoding = BECH32_COSMOS;
+
+    // Check if HRP was sent
+    if ((rx - offset) > sizeof(uint32_t) * HDPATH_LEN_DEFAULT) {
+        extractHRP(rx, offset + sizeof(uint32_t) * HDPATH_LEN_DEFAULT);
+        encoding = checkChainConfig(hdPath[1], bech32_hrp, bech32_hrp_len);
+        if (encoding == UNSUPPORTED) {
+            ZEMU_LOGF(50, "Chain config not supported for: %s\n", bech32_hrp)
+            THROW(APDU_CODE_COMMAND_NOT_ALLOWED);
+        }
+    } else if (hdPath[1] == HDPATH_ETH_1_DEFAULT) {
+        THROW(APDU_CODE_COMMAND_NOT_ALLOWED);
+    }
+}
+
 static bool process_chunk(volatile uint32_t *tx, uint32_t rx) {
     UNUSED(tx);
 
@@ -103,7 +135,7 @@ static bool process_chunk(volatile uint32_t *tx, uint32_t rx) {
         case P1_INIT:
             tx_initialize();
             tx_reset();
-            extractHDPath(rx, OFFSET_DATA);
+            extractHDPath_HRP(rx, OFFSET_DATA);
             return false;
         case P1_ADD:
             added = tx_append(&(G_io_apdu_buffer[OFFSET_DATA]), rx - OFFSET_DATA);
@@ -126,6 +158,13 @@ __Z_INLINE void handleGetAddrSecp256K1(volatile uint32_t *flags, volatile uint32
     uint8_t len = extractHRP(rx, OFFSET_DATA);
     extractHDPath(rx, OFFSET_DATA + 1 + len);
 
+    // Verify encoding
+    encoding = checkChainConfig(hdPath[1], bech32_hrp, bech32_hrp_len);
+    if (encoding == UNSUPPORTED) {
+        ZEMU_LOGF(50, "Chain config not supported for: %s\n", bech32_hrp)
+        THROW(APDU_CODE_COMMAND_NOT_ALLOWED);
+    }
+
     uint8_t requireConfirmation = G_io_apdu_buffer[OFFSET_P1];
     zxerr_t zxerr = app_fill_address();
     if (zxerr != zxerr_ok) {
@@ -152,19 +191,20 @@ __Z_INLINE void handleSign(volatile uint32_t *flags, volatile uint32_t *tx, uint
     // Let grab P2 value and if it's not valid, the parser should reject it
     const tx_type_e sign_type = (tx_type_e) G_io_apdu_buffer[OFFSET_P2];
 
+    if ((hdPath[1] == HDPATH_ETH_1_DEFAULT) && !app_mode_expert()) {
+        *flags |= IO_ASYNCH_REPLY;
+        view_custom_error_show(PIC(msg_error1),PIC(msg_error2));
+        THROW(APDU_CODE_DATA_INVALID);
+    }
+
     // Put address in output buffer, we will use it to confirm source address
     zxerr_t zxerr = app_fill_address();
     if (zxerr != zxerr_ok) {
         *tx = 0;
         THROW(APDU_CODE_DATA_INVALID);
     }
-    parser_tx_obj.tx_json.own_addr = (const char *) (G_io_apdu_buffer + VIEW_ADDRESS_OFFSET_SECP256K1);
 
-    if ((encoding != BECH32_COSMOS) && !app_mode_expert()) {
-        *flags |= IO_ASYNCH_REPLY;
-        view_custom_error_show(PIC(msg_error1),PIC(msg_error2));
-        THROW(APDU_CODE_DATA_INVALID);
-    }
+    parser_tx_obj.tx_json.own_addr = (const char *) (G_io_apdu_buffer + VIEW_ADDRESS_OFFSET_SECP256K1);
     const char *error_msg = tx_parse(sign_type);
 
     if (error_msg != NULL) {

app/src/coin.h

@@ -14,12 +14,12 @@
 *  limitations under the License.
 ********************************************************************************/
 #pragma once
-#include "stdbool.h"
+
 #ifdef __cplusplus
 extern "C" {
 #endif
 
-#define CLA                  0x55
+#define CLA                  0x55u
 
 #define HDPATH_LEN_DEFAULT   5
 
@@ -60,27 +60,27 @@ typedef enum {
 // In non-expert mode, the app will convert from uatom to ATOM
 #define COIN_DEFAULT_DENOM_BASE             "uatom"
 #define COIN_DEFAULT_DENOM_REPR             "ATOM"
-#define COIN_DEFAULT_DENOM_FACTOR           6
-#define COIN_DEFAULT_DENOM_TRIMMING         6
+#define COIN_DEFAULT_DENOM_FACTOR           6u
+#define COIN_DEFAULT_DENOM_TRIMMING         6u
 
 // Coin denoms may be up to 128 characters long
 // https://github.com/cosmos/cosmos-sdk/blob/master/types/coin.go#L780
 // https://github.com/cosmos/ibc-go/blob/main/docs/architecture/adr-001-coin-source-tracing.md
 #define COIN_DENOM_MAXSIZE                  129
 #define COIN_AMOUNT_MAXSIZE                 50
 
-#define COIN_MAX_CHAINID_LEN                20
-#define INDEXING_TMP_KEYSIZE 70
-#define INDEXING_TMP_VALUESIZE 70
-#define INDEXING_GROUPING_REF_TYPE_SIZE 70
-#define INDEXING_GROUPING_REF_FROM_SIZE 70
+#define COIN_MAX_CHAINID_LEN                20u
+#define INDEXING_TMP_KEYSIZE                70u
+#define INDEXING_TMP_VALUESIZE              70u
+#define INDEXING_GROUPING_REF_TYPE_SIZE     70u
+#define INDEXING_GROUPING_REF_FROM_SIZE     70u
 
 #define MENU_MAIN_APP_LINE2_SECRET         "?"
 #define COIN_SECRET_REQUIRED_CLICKS         0
 
 #define INS_GET_VERSION                 0x00
-#define INS_SIGN_SECP256K1              0x02
-#define INS_GET_ADDR_SECP256K1          0x04
+#define INS_SIGN_SECP256K1              0x02u
+#define INS_GET_ADDR_SECP256K1          0x04u
 
 #ifdef __cplusplus
 }

app/src/common/actions.h

@@ -25,15 +25,11 @@
 
 extern uint16_t action_addrResponseLen;
 
-__Z_INLINE void app_set_hrp(char *p) {
-    crypto_set_hrp(p);
-}
-
 __Z_INLINE void app_sign() {
     uint16_t replyLen = 0;
 
     MEMZERO(G_io_apdu_buffer, IO_APDU_BUFFER_SIZE);
-    zxerr_t err = crypto_sign(G_io_apdu_buffer, IO_APDU_BUFFER_SIZE - 3, &replyLen);
+    const zxerr_t err = crypto_sign(G_io_apdu_buffer, IO_APDU_BUFFER_SIZE - 3, &replyLen);
 
     if (err != zxerr_ok || replyLen == 0) {
         set_code(G_io_apdu_buffer, 0, APDU_CODE_SIGN_VERIFY_ERROR);
@@ -49,7 +45,7 @@ __Z_INLINE zxerr_t app_fill_address() {
     MEMZERO(G_io_apdu_buffer, IO_APDU_BUFFER_SIZE);
 
     action_addrResponseLen = 0;
-    zxerr_t err = crypto_fillAddress(G_io_apdu_buffer, IO_APDU_BUFFER_SIZE - 2, &action_addrResponseLen);
+    const zxerr_t err = crypto_fillAddress(G_io_apdu_buffer, IO_APDU_BUFFER_SIZE - 2, &action_addrResponseLen);
 
     if (err != zxerr_ok || action_addrResponseLen == 0) {
         THROW(APDU_CODE_EXECUTION_ERROR);

app/src/common/tx.c

@@ -21,7 +21,7 @@
 #include <string.h>
 #include "zxmacros.h"
 
-#if defined(TARGET_NANOX) || defined(TARGET_NANOS2)
+#if defined(TARGET_NANOX) || defined(TARGET_NANOS2) || defined(TARGET_STAX)
 #define RAM_BUFFER_SIZE 8192
 #define FLASH_BUFFER_SIZE 16384
 #elif defined(TARGET_NANOS)
@@ -38,7 +38,7 @@ typedef struct
     uint8_t buffer[FLASH_BUFFER_SIZE];
 } storage_t;
 
-#if defined(TARGET_NANOS) || defined(TARGET_NANOX) || defined(TARGET_NANOS2)
+#if defined(TARGET_NANOS) || defined(TARGET_NANOX) || defined(TARGET_NANOS2) || defined(TARGET_STAX)
 storage_t NV_CONST N_appdata_impl __attribute__((aligned(64)));
 #define N_appdata (*(NV_VOLATILE storage_t *)PIC(&N_appdata_impl))
 #endif