Merge pull request #34 from Zondax/fuzzing

Adding fuzzing targets + fixes
Zondax · Aug 13, 2020 · af8048a · af8048a
2 parents 67e6479 + 2b7f1c7
commit af8048a
Show file tree

Hide file tree

Showing 20 changed files with 552 additions and 113 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -2,7 +2,7 @@ version: 2
 jobs:
   build:
     docker:
-      - image: zondax/circleci:latest
+      - image: zondax/circleci@sha256:37f78ab294b35a055768c2305b3f13813e55fb9db4e65f72745ede61dd842c08
     steps:
       - checkout
       - run: git submodule update --init --recursive
@@ -13,7 +13,7 @@ jobs:
 
   build_ledger:
     docker:
-      - image: zondax/builder-bolos:latest
+      - image: zondax/builder-bolos@sha256:5af9542b68b92c12c5c6ae5bd862ff9dbcce063b38755fe9d8153175f6a53338
     environment:
       - BOLOS_SDK=/home/zondax/project/deps/nanos-secure-sdk
       - BOLOS_ENV=/opt/bolos
@@ -28,6 +28,26 @@ jobs:
             cd /home/zondax/project
             make
 
+  test_fuzz_crash_fixes:
+    docker:
+      - image: zondax/circleci@sha256:37f78ab294b35a055768c2305b3f13813e55fb9db4e65f72745ede61dd842c08
+    steps:
+      - checkout
+      - run: git submodule update --init --recursive
+      - run: sudo apt update && sudo apt -y install clang-10
+      - run:
+          name: Build
+          command: |
+            cmake -B build \
+                    -DCMAKE_C_COMPILER=clang-10 \
+                    -DCMAKE_CXX_COMPILER=clang++-10 \
+                    -DCMAKE_BUILD_TYPE=Debug \
+                    -DENABLE_FUZZING=1 \
+                    -DENABLE_SANITIZERS=1 \
+                    .
+            make -C build
+      - run: ./run-fuzz-crashes.py
+
   build_ledger_ledgeracio:
     docker:
       - image: zondax/builder-bolos:latest
@@ -48,12 +68,14 @@ jobs:
   test_zemu:
     machine:
       image: ubuntu-1604:201903-01
+    resource_class: large
     working_directory: ~/repo
     environment:
       BASH_ENV: "/opt/circleci/.nvm/nvm.sh"
     steps:
       - checkout
       - run: git submodule update --init --recursive
+      - run: sudo apt-get update -y && sudo apt-get install -y libusb-1.0.0 libudev-dev
       - run:
           name: Build Ledger app
           command: |
@@ -108,7 +130,7 @@ jobs:
 
   build_package:
     docker:
-      - image: zondax/builder-bolos:latest
+      - image: zondax/builder-bolos@sha256:5af9542b68b92c12c5c6ae5bd862ff9dbcce063b38755fe9d8153175f6a53338
     environment:
       - BOLOS_SDK=/home/zondax/project/deps/nanos-secure-sdk
       - BOLOS_ENV=/opt/bolos

diff --git a/.gitignore b/.gitignore
@@ -79,3 +79,6 @@ cmake-build-fuzz/
 
 tests_zemu/yarn.lock
 /tests_zemu/snapshots-tmp/
+/build
+/fuzz-*.log
+/fuzz/corpora
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -20,15 +20,53 @@ enable_testing()
 cmake_policy(SET CMP0025 NEW)
 set(CMAKE_CXX_STANDARD 11)
 
-include(cmake/conan/CMakeLists.txt)
-add_subdirectory(cmake/gtest)
+option(ENABLE_FUZZING "Build with fuzzing instrumentation and build fuzz targets" OFF)
+option(ENABLE_COVERAGE "Build with source code coverage instrumentation" OFF)
+option(ENABLE_SANITIZERS "Build with ASAN and UBSAN" OFF)
 
-string(APPEND CMAKE_C_FLAGS " -fsanitize=address -fno-omit-frame-pointer -Wno-extern-c-compat")
-string(APPEND CMAKE_CXX_FLAGS " -fsanitize=address -fno-omit-frame-pointer -Wno-extern-c-compat")
-string(APPEND CMAKE_LINKER_FLAGS " -fsanitize=address -fno-omit-frame-pointer")
+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")
 
 add_definitions(-DAPP_STANDARD)
 
+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 ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang")
+        # require at least clang 3.2
+        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")
+    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()
+
+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'"
 )
@@ -65,6 +103,7 @@ file(GLOB_RECURSE LIB_SRC
         ${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/zbuffer.c
+        ${CMAKE_CURRENT_SOURCE_DIR}/deps/ledger-zxlib/src/zxformat.c
         ####
         ${CMAKE_CURRENT_SOURCE_DIR}/app/src/crypto.c
         ${CMAKE_CURRENT_SOURCE_DIR}/app/src/parser.c
@@ -79,8 +118,8 @@ target_include_directories(app_lib PUBLIC
         ${CMAKE_CURRENT_SOURCE_DIR}/deps/BLAKE2/ref
         ${CMAKE_CURRENT_SOURCE_DIR}/deps/ledger-zxlib/include
         ${CMAKE_CURRENT_SOURCE_DIR}/app/src
-        ${CMAKE_CURRENT_SOURCE_DIR}/app/src/common
         ${CMAKE_CURRENT_SOURCE_DIR}/app/src/lib
+        ${CMAKE_CURRENT_SOURCE_DIR}/app/src/common
         )
 
 ##############################################################
@@ -107,3 +146,18 @@ target_link_libraries(unittests PRIVATE
 
 add_test(unittests ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/unittests)
 set_tests_properties(unittests PROPERTIES WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/tests)
+
+##############################################################
+##############################################################
+#  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)
+        target_link_libraries(fuzz-${target} PRIVATE app_lib)
+        target_link_options(fuzz-${target} PRIVATE "-fsanitize=fuzzer")
+    endforeach()
+endif()
diff --git a/app/Makefile.version b/app/Makefile.version
@@ -1,3 +1,3 @@
 APPVERSION_M=2
 APPVERSION_N=2019
-APPVERSION_P=2
+APPVERSION_P=3
diff --git a/app/src/parser.c b/app/src/parser.c
@@ -83,6 +83,18 @@ parser_error_t parser_validate_vecLookupSource(pd_VecLookupSource_t *targets) {
 #endif
 
 parser_error_t parser_validate(const parser_context_t *ctx) {
+    // Iterate through all items to check that all can be shown and are valid
+    uint8_t numItems = 0;
+    CHECK_PARSER_ERR(parser_getNumItems(ctx, &numItems));
+
+    char tmpKey[40];
+    char tmpVal[40];
+
+    for (uint8_t idx = 0; idx < numItems; idx++) {
+        uint8_t pageCount = 0;
+        CHECK_PARSER_ERR(parser_getItem(ctx, idx, tmpKey, sizeof(tmpKey), tmpVal, sizeof(tmpVal), 0, &pageCount))
+    }
+
 #if defined(APP_RESTRICTED)
     if (hdPath[2] == HDPATH_2_STASH) {
         if (ctx->tx_obj->callIndex.moduleIdx == PD_CALL_STAKING) {

diff --git a/app/src/parser_impl.c b/app/src/parser_impl.c
@@ -190,65 +190,31 @@ parser_error_t _toStringCompactInt(const compactInt_t *c,
     if (c->len <= 4) {
         uint64_t v;
         _getValue(c, &v);
-
-        if (decimalPlaces == 0) {
-            if (uint64_to_str(bufferUI, sizeof(bufferUI), v) != NULL){
-                return parser_unexpected_value;
-            }
-        } else {
-            if (fpuint64_to_str(bufferUI, sizeof(bufferUI), v, decimalPlaces) == 0) {
-                return parser_unexpected_value;
-            }
-        }
-
-        // Add postfix
-        if (postfix > 32 && postfix < 127) {
-            const uint16_t p = strlen(bufferUI);
-            bufferUI[p] = postfix;
+        if (uint64_to_str(bufferUI, sizeof(bufferUI), v) != NULL) {
+            return parser_unexpected_value;
         }
-
-        pageString(outValue, outValueLen, bufferUI, pageIdx, pageCount);
     } else {
+        // This is longer number
         uint8_t bcdOut[100];
         const uint16_t bcdOutLen = sizeof(bcdOut);
-
         bignumLittleEndian_to_bcd(bcdOut, bcdOutLen, c->ptr + 1, c->len - 1);
         if (!bignumLittleEndian_bcdprint(bufferUI, sizeof(bufferUI), bcdOut, bcdOutLen))
             return parser_unexpected_buffer_end;
+    }
 
-        if (decimalPlaces > 0) {
-            uint16_t numChars = strlen(bufferUI);
-
-//        0123456789012     <-decimal places
-//        abcd              < numChars = 4
-//                 abcd     < shift
-//        000000000abcd     < fill
-//        0.00000000abcd    < add decimal point
-
-            if (numChars < decimalPlaces) {
-                for (uint16_t j = 0; j < numChars; j++) {
-                    bufferUI[decimalPlaces + 1 - j] = bufferUI[numChars - 1 - j];
-                }
-                MEMSET(bufferUI, '0', decimalPlaces - numChars + 1);
-                numChars = strlen(bufferUI);
-            }
-
-            // add decimal point
-            for (uint16_t j = 0; j < decimalPlaces + 1; j++) {
-                bufferUI[numChars + 1 - j] = bufferUI[numChars - j];
-            }
-            bufferUI[numChars - decimalPlaces] = '.';
-        }
-
-        // Add postfix
-        if (postfix > 32 && postfix < 127) {
-            const uint16_t p = strlen(bufferUI);
-            bufferUI[p] = postfix;
-        }
+    // Format number
+    if (intstr_to_fpstr_inplace(bufferUI, sizeof(bufferUI), decimalPlaces) == 0){
+        return parser_unexpected_value;
+    }
 
-        pageString(outValue, outValueLen, bufferUI, pageIdx, pageCount);
+    // Add postfix
+    if (postfix > 32 && postfix < 127) {
+        const uint16_t p = strlen(bufferUI);
+        bufferUI[p] = postfix;
     }
 
+    pageString(outValue, outValueLen, bufferUI, pageIdx, pageCount);
+
     return parser_ok;
 }
 
@@ -346,17 +312,17 @@ parser_error_t _checkVersions(parser_context_t *c) {
 
     uint8_t *p = (uint8_t *) (c->buffer + c->bufferLen - specOffsetFromBack);
     uint32_t specVersion = 0;
-    specVersion += p[0] << 0u;
-    specVersion += p[1] << 8u;
-    specVersion += p[2] << 16u;
-    specVersion += p[3] << 24u;
+    specVersion += (uint32_t) p[0] << 0u;
+    specVersion += (uint32_t) p[1] << 8u;
+    specVersion += (uint32_t) p[2] << 16u;
+    specVersion += (uint32_t) p[3] << 24u;
 
     p += 4;
     uint32_t transactionVersion = 0;
-    transactionVersion += p[0] << 0u;
-    transactionVersion += p[1] << 8u;
-    transactionVersion += p[2] << 16u;
-    transactionVersion += p[3] << 24u;
+    transactionVersion += (uint32_t) p[0] << 0u;
+    transactionVersion += (uint32_t) p[1] << 8u;
+    transactionVersion += (uint32_t) p[2] << 16u;
+    transactionVersion += (uint32_t) p[3] << 24u;
 
     if (specVersion < SUPPORTED_MINIMUM_SPEC_VERSION) {
         return parser_spec_not_supported;

diff --git a/app/src/parser_impl.h b/app/src/parser_impl.h
@@ -42,7 +42,8 @@ extern "C" {
     if (v == NULL) { return parser_no_data; } \
     CTX_CHECK_AVAIL(c, 1) // Checks that there is something available in the buffer
 
-#define CLEAN_AND_CHECK() MEMZERO(outValue, outValueLen);  \
+#define CLEAN_AND_CHECK() \
+    MEMZERO(outValue, outValueLen);  \
     if (v == NULL) { *pageCount = 0; return parser_no_data; }
 
 #define GEN_DEF_READARRAY(SIZE) \
@@ -51,24 +52,21 @@ extern "C" {
     return parser_ok;
 
 #define GEN_DEF_TOSTRING_ARRAY(SIZE) \
-    CLEAN_AND_CHECK();\
-    if (v->_ptr == NULL) return parser_unexpected_buffer_end; \
-    const uint16_t outLenNormalized = ((outValueLen - 1u) >> 1u);\
-    const uint16_t pageOffset = pageIdx * outLenNormalized;\
-    *pageCount = SIZE / outLenNormalized;    \
-    if (SIZE % outLenNormalized != 0)    \
-    (*pageCount)++; \
-    uint16_t loopmax = outLenNormalized;    \
-    if (loopmax > SIZE - pageOffset) { \
-        loopmax = SIZE - pageOffset; \
-    };\
-    for (uint16_t i = 0; i < loopmax; i++) {\
-        const uint16_t offset = i << 1u;\
-        snprintf(outValue + offset,\
-        outValueLen - offset,\
-        "%02x", *(v->_ptr + pageOffset + i));\
-    }\
+    CLEAN_AND_CHECK();                                                                  \
+    if (v->_ptr == NULL || outValueLen == 0 ) return parser_unexpected_buffer_end;      \
+    const uint16_t outLenNormalized = (outValueLen - 1) / 2;                            \
+    *pageCount = SIZE / outLenNormalized;                                               \
+    if (SIZE % outLenNormalized != 0) *pageCount+=1;                                    \
+    const uint16_t pageOffset = pageIdx * outLenNormalized;                             \
+    uint16_t loopmax = outLenNormalized;                                                \
+    if (loopmax > SIZE - pageOffset)  loopmax = SIZE - pageOffset;                      \
+    for (uint16_t i = 0; i < loopmax; i++) {                                            \
+        const uint16_t offset = i << 1u;                                                \
+        const uint8_t *c = v->_ptr + pageOffset;                                        \
+        snprintf(outValue + offset, outValueLen - offset, "%02x", c[i]);                \
+    }                                                                                   \
     return parser_ok;
+
 #define GEN_DEC_READFIX_UNSIGNED(BITS) parser_error_t _readUInt ## BITS(parser_context_t *ctx, uint ## BITS ##_t *value)
 #define GEN_DEF_READFIX_UNSIGNED(BITS) parser_error_t _readUInt ## BITS(parser_context_t *ctx, uint ## BITS ##_t *value) \
 {                                                                                           \

diff --git a/app/src/substrate_methods.h b/app/src/substrate_methods.h
@@ -13,6 +13,8 @@
 *  See the License for the specific language governing permissions and
 *  limitations under the License.
 ********************************************************************************/
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wextern-c-compat"
 #pragma once
 
 #ifdef __cplusplus
@@ -1362,3 +1364,5 @@ typedef union {
 #ifdef __cplusplus
 }
 #endif
+
+#pragma clang diagnostic pop