fuzz: rule-out too deep derivation paths in descriptor parsing targets

This fixes the reported timeouts and direct the target cycles toward what it's intended to fuzz: the descriptor syntax.
This commit is contained in:
Antoine Poinsot 2023-11-09 14:53:15 +01:00
parent 4b1196a985
commit a44808fb43
No known key found for this signature in database
GPG Key ID: E13FC145CD3F4304
4 changed files with 44 additions and 0 deletions

View File

@ -67,6 +67,11 @@ void initialize_mocked_descriptor_parse()
FUZZ_TARGET(mocked_descriptor_parse, .init = 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()}; const std::string mocked_descriptor{buffer.begin(), buffer.end()};
if (const auto descriptor = MOCKED_DESC_CONVERTER.GetDescriptor(mocked_descriptor)) { if (const auto descriptor = MOCKED_DESC_CONVERTER.GetDescriptor(mocked_descriptor)) {
FlatSigningProvider signing_provider; FlatSigningProvider signing_provider;
@ -78,6 +83,9 @@ FUZZ_TARGET(mocked_descriptor_parse, .init = initialize_mocked_descriptor_parse)
FUZZ_TARGET(descriptor_parse, .init = initialize_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()); const std::string descriptor(buffer.begin(), buffer.end());
FlatSigningProvider signing_provider; FlatSigningProvider signing_provider;
std::string error; std::string error;

View File

@ -70,3 +70,17 @@ std::optional<std::string> MockedDescriptorConverter::GetDescriptor(std::string_
return desc; 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;
}

View File

@ -8,6 +8,7 @@
#include <key_io.h> #include <key_io.h>
#include <util/strencodings.h> #include <util/strencodings.h>
#include <script/descriptor.h> #include <script/descriptor.h>
#include <test/fuzz/fuzz.h>
#include <functional> #include <functional>
@ -45,4 +46,13 @@ public:
std::optional<std::string> GetDescriptor(std::string_view mocked_desc) const; 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 #endif // BITCOIN_TEST_FUZZ_UTIL_DESCRIPTOR_H

View File

@ -49,9 +49,21 @@ void initialize_spkm()
MOCKED_DESC_CONVERTER.Init(); MOCKED_DESC_CONVERTER.Init();
} }
/**
* 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 actually fuzzing the DescriptorScriptPubKeyMan. So rule out strings which could
* correspond to a descriptor containing a too large derivation path.
*/
static bool TooDeepDerivPath(std::string_view desc)
{
const FuzzBufferType desc_buf{reinterpret_cast<const unsigned char *>(desc.data()), desc.size()};
return HasDeepDerivPath(desc_buf);
}
static std::optional<std::pair<WalletDescriptor, FlatSigningProvider>> CreateWalletDescriptor(FuzzedDataProvider& fuzzed_data_provider) static std::optional<std::pair<WalletDescriptor, FlatSigningProvider>> CreateWalletDescriptor(FuzzedDataProvider& fuzzed_data_provider)
{ {
const std::string mocked_descriptor{fuzzed_data_provider.ConsumeRandomLengthString()}; const std::string mocked_descriptor{fuzzed_data_provider.ConsumeRandomLengthString()};
if (TooDeepDerivPath(mocked_descriptor)) return {};
const auto desc_str{MOCKED_DESC_CONVERTER.GetDescriptor(mocked_descriptor)}; const auto desc_str{MOCKED_DESC_CONVERTER.GetDescriptor(mocked_descriptor)};
if (!desc_str.has_value()) return std::nullopt; if (!desc_str.has_value()) return std::nullopt;