fuzz: Exclude too expensive inputs in descriptor_parse targets
What changed, and why it matters
This commit is a code-quality and fuzz-testing efficiency change. It refactors how Bitcoin Core's fuzz tests skip descriptor inputs that are too computationally expensive to process, combining several checks into one helper function and adding a new leaf-size check. It does not change production wallet or node behavior, and there is no indication it fixes a security vulnerability in live software.
No security action required. Treat as a normal fuzz-harness maintainability improvement. If reviewing, verify the new `HasTooLargeLeafSize` check behaves equivalently to the previous inline check in `scriptpubkeyman.cpp`.
Security signals we found
No production code paths modified
Change is confined to fuzz-test harnesses and utilities
No mention of vulnerability, CVE, bug bounty, or security fix in commit message
Refactoring of existing input-filtering logic, not introduction of a new security boundary
Evidence from the diff
The patch consolidates descriptor cost checks (deep derivation paths, too many sub-fragments, too many nested wrappers, and now too-large leaf size) into a single inline helper IsTooExpensive() in src/test/fuzz/util/descriptor.h. Fuzz targets descriptor_parse, mocked_descriptor_parse, and scriptpubkeyman are updated to use it. The change is purely within fuzz-test harness code; no consensus, P2P, wallet, or descriptor parsing logic in the production binary is modified.
Changed components
src/test/fuzz/descriptor_parse.cppsrc/test/fuzz/util/descriptor.cppsrc/test/fuzz/util/descriptor.hsrc/wallet/test/fuzz/scriptpubkeyman.cppInspect captured patch +34 / −30
diff --git a/src/test/fuzz/descriptor_parse.cpp b/src/test/fuzz/descriptor_parse.cpp
index daad5e80..eeba5f13 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 1db3c2f2..1db66132 100644
--- a/src/test/fuzz/util/descriptor.cpp
+++ b/src/test/fuzz/util/descriptor.cpp
@@ -7,12 +7,15 @@
#include <key.h>
#include <key_io.h>
#include <pubkey.h>
+#include <span.h>
#include <util/strencodings.h>
#include <ranges>
#include <stack>
+#include <vector>
-void MockedDescriptorConverter::Init() {
+void MockedDescriptorConverter::Init()
+{
// The data to use as a private key or a seed for an xprv.
std::array<std::byte, 32> 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 17a91e96..974e9ae8 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<const uint8_t> 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<const uint8_t> 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 deb1a579..243c9c69 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<const uint8_t> desc)
-{
- return HasDeepDerivPath(desc) || HasTooManySubFrag(desc) || HasTooManyWrappers(desc);
-}
-
static std::optional<std::pair<WalletDescriptor, FlatSigningProvider>> 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;
Why this scored 13/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.