Merge bitcoin/bitcoin#28832: fuzz: rule-out too deep derivation paths in descriptor parsing targets

a44808fb437864878c2d9696b8a96193091446ee fuzz: rule-out too deep derivation paths in descriptor parsing targets (Antoine Poinsot)

Pull request description:

  This fixes the `mocked_descriptor_parse` timeout reported in #28812 and direct the targets more toward what they are intended to fuzz: the descriptor syntax.

ACKs for top commit:
  sipa:
    utACK a44808fb437864878c2d9696b8a96193091446ee
  achow101:
    ACK a44808fb437864878c2d9696b8a96193091446ee
  dergoegge:
    ACK a44808fb437864878c2d9696b8a96193091446ee - Not running into timeouts anymore
  TheCharlatan:
    ACK a44808fb437864878c2d9696b8a96193091446ee

Tree-SHA512: a5dd1dbe9adf8f088bdc435addab88b56f435e6d7d2065bd6d5c6d80a32e3f1f97d3d2323131ab233618cd6dcc477c458abe3c4c865ab569449b8bc176231e93

src/test/fuzz/descriptor_parse.cpp

@@ -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;
@@ -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;

src/test/fuzz/util/descriptor.cpp

@@ -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;
+}

src/test/fuzz/util/descriptor.h

@@ -8,6 +8,7 @@
 #include <key_io.h>
 #include <util/strencodings.h>
 #include <script/descriptor.h>
+#include <test/fuzz/fuzz.h>
 
 #include <functional>
 
@@ -45,4 +46,13 @@ public:
     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

src/wallet/test/fuzz/scriptpubkeyman.cpp

@@ -49,9 +49,21 @@ void initialize_spkm()
     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)
 {
     const std::string mocked_descriptor{fuzzed_data_provider.ConsumeRandomLengthString()};
+    if (TooDeepDerivPath(mocked_descriptor)) return {};
     const auto desc_str{MOCKED_DESC_CONVERTER.GetDescriptor(mocked_descriptor)};
     if (!desc_str.has_value()) return std::nullopt;