From bad1433ef2b5b02ac4b1c6c1d9482c513e5b2192 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Thu, 20 Feb 2025 16:48:28 +0100 Subject: [PATCH 1/2] fuzz: Always restrict base conversion input lengths They seem to cause timeouts: > Issue 397734700: bitcoin-core:base58check_encode_decode: Timeout in base58check_encode_decode The `encoded_string.empty()` check was corrected here to `decoded.empty()` to make sure the `(0, decoded.size() - 1)` range is always valid. Co-authored-by: maflcko <6399679+maflcko@users.noreply.github.com> Co-authored-by: marcofleon Co-authored-by: Martin Zumsande --- src/test/fuzz/base_encode_decode.cpp | 35 ++++++++++++++-------------- 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/src/test/fuzz/base_encode_decode.cpp b/src/test/fuzz/base_encode_decode.cpp index 06b249fb8d3..69caac58100 100644 --- a/src/test/fuzz/base_encode_decode.cpp +++ b/src/test/fuzz/base_encode_decode.cpp @@ -6,6 +6,7 @@ #include #include +#include #include #include #include @@ -19,42 +20,40 @@ using util::TrimStringView; FUZZ_TARGET(base58_encode_decode) { - FuzzedDataProvider provider(buffer.data(), buffer.size()); - const std::string random_string{provider.ConsumeRandomLengthString(1000)}; - const int max_ret_len{provider.ConsumeIntegralInRange(-1, 1000)}; + FuzzedDataProvider provider{buffer.data(), buffer.size()}; + const auto random_string{provider.ConsumeRandomLengthString(100)}; + const int max_ret_len{provider.ConsumeIntegralInRange(-1, 100)}; // Decode/Encode roundtrip - std::vector decoded; - if (DecodeBase58(random_string, decoded, max_ret_len)) { + if (std::vector decoded; DecodeBase58(random_string, decoded, max_ret_len)) { const auto encoded_string{EncodeBase58(decoded)}; assert(encoded_string == TrimStringView(random_string)); - assert(encoded_string.empty() || !DecodeBase58(encoded_string, decoded, provider.ConsumeIntegralInRange(0, decoded.size() - 1))); + assert(decoded.empty() || !DecodeBase58(encoded_string, decoded, provider.ConsumeIntegralInRange(0, decoded.size() - 1))); } // Encode/Decode roundtrip - const auto encoded{EncodeBase58(buffer)}; + const auto encoded{EncodeBase58(MakeUCharSpan(random_string))}; std::vector roundtrip_decoded; - assert(DecodeBase58(encoded, roundtrip_decoded, buffer.size()) - && std::ranges::equal(roundtrip_decoded, buffer)); + assert(DecodeBase58(encoded, roundtrip_decoded, random_string.size()) + && std::ranges::equal(roundtrip_decoded, MakeUCharSpan(random_string))); } FUZZ_TARGET(base58check_encode_decode) { - FuzzedDataProvider provider(buffer.data(), buffer.size()); - const std::string random_string{provider.ConsumeRandomLengthString(1000)}; - const int max_ret_len{provider.ConsumeIntegralInRange(-1, 1000)}; + FuzzedDataProvider provider{buffer.data(), buffer.size()}; + const auto random_string{provider.ConsumeRandomLengthString(100)}; + const int max_ret_len{provider.ConsumeIntegralInRange(-1, 100)}; // Decode/Encode roundtrip - std::vector decoded; - if (DecodeBase58Check(random_string, decoded, max_ret_len)) { + if (std::vector decoded; DecodeBase58Check(random_string, decoded, max_ret_len)) { const auto encoded_string{EncodeBase58Check(decoded)}; assert(encoded_string == TrimStringView(random_string)); - assert(encoded_string.empty() || !DecodeBase58Check(encoded_string, decoded, provider.ConsumeIntegralInRange(0, decoded.size() - 1))); + assert(decoded.empty() || !DecodeBase58Check(encoded_string, decoded, provider.ConsumeIntegralInRange(0, decoded.size() - 1))); } // Encode/Decode roundtrip - const auto encoded{EncodeBase58Check(buffer)}; + const auto encoded{EncodeBase58Check(MakeUCharSpan(random_string))}; std::vector roundtrip_decoded; - assert(DecodeBase58Check(encoded, roundtrip_decoded, buffer.size()) - && std::ranges::equal(roundtrip_decoded, buffer)); + assert(DecodeBase58Check(encoded, roundtrip_decoded, random_string.size()) + && std::ranges::equal(roundtrip_decoded, MakeUCharSpan(random_string))); } FUZZ_TARGET(base32_encode_decode) From d5537c18a9034647ba4c9ed4008abd7fee33989e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Thu, 20 Feb 2025 16:57:26 +0100 Subject: [PATCH 2/2] fuzz: make sure DecodeBase58(Check) is called with valid values more often In Base58 fuzz the two roundtrips are merged now, the new `decode_input` switches between a completely random input and a valid encoded one, to make sure the decoding passes more often. The `max_ret_len` can also exceed the original length now and is being validated more thoroughly. Co-authored-by: maflcko <6399679+maflcko@users.noreply.github.com> Co-authored-by: marcofleon --- src/test/fuzz/base_encode_decode.cpp | 44 ++++++++++++++-------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/src/test/fuzz/base_encode_decode.cpp b/src/test/fuzz/base_encode_decode.cpp index 69caac58100..2ffcbdf7207 100644 --- a/src/test/fuzz/base_encode_decode.cpp +++ b/src/test/fuzz/base_encode_decode.cpp @@ -22,38 +22,38 @@ FUZZ_TARGET(base58_encode_decode) { FuzzedDataProvider provider{buffer.data(), buffer.size()}; const auto random_string{provider.ConsumeRandomLengthString(100)}; - const int max_ret_len{provider.ConsumeIntegralInRange(-1, 100)}; - // Decode/Encode roundtrip - if (std::vector decoded; DecodeBase58(random_string, decoded, max_ret_len)) { - const auto encoded_string{EncodeBase58(decoded)}; - assert(encoded_string == TrimStringView(random_string)); - assert(decoded.empty() || !DecodeBase58(encoded_string, decoded, provider.ConsumeIntegralInRange(0, decoded.size() - 1))); - } - // Encode/Decode roundtrip const auto encoded{EncodeBase58(MakeUCharSpan(random_string))}; - std::vector roundtrip_decoded; - assert(DecodeBase58(encoded, roundtrip_decoded, random_string.size()) - && std::ranges::equal(roundtrip_decoded, MakeUCharSpan(random_string))); + const auto decode_input{provider.ConsumeBool() ? random_string : encoded}; + const int max_ret_len{provider.ConsumeIntegralInRange(-1, decode_input.size() + 1)}; + if (std::vector decoded; DecodeBase58(decode_input, decoded, max_ret_len)) { + const auto encoded_string{EncodeBase58(decoded)}; + assert(encoded_string == TrimStringView(decode_input)); + if (decoded.size() > 0) { + assert(max_ret_len > 0); + assert(decoded.size() <= static_cast(max_ret_len)); + assert(!DecodeBase58(encoded_string, decoded, provider.ConsumeIntegralInRange(0, decoded.size() - 1))); + } + } } FUZZ_TARGET(base58check_encode_decode) { FuzzedDataProvider provider{buffer.data(), buffer.size()}; const auto random_string{provider.ConsumeRandomLengthString(100)}; - const int max_ret_len{provider.ConsumeIntegralInRange(-1, 100)}; - // Decode/Encode roundtrip - if (std::vector decoded; DecodeBase58Check(random_string, decoded, max_ret_len)) { - const auto encoded_string{EncodeBase58Check(decoded)}; - assert(encoded_string == TrimStringView(random_string)); - assert(decoded.empty() || !DecodeBase58Check(encoded_string, decoded, provider.ConsumeIntegralInRange(0, decoded.size() - 1))); - } - // Encode/Decode roundtrip const auto encoded{EncodeBase58Check(MakeUCharSpan(random_string))}; - std::vector roundtrip_decoded; - assert(DecodeBase58Check(encoded, roundtrip_decoded, random_string.size()) - && std::ranges::equal(roundtrip_decoded, MakeUCharSpan(random_string))); + const auto decode_input{provider.ConsumeBool() ? random_string : encoded}; + const int max_ret_len{provider.ConsumeIntegralInRange(-1, decode_input.size() + 1)}; + if (std::vector decoded; DecodeBase58Check(decode_input, decoded, max_ret_len)) { + const auto encoded_string{EncodeBase58Check(decoded)}; + assert(encoded_string == TrimStringView(decode_input)); + if (decoded.size() > 0) { + assert(max_ret_len > 0); + assert(decoded.size() <= static_cast(max_ret_len)); + assert(!DecodeBase58Check(encoded_string, decoded, provider.ConsumeIntegralInRange(0, decoded.size() - 1))); + } + } } FUZZ_TARGET(base32_encode_decode)