$ 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&&)
...
fuzz: `scriptpubkeyman`: heap-buffer-overflow miniscript.cpp in CScript BuildScript #30864
issue dergoegge opened this issue on September 10, 2024-
dergoegge commented at 5:25 PM on September 10, 2024: member
-
achow101 commented at 5:43 PM on September 10, 2024: member
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'sm_nodeis 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.
- fanquake closed this on Jan 22, 2025
- fanquake referenced this in commit 523520f827 on Jan 22, 2025
- bitcoin locked this on Jan 22, 2026