From fab2f3df4beb230eef63bdcf5042b6417c0012dc Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Fri, 16 Jan 2026 12:41:59 +0100 Subject: [PATCH] fuzz: Exclude too expensive inputs in descriptor_parse targets Also, fixup iwyu warnings in the util module. Also, fixup a typo. The moved part can be reviewed with the git option: --color-moved=dimmed-zebra --- src/test/fuzz/descriptor_parse.cpp | 19 ++--------------- src/test/fuzz/util/descriptor.cpp | 5 ++++- src/test/fuzz/util/descriptor.h | 27 ++++++++++++++++++++++++ src/wallet/test/fuzz/scriptpubkeyman.cpp | 13 +----------- 4 files changed, 34 insertions(+), 30 deletions(-) diff --git a/src/test/fuzz/descriptor_parse.cpp b/src/test/fuzz/descriptor_parse.cpp index daad5e805c4..eeba5f1369e 100644 --- a/src/test/fuzz/descriptor_parse.cpp +++ b/src/test/fuzz/descriptor_parse.cpp @@ -77,21 +77,9 @@ void initialize_mocked_descriptor_parse() FUZZ_TARGET(mocked_descriptor_parse, .init = initialize_mocked_descriptor_parse) { - // 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 the actual descriptor syntax. So rule - // out strings which could correspond to a descriptor containing a too large derivation path. - if (HasDeepDerivPath(buffer)) return; - - // Some fragments can take a virtually unlimited number of sub-fragments (thresh, multi_a) but - // may perform quadratic operations on them. Limit the number of sub-fragments per fragment. - if (HasTooManySubFrag(buffer)) return; - - // The script building logic performs quadratic copies in the number of nested wrappers. Limit - // the number of nested wrappers per fragment. - if (HasTooManyWrappers(buffer)) return; - const std::string mocked_descriptor{buffer.begin(), buffer.end()}; if (const auto descriptor = MOCKED_DESC_CONVERTER.GetDescriptor(mocked_descriptor)) { + if (IsTooExpensive(MakeUCharSpan(*descriptor))) return; FlatSigningProvider signing_provider; std::string error; const auto desc = Parse(*descriptor, signing_provider, error); @@ -106,10 +94,7 @@ FUZZ_TARGET(mocked_descriptor_parse, .init = initialize_mocked_descriptor_parse) FUZZ_TARGET(descriptor_parse, .init = initialize_descriptor_parse) { - // See comments above for rationales. - if (HasDeepDerivPath(buffer)) return; - if (HasTooManySubFrag(buffer)) return; - if (HasTooManyWrappers(buffer)) return; + if (IsTooExpensive(buffer)) return; const std::string descriptor(buffer.begin(), buffer.end()); FlatSigningProvider signing_provider; diff --git a/src/test/fuzz/util/descriptor.cpp b/src/test/fuzz/util/descriptor.cpp index 1db3c2f265a..1db66132a87 100644 --- a/src/test/fuzz/util/descriptor.cpp +++ b/src/test/fuzz/util/descriptor.cpp @@ -7,12 +7,15 @@ #include #include #include +#include #include #include #include +#include -void MockedDescriptorConverter::Init() { +void MockedDescriptorConverter::Init() +{ // The data to use as a private key or a seed for an xprv. std::array key_data{std::byte{1}}; // Generate keys of all kinds and store them in the keys array. diff --git a/src/test/fuzz/util/descriptor.h b/src/test/fuzz/util/descriptor.h index 17a91e969cf..974e9ae83a7 100644 --- a/src/test/fuzz/util/descriptor.h +++ b/src/test/fuzz/util/descriptor.h @@ -86,4 +86,31 @@ constexpr uint32_t MAX_LEAF_SIZE{200}; /// MockedDescriptorConverter) has a leaf size too large. bool HasTooLargeLeafSize(std::span buff, uint32_t max_leaf_size = MAX_LEAF_SIZE); +/// 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. +/// +/// Use this function after MockedDescriptorConverter::GetDescriptor() +inline bool IsTooExpensive(std::span buffer) +{ + // Key derivation is expensive. Deriving deep derivation paths takes a lot of compute and we'd + // rather spend time elsewhere in this target, like on the actual descriptor syntax. So rule + // out strings which could correspond to a descriptor containing a too large derivation path. + if (HasDeepDerivPath(buffer)) return true; + + // Some fragments can take a virtually unlimited number of sub-fragments (thresh, multi_a) but + // may perform quadratic operations on them. Limit the number of sub-fragments per fragment. + if (HasTooManySubFrag(buffer)) return true; + + // The script building logic performs quadratic copies in the number of nested wrappers. Limit + // the number of nested wrappers per fragment. + if (HasTooManyWrappers(buffer)) return true; + + // If any suspected leaf is too large, it will likely not represent a valid + // use-case. Also, possible base58 parsing in the leaf is quadratic. So + // limit the leaf size. + if (HasTooLargeLeafSize(buffer)) return true; + + return false; +} #endif // BITCOIN_TEST_FUZZ_UTIL_DESCRIPTOR_H diff --git a/src/wallet/test/fuzz/scriptpubkeyman.cpp b/src/wallet/test/fuzz/scriptpubkeyman.cpp index deb1a57983d..243c9c69114 100644 --- a/src/wallet/test/fuzz/scriptpubkeyman.cpp +++ b/src/wallet/test/fuzz/scriptpubkeyman.cpp @@ -50,23 +50,12 @@ void initialize_spkm() MOCKED_DESC_CONVERTER.Init(); } -/** - * 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 IsTooExpensive(std::span desc) -{ - 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 (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 {}; + if (IsTooExpensive(MakeUCharSpan(*desc_str))) return {}; FlatSigningProvider keys; std::string error;