return and apduspec improvements

Zondax · Dec 28, 2023 · 2060ee1 · 2060ee1
1 parent fbcd06b
commit 2060ee1
Show file tree

Hide file tree

Showing 8 changed files with 145 additions and 170 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -1,18 +1,18 @@
-#*******************************************************************************
-#*   (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.
-#*  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) 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.
+# *  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.
+# ********************************************************************************
 cmake_minimum_required(VERSION 3.0)
 project(ledger-flare VERSION 0.0.0)
 enable_testing()
@@ -36,23 +36,23 @@ if(ENABLE_FUZZING)
     SET(ENABLE_SANITIZERS ON CACHE BOOL "Sanitizer automatically enabled" FORCE)
     SET(CMAKE_BUILD_TYPE Debug)
 
-    if (DEFINED ENV{FUZZ_LOGGING})
+    if(DEFINED ENV{FUZZ_LOGGING})
         add_definitions(-DFUZZING_LOGGING)
         message(FATAL_ERROR "Fuzz logging enabled")
     endif()
 
     set(CMAKE_CXX_CLANG_TIDY clang-tidy -checks=-*,bugprone-*,cert-*,clang-analyzer-*,-cert-err58-cpp,misc-*)
 
-    if ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang")
+    if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang")
         # require at least clang 3.2
-        if (CMAKE_CXX_COMPILER_VERSION VERSION_LESS 10.0)
+        if(CMAKE_CXX_COMPILER_VERSION VERSION_LESS 10.0)
             message(FATAL_ERROR "Clang version must be at least 10.0!")
         endif()
     else()
         message(FATAL_ERROR
-                "You are using an unsupported compiler! Fuzzing only works with Clang 10.\n"
-                "1. Install clang-10 \n"
-                "2. Pass -DCMAKE_C_COMPILER=clang-10 -DCMAKE_CXX_COMPILER=clang++-10")
+            "You are using an unsupported compiler! Fuzzing only works with Clang 10.\n"
+            "1. Install clang-10 \n"
+            "2. Pass -DCMAKE_C_COMPILER=clang-10 -DCMAKE_CXX_COMPILER=clang++-10")
     endif()
 
     string(APPEND CMAKE_C_FLAGS " -fsanitize=fuzzer-no-link")
@@ -75,102 +75,103 @@ endif()
 include(cmake/conan/CMakeLists.txt)
 add_subdirectory(cmake/gtest)
 
-set (RETRIEVE_MAJOR_CMD
-        "cat ${CMAKE_CURRENT_SOURCE_DIR}/app/Makefile.version | grep APPVERSION_M | cut -b 14- | tr -d '\n'"
+set(RETRIEVE_MAJOR_CMD
+    "cat ${CMAKE_CURRENT_SOURCE_DIR}/app/Makefile.version | grep APPVERSION_M | cut -b 14- | tr -d '\n'"
 )
-set (RETRIEVE_MINOR_CMD
-        "cat ${CMAKE_CURRENT_SOURCE_DIR}/app/Makefile.version | grep APPVERSION_N | cut -b 14- | tr -d '\n'"
+set(RETRIEVE_MINOR_CMD
+    "cat ${CMAKE_CURRENT_SOURCE_DIR}/app/Makefile.version | grep APPVERSION_N | cut -b 14- | tr -d '\n'"
 )
 execute_process(
-        COMMAND bash "-c" ${RETRIEVE_MAJOR_CMD}
-        RESULT_VARIABLE MAJOR_RESULT
-        OUTPUT_VARIABLE MAJOR_VERSION
+    COMMAND bash "-c" ${RETRIEVE_MAJOR_CMD}
+    RESULT_VARIABLE MAJOR_RESULT
+    OUTPUT_VARIABLE MAJOR_VERSION
 )
 execute_process(
-        COMMAND bash "-c" ${RETRIEVE_MINOR_CMD}
-        RESULT_VARIABLE MINOR_RESULT
-        OUTPUT_VARIABLE MINOR_VERSION
+    COMMAND bash "-c" ${RETRIEVE_MINOR_CMD}
+    RESULT_VARIABLE MINOR_RESULT
+    OUTPUT_VARIABLE MINOR_VERSION
 )
 
-message(STATUS "LEDGER_MAJOR_VERSION [${MAJOR_RESULT}]: ${MAJOR_VERSION}" )
-message(STATUS "LEDGER_MINOR_VERSION [${MINOR_RESULT}]: ${MINOR_VERSION}" )
+message(STATUS "LEDGER_MAJOR_VERSION [${MAJOR_RESULT}]: ${MAJOR_VERSION}")
+message(STATUS "LEDGER_MINOR_VERSION [${MINOR_RESULT}]: ${MINOR_VERSION}")
 
 add_definitions(
     -DLEDGER_MAJOR_VERSION=${MAJOR_VERSION}
     -DLEDGER_MINOR_VERSION=${MINOR_VERSION}
 )
 
-
-##############################################################
-##############################################################
-#  static libs
+# #############################################################
+# #############################################################
+# static libs
 file(GLOB_RECURSE LIB_SRC
-        ${CMAKE_CURRENT_SOURCE_DIR}/deps/ledger-zxlib/src/hexutils.c
-        ${CMAKE_CURRENT_SOURCE_DIR}/deps/ledger-zxlib/src/app_mode.c
-        ${CMAKE_CURRENT_SOURCE_DIR}/deps/ledger-zxlib/src/bignum.c
-        ${CMAKE_CURRENT_SOURCE_DIR}/deps/ledger-zxlib/src/hexutils.c
-        ${CMAKE_CURRENT_SOURCE_DIR}/deps/ledger-zxlib/src/zxmacros.c
-        ${CMAKE_CURRENT_SOURCE_DIR}/deps/ledger-zxlib/src/zxformat.c
-        ${CMAKE_CURRENT_SOURCE_DIR}/deps/ledger-zxlib/src/bech32.c
-        ${CMAKE_CURRENT_SOURCE_DIR}/deps/ledger-zxlib/src/segwit_addr.c
-        ${CMAKE_CURRENT_SOURCE_DIR}/deps/ledger-zxlib/src/timeutils.c
-        ####
-        ${CMAKE_CURRENT_SOURCE_DIR}/app/src/parser.c
-        ${CMAKE_CURRENT_SOURCE_DIR}/app/src/parser_impl.c
-        ${CMAKE_CURRENT_SOURCE_DIR}/app/src/crypto_helper.c
-        ${CMAKE_CURRENT_SOURCE_DIR}/deps/ripemd160/ripemd160.c
-        ${CMAKE_CURRENT_SOURCE_DIR}/app/src/parser_impl_common.c
-        ${CMAKE_CURRENT_SOURCE_DIR}/app/src/parser_print_common.c
-        ${CMAKE_CURRENT_SOURCE_DIR}/app/src/tx_cchain.c
-        ${CMAKE_CURRENT_SOURCE_DIR}/app/src/tx_pchain.c
-        )
+    ${CMAKE_CURRENT_SOURCE_DIR}/deps/ledger-zxlib/src/hexutils.c
+    ${CMAKE_CURRENT_SOURCE_DIR}/deps/ledger-zxlib/src/app_mode.c
+    ${CMAKE_CURRENT_SOURCE_DIR}/deps/ledger-zxlib/src/bignum.c
+    ${CMAKE_CURRENT_SOURCE_DIR}/deps/ledger-zxlib/src/hexutils.c
+    ${CMAKE_CURRENT_SOURCE_DIR}/deps/ledger-zxlib/src/zxmacros.c
+    ${CMAKE_CURRENT_SOURCE_DIR}/deps/ledger-zxlib/src/zxformat.c
+    ${CMAKE_CURRENT_SOURCE_DIR}/deps/ledger-zxlib/src/bech32.c
+    ${CMAKE_CURRENT_SOURCE_DIR}/deps/ledger-zxlib/src/base58.c
+    ${CMAKE_CURRENT_SOURCE_DIR}/deps/ledger-zxlib/src/segwit_addr.c
+    ${CMAKE_CURRENT_SOURCE_DIR}/deps/ledger-zxlib/src/timeutils.c
+
+    # ###
+    ${CMAKE_CURRENT_SOURCE_DIR}/app/src/parser.c
+    ${CMAKE_CURRENT_SOURCE_DIR}/app/src/parser_impl.c
+    ${CMAKE_CURRENT_SOURCE_DIR}/app/src/crypto_helper.c
+    ${CMAKE_CURRENT_SOURCE_DIR}/deps/ripemd160/ripemd160.c
+    ${CMAKE_CURRENT_SOURCE_DIR}/app/src/parser_impl_common.c
+    ${CMAKE_CURRENT_SOURCE_DIR}/app/src/parser_print_common.c
+    ${CMAKE_CURRENT_SOURCE_DIR}/app/src/tx_cchain.c
+    ${CMAKE_CURRENT_SOURCE_DIR}/app/src/tx_pchain.c
+)
 
 add_library(app_lib STATIC ${LIB_SRC})
 
 target_include_directories(app_lib PUBLIC
-        ${CMAKE_CURRENT_SOURCE_DIR}/deps/ledger-zxlib/include
-        ${CMAKE_CURRENT_SOURCE_DIR}/deps/ledger-zxlib/app/common
-        ${CMAKE_CURRENT_SOURCE_DIR}/app/src
-        ${CMAKE_CURRENT_SOURCE_DIR}/app/src/lib
-        ${CMAKE_CURRENT_SOURCE_DIR}/app/src/common
-        ${CMAKE_CURRENT_SOURCE_DIR}/deps/picohash/
-        ${CMAKE_CURRENT_SOURCE_DIR}/deps/ripemd160
-        )
-
-##############################################################
-##############################################################
-#  Tests
+    ${CMAKE_CURRENT_SOURCE_DIR}/deps/ledger-zxlib/include
+    ${CMAKE_CURRENT_SOURCE_DIR}/deps/ledger-zxlib/app/common
+    ${CMAKE_CURRENT_SOURCE_DIR}/app/src
+    ${CMAKE_CURRENT_SOURCE_DIR}/app/src/lib
+    ${CMAKE_CURRENT_SOURCE_DIR}/app/src/common
+    ${CMAKE_CURRENT_SOURCE_DIR}/deps/picohash/
+    ${CMAKE_CURRENT_SOURCE_DIR}/deps/ripemd160
+)
+
+# #############################################################
+# #############################################################
+# Tests
 file(GLOB_RECURSE TESTS_SRC
-        ${CMAKE_CURRENT_SOURCE_DIR}/tests/*.cpp)
+    ${CMAKE_CURRENT_SOURCE_DIR}/tests/*.cpp)
 
 add_executable(unittests ${TESTS_SRC})
 target_include_directories(unittests PRIVATE
-        ${gtest_SOURCE_DIR}/include
-        ${gmock_SOURCE_DIR}/include
-        ${CONAN_INCLUDE_DIRS_FMT}
-        ${CONAN_INCLUDE_DIRS_JSONCPP}
-        ${CMAKE_CURRENT_SOURCE_DIR}/app/src
-        ${CMAKE_CURRENT_SOURCE_DIR}/app/src/lib
-        ${CMAKE_CURRENT_SOURCE_DIR}/deps/ripemd160
-        )
+    ${gtest_SOURCE_DIR}/include
+    ${gmock_SOURCE_DIR}/include
+    ${CONAN_INCLUDE_DIRS_FMT}
+    ${CONAN_INCLUDE_DIRS_JSONCPP}
+    ${CMAKE_CURRENT_SOURCE_DIR}/app/src
+    ${CMAKE_CURRENT_SOURCE_DIR}/app/src/lib
+    ${CMAKE_CURRENT_SOURCE_DIR}/deps/ripemd160
+)
 
 target_link_libraries(unittests PRIVATE
-        gtest_main
-        app_lib
-        CONAN_PKG::fmt
-        CONAN_PKG::jsoncpp)
+    gtest_main
+    app_lib
+    CONAN_PKG::fmt
+    CONAN_PKG::jsoncpp)
 
 add_compile_definitions(TESTVECTORS_DIR="${CMAKE_CURRENT_SOURCE_DIR}/tests/")
 add_test(unittests ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/unittests)
 set_tests_properties(unittests PROPERTIES WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/tests)
 
-##############################################################
-##############################################################
-#  Fuzz Targets
+# #############################################################
+# #############################################################
+# Fuzz Targets
 if(ENABLE_FUZZING)
     set(FUZZ_TARGETS
         parser_parse
-        )
+    )
 
     foreach(target ${FUZZ_TARGETS})
         add_executable(fuzz-${target} ${CMAKE_CURRENT_SOURCE_DIR}/fuzz/${target}.cpp)

diff --git a/app/src/apdu_handler.c b/app/src/apdu_handler.c
@@ -42,7 +42,6 @@ void extractHDPath(uint32_t rx, uint32_t offset) {
 
     memcpy(hdPath, G_io_apdu_buffer + offset, sizeof(uint32_t) * HDPATH_LEN_DEFAULT);
 
-    // #{TODO} --> testnet necessary?
     const bool mainnet = hdPath[0] == HDPATH_0_DEFAULT && hdPath[1] == HDPATH_1_DEFAULT;
 
     if (!mainnet) {
@@ -116,7 +115,7 @@ __Z_INLINE void handleGetAddr(volatile uint32_t *flags, volatile uint32_t *tx, u
 
     const uint8_t requireConfirmation = G_io_apdu_buffer[OFFSET_P1];
 
-    zxerr_t zxerr = app_fill_address();
+    const zxerr_t zxerr = app_fill_address();
     if (zxerr != zxerr_ok) {
         *tx = 0;
         THROW(APDU_CODE_DATA_INVALID);
@@ -139,7 +138,7 @@ __Z_INLINE void handleSign(volatile uint32_t *flags, volatile uint32_t *tx, uint
         THROW(APDU_CODE_OK);
     }
 
-    // GET addres to test later which outputs the app should show
+    // GET address to test later which outputs the app should show
     zxerr_t zxerr = app_get_address();
     if (zxerr != zxerr_ok) {
         *tx = 0;

diff --git a/app/src/common/actions.h b/app/src/common/actions.h
@@ -33,7 +33,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, &action_addrResponseLen);
+    const zxerr_t err = crypto_fillAddress(G_io_apdu_buffer, IO_APDU_BUFFER_SIZE, &action_addrResponseLen);
 
     if (err != zxerr_ok || action_addrResponseLen == 0) {
         THROW(APDU_CODE_EXECUTION_ERROR);
@@ -76,7 +76,7 @@ __Z_INLINE void app_reply_error() {
 __Z_INLINE zxerr_t app_get_address() {
     zemu_log("app_get_address\n");
 
-    zxerr_t err = crypto_get_address();
+    const zxerr_t err = crypto_get_address();
 
     if (err != zxerr_ok) {
         THROW(APDU_CODE_EXECUTION_ERROR);

diff --git a/app/src/parser_impl.c b/app/src/parser_impl.c
@@ -159,7 +159,6 @@ parser_error_t getNumItems(const parser_context_t *ctx, uint8_t *numItems) {
             // Tx + fee + Amounts(= n_outs) + Addresses
             *numItems = 2 + ctx->tx_obj->tx.p_export_tx.secp_outs.n_addrs +
                         parser_get_renderable_outputs_number(ctx->tx_obj->tx.p_export_tx.secp_outs.out_render_mask);
-            ;
             break;
         case p_import_tx:
             // Tx + fee + Amounts(= n_outs) + Addresses

diff --git a/app/src/parser_print_common.c b/app/src/parser_print_common.c
@@ -15,6 +15,7 @@
  ********************************************************************************/
 #include "parser_print_common.h"
 
+#include "base58.h"
 #include "bech32.h"
 #include "parser_common.h"
 #include "timeutils.h"
@@ -147,53 +148,6 @@ parser_error_t printTimestamp(uint64_t timestamp, char *outVal, uint16_t outValL
     return parser_ok;
 }
 
-static parser_error_t bs58_encode(const uint8_t *input, size_t input_len, uint8_t *output, size_t output_len) {
-    if (input == NULL || output == NULL) {
-        return parser_unexpected_error;
-    }
-
-    const size_t alphabet_size = 58;
-    const uint8_t *table = (const uint8_t *)ALPHABET_ENCODE;
-
-    size_t index = 0;
-    for (size_t i = 0; i < input_len; ++i) {
-        size_t carry = input[i];
-        for (size_t j = 0; j < index; ++j) {
-            carry += output[j] << 8;
-            output[j] = carry % alphabet_size;
-            carry /= alphabet_size;
-        }
-        while (carry > 0) {
-            if (index == output_len) {
-                return parser_unexpected_error;  // Output buffer too small
-            }
-            output[index++] = carry % alphabet_size;
-            carry /= alphabet_size;
-        }
-    }
-
-    for (size_t i = 0; i < input_len && input[i] == 0; ++i) {
-        if (index == output_len) {
-            return parser_unexpected_error;  // Output buffer too small
-        }
-        output[index++] = 0;
-    }
-
-    for (size_t i = 0; i < index; ++i) {
-        output[i] = table[output[i]];
-    }
-
-    // Reverse the output
-    for (size_t i = 0, j = index - 1; i < j; ++i, --j) {
-        uint8_t temp = output[i];
-        output[i] = output[j];
-        output[j] = temp;
-    }
-
-    return parser_ok;
-    ;
-}
-
 parser_error_t printNodeId(const uint8_t *nodeId, char *outVal, uint16_t outValLen, uint8_t pageIdx, uint8_t *pageCount) {
     if (nodeId == NULL) {
         return parser_unexpected_error;
@@ -223,11 +177,10 @@ parser_error_t printNodeId(const uint8_t *nodeId, char *outVal, uint16_t outValL
 
     // Prepare node_id by appending prefix
     uint8_t node_id[NODE_ID_MAX_SIZE] = {0};
+    size_t outlen = sizeof(node_id) - sizeof(prefix) + 1;
     memcpy(node_id, prefix, sizeof(prefix) - 1);
 
-    CHECK_ERROR(bs58_encode((const uint8_t *)data, sizeof(data), node_id + sizeof(prefix) - 1,
-                            sizeof(node_id) - sizeof(prefix) + 1));
-
+    CHECK_ERROR(encode_base58((const unsigned char *)data, sizeof(data), node_id + sizeof(prefix) - 1, &outlen))
     pageString(outVal, outValLen, (const char *)node_id, pageIdx, pageCount);
 
     return parser_ok;