Merge bitcoin/bitcoin#34317: fuzz: Exclude too expensive inputs in descriptor_parse targets

fab2f3df4beb230eef63bdcf5042b6417c0012dc fuzz: Exclude too expensive inputs in descriptor_parse targets (MarcoFalke)

Pull request description:

  Accepting "expensive" fuzz inputs which have no real use-case is problematic, because it prevents the fuzz engine from spending time on the next useful fuzz input.

  For example, those will take several seconds (!) and the flamegraph shows that base58 encoding is the cause:

  ```
  curl -fLO 'f5abf41608'
  curl -fLO '78cb317546'

  FUZZ=mocked_descriptor_parse ./bld-cmake/bin/fuzz ./f5abf41608addcef3538da61d8096c2050235032
  FUZZ=descriptor_parse ./bld-cmake/bin/fuzz ./78cb3175467f53b467b949883ee6072e92dbb267
  ```

  This will also break 32-bit fuzzing, see https://github.com/bitcoin/bitcoin/issues/34110#issuecomment-3759461248.

  Fix all issues by checking for `HasTooLargeLeafSize`.

  Sorry for creating several pull requests to fix this class of issue, but I think this one should be the last one. 😅

ACKs for top commit:
  brunoerg:
    reACK fab2f3df4beb230eef63bdcf5042b6417c0012dc
  frankomosh:
    re-ACK fab2f3df4beb230eef63bdcf5042b6417c0012dc

Tree-SHA512: 4ecf98ec4adc39f6e014370945fb1598cdd3ceba60f7209b00789ac1164b6d20e82a69d71f8419d9a40d57ee3fea36ef593c47fe48b584b6e8344c44f20a15c1

src/test/fuzz/descriptor_parse.cpp

@@ -77,21 +77,9 @@ 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;
-
-    // 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;
-
-    // The script building logic performs quadratic copies in the number of nested wrappers. Limit
-    // the number of nested wrappers per fragment.
-    if (HasTooManyWrappers(buffer)) return;
-
     const std::string mocked_descriptor{buffer.begin(), buffer.end()};
     if (const auto descriptor = MOCKED_DESC_CONVERTER.GetDescriptor(mocked_descriptor)) {
+        if (IsTooExpensive(MakeUCharSpan(*descriptor))) return;
         FlatSigningProvider signing_provider;
         std::string error;
         const auto desc = Parse(*descriptor, signing_provider, error);
@@ -106,10 +94,7 @@ FUZZ_TARGET(mocked_descriptor_parse, .init = initialize_mocked_descriptor_parse)
 
 FUZZ_TARGET(descriptor_parse, .init = initialize_descriptor_parse)
 {
-    // See comments above for rationales.
-    if (HasDeepDerivPath(buffer)) return;
-    if (HasTooManySubFrag(buffer)) return;
-    if (HasTooManyWrappers(buffer)) return;
+    if (IsTooExpensive(buffer)) return;
 
     const std::string descriptor(buffer.begin(), buffer.end());
     FlatSigningProvider signing_provider;

src/test/fuzz/util/descriptor.cpp

@@ -7,12 +7,15 @@
 #include <key.h>
 #include <key_io.h>
 #include <pubkey.h>
+#include <span.h>
 #include <util/strencodings.h>
 
 #include <ranges>
 #include <stack>
+#include <vector>
 
-void MockedDescriptorConverter::Init() {
+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}};
     // Generate keys of all kinds and store them in the keys array.

src/test/fuzz/util/descriptor.h

@@ -86,4 +86,31 @@ constexpr uint32_t MAX_LEAF_SIZE{200};
 /// MockedDescriptorConverter) has a leaf size too large.
 bool HasTooLargeLeafSize(std::span<const uint8_t> buff, uint32_t max_leaf_size = MAX_LEAF_SIZE);
 
+/// Deriving "expensive" descriptors will consume useful fuzz compute. The
+/// compute is better spent on a smaller subset of descriptors, which still
+/// covers all real end-user settings.
+///
+/// Use this function after MockedDescriptorConverter::GetDescriptor()
+inline bool IsTooExpensive(std::span<const uint8_t> buffer)
+{
+    // Key derivation is expensive. Deriving deep derivation paths takes 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 true;
+
+    // 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 true;
+
+    // The script building logic performs quadratic copies in the number of nested wrappers. Limit
+    // the number of nested wrappers per fragment.
+    if (HasTooManyWrappers(buffer)) return true;
+
+    // If any suspected leaf is too large, it will likely not represent a valid
+    // use-case. Also, possible base58 parsing in the leaf is quadratic. So
+    // limit the leaf size.
+    if (HasTooLargeLeafSize(buffer)) return true;
+
+    return false;
+}
 #endif // BITCOIN_TEST_FUZZ_UTIL_DESCRIPTOR_H

src/wallet/test/fuzz/scriptpubkeyman.cpp

@@ -50,23 +50,12 @@ void initialize_spkm()
     MOCKED_DESC_CONVERTER.Init();
 }
 
-/**
- * Deriving "expensive" descriptors will consume useful fuzz compute. The
- * compute is better spent on a smaller subset of descriptors, which still
- * covers all real end-user settings.
- */
-static bool IsTooExpensive(std::span<const uint8_t> desc)
-{
-    return HasDeepDerivPath(desc) || HasTooManySubFrag(desc) || HasTooManyWrappers(desc);
-}
-
 static std::optional<std::pair<WalletDescriptor, FlatSigningProvider>> CreateWalletDescriptor(FuzzedDataProvider& fuzzed_data_provider)
 {
     const std::string mocked_descriptor{fuzzed_data_provider.ConsumeRandomLengthString()};
-    if (IsTooExpensive(MakeUCharSpan(mocked_descriptor))) return {};
     const auto desc_str{MOCKED_DESC_CONVERTER.GetDescriptor(mocked_descriptor)};
     if (!desc_str.has_value()) return std::nullopt;
-    if (HasTooLargeLeafSize(MakeUCharSpan(*desc_str))) return {};
+    if (IsTooExpensive(MakeUCharSpan(*desc_str))) return {};
 
     FlatSigningProvider keys;
     std::string error;