fuzz: `scriptpubkeyman`: heap-buffer-overflow miniscript.cpp in CScript BuildScript

[dergoegge]

$ echo "dHIoJTE3LzwyOzM+LGw6cGsoJTA4KSk=" | base64 --decode > scriptpubkeyman.crash
$ FUZZ=scriptpubkeyman src/test/fuzz/fuzz scriptpubkeyman.crash
...
SUMMARY: AddressSanitizer: heap-buffer-overflow miniscript.cpp in CScript BuildScript<opcodetype, CScript&, opcodetype, CScript&, opcodetype>(opcodetype&&, CScript&, opcodetype&&, CScript&, opcodetype&&)
...

[achow101]

I believe this is only an issue with the fuzzer as I can't trigger this crash outside of it. However, it does reveal an actual issue in the handling of multipath key expressions with miniscript.

The issue appears to be because MiniscriptDescriptor's m_node is shallow copied, and when cloned fragments belonging to the multipath components are destroyed later, various shared_ptrs end up also being destroyed.

The following diff fixes this particular crash, but I think it is insufficient and possibly incorrect:

diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp
index 5026470edcf..c1b898610c7 100644
--- a/src/script/descriptor.cpp
+++ b/src/script/descriptor.cpp
@@ -1360,7 +1360,7 @@ public:
         for (const auto& arg : m_pubkey_args) {
             providers.push_back(arg->Clone());
         }
-        return std::make_unique<MiniscriptDescriptor>(std::move(providers), miniscript::MakeNodeRef<uint32_t>(*m_node));
+        return std::make_unique<MiniscriptDescriptor>(std::move(providers), m_node);
     }
 };

I think the proper way to fix this is to deep copy the entire node, but this has the possibility of triggering a stack overflow.

[achow101]

30866