diff --git a/src/test/fuzz/util/descriptor.cpp b/src/test/fuzz/util/descriptor.cpp index 9e52e990a2a..08ab7104c19 100644 --- a/src/test/fuzz/util/descriptor.cpp +++ b/src/test/fuzz/util/descriptor.cpp @@ -74,7 +74,7 @@ std::optional MockedDescriptorConverter::GetDescriptor(std::string_ return desc; } -bool HasDeepDerivPath(const FuzzBufferType& buff, const int max_depth) +bool HasDeepDerivPath(std::span buff, const int max_depth) { auto depth{0}; for (const auto& ch: buff) { @@ -88,7 +88,7 @@ bool HasDeepDerivPath(const FuzzBufferType& buff, const int max_depth) return false; } -bool HasTooManySubFrag(const FuzzBufferType& buff, const int max_subs, const size_t max_nested_subs) +bool HasTooManySubFrag(std::span buff, const int max_subs, const size_t max_nested_subs) { // We use a stack because there may be many nested sub-frags. std::stack counts; @@ -112,7 +112,7 @@ bool HasTooManySubFrag(const FuzzBufferType& buff, const int max_subs, const siz return false; } -bool HasTooManyWrappers(const FuzzBufferType& buff, const int max_wrappers) +bool HasTooManyWrappers(std::span buff, const int max_wrappers) { // The number of nested wrappers. Nested wrappers are always characters which follow each other so we don't have to // use a stack as we do above when counting the number of sub-fragments. @@ -143,3 +143,23 @@ bool HasTooManyWrappers(const FuzzBufferType& buff, const int max_wrappers) return false; } + +bool HasTooLargeLeafSize(std::span buff, const uint32_t max_leaf_size) +{ + uint32_t leaf_len{0}; + for (auto c : buff) { + if (c == '(' || c == ')' || c == ',' || c == '{' || c == '}') { + // Possibly start a fresh leaf, or a fresh function name (with + // wrappers), or terminate a prior leaf. + leaf_len = 0; + } else { + // Just treat everything else as a leaf. This will also reject long + // function names, but this should be fine if the max_leaf_size is + // set large enough. + if (++leaf_len > max_leaf_size) { + return true; + } + } + } + return false; +} diff --git a/src/test/fuzz/util/descriptor.h b/src/test/fuzz/util/descriptor.h index ea928c39f04..82cc967cb49 100644 --- a/src/test/fuzz/util/descriptor.h +++ b/src/test/fuzz/util/descriptor.h @@ -53,7 +53,7 @@ constexpr int MAX_DEPTH{2}; * Whether the buffer, if it represents a valid descriptor, contains a derivation path deeper than * a given maximum depth. Note this may also be hit for deriv paths in origins. */ -bool HasDeepDerivPath(const FuzzBufferType& buff, const int max_depth = MAX_DEPTH); +bool HasDeepDerivPath(std::span buff, int max_depth = MAX_DEPTH); //! Default maximum number of sub-fragments. constexpr int MAX_SUBS{1'000}; @@ -64,8 +64,8 @@ constexpr size_t MAX_NESTED_SUBS{10'000}; * Whether the buffer, if it represents a valid descriptor, contains a fragment with more * sub-fragments than the given maximum. */ -bool HasTooManySubFrag(const FuzzBufferType& buff, const int max_subs = MAX_SUBS, - const size_t max_nested_subs = MAX_NESTED_SUBS); +bool HasTooManySubFrag(std::span buff, int max_subs = MAX_SUBS, + size_t max_nested_subs = MAX_NESTED_SUBS); //! Default maximum number of wrappers per fragment. constexpr int MAX_WRAPPERS{100}; @@ -74,6 +74,14 @@ constexpr int MAX_WRAPPERS{100}; * Whether the buffer, if it represents a valid descriptor, contains a fragment with more * wrappers than the given maximum. */ -bool HasTooManyWrappers(const FuzzBufferType& buff, const int max_wrappers = MAX_WRAPPERS); +bool HasTooManyWrappers(std::span buff, int max_wrappers = MAX_WRAPPERS); + +/// Default maximum leaf size. This should be large enough to cover an extended +/// key, including paths "/", inside and outside of "[]". +constexpr uint32_t MAX_LEAF_SIZE{200}; + +/// Whether the expanded buffer (after calling GetDescriptor() in +/// MockedDescriptorConverter) has a leaf size too large. +bool HasTooLargeLeafSize(std::span buff, uint32_t max_leaf_size = MAX_LEAF_SIZE); #endif // BITCOIN_TEST_FUZZ_UTIL_DESCRIPTOR_H diff --git a/src/wallet/test/fuzz/scriptpubkeyman.cpp b/src/wallet/test/fuzz/scriptpubkeyman.cpp index ea1431a7cf0..deb1a57983d 100644 --- a/src/wallet/test/fuzz/scriptpubkeyman.cpp +++ b/src/wallet/test/fuzz/scriptpubkeyman.cpp @@ -51,22 +51,22 @@ void initialize_spkm() } /** - * Key derivation is expensive. Deriving deep derivation paths take a lot of compute and we'd rather spend time - * elsewhere in this target, like on actually fuzzing the DescriptorScriptPubKeyMan. So rule out strings which could - * correspond to a descriptor containing a too large derivation path. + * Deriving "expensive" descriptors will consume useful fuzz compute. The + * compute is better spent on a smaller subset of descriptors, which still + * covers all real end-user settings. */ -static bool TooDeepDerivPath(std::string_view desc) +static bool IsTooExpensive(std::span desc) { - const FuzzBufferType desc_buf{reinterpret_cast(desc.data()), desc.size()}; - return HasDeepDerivPath(desc_buf); + return HasDeepDerivPath(desc) || HasTooManySubFrag(desc) || HasTooManyWrappers(desc); } static std::optional> CreateWalletDescriptor(FuzzedDataProvider& fuzzed_data_provider) { const std::string mocked_descriptor{fuzzed_data_provider.ConsumeRandomLengthString()}; - if (TooDeepDerivPath(mocked_descriptor)) return {}; + if (IsTooExpensive(MakeUCharSpan(mocked_descriptor))) return {}; const auto desc_str{MOCKED_DESC_CONVERTER.GetDescriptor(mocked_descriptor)}; if (!desc_str.has_value()) return std::nullopt; + if (HasTooLargeLeafSize(MakeUCharSpan(*desc_str))) return {}; FlatSigningProvider keys; std::string error;