Skip to content

Commit 8d73401

Browse files
committed
fuzz: limit the number of sub-fragments per fragment for descriptors
This target may call into logic quadratic over the number of sub-fragments. Limit the number of sub-fragments to keep the runtime reasonable. Thanks to Marco Falke for reporting the fuzz timeouts with a minimized input.
1 parent 00feabf commit 8d73401

File tree

3 files changed

+44
-1
lines changed

3 files changed

+44
-1
lines changed

src/test/fuzz/descriptor_parse.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,10 @@ FUZZ_TARGET(mocked_descriptor_parse, .init = initialize_mocked_descriptor_parse)
7272
// out strings which could correspond to a descriptor containing a too large derivation path.
7373
if (HasDeepDerivPath(buffer)) return;
7474

75+
// Some fragments can take a virtually unlimited number of sub-fragments (thresh, multi_a) but
76+
// may perform quadratic operations on them. Limit the number of sub-fragments per fragment.
77+
if (HasTooManySubFrag(buffer)) return;
78+
7579
const std::string mocked_descriptor{buffer.begin(), buffer.end()};
7680
if (const auto descriptor = MOCKED_DESC_CONVERTER.GetDescriptor(mocked_descriptor)) {
7781
FlatSigningProvider signing_provider;
@@ -83,8 +87,9 @@ FUZZ_TARGET(mocked_descriptor_parse, .init = initialize_mocked_descriptor_parse)
8387

8488
FUZZ_TARGET(descriptor_parse, .init = initialize_descriptor_parse)
8589
{
86-
// See comment above for rationale.
90+
// See comments above for rationales.
8791
if (HasDeepDerivPath(buffer)) return;
92+
if (HasTooManySubFrag(buffer)) return;
8893

8994
const std::string descriptor(buffer.begin(), buffer.end());
9095
FlatSigningProvider signing_provider;

src/test/fuzz/util/descriptor.cpp

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44

55
#include <test/fuzz/util/descriptor.h>
66

7+
#include <stack>
8+
79
void MockedDescriptorConverter::Init() {
810
// The data to use as a private key or a seed for an xprv.
911
std::array<std::byte, 32> key_data{std::byte{1}};
@@ -84,3 +86,27 @@ bool HasDeepDerivPath(const FuzzBufferType& buff, const int max_depth)
8486
}
8587
return false;
8688
}
89+
90+
bool HasTooManySubFrag(const FuzzBufferType& buff, const int max_subs, const size_t max_nested_subs)
91+
{
92+
// We use a stack because there may be many nested sub-frags.
93+
std::stack<int> counts;
94+
for (const auto& ch: buff) {
95+
// The fuzzer may generate an input with a ton of parentheses. Rule out pathological cases.
96+
if (counts.size() > max_nested_subs) return true;
97+
98+
if (ch == '(') {
99+
// A new fragment was opened, create a new sub-count for it and start as one since any fragment with
100+
// parentheses has at least one sub.
101+
counts.push(1);
102+
} else if (ch == ',' && !counts.empty()) {
103+
// When encountering a comma, account for an additional sub in the last opened fragment. If it exceeds the
104+
// limit, bail.
105+
if (++counts.top() > max_subs) return true;
106+
} else if (ch == ')' && !counts.empty()) {
107+
// Fragment closed! Drop its sub count and resume to counting the number of subs for its parent.
108+
counts.pop();
109+
}
110+
}
111+
return false;
112+
}

src/test/fuzz/util/descriptor.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,4 +55,16 @@ constexpr int MAX_DEPTH{2};
5555
*/
5656
bool HasDeepDerivPath(const FuzzBufferType& buff, const int max_depth = MAX_DEPTH);
5757

58+
//! Default maximum number of sub-fragments.
59+
constexpr int MAX_SUBS{1'000};
60+
//! Maximum number of nested sub-fragments we'll allow in a descriptor.
61+
constexpr size_t MAX_NESTED_SUBS{10'000};
62+
63+
/**
64+
* Whether the buffer, if it represents a valid descriptor, contains a fragment with more
65+
* sub-fragments than the given maximum.
66+
*/
67+
bool HasTooManySubFrag(const FuzzBufferType& buff, const int max_subs = MAX_SUBS,
68+
const size_t max_nested_subs = MAX_NESTED_SUBS);
69+
5870
#endif // BITCOIN_TEST_FUZZ_UTIL_DESCRIPTOR_H

0 commit comments

Comments
 (0)