Skip to content

Commit

Permalink
fuzz: rule-out too deep derivation paths in descriptor parsing targets
Browse files Browse the repository at this point in the history
This fixes the reported timeouts and direct the target cycles toward what it's intended to fuzz: the descriptor syntax.
  • Loading branch information
darosior committed Dec 31, 2023
1 parent 4b1196a commit f5942c5
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 0 deletions.
8 changes: 8 additions & 0 deletions src/test/fuzz/descriptor_parse.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,11 @@ 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;

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

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

const std::string descriptor(buffer.begin(), buffer.end());
FlatSigningProvider signing_provider;
std::string error;
Expand Down
14 changes: 14 additions & 0 deletions src/test/fuzz/util/descriptor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,17 @@ std::optional<std::string> MockedDescriptorConverter::GetDescriptor(std::string_

return desc;
}

bool HasDeepDerivPath(const FuzzBufferType& buff, const int max_depth)
{
auto depth{0};
for (const auto& ch: buff) {
if (ch == ',') {
// A comma is always present between two key expressions, so we use that as a delimiter.
depth = 0;
} else if (ch == '/') {
if (++depth > max_depth) return true;
}
}
return false;
}
10 changes: 10 additions & 0 deletions src/test/fuzz/util/descriptor.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <key_io.h>
#include <util/strencodings.h>
#include <script/descriptor.h>
#include <test/fuzz/fuzz.h>

#include <functional>

Expand Down Expand Up @@ -45,4 +46,13 @@ class MockedDescriptorConverter {
std::optional<std::string> GetDescriptor(std::string_view mocked_desc) const;
};

//! Default maximum number of derivation indexes in a single derivation path when limiting its depth.
constexpr int MAX_DEPTH{2};

/**
* Whether the buffer, if it represents a valid descriptor, contains a derivation path deeper than
* a given maximum depth. Note this may also be hit for deriv paths in origins.
*/
bool HasDeepDerivPath(const FuzzBufferType& buff, const int max_depth = MAX_DEPTH);

#endif // BITCOIN_TEST_FUZZ_UTIL_DESCRIPTOR_H

0 comments on commit f5942c5

Please sign in to comment.