Merge pull request #197 from Zondax/update-9420

.github/workflows/check_version.yml

@@ -22,8 +22,6 @@ jobs:
     container:
       image: zondax/ledger-app-builder:latest
       options: --user ${{ needs.configure.outputs.uid_gid }}
-    env:
-      SDK_VARNAME: NANOSP_SDK
     outputs:
       version: ${{ steps.store-version.outputs.version }}
     steps:

.github/workflows/codeql.yml

@@ -0,0 +1,37 @@
+name: "CodeQL"
+
+on:
+  workflow_dispatch:
+  push:
+  pull_request:
+    branches:
+      - main
+      - develop
+
+jobs:
+  analyse:
+    name: Analyse
+    strategy:
+      matrix:
+        sdk: ["$NANOS_SDK", "$NANOX_SDK", "$NANOSP_SDK"]
+    runs-on: ubuntu-latest
+    container:
+      image: ghcr.io/ledgerhq/ledger-app-builder/ledger-app-builder-legacy:latest
+
+    steps:
+      - name: Clone
+        uses: actions/checkout@v3
+        with:
+          submodules: recursive
+
+      - name: Initialize CodeQL
+        uses: github/codeql-action/init@v2
+        with:
+          languages: cpp
+          queries: security-and-quality
+
+      - name: Build
+        run: |
+          make -j BOLOS_SDK=${{ matrix.sdk }}
+      - name: Perform CodeQL Analysis
+        uses: github/codeql-action/analyze@v2

.github/workflows/guidelines_enforcer.yml

@@ -0,0 +1,26 @@
+name: Ensure compliance with Ledger guidelines
+
+# This workflow is mandatory in all applications
+# It calls a reusable workflow guidelines_enforcer developed by Ledger's internal developer team.
+# The successful completion of the reusable workflow is a mandatory step for an app to be available on the Ledger
+# application store.
+#
+# More information on the guidelines can be found in the repository:
+# LedgerHQ/ledger-app-workflows/
+
+on:
+  workflow_dispatch:
+  push:
+    branches:
+      - master
+      - main
+      - develop
+  pull_request:
+
+jobs:
+  guidelines_enforcer:
+    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"]'

Makefile

@@ -47,5 +47,6 @@ zemu_install: tests_tools_build
 test_all:
 	make zemu_install
 	SUBSTRATE_PARSER_FULL=1 make
+	make clean_glyphs
 	SUBSTRATE_PARSER_FULL=1 SUPPORT_SR25519=1 make buildS
 	make zemu_test

app/Makefile

@@ -90,20 +90,17 @@ endif
 
 include $(CURDIR)/../deps/ledger-zxlib/makefiles/Makefile.platform
 LDFLAGS += -z muldefs
+
+ifeq ($(SUPPORT_SR25519),1)
+APP_CUSTOM_LINK_DEPENDENCIES = rust
 LDLIBS += -Lrust/target/thumbv6m-none-eabi/release -lrslib
-CFLAGS += -Wvla
 APP_SOURCE_PATH += $(CURDIR)/rust/include
+endif
 
 .PHONY: rust
 rust:
 	cd rust && CARGO_HOME="$(CURDIR)/rust/.cargo" cargo build --target thumbv6m-none-eabi --release
 
-# Force rust dependency before c compilation in parallel mode (-j option).
-.DEFAULT_GOAL := build_with_rust
-.PHONY: build_with_rust
-build_with_rust: rust
-	$(MAKE) all
-
 .PHONY: rust_clean
 rust_clean:
 	cd rust && CARGO_HOME="$(CURDIR)/rust/.cargo" cargo clean

app/Makefile.version

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

app/src/addr.c

@@ -35,9 +35,7 @@ zxerr_t addr_getItem(int8_t displayIdx,
                      char *outKey, uint16_t outKeyLen,
                      char *outVal, uint16_t outValLen,
                      uint8_t pageIdx, uint8_t *pageCount) {
-    char buffer[30];
-    snprintf(buffer, sizeof(buffer), "addr_getItem %d/%d", displayIdx, pageIdx);
-    zemu_log_stack(buffer);
+    ZEMU_LOGF(50, "addr_getItem %d/%d", displayIdx, pageIdx)
 
     switch (displayIdx) {
         case 0:

app/src/apdu_handler.c

@@ -31,6 +31,7 @@
 #include "zxmacros.h"
 #include "secret.h"
 #include "app_mode.h"
+#include "view.h"
 
 static bool tx_initialized = false;
 
@@ -103,7 +104,7 @@ __Z_INLINE bool process_chunk(__Z_UNUSED volatile uint32_t *tx, uint32_t rx) {
     THROW(APDU_CODE_INVALIDP1P2);
 }
 
-__Z_INLINE void handle_getversion(volatile uint32_t *flags, volatile uint32_t *tx) {
+__Z_INLINE void handle_getversion(__Z_UNUSED volatile uint32_t *flags, volatile uint32_t *tx) {
     G_io_apdu_buffer[0] = 0;
 
 #if defined(APP_TESTING)
@@ -144,7 +145,7 @@ __Z_INLINE void handleGetAddr(volatile uint32_t *flags, volatile uint32_t *tx, u
     }
     if (requireConfirmation) {
         view_review_init(addr_getItem, addr_getNumItems, app_reply_address);
-        view_review_show(0x03);
+        view_review_show(REVIEW_ADDRESS);
         *flags |= IO_ASYNCH_REPLY;
         return;
     }
@@ -166,14 +167,14 @@ __Z_INLINE void handleSignSr25519(volatile uint32_t *flags, volatile uint32_t *t
     CHECK_APP_CANARY()
 
     if (error_msg != NULL) {
-        int error_msg_length = strlen(error_msg);
+        const int error_msg_length = strnlen(error_msg, sizeof(G_io_apdu_buffer));
         memcpy(G_io_apdu_buffer, error_msg, error_msg_length);
         *tx += (error_msg_length);
         THROW(APDU_CODE_DATA_INVALID);
     }
 
     view_review_init(tx_getItem, tx_getNumItems, app_return_sr25519);
-    view_review_show(0x03);
+    view_review_show(REVIEW_TXN);
     *flags |= IO_ASYNCH_REPLY;
 }
 #endif
@@ -182,14 +183,14 @@ __Z_INLINE void handleSignEd25519(volatile uint32_t *flags, volatile uint32_t *t
     const char *error_msg = tx_parse();
     CHECK_APP_CANARY()
     if (error_msg != NULL) {
-        int error_msg_length = strlen(error_msg);
+        const int error_msg_length = strnlen(error_msg, sizeof(G_io_apdu_buffer));
         memcpy(G_io_apdu_buffer, error_msg, error_msg_length);
         *tx += (error_msg_length);
         THROW(APDU_CODE_DATA_INVALID);
     }
 
     view_review_init(tx_getItem, tx_getNumItems, app_sign_ed25519);
-    view_review_show(0x03);
+    view_review_show(REVIEW_TXN);
     *flags |= IO_ASYNCH_REPLY;
 }
 

app/src/common/actions.h

@@ -50,11 +50,16 @@ __Z_INLINE void app_sign_ed25519() {
 
 #ifdef SUPPORT_SR25519
 __Z_INLINE void app_return_sr25519() {
-    copy_sr25519_signdata(G_io_apdu_buffer);
+    const zxerr_t err = copy_sr25519_signdata(G_io_apdu_buffer, sizeof(G_io_apdu_buffer) - 2);
     zeroize_sr25519_signdata();
 
-    set_code(G_io_apdu_buffer, SIG_PLUS_TYPE_LEN, APDU_CODE_OK);
-    io_exchange(CHANNEL_APDU | IO_RETURN_AFTER_TX, SIG_PLUS_TYPE_LEN + 2);
+    if (err != zxerr_ok) {
+        set_code(G_io_apdu_buffer, 0, APDU_CODE_SIGN_VERIFY_ERROR);
+        io_exchange(CHANNEL_APDU | IO_RETURN_AFTER_TX, 2);
+    } else {
+        set_code(G_io_apdu_buffer, SIG_PLUS_TYPE_LEN, APDU_CODE_OK);
+        io_exchange(CHANNEL_APDU | IO_RETURN_AFTER_TX, SIG_PLUS_TYPE_LEN + 2);
+    }
 }
 #endif
 
@@ -83,6 +88,7 @@ __Z_INLINE key_kind_e get_key_type(uint8_t num) {
     }
     return 0xff;
 #else
+    UNUSED(num);
     return key_ed25519;
 #endif
 }

app/src/crypto.c

@@ -18,10 +18,13 @@
 #include "base58.h"
 #include "coin.h"
 #include "cx.h"
-#include "rslib.h"
 #include "zxmacros.h"
 #include "ristretto.h"
 
+#ifdef SUPPORT_SR25519
+#include "rslib.h"
+#endif
+
 uint32_t hdPath[HDPATH_LEN_DEFAULT];
 
 zxerr_t crypto_extractPublicKey(key_kind_e addressKind, const uint32_t path[HDPATH_LEN_DEFAULT],
@@ -161,8 +164,13 @@ void zeroize_sr25519_signdata(void) {
     explicit_bzero(sr25519_signature, sizeof(sr25519_signature));
 }
 
-void copy_sr25519_signdata(uint8_t *buffer) {
+zxerr_t copy_sr25519_signdata(uint8_t *buffer, uint16_t bufferLen) {
+    if (SIG_PLUS_TYPE_LEN > bufferLen) {
+        return zxerr_buffer_too_small;
+    }
+
     memcpy(buffer, sr25519_signature, SIG_PLUS_TYPE_LEN);
+    return zxerr_ok;
 }
 
 static zxerr_t crypto_sign_sr25519_helper(const uint8_t *data, size_t len) {

app/src/crypto.h

@@ -48,7 +48,7 @@ zxerr_t crypto_sign_ed25519(uint8_t *signature, uint16_t signatureMaxlen, const
 #ifdef SUPPORT_SR25519
 void zeroize_sr25519_signdata(void);
 
-void copy_sr25519_signdata(uint8_t *buffer);
+zxerr_t copy_sr25519_signdata(uint8_t *buffer, uint16_t bufferLen);
 
 zxerr_t crypto_sign_sr25519(const uint8_t *message, size_t messageLen);
 #endif

app/src/parser.c

@@ -22,13 +22,6 @@
 #include "coin.h"
 #include "substrate_dispatch.h"
 
-#if defined(TARGET_NANOX) || defined(TARGET_NANOS2)
-// For some reason NanoX requires this function
-void __assert_fail(const char * assertion, const char * file, unsigned int line, const char * function){
-    while(1) {};
-}
-#endif
-
 #define FIELD_FIXED_TOTAL_COUNT 7
 
 #define FIELD_METHOD        0

app/src/parser_impl.h

@@ -142,14 +142,14 @@ GEN_DEC_READFIX_UNSIGNED(64);
     /* We need to do it twice because there is no memory to keep intermediate results*/ \
     /* First count*/ \
     parser_init(&ctx, v->_ptr, v->_lenBuffer);\
-    for (uint16_t i = 0; i < v->_len; i++) {\
+    for (uint64_t i = 0; i < v->_len; i++) {\
         CHECK_ERROR(_read##TYPE(&ctx, &tmp));\
         CHECK_ERROR(_toString##TYPE(&tmp, outValue, outValueLen, 0, &chunkPageCount));\
         (*pageCount)+=chunkPageCount;\
     }\
     /* Then iterate until we can print the corresponding chunk*/ \
     parser_init(&ctx, v->_ptr, v->_lenBuffer);\
-    for (uint16_t i = 0; i < v->_len; i++) {\
+    for (uint64_t i = 0; i < v->_len; i++) {\
         CHECK_ERROR(_read##TYPE(&ctx, &tmp));\
         chunkPageCount = 1;\
         currentPage = 0;\
@@ -188,8 +188,8 @@ uint16_t _detectAddressType(const parser_context_t *c);
 
 parser_error_t _toStringCompactInt(const compactInt_t *c, uint8_t decimalPlaces,
                                    bool trimTrailingZeros,
-                                   char postfix[],
-                                   char prefix[],
+                                   const char postfix[],
+                                   const char prefix[],
                                    char *outValue, uint16_t outValueLen,
                                    uint8_t pageIdx, uint8_t *pageCount);
 

app/src/parser_impl_common.c

@@ -199,8 +199,8 @@ parser_error_t _getValue(const compactInt_t *c, uint64_t *v) {
 parser_error_t _toStringCompactInt(const compactInt_t *c,
                                    uint8_t decimalPlaces,
                                    bool trimTrailingZeros,
-                                   char postfix[],
-                                   char prefix[],
+                                   const char postfix[],
+                                   const char prefix[],
                                    char *outValue, uint16_t outValueLen,
                                    uint8_t pageIdx, uint8_t *pageCount) {
     char bufferUI[200];

app/src/ristretto.c

@@ -16,7 +16,6 @@
 #include <zxmacros.h>
 #include "ristretto.h"
 #include "cx.h"
-#include "rslib.h"
 
 unsigned char const ED25519_GEN[ED25519_SDKPOINT_BYTES] = {
         //uncompressed

app/src/secret.c

@@ -30,7 +30,7 @@ void secret_accept() {
 #endif
 }
 
-static char *secret_message =
+static const char *secret_message =
         "USE AT YOUR OWN RISK!! "
         "You are about to enable the KSM recovery mode."
         "If you are not sure why you are here, reject or unplug your device immediately."

app/src/substrate/substrate_coin.h

@@ -51,7 +51,7 @@ typedef enum {
 // Coin Specific
 #define PK_ADDRESS_TYPE COIN_ADDR_TYPE
 #define SUPPORTED_TX_VERSION_CURRENT LEDGER_MAJOR_VERSION
-#define SUPPORTED_TX_VERSION_PREVIOUS (LEDGER_MAJOR_VERSION - 1)
+#define SUPPORTED_TX_VERSION_PREVIOUS 20
 #define SUPPORTED_SPEC_VERSION (LEDGER_MINOR_VERSION + 0)
 #define SUPPORTED_MINIMUM_SPEC_VERSION 9000