From 31523aab5b74c8f956a4cba8adf1425ff7a7d4af Mon Sep 17 00:00:00 2001
From: Antoine Poinsot <darosior@protonmail.com>
Date: Wed, 29 Sep 2021 11:30:24 +0200
Subject: [PATCH 1/2] fuzz: add a fuzz target for Miniscript decoding

Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
---
 bitcoin/test/fuzz/miniscript_decode.cpp | 72 +++++++++++++++++++++++++
 1 file changed, 72 insertions(+)
 create mode 100644 bitcoin/test/fuzz/miniscript_decode.cpp

diff --git a/bitcoin/test/fuzz/miniscript_decode.cpp b/bitcoin/test/fuzz/miniscript_decode.cpp
new file mode 100644
index 0000000..4cc0a1b
--- /dev/null
+++ b/bitcoin/test/fuzz/miniscript_decode.cpp
@@ -0,0 +1,72 @@
+// Copyright (c) 2022 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <core_io.h>
+#include <hash.h>
+#include <key.h>
+#include <script/miniscript.h>
+#include <script/script.h>
+#include <span.h>
+#include <test/fuzz/FuzzedDataProvider.h>
+#include <test/fuzz/fuzz.h>
+#include <test/fuzz/util.h>
+#include <util/strencodings.h>
+
+#include <optional>
+
+using miniscript::operator""_mst;
+
+
+struct Converter {
+    typedef CPubKey Key;
+
+    bool ToString(const Key& key, std::string& ret) const {
+        ret = HexStr(key);
+        return true;
+    }
+    const std::vector<unsigned char> ToPKBytes(const Key& key) const {
+        return {key.begin(), key.end()};
+    }
+    const std::vector<unsigned char> ToPKHBytes(const Key& key) const {
+        const auto h = Hash160(key);
+        return {h.begin(), h.end()};
+    }
+
+    template<typename I>
+    bool FromString(I first, I last, Key& key) const {
+        const auto bytes = ParseHex(std::string(first, last));
+        key.Set(bytes.begin(), bytes.end());
+        return key.IsValid();
+    }
+    template<typename I>
+    bool FromPKBytes(I first, I last, CPubKey& key) const {
+        key.Set(first, last);
+        return key.IsValid();
+    }
+    template<typename I>
+    bool FromPKHBytes(I first, I last, CPubKey& key) const {
+        assert(last - first == 20);
+        return false;
+    }
+};
+
+const Converter CONVERTER;
+
+FUZZ_TARGET(miniscript_decode)
+{
+    FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
+    const std::optional<CScript> script = ConsumeDeserializable<CScript>(fuzzed_data_provider);
+    if (!script) return;
+
+    const auto ms = miniscript::FromScript(*script, CONVERTER);
+    if (!ms) return;
+
+    // We can roundtrip it to its string representation.
+    std::string ms_str;
+    assert(ms->ToString(CONVERTER, ms_str));
+    assert(*miniscript::FromString(ms_str, CONVERTER) == *ms);
+    // The Script representation must roundtrip since we parsed it this way the first time.
+    const CScript ms_script = ms->ToScript(CONVERTER);
+    assert(ms_script == *script);
+}

From 44b47731ed9e6b9299a74c645a4a89d30aa7a7e7 Mon Sep 17 00:00:00 2001
From: Antoine Poinsot <darosior@protonmail.com>
Date: Fri, 11 Feb 2022 15:00:27 +0100
Subject: [PATCH 2/2] miniscript: constructed fragments stack size checks in
 DecodeScript()

The miniscript_decode fuzz target demonstrated this could lead to
crashes.
---
 bitcoin/script/miniscript.h | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/bitcoin/script/miniscript.h b/bitcoin/script/miniscript.h
index 7d4aa00..59aa130 100644
--- a/bitcoin/script/miniscript.h
+++ b/bitcoin/script/miniscript.h
@@ -1634,58 +1634,69 @@ inline NodeRef<Key> DecodeScript(I& in, I last, const Ctx& ctx)
             break;
         }
         case DecodeContext::SWAP: {
-            if (in >= last || in[0].first != OP_SWAP) return {};
+            if (in >= last || in[0].first != OP_SWAP || constructed.empty()) return {};
             ++in;
             constructed.back() = MakeNodeRef<Key>(NodeType::WRAP_S, Vector(std::move(constructed.back())));
             break;
         }
         case DecodeContext::ALT: {
-            if (in >= last || in[0].first != OP_TOALTSTACK) return {};
+            if (in >= last || in[0].first != OP_TOALTSTACK || constructed.empty()) return {};
             ++in;
             constructed.back() = MakeNodeRef<Key>(NodeType::WRAP_A, Vector(std::move(constructed.back())));
             break;
         }
         case DecodeContext::CHECK: {
+            if (constructed.empty()) return {};
             constructed.back() = MakeNodeRef<Key>(NodeType::WRAP_C, Vector(std::move(constructed.back())));
             break;
         }
         case DecodeContext::DUP_IF: {
+            if (constructed.empty()) return {};
             constructed.back() = MakeNodeRef<Key>(NodeType::WRAP_D, Vector(std::move(constructed.back())));
             break;
         }
         case DecodeContext::VERIFY: {
+            if (constructed.empty()) return {};
             constructed.back() = MakeNodeRef<Key>(NodeType::WRAP_V, Vector(std::move(constructed.back())));
             break;
         }
         case DecodeContext::NON_ZERO: {
+            if (constructed.empty()) return {};
             constructed.back() = MakeNodeRef<Key>(NodeType::WRAP_J, Vector(std::move(constructed.back())));
             break;
         }
         case DecodeContext::ZERO_NOTEQUAL: {
+            if (constructed.empty()) return {};
             constructed.back() = MakeNodeRef<Key>(NodeType::WRAP_N, Vector(std::move(constructed.back())));
             break;
         }
         case DecodeContext::AND_V: {
+            if (constructed.size() < 2) return {};
             BuildBack(NodeType::AND_V, constructed, /* reverse */ true);
             break;
         }
         case DecodeContext::AND_B: {
+            if (constructed.size() < 2) return {};
             BuildBack(NodeType::AND_B, constructed, /* reverse */ true);
             break;
         }
         case DecodeContext::OR_B: {
+            if (constructed.size() < 2) return {};
             BuildBack(NodeType::OR_B, constructed, /* reverse */ true);
             break;
         }
         case DecodeContext::OR_C: {
+            if (constructed.size() < 2) return {};
             BuildBack(NodeType::OR_C, constructed, /* reverse */ true);
             break;
         }
         case DecodeContext::OR_D: {
+            if (constructed.size() < 2) return {};
             BuildBack(NodeType::OR_D, constructed, /* reverse */ true);
             break;
         }
         case DecodeContext::ANDOR: {
+            if (constructed.size() < 3) return {};
             NodeRef<Key> left = std::move(constructed.back());
             constructed.pop_back();
             NodeRef<Key> right = std::move(constructed.back());
@@ -1708,7 +1719,7 @@ inline NodeRef<Key> DecodeScript(I& in, I last, const Ctx& ctx)
             break;
         }
         case DecodeContext::THRESH_E: {
-            if (k < 1 || k > n) return {};
+            if (k < 1 || k > n || constructed.size() < static_cast<size_t>(n)) return {};
             std::vector<NodeRef<Key>> subs;
             for (int i = 0; i < n; ++i) {
                 NodeRef<Key> sub = std::move(constructed.back());