Skip to content

Commit

Permalink
fuzz: limit the number of sub-fragments per fragment for descriptors
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
darosior committed Jul 11, 2024
1 parent 00feabf commit 0d961c1
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 1 deletion.
7 changes: 6 additions & 1 deletion src/test/fuzz/descriptor_parse.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,10 @@ FUZZ_TARGET(mocked_descriptor_parse, .init = initialize_mocked_descriptor_parse)
// 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;

const std::string mocked_descriptor{buffer.begin(), buffer.end()};
if (const auto descriptor = MOCKED_DESC_CONVERTER.GetDescriptor(mocked_descriptor)) {
FlatSigningProvider signing_provider;
Expand All @@ -83,8 +87,9 @@ FUZZ_TARGET(mocked_descriptor_parse, .init = initialize_mocked_descriptor_parse)

FUZZ_TARGET(descriptor_parse, .init = initialize_descriptor_parse)
{
// See comment above for rationale.
// See comments above for rationales.
if (HasDeepDerivPath(buffer)) return;
if (HasTooManySubFrag(buffer)) return;

const std::string descriptor(buffer.begin(), buffer.end());
FlatSigningProvider signing_provider;
Expand Down
29 changes: 29 additions & 0 deletions src/test/fuzz/util/descriptor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

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

#include <stack>

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}};
Expand Down Expand Up @@ -84,3 +86,30 @@ bool HasDeepDerivPath(const FuzzBufferType& buff, const int max_depth)
}
return false;
}

//! Maximum number of nested sub-fragments we'll allow in a descriptor.
constexpr int MAX_NESTED_SUBS{10'000};

bool HasTooManySubFrag(const FuzzBufferType& buff, const int max_subs)
{
// We use a stack because there may be many nested sub-frags.
std::stack<int> counts;
for (const auto& ch: buff) {
// The fuzzer may generate an input with a ton of parenthesis. Rule out pathological cases.
if (counts.size() > MAX_NESTED_SUBS) return false;

if (ch == '(') {
// A new fragment was opened, create a new sub-count for it and start as one since any fragment with
// parenthesis has at least one sub.
counts.push(1);
} else if (ch == ',' && !counts.empty()) {
// When encountering a comma, account for an additional sub in the last opened fragment. If it exceeds the
// limit, bail.
if (++counts.top() > max_subs) return true;
} else if (ch == ')' && !counts.empty()) {
// Fragment closed! Drop its sub count and resume to counting the number of subs for its parent.
counts.pop();
}
}
return false;
}
9 changes: 9 additions & 0 deletions src/test/fuzz/util/descriptor.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,4 +55,13 @@ constexpr int MAX_DEPTH{2};
*/
bool HasDeepDerivPath(const FuzzBufferType& buff, const int max_depth = MAX_DEPTH);

//! Default maximum number of sub-fragments.
constexpr int MAX_SUBS{1'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);

#endif // BITCOIN_TEST_FUZZ_UTIL_DESCRIPTOR_H

0 comments on commit 0d961c1

Please sign in to comment.