Merge pull request #262 from Zondax/dev

Fix secp256k1 for long transactions (#261)

Author: abenso

.gitignore

@@ -78,3 +78,15 @@ fuzz-*.log
 /scan-build
 
 .DS_Store
+
+# Additional fuzzing artifacts
+fuzz/corpora/
+fuzz/build/
+fuzz/coverage/
+fuzz/logs/
+fuzz-*.log
+
+# Python bytecode
+__pycache__/
+*.pyc
+*.pyo

CMakeLists.txt

@@ -30,6 +30,10 @@ endif()
 ########################################################
 
 project(ledger-polkadot VERSION 0.0.0)
+
+# Include common fuzzing configuration
+include(deps/ledger-zxlib/fuzzing/FuzzingCommon.cmake)
+
 set(CMAKE_CXX_STANDARD 17)
 cmake_policy(SET CMP0025 NEW)
 cmake_policy(SET CMP0144 NEW)
@@ -39,10 +43,6 @@ set(HUNTER_TLS_VERIFY OFF)
 
 enable_testing()
 
-option(ENABLE_FUZZING "Enable fuzzing instrumentation and build fuzz targets" OFF)
-option(ENABLE_COVERAGE "Enable source code coverage instrumentation" OFF)
-option(ENABLE_SANITIZERS "Enable ASAN and UBSAN" OFF)
-
 string(APPEND CMAKE_C_FLAGS " -fno-omit-frame-pointer -g")
 string(APPEND CMAKE_CXX_FLAGS " -fno-omit-frame-pointer -g")
 string(APPEND CMAKE_LINKER_FLAGS " -fno-omit-frame-pointer -g")
@@ -64,46 +64,7 @@ FetchContent_MakeAvailable(nlohmann_json)
 hunter_add_package(GTest)
 find_package(GTest CONFIG REQUIRED)
 
-if(ENABLE_FUZZING)
-        add_definitions(-DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION=1)
-        SET(ENABLE_SANITIZERS ON CACHE BOOL "Sanitizer automatically enabled" FORCE)
-        SET(CMAKE_BUILD_TYPE Debug)
-
-        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_VERSION VERSION_LESS 10.0)
-                        message(FATAL_ERROR "Clang version must be at least 10.0!")
-                endif()
-        else()
-                message(FATAL_ERROR
-                        "Unsupported compiler! Fuzzing requires Clang 10.\n"
-                        "1. Install clang-10 \n"
-                        "2. Use -DCMAKE_C_COMPILER=clang-10 -DCMAKE_CXX_COMPILER=clang++-10")
-        endif()
-
-        string(APPEND CMAKE_C_FLAGS " -fsanitize=fuzzer-no-link")
-        string(APPEND CMAKE_CXX_FLAGS " -fsanitize=fuzzer-no-link")
-        string(APPEND CMAKE_LINKER_FLAGS " -fsanitize=fuzzer-no-link")
-endif()
-
-if(ENABLE_COVERAGE)
-        string(APPEND CMAKE_C_FLAGS " -fprofile-instr-generate -fcoverage-mapping")
-        string(APPEND CMAKE_CXX_FLAGS " -fprofile-instr-generate -fcoverage-mapping")
-        string(APPEND CMAKE_LINKER_FLAGS " -fprofile-instr-generate -fcoverage-mapping")
-endif()
-
-if(ENABLE_SANITIZERS)
-        string(APPEND CMAKE_C_FLAGS " -fsanitize=address,undefined -fsanitize-recover=address,undefined")
-        string(APPEND CMAKE_CXX_FLAGS " -fsanitize=address,undefined -fsanitize-recover=address,undefined")
-        string(APPEND CMAKE_LINKER_FLAGS " -fsanitize=address,undefined -fsanitize-recover=address,undefined")
-endif()
-
+# Fuzzing configuration is now handled by FuzzingCommon.cmake
 set(RETRIEVE_MAJOR_CMD
         "cat ${CMAKE_CURRENT_SOURCE_DIR}/app/Makefile.version | grep APPVERSION_M | cut -b 14- | tr -d '\n'"
 )
@@ -171,25 +132,24 @@ target_include_directories(app_lib PUBLIC
 ##############################################################
 ##############################################################
 #  Fuzz Targets
-if(ENABLE_FUZZING)
-        set(FUZZ_TARGETS
-                parser_parse
-                metadata_parser_array
-                metadata_parser_composite
-                metadata_parser_primitives
-                metadata_parser_sequence
-                metadata_parser_variant
-        )
-
-        foreach(target ${FUZZ_TARGETS})
-                add_executable(fuzz-${target} ${CMAKE_CURRENT_SOURCE_DIR}/fuzz/${target}.cpp)
-                target_link_libraries(fuzz-${target} PRIVATE app_lib)
-                target_link_options(fuzz-${target} PRIVATE "-fsanitize=fuzzer")
-        endforeach()
+if(ENABLE_FUZZING) # Fuzz Targets
+    # Setup fuzzing directories
+    setup_fuzz_directories()
+    
+    # Add fuzz targets using the common macro
+    add_fuzz_target(parser_parse ${CMAKE_CURRENT_SOURCE_DIR}/fuzz/parser_parse.cpp app_lib)
+    add_fuzz_target(metadata_parser_composite ${CMAKE_CURRENT_SOURCE_DIR}/fuzz/metadata_parser_composite.cpp app_lib)
+    add_fuzz_target(metadata_parser_sequence ${CMAKE_CURRENT_SOURCE_DIR}/fuzz/metadata_parser_sequence.cpp app_lib)
+    add_fuzz_target(metadata_parser_array ${CMAKE_CURRENT_SOURCE_DIR}/fuzz/metadata_parser_array.cpp app_lib)
+    add_fuzz_target(metadata_parser_tuple ${CMAKE_CURRENT_SOURCE_DIR}/fuzz/metadata_parser_tuple.cpp app_lib)
+    add_fuzz_target(metadata_parser_primitives ${CMAKE_CURRENT_SOURCE_DIR}/fuzz/metadata_parser_primitives.cpp app_lib)
+    add_fuzz_target(metadata_parser_variant ${CMAKE_CURRENT_SOURCE_DIR}/fuzz/metadata_parser_variant.cpp app_lib)
+    
 else()
-#############################################################
-# Tests
-        file(GLOB_RECURSE TESTS_SRC
+    # #############################################################
+    # #############################################################
+    # Tests
+            file(GLOB_RECURSE TESTS_SRC
                 ${CMAKE_CURRENT_SOURCE_DIR}/tests/*.cpp)
 
         add_executable(unittests ${TESTS_SRC})
@@ -208,4 +168,4 @@ else()
         add_compile_definitions(TESTVECTORS_DIR="${CMAKE_CURRENT_SOURCE_DIR}/tests/")
         add_test(NAME unittests COMMAND unittests)
         set_tests_properties(unittests PROPERTIES WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/tests)
-endif()
+endif()

app/Makefile.version

@@ -3,4 +3,4 @@ APPVERSION_M=100
 # This is the `spec_version` field of `Runtime`
 APPVERSION_N=0
 # This is the patch version of this release
-APPVERSION_P=17
+APPVERSION_P=18

app/src/crypto.c

@@ -155,7 +155,23 @@ zxerr_t crypto_sign_secp256k1(
 
     // Hash the message
     uint8_t messageDigest[CX_KECCAK_256_SIZE] = {0};
-    CHECK_CX_OK(cx_keccak_256_hash(message, messageLen, messageDigest));
+    zxerr_t error = zxerr_unknown;
+
+    // When the payload is bigger than MAX_SIGN_SIZE, it needs to be hashed with blake2b_256 before hashing with keccak_256.
+    // https://github.com/paritytech/polkadot-sdk/blob/1a512570552119a49a8ecb2abfb7021954c4422d/substrate/primitives/runtime/src/generic/unchecked_extrinsic.rs#L567
+    if (messageLen > MAX_SIGN_SIZE) {
+        uint8_t intermediate_digest[BLAKE2B_DIGEST_SIZE];
+        // Hash it with blake2b
+        cx_blake2b_t ctx;
+        CATCH_CXERROR(cx_blake2b_init_no_throw(&ctx, 256));
+        CATCH_CXERROR(cx_hash_no_throw(&ctx.header, CX_LAST, message, messageLen, intermediate_digest, BLAKE2B_DIGEST_SIZE));
+
+        // Then hash with keccak_256
+        CHECK_CX_OK(cx_keccak_256_hash(intermediate_digest, BLAKE2B_DIGEST_SIZE, messageDigest));
+    } else {
+        // Mandatory, hash the message with keccak_256
+        CHECK_CX_OK(cx_keccak_256_hash(message, messageLen, messageDigest));
+    }
 
     cx_ecfp_private_key_t cx_privateKey;
     uint8_t privateKeyData[SECP256K1_PK_LEN_UNCOMPRESSED - 1] = {0};
@@ -164,7 +180,6 @@ zxerr_t crypto_sign_secp256k1(
     *sigSize = 0;
 
     signature_t *const signature = (signature_t *)output;
-    zxerr_t error = zxerr_unknown;
 
     CATCH_CXERROR(os_derive_bip32_with_seed_no_throw(HDW_NORMAL, CX_CURVE_SECP256K1, hdPath, HDPATH_LEN_DEFAULT,
                                                      privateKeyData, NULL, NULL, 0));

app/src/metadata_reader.c

@@ -335,9 +335,21 @@ static parser_error_t readMerkleIndices(parser_context_t *ctx, MerkleIndices_t *
     }
 
     CHECK_ERROR(readCompactU32(ctx, &indices->entries));
+
+    // Check for overflow when computing buffer length
+    // Each index is 4 bytes (u32), so total size = 4 * entries
+    // First check if multiplication would overflow uint32_t
+    if (indices->entries > UINT32_MAX / 4) {
+        return parser_value_out_of_range;
+    }
+    const uint32_t totalSize = 4 * indices->entries;
+    if (totalSize > UINT16_MAX) {
+        return parser_value_out_of_range;
+    }
+
     indices->indices.buffer = ctx->buffer + ctx->offset;
     indices->indices.offset = 0;
-    indices->indices.bufferLen = 4 * indices->entries;
+    indices->indices.bufferLen = (uint16_t)totalSize;
 
     uint32_t tmpIndex = 0;
     for (uint32_t idx = 0; idx < indices->entries; idx++) {
@@ -360,9 +372,20 @@ static parser_error_t readMerkleLemmas(parser_context_t *ctx, MerkleLemmas_t *le
 
     CHECK_ERROR(readCompactU32(ctx, &lemmas->entries));
 
+    // Check for overflow when computing buffer length
+    // Each lemma is 32 bytes, so total size = 32 * entries
+    // First check if multiplication would overflow uint32_t
+    if (lemmas->entries > UINT32_MAX / 32) {
+        return parser_value_out_of_range;
+    }
+    const uint32_t totalSize = 32 * lemmas->entries;
+    if (totalSize > UINT16_MAX) {
+        return parser_value_out_of_range;
+    }
+
     lemmas->lemmas.buffer = ctx->buffer + ctx->offset;
     lemmas->lemmas.offset = 0;
-    lemmas->lemmas.bufferLen = 32 * lemmas->entries;
+    lemmas->lemmas.bufferLen = (uint16_t)totalSize;
     for (uint32_t idx = 0; idx < lemmas->entries; idx++) {
         CTX_CHECK_AND_ADVANCE(ctx, 32);  // lemmas are [u8;32]
     }

deps/BLAKE3-c/blake3_portable.c

@@ -1,10 +1,24 @@
 #include "blake3_impl.h"
 #include <string.h>
 
+// Disable UndefinedBehaviorSanitizer for BLAKE3 cryptographic operations
+// Unsigned integer overflow is intentional (modular arithmetic)
+#if defined(__has_attribute)
+#if __has_attribute(no_sanitize)
+#define NO_SANITIZE_UNSIGNED __attribute__((no_sanitize("unsigned-integer-overflow", "unsigned-shift-base")))
+#else
+#define NO_SANITIZE_UNSIGNED
+#endif
+#else
+#define NO_SANITIZE_UNSIGNED
+#endif
+
+NO_SANITIZE_UNSIGNED
 INLINE uint32_t rotr32(uint32_t w, uint32_t c) {
   return (w >> c) | (w << (32 - c));
 }
 
+NO_SANITIZE_UNSIGNED
 INLINE void g(uint32_t *state, size_t a, size_t b, size_t c, size_t d,
               uint32_t x, uint32_t y) {
   state[a] = state[a] + state[b] + x;
@@ -17,6 +31,7 @@ INLINE void g(uint32_t *state, size_t a, size_t b, size_t c, size_t d,
   state[b] = rotr32(state[b] ^ state[c], 7);
 }
 
+NO_SANITIZE_UNSIGNED
 INLINE void round_fn(uint32_t state[16], const uint32_t *msg, size_t round) {
   // Select the message schedule based on the round.
   const uint8_t *schedule = MSG_SCHEDULE[round];
@@ -34,6 +49,7 @@ INLINE void round_fn(uint32_t state[16], const uint32_t *msg, size_t round) {
   g(state, 3, 4, 9, 14, msg[schedule[14]], msg[schedule[15]]);
 }
 
+NO_SANITIZE_UNSIGNED
 INLINE void compress_pre(uint32_t state[16], const uint32_t cv[8],
                          const uint8_t block[BLAKE3_BLOCK_LEN],
                          uint8_t block_len, uint64_t counter, uint8_t flags) {
@@ -81,6 +97,7 @@ INLINE void compress_pre(uint32_t state[16], const uint32_t cv[8],
   round_fn(state, &block_words[0], 6);
 }
 
+NO_SANITIZE_UNSIGNED
 void blake3_compress_in_place_portable(uint32_t cv[8],
                                        const uint8_t block[BLAKE3_BLOCK_LEN],
                                        uint8_t block_len, uint64_t counter,
@@ -97,6 +114,7 @@ void blake3_compress_in_place_portable(uint32_t cv[8],
   cv[7] = state[7] ^ state[15];
 }
 
+NO_SANITIZE_UNSIGNED
 void blake3_compress_xof_portable(const uint32_t cv[8],
                                   const uint8_t block[BLAKE3_BLOCK_LEN],
                                   uint8_t block_len, uint64_t counter,

deps/ledger-zxlib

@@ -23,16 +23,28 @@ You can get pre-releases in two ways (*THESE ARE UNVETTED DEVELOPMENT RELEASES*)
 - Install Docker CE
     - Instructions can be found here: https://docs.docker.com/install/
 
-- We typically use Ubuntu or MacOS. Install the following packages:
+- We typically use Ubuntu. Install the following packages:
    ```sh
    sudo apt update && apt-get -y install build-essential git wget cmake \
   libssl-dev libgmp-dev autoconf libtool
    ```
 
+  Or for MacOS, install the following:
+   ```sh
+    brew install python@3.11 cmake
+    echo 'export PATH="$PATH:/opt/homebrew/opt/python@3.11/libexec/bin"' >> .zprofile
+    source ~/.zshrc
+   ```
+
 - Install [poetry](https://python-poetry.org/docs/#installing-with-the-official-installer). This simplifies python environments
 
     ```sh
-    https://python-poetry.org/docs/#installing-with-the-official-installer
+    curl -sSL https://install.python-poetry.org | python3 -
+    ```
+
+    After installing Poetry, restart your shell or run:
+    ```sh
+    source ~/.bashrc  # or ~/.zshrc on MacOS
     ```
 
 - Install [volta](https://docs.volta.sh/guide/getting-started)
@@ -47,6 +59,7 @@ You can get pre-releases in two ways (*THESE ARE UNVETTED DEVELOPMENT RELEASES*)
     ```
 
 - You will need python 3 and then run
+    - `poetry shell`
     - `make deps`
 
     - The current repository keeps track of Ledger's SDK but it is possible to override it by changing the git submodule.

docs/build.md

@@ -0,0 +1,45 @@
+#!/usr/bin/env python3
+"""
+Project-specific fuzzing configuration
+"""
+
+import sys
+import os
+
+# Configuration constants
+MAX_SECONDS = 3600  # 1 hour for quick test
+FUZZER_JOBS = 8  # Number of parallel jobs for fuzzing
+
+# Add the common fuzzing module to path
+sys.path.append(os.path.join(os.path.dirname(__file__), "..", "deps", "ledger-zxlib", "fuzzing"))
+
+try:
+    from run_fuzzers import FuzzConfig
+except ImportError:
+    # Fallback if common module is not available
+    class FuzzConfig:
+        def __init__(self, name: str, max_len: int = 17000):
+            self.name = name
+            self.max_len = max_len
+
+
+def get_fuzzer_configs():
+    """Get project-specific fuzzer configurations"""
+    return [
+        FuzzConfig(name="metadata_parser_array", max_len=17000),  # Tests metadata_parser_array functionality
+        FuzzConfig(name="metadata_parser_composite", max_len=17000),  # Tests metadata_parser_composite functionality
+        FuzzConfig(name="metadata_parser_primitives", max_len=17000),  # Tests metadata_parser_primitives functionality
+        FuzzConfig(name="metadata_parser_sequence", max_len=17000),  # Tests metadata_parser_sequence functionality
+        FuzzConfig(name="metadata_parser_variant", max_len=17000),  # Tests metadata_parser_variant functionality
+        FuzzConfig(name="metadata_parser_tuple", max_len=17000),  # Tests metadata_parser_variant functionality
+        FuzzConfig(name="parser_parse", max_len=17000),  # Tests parser_parse functionality
+        # Add more fuzzers here as needed
+    ]
+
+
+if __name__ == "__main__":
+    # Show available configurations
+    configs = get_fuzzer_configs()
+    print("Available fuzzer configurations:")
+    for config in configs:
+        print(f"  - {config.name}: max_len={config.max_len}")