From 333333356f431d8ef318f685860d25ff99d4b457 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Thu, 8 Jan 2026 11:05:59 +0100 Subject: [PATCH 1/3] fuzz: [refactor] Use std::span over FuzzBufferType in descriptor utils They are exactly the same, but the descriptor utils should not prescribe to use the FuzzBufferType. Using a dedicated type for them clarifies that the utils are not tied to FuzzBufferType. Also, while touching the lines, use `const` only where it is meaningful. --- src/test/fuzz/util/descriptor.cpp | 6 +++--- src/test/fuzz/util/descriptor.h | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/test/fuzz/util/descriptor.cpp b/src/test/fuzz/util/descriptor.cpp index 9e52e990a2a..4e563eac4d7 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. diff --git a/src/test/fuzz/util/descriptor.h b/src/test/fuzz/util/descriptor.h index ea928c39f04..41605dd98d6 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,6 @@ 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); #endif // BITCOIN_TEST_FUZZ_UTIL_DESCRIPTOR_H From fabac1b3950e4bc9716f9b3c17b8f02952d6b974 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Thu, 8 Jan 2026 11:31:20 +0100 Subject: [PATCH 2/3] fuzz: Reject some more "expensive" descriptors in the scriptpubkeyman target The same are rejected in the descriptor_parse target, so it makes sense to reject them here as well. --- src/wallet/test/fuzz/scriptpubkeyman.cpp | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/wallet/test/fuzz/scriptpubkeyman.cpp b/src/wallet/test/fuzz/scriptpubkeyman.cpp index ea1431a7cf0..ff9d1cc0d33 100644 --- a/src/wallet/test/fuzz/scriptpubkeyman.cpp +++ b/src/wallet/test/fuzz/scriptpubkeyman.cpp @@ -51,20 +51,19 @@ 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; From fa8d56f9f092fceab7dfb10533c4187e1b5fabfe Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Thu, 8 Jan 2026 12:27:03 +0100 Subject: [PATCH 3/3] fuzz: Reject too large descriptor leaf sizes in scriptpubkeyman target --- src/test/fuzz/util/descriptor.cpp | 20 ++++++++++++++++++++ src/test/fuzz/util/descriptor.h | 8 ++++++++ src/wallet/test/fuzz/scriptpubkeyman.cpp | 1 + 3 files changed, 29 insertions(+) diff --git a/src/test/fuzz/util/descriptor.cpp b/src/test/fuzz/util/descriptor.cpp index 4e563eac4d7..08ab7104c19 100644 --- a/src/test/fuzz/util/descriptor.cpp +++ b/src/test/fuzz/util/descriptor.cpp @@ -143,3 +143,23 @@ bool HasTooManyWrappers(std::span 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 41605dd98d6..82cc967cb49 100644 --- a/src/test/fuzz/util/descriptor.h +++ b/src/test/fuzz/util/descriptor.h @@ -76,4 +76,12 @@ constexpr int MAX_WRAPPERS{100}; */ 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 ff9d1cc0d33..deb1a57983d 100644 --- a/src/wallet/test/fuzz/scriptpubkeyman.cpp +++ b/src/wallet/test/fuzz/scriptpubkeyman.cpp @@ -66,6 +66,7 @@ static std::optional> CreateWal 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;