fuzzer improvements

CMakeLists.txt

@@ -41,18 +41,18 @@ if(ENABLE_FUZZING)
         message(FATAL_ERROR "Fuzz logging enabled")
     endif()
 
-    set(CMAKE_CXX_CLANG_TIDY clang-tidy -checks=-*,bugprone-*,cert-*,clang-analyzer-*,-cert-err58-cpp,misc-*)
+    set(CMAKE_CXX_CLANG_TIDY clang-tidy -checks=-*,bugprone-*,cert-*,clang-analyzer-*,-cert-err58-cpp,misc-*,-bugprone-suspicious-include)
 
     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!")
+        if(CMAKE_CXX_COMPILER_VERSION VERSION_LESS 11.0)
+            message(FATAL_ERROR "Clang version must be at least 11.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 11.\n"
+            "1. Install clang-11 \n"
+            "2. Pass -DCMAKE_C_COMPILER=clang-10 -DCMAKE_CXX_COMPILER=clang++-11")
     endif()
 
     string(APPEND CMAKE_C_FLAGS " -fsanitize=fuzzer-no-link")

app/src/eth_erc20.c

@@ -97,7 +97,7 @@ parser_error_t printERC20Value(const rlp_t *data, char *outVal, uint16_t outValL
 
 bool validateERC20(rlp_t data) {
     // Check that data start with ERC20 prefix
-    if (data.rlpLen != ERC20_DATA_LENGTH || memcmp(data.ptr, ERC20_TRANSFER_PREFIX, 4) != 0) {
+    if (data.ptr == NULL || data.rlpLen != ERC20_DATA_LENGTH || memcmp(data.ptr, ERC20_TRANSFER_PREFIX, 4) != 0) {
         return false;
     }
 

app/src/parser_impl_common.c

@@ -197,6 +197,7 @@ bool parser_output_contains_change_address(parser_context_t *c) {
     bool contains = false;
     // verify address is renderable  compare with CHANGE ADDRESS
 #if defined(TARGET_NANOS) || defined(TARGET_NANOS2) || defined(TARGET_NANOX) || defined(TARGET_STAX)
+    CTX_CHECK_AVAIL(c, ADDRESS_LEN)
     if (MEMCMP(c->buffer + c->offset, change_address, ADDRESS_LEN) == 0) {
         contains = false;
     } else {
@@ -205,6 +206,8 @@ bool parser_output_contains_change_address(parser_context_t *c) {
 #else
     uint8_t test_change_address[ADDRESS_LEN] = {0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa,
                                                 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa};
+
+    CTX_CHECK_AVAIL(c, ADDRESS_LEN)
     if (MEMCMP(c->buffer + c->offset, test_change_address, ADDRESS_LEN) == 0) {
         contains = false;
     } else {

app/src/parser_impl_eth.c

@@ -41,9 +41,11 @@ static parser_error_t readChainID(parser_context_t *ctx, rlp_t *chainId) {
     uint64_t tmpChainId = 0;
     if (chainId->rlpLen > 0) {
         CHECK_ERROR(be_bytes_to_u64(chainId->ptr, chainId->rlpLen, &tmpChainId))
-    } else {
+    } else if (chainId->kind == RLP_KIND_BYTE) {
         // case were the prefix is the byte itself
         tmpChainId = chainId->ptr[0];
+    } else {
+        return parser_unexpected_error;
     }
 
     // Check allowed values for chain id

fuzz/parser_parse.cpp

@@ -9,21 +9,20 @@
 #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;
     parser_error_t rc;
 
+    ctx.tx_type = flr_tx;
     rc = parser_parse(&ctx, data, size, &txObj);
     if (rc != parser_ok) {
         return 0;
@@ -37,9 +36,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));
+        (void)fprintf(stderr, "error in parser_getNumItems: %s\n", parser_getErrorDescription(rc));
         assert(false);
     }
 
@@ -49,19 +46,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);
             }
 

tests/expected_output.cpp

@@ -13,6 +13,7 @@
  *  See the License for the specific language governing permissions and
  *  limitations under the License.
  ********************************************************************************/
+
 #include <coin.h>
 #include <fmt/core.h>
 
@@ -28,7 +29,7 @@ void addTo(std::vector<std::string> &answer, const S &format_str, Args &&...args
     answer.push_back(fmt::format(format_str, args...));
 }
 
-std::vector<std::string> FormatEthAddress(const uint32_t idx, const std::string &name, const std::string &address) {
+std::vector<std::string> FormatEthAddress(const std::string &name, const uint32_t idx, const std::string &address) {
     auto answer = std::vector<std::string>();
     uint8_t numPages = 0;
     char outBuffer[100];
@@ -72,12 +73,12 @@ std::vector<std::string> EVMGenerateExpectedUIOutput(const Json::Value &json, bo
     ///
 
     uint8_t idx = 0;
-    auto destAddress = FormatEthAddress(idx, "To", to);
+    auto destAddress = FormatEthAddress("To", idx, to);
     answer.insert(answer.end(), destAddress.begin(), destAddress.end());
 
     if (value.compare(0, 2, "??") == 0) {
         idx++;
-        auto contractAddress = FormatEthAddress(idx, "Contract", contract);
+        auto contractAddress = FormatEthAddress("Contract", idx, contract);
         answer.insert(answer.end(), contractAddress.begin(), contractAddress.end());
     }
 

tests/ui_tests.cpp

@@ -47,10 +47,10 @@ class JsonTestsA : public ::testing::TestWithParam<testcase_t> {
 std::vector<testcase_t> GetJsonTestCases(std::string jsonFile) {
     auto answer = std::vector<testcase_t>();
 
-    Json::CharReaderBuilder builder;
+    const Json::CharReaderBuilder builder;
     Json::Value obj;
 
-    std::string fullPathJsonFile = std::string(TESTVECTORS_DIR) + jsonFile;
+    const std::string fullPathJsonFile = std::string(TESTVECTORS_DIR) + jsonFile;
 
     std::ifstream inFile(fullPathJsonFile);
     if (!inFile.is_open()) {
@@ -94,10 +94,10 @@ template <typename Generator>
 std::vector<testcase_t> GetEVMJsonTestCases(const std::string &jsonFile, Generator gen_ui_output) {
     auto answer = std::vector<testcase_t>();
 
-    Json::CharReaderBuilder builder;
+    const Json::CharReaderBuilder builder;
     Json::Value obj;
 
-    std::string fullPathJsonFile = std::string(TESTVECTORS_DIR) + jsonFile;
+    const std::string fullPathJsonFile = std::string(TESTVECTORS_DIR) + jsonFile;
 
     std::ifstream inFile(fullPathJsonFile);
     if (!inFile.is_open()) {
@@ -129,7 +129,7 @@ void check_testcase(const testcase_t &tc, bool expert_mode, parser_context_t ctx
     parser_error_t err;
 
     uint8_t buffer[5000];
-    uint16_t bufferLen = parseHexString(buffer, sizeof(buffer), tc.blob.c_str());
+    const uint16_t bufferLen = parseHexString(buffer, sizeof(buffer), tc.blob.c_str());
 
     parser_tx_t tx_obj;
     memset(&tx_obj, 0, sizeof(tx_obj));