fuzz: Reject too large descriptor leaf sizes in scriptpubkeyman target

maflcko commented at 11:23 am on January 8, 2026: member

Accepting “expensive” fuzz inputs which have no real use-case is problematic, because it prevents the fuzz engine from spending time on the next useful fuzz input.

Also, this may lead to problems, where the fuzz target can not be run at all on some platforms. See #34110.

Fixes #34110 by rejecting those useless and expensive inputs (via the third commit)

Can be tested by running the input and checking the time before and after the changes here:

0curl -fLO 'https://github.com/bitcoin-core/qa-assets/raw/b5ad78e070e4cf36beb415d7b490d948d70ba73f/fuzz_corpora/scriptpubkeyman/1cf91e0c6bfff9dafcd4db5b0ba36b1e906f4cf5'
1FUZZ=scriptpubkeyman time ./bld-cmake/bin/fuzz ./1cf91e0c6bfff9dafcd4db5b0ba36b1e906f4cf5

Also, the second commit fixes #31066.

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.

333333356f

DrahtBot renamed this:
~~fuzz: Reject too large descriptor leaf sizes in scriptpubkeyman target~~
fuzz: Reject too large descriptor leaf sizes in scriptpubkeyman target
on Jan 8, 2026

DrahtBot added the label Fuzzing on Jan 8, 2026

DrahtBot commented at 11:23 am on January 8, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/34230.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	brunoerg, marcofleon, sipa

If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#31650 (refactor: Avoid copies by using const references or by move-construction by maflcko)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

maflcko force-pushed on Jan 8, 2026

DrahtBot added the label CI failed on Jan 8, 2026

brunoerg commented at 12:37 pm on January 8, 2026: contributor

Concept ACK

DrahtBot removed the label CI failed on Jan 8, 2026

in src/test/fuzz/util/descriptor.h:79 in fac0254014

75@@ -76,4 +76,11 @@ constexpr int MAX_WRAPPERS{100};
76  */
77 bool HasTooManyWrappers(std::span<const uint8_t> buff, int max_wrappers = MAX_WRAPPERS);
78 
79+/// Default maximum leaf size.

brunoerg commented at 12:56 pm on January 8, 2026:

fac025401466204371ca59de08d1ebde4c2c0b9b: Perhaps it would be good to put in the comment where 200 comes from? I suppose it’s a comfortable margin, especially considering the maximum size of an extended public key (xpub) that would be our longest case, right?

maflcko commented at 1:05 pm on January 8, 2026:

Yeah, even with an extended key, and [] as well as /, 200 should be sufficient.

in src/test/fuzz/util/descriptor.cpp:151 in fac0254014 outdated

142@@ -143,3 +143,23 @@ bool HasTooManyWrappers(std::span<const uint8_t> buff, const int max_wrappers)
143 
144     return false;
145 }
146+
147+bool HasTooLargeLeafSize(std::span<const uint8_t> buff, const uint32_t max_leaf_size)
148+{
149+    uint32_t leaf_len{0};
150+    for (auto c : buff) {
151+        if (c == '(' || c == ')' || c == ',' || c == '{' || c == '}') {

brunoerg commented at 12:59 pm on January 8, 2026:

fac025401466204371ca59de08d1ebde4c2c0b9b: Should we also include [ and ] here?

maflcko commented at 1:05 pm on January 8, 2026:

I’d say no, because this is part of the leaf, but I can see it go both ways. No strong opinion. Though, / should be included as well, if [ is.

brunoerg commented at 1:08 pm on January 8, 2026:

It’s fine to keep as is.

maflcko force-pushed on Jan 8, 2026

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.

fabac1b395

fuzz: Reject too large descriptor leaf sizes in scriptpubkeyman target fa8d56f9f0

maflcko force-pushed on Jan 8, 2026

DrahtBot added the label CI failed on Jan 8, 2026

DrahtBot removed the label CI failed on Jan 8, 2026

brunoerg approved

brunoerg commented at 4:57 pm on January 8, 2026: contributor

code review ACK fa8d56f9f092fceab7dfb10533c4187e1b5fabfe

in src/test/fuzz/util/descriptor.h:11 in 333333356f

marcofleon commented at 1:48 pm on January 9, 2026:

nit: Since FuzzBufferType is no longer used, could drop fuzz.h and include <span> here directly.

maflcko commented at 2:00 pm on January 9, 2026:

thx, I can just do this in the next follow-up (there likely will be one for other fuzz targets). For reference, the iwyu output seems to be:

 02026-01-08T13:36:25.5603340Z /home/admin/actions-runner/_work/_temp/src/test/fuzz/util/descriptor.h should add these lines:
 12026-01-08T13:36:25.5603600Z #include <array>                // for array
 22026-01-08T13:36:25.5603782Z #include <cinttypes>            // for uint8_t, uint32_t
 32026-01-08T13:36:25.5603961Z #include <cstddef>              // for size_t
 42026-01-08T13:36:25.5604137Z #include <limits>               // for numeric_limits
 52026-01-08T13:36:25.5604322Z #include <optional>             // for optional
 62026-01-08T13:36:25.5604479Z #include <span>                 // for span
 72026-01-08T13:36:25.5604579Z 
 82026-01-08T13:36:25.5604740Z /home/admin/actions-runner/_work/_temp/src/test/fuzz/util/descriptor.h should remove these lines:
 92026-01-08T13:36:25.5604991Z - #include <key_io.h>  // lines 8-8
102026-01-08T13:36:25.5605156Z - #include <script/descriptor.h>  // lines 10-10
112026-01-08T13:36:25.5605324Z - #include <test/fuzz/fuzz.h>  // lines 11-11
122026-01-08T13:36:25.5605482Z - #include <functional>  // lines 13-13
132026-01-08T13:36:25.5605583Z 
14
152026-01-08T13:36:25.5607883Z /home/admin/actions-runner/_work/_temp/src/test/fuzz/util/descriptor.cpp should add these lines:
162026-01-08T13:36:25.5608180Z #include <key.h>     // for CKey, CExtKey
172026-01-08T13:36:25.5608407Z #include <key_io.h>  // for EncodeExtKey, EncodeExtPubKey, EncodeSecret
182026-01-08T13:36:25.5608746Z #include <pubkey.h>  // for XOnlyPubKey, CExtPubKey, CPubKey
192026-01-08T13:36:25.5608961Z #include <span.h>    // for UCharCast
202026-01-08T13:36:25.5609119Z #include <vector>    // for vector
212026-01-08T13:36:25.5609242Z 
222026-01-08T13:36:25.5609520Z /home/admin/actions-runner/_work/_temp/src/test/fuzz/util/descriptor.cpp should remove these lines:
232026-01-08T13:36:25.5609984Z

Leaving as-is here for now, because several includes seem to be affected.

maflcko commented at 9:35 pm on January 14, 2026:

fixed includes in https://github.com/bitcoin/bitcoin/pull/34288

marcofleon commented at 1:50 pm on January 9, 2026: contributor

ACK fa8d56f9f092fceab7dfb10533c4187e1b5fabfe

Tested with the input in the PR description and saw a drop from ~500ms to 15ms.

maflcko commented at 7:02 pm on January 13, 2026: member

is this fuzz-only change rfm with two acks, or would it need more?

sipa commented at 7:44 pm on January 13, 2026: member

ACK fa8d56f9f092fceab7dfb10533c4187e1b5fabfe

glozow merged this on Jan 13, 2026

glozow closed this on Jan 13, 2026

maflcko deleted the branch on Jan 14, 2026

fuzz: Reject too large descriptor leaf sizes in scriptpubkeyman target #34230

Code Coverage & Benchmarks

Reviews

Conflicts