Merge pull request #18 from Zondax/dev

Ledger findings

Author: abenso

.github/workflows/main.yml

@@ -53,16 +53,6 @@ jobs:
           make
           echo "size=$(python3 deps/ledger-zxlib/scripts/getSize.py s)" >> $GITHUB_OUTPUT
 
-  size_nano_s:
-    needs: build_ledger
-    runs-on: ubuntu-latest
-    env:
-      NANOS_LIMIT_SIZE: 136
-    steps:
-      - run: |
-          echo "LNS app size: ${{needs.build_ledger.outputs.size}} KiB"
-          [ ${{needs.build_ledger.outputs.size}} -le $NANOS_LIMIT_SIZE ]
-
   test_zemu:
     runs-on: ubuntu-latest
     steps:
@@ -88,47 +78,11 @@ jobs:
           make test_all
       - name: Upload Snapshots (only failure)
         if: ${{ failure() }}
-        uses: actions/upload-artifact@v3
+        uses: actions/upload-artifact@v4
         with:
           name: snapshots-tmp
           path: tests_zemu/snapshots-tmp/
 
-  build_package_nanos:
-    needs: [configure, build, build_ledger, test_zemu]
-    if: ${{ github.ref == 'refs/heads/main' }}
-    runs-on: ubuntu-latest
-    container:
-      image: zondax/ledger-app-builder:latest
-      options: --user ${{ needs.configure.outputs.uid_gid }}
-      env:
-        BOLOS_SDK: /opt/nanos-secure-sdk
-    steps:
-      - name: Checkout
-        uses: actions/checkout@v4
-        with:
-          submodules: true
-      - name: Install deps
-        run: pip install ledgerblue
-
-      - name: Build NanoS
-        shell: bash -l {0}
-        run: |
-          PRODUCTION_BUILD=0 make
-          mv ./app/pkg/installer_s.sh ./app/pkg/installer_nanos.sh
-      - name: Set tag
-        id: nanos
-        run: echo "tag_name=$(./app/pkg/installer_nanos.sh version)" >> $GITHUB_OUTPUT
-      - name: Create or Update Release (1)
-        id: create_release_0
-        uses: softprops/action-gh-release@v1
-        env:
-          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # This token is provided by Actions, you do not need to create your own token
-        with:
-          files: ./app/pkg/installer_nanos.sh
-          tag_name: ${{ steps.nanos.outputs.tag_name }}
-          draft: false
-          prerelease: false
-
   build_package_nanosp:
     needs: [configure, build, build_ledger, test_zemu]
     if: ${{ github.ref == 'refs/heads/main' }}

Makefile

@@ -25,8 +25,13 @@ ifeq ($(BOLOS_SDK),)
 # In this case, there is not predefined SDK and we run dockerized
 # When not using the SDK, we override and build the XL complete app
 
-ZXLIB_COMPILE_STAX ?= 1
 PRODUCTION_BUILD ?= 1
+SKIP_NANOS = 1
+
+ifeq ($(SKIP_NANOS), 0)
+$(error "NanoS device is not supported")
+endif
+
 include $(CURDIR)/deps/ledger-zxlib/dockerized_build.mk
 
 else

app/Makefile

@@ -22,14 +22,8 @@ endif
 
 MY_DIR := $(dir $(lastword $(MAKEFILE_LIST)))
 
-include $(CURDIR)/../deps/ledger-zxlib/makefiles/Makefile.installer_script
-
-include $(BOLOS_SDK)/Makefile.defines
-
 # Set the default value for PRODUCTION_BUILD to 0 if not already defined
-PRODUCTION_BUILD ?= 0
-
-$(info ************ TARGET_NAME  = [$(TARGET_NAME)])
+PRODUCTION_BUILD ?= 1
 
 # Display whether this is a production build or for internal use
 ifeq ($(PRODUCTION_BUILD), 1)
@@ -38,9 +32,15 @@ else
     $(info ************ PRODUCTION_BUILD  = [INTERNAL USE])
 endif
 
-DEFINES += APP_BLINDSIGN_MODE_ENABLED
+# Display whether swap functionality is enabled or not
+ifeq ($(ENABLE_SWAP), 1)
+    $(info ************ HAVE_SWAP  = [ENABLED])
+else
+    $(info ************ HAVE_SWAP  = [DISABLED])
+endif
 
 # Add the PRODUCTION_BUILD definition to the compiler flags
+DEFINES += APP_BLINDSIGN_MODE_ENABLED
 DEFINES += PRODUCTION_BUILD=$(PRODUCTION_BUILD)
 
 include $(CURDIR)/../deps/ledger-zxlib/makefiles/Makefile.app_testing
@@ -49,6 +49,9 @@ ifndef COIN
 COIN=SEI
 endif
 
+VARIANT_PARAM=COIN
+VARIANT_VALUES=$(COIN)
+
 include $(CURDIR)/Makefile.version
 
 $(info COIN  = [$(COIN)])
@@ -65,20 +68,32 @@ endef
 $(error "$(error_message)")
 endif
 
+$(info PATHS LIST = $(APPPATH))
+
 APP_LOAD_PARAMS =  --curve secp256k1 --delete $(COMMON_LOAD_PARAMS) --path $(APPPATH)
+APP_SOURCE_PATH += $(MY_DIR)/../deps/jsmn/src
+INCLUDES_PATH += $(CURDIR)/src/common
+
+ENABLE_NBGL_QRCODE ?= 1
+ENABLE_BLUETOOTH = 1
+
+include $(BOLOS_SDK)/Makefile.target
 
 include $(CURDIR)/../deps/ledger-zxlib/makefiles/Makefile.devices
 
+include $(CURDIR)/../deps/ledger-zxlib/makefiles/Makefile.platform
+
+CFLAGS += -Wvla
+
+include $(CURDIR)/../deps/ledger-zxlib/makefiles/Makefile.installer_script
+
 $(info TARGET_NAME  = [$(TARGET_NAME)])
 $(info ICONNAME  = [$(ICONNAME)])
 
 ifndef ICONNAME
 $(error ICONNAME is not set)
 endif
 
-include $(CURDIR)/../deps/ledger-zxlib/makefiles/Makefile.platform
-APP_SOURCE_PATH += $(MY_DIR)/../deps/jsmn/src
-CFLAGS += -Wvla
 
 .PHONY: rust
 rust:
@@ -93,17 +108,10 @@ rust_clean:
 
 clean: rust_clean
 
-include $(CURDIR)/../deps/ledger-zxlib/makefiles/Makefile.side_loading
-
-# Import generic rules from the SDK
-include $(BOLOS_SDK)/Makefile.rules
 
 #add dependency on custom makefile filename
 dep/%.d: %.c Makefile
 
-listvariants:
-	@echo VARIANTS COIN SEI
-
 .PHONY: version
 version:
 	@echo "v$(APPVERSION)" > app.version

app/Makefile.version

@@ -3,4 +3,4 @@ APPVERSION_M=0
 # This is the minor version
 APPVERSION_N=1
 # This is the patch version
-APPVERSION_P=3
+APPVERSION_P=4

app/src/apdu_handler.c

@@ -97,14 +97,14 @@ __Z_INLINE void handle_getversion(__Z_UNUSED volatile uint32_t *flags, volatile
     G_io_apdu_buffer[0] = 0x01;
 #endif
 
-    G_io_apdu_buffer[1] = (LEDGER_MAJOR_VERSION >> 8) & 0xFF;
-    G_io_apdu_buffer[2] = (LEDGER_MAJOR_VERSION >> 0) & 0xFF;
+    G_io_apdu_buffer[1] = (MAJOR_VERSION >> 8) & 0xFF;
+    G_io_apdu_buffer[2] = (MAJOR_VERSION >> 0) & 0xFF;
 
-    G_io_apdu_buffer[3] = (LEDGER_MINOR_VERSION >> 8) & 0xFF;
-    G_io_apdu_buffer[4] = (LEDGER_MINOR_VERSION >> 0) & 0xFF;
+    G_io_apdu_buffer[3] = (MINOR_VERSION >> 8) & 0xFF;
+    G_io_apdu_buffer[4] = (MINOR_VERSION >> 0) & 0xFF;
 
-    G_io_apdu_buffer[5] = (LEDGER_PATCH_VERSION >> 8) & 0xFF;
-    G_io_apdu_buffer[6] = (LEDGER_PATCH_VERSION >> 0) & 0xFF;
+    G_io_apdu_buffer[5] = (PATCH_VERSION >> 8) & 0xFF;
+    G_io_apdu_buffer[6] = (PATCH_VERSION >> 0) & 0xFF;
 
     G_io_apdu_buffer[7] = !IS_UX_ALLOWED;
 

app/src/evm/apdu_handler_evm.c

@@ -304,6 +304,6 @@ void handleSignEip191(volatile uint32_t *flags, volatile uint32_t *tx, uint32_t
     CHECK_APP_CANARY()
 
     view_review_init(eip191_msg_getItem, eip191_msg_getNumItems, app_sign_eip191);
-    view_review_show(REVIEW_TXN);
+    view_review_show(REVIEW_MSG);
     *flags |= IO_ASYNCH_REPLY;
 }

app/src/evm/evm_eip191.c

@@ -65,15 +65,17 @@ zxerr_t eip191_msg_getItem(int8_t displayIdx, char *outKey, uint16_t outKeyLen,
         }
         case 1: {
             snprintf(outKey, outKeyLen, "Msg hex");
-            uint16_t npc = 0;  // Non Printable Chars Counter
+            uint8_t is_printable = 1;
 
+            // Check if all characters are printable
             for (uint16_t i = 0; i < messageLength; i++) {
-                npc += IS_PRINTABLE(message[i]) ? 0 /* Printable Char */ : 1 /* Non Printable Char */;
+                if (!IS_PRINTABLE(message[i])) {
+                    is_printable = 0;
+                    break;
+                }
             }
 
-            // msg in hex in case >= than 40% is non printable
-            // or first char is not printable.
-            if (messageLength > 0 && (npc * 100) / messageLength >= 40) {
+            if (messageLength > 0 && is_printable == 0) {
                 pageStringHex(outVal, outValLen, (const char *)message, messageLength, pageIdx, pageCount);
                 return zxerr_ok;
             }
@@ -93,16 +95,14 @@ zxerr_t eip191_msg_getItem(int8_t displayIdx, char *outKey, uint16_t outKeyLen,
 bool eip191_msg_parse() {
     const uint8_t *message = tx_get_buffer() + sizeof(uint32_t);
     const uint16_t messageLength = tx_get_buffer_length() - sizeof(uint32_t);
-    uint16_t npc = 0;  // Non Printable Chars Counter
+    // Check if all characters are printable
     for (uint16_t i = 0; i < messageLength; i++) {
-        npc += IS_PRINTABLE(message[i]) ? 0 /* Printable Char */ : 1 /* Non Printable Char */;
-    }
-    // msg in hex in case >= than 40% is non printable
-    // or first char is not printable.
-    if (messageLength > 0 && (npc * 100) / messageLength >= 40 && !app_mode_blindsign()) {
-        return false;
-    } else if (messageLength > 0 && (npc * 100) / messageLength < 40) {
-        app_mode_skip_blindsign_ui();
+        if (!IS_PRINTABLE(message[i])) {
+            if (!app_mode_blindsign()) {
+                return false;
+            }
+            break;
+        }
     }
 
     return true;

deps/ledger-zxlib

@@ -9,16 +9,14 @@
 #error "This fuzz target won't work correctly with NDEBUG defined, which will cause asserts to be eliminated"
 #endif
 
-
 using std::size_t;
 
 namespace {
-    char PARSER_KEY[16384];
-    char PARSER_VALUE[16384];
-}
+char PARSER_KEY[16384];
+char PARSER_VALUE[16384];
+}  // namespace
 
-extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
-{
+extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
     parser_tx_t txObj;
     MEMZERO(&txObj, sizeof(txObj));
     parser_context_t ctx;
@@ -37,9 +35,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
     uint8_t num_items;
     rc = parser_getNumItems(&ctx, &num_items);
     if (rc != parser_ok) {
-        fprintf(stderr,
-                "error in parser_getNumItems: %s\n",
-                parser_getErrorDescription(rc));
+        fprintf(stderr, "error in parser_getNumItems: %s\n", parser_getErrorDescription(rc));
         assert(false);
     }
 
@@ -49,19 +45,14 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
         uint8_t page_idx = 0;
         uint8_t page_count = 1;
         while (page_idx < page_count) {
-            rc = parser_getItem(&ctx, i,
-                                PARSER_KEY, sizeof(PARSER_KEY),
-                                PARSER_VALUE, sizeof(PARSER_VALUE),
-                                page_idx, &page_count);
+            rc = parser_getItem(&ctx, i, PARSER_KEY, sizeof(PARSER_KEY), PARSER_VALUE, sizeof(PARSER_VALUE), page_idx,
+                                &page_count);
 
-//            (void)fprintf(stderr, "%s = %s\n", PARSER_KEY, PARSER_VALUE);
+            //            (void)fprintf(stderr, "%s = %s\n", PARSER_KEY, PARSER_VALUE);
 
             if (rc != parser_ok) {
-                (void)fprintf(stderr,
-                        "error getting item %u at page index %u: %s\n",
-                        (unsigned)i,
-                        (unsigned)page_idx,
-                        parser_getErrorDescription(rc));
+                (void)fprintf(stderr, "error getting item %u at page index %u: %s\n", (unsigned)i, (unsigned)page_idx,
+                              parser_getErrorDescription(rc));
                 assert(false);
             }
 

fuzz/parser_parse.cpp

@@ -1,21 +1,22 @@
 /*******************************************************************************
-*   (c) 2019 Zondax GmbH
-*
-*  Licensed under the Apache License, Version 2.0 (the "License");
-*  you may not use this file except in compliance with the License.
-*  You may obtain a copy of the License at
-*
-*      http://www.apache.org/licenses/LICENSE-2.0
-*
-*  Unless required by applicable law or agreed to in writing, software
-*  distributed under the License is distributed on an "AS IS" BASIS,
-*  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-*  See the License for the specific language governing permissions and
-*  limitations under the License.
-********************************************************************************/
+ *   (c) 2019 Zondax GmbH
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ ********************************************************************************/
 #pragma once
+#include <json/json.h>
+
 #include <fstream>
 #include <vector>
-#include <json/json.h>
 
 std::vector<std::string> EVMGenerateExpectedUIOutput(const Json::Value &json, bool expertMode);