miniscript: check validity when decoding a Script

darosior · darosior · commit c1ecd137aab9 · 2021-09-28T18:52:07.000+02:00
diff --git a/bitcoin/script/miniscript.h b/bitcoin/script/miniscript.h
@@ -1089,6 +1089,8 @@ inline NodeRef<Key> DecodeScript(I& in, I last, const Ctx& ctx) {
     to_parse.emplace_back(DecodeContext::BKV_EXPR, -1, -1);
 
     while (!to_parse.empty()) {
+        // Exit early if the Miniscript is not going to be valid.
+        if (!constructed.empty() && !constructed.back()->IsValid()) return {};
 
         // Get the current context we are decoding within
         auto [cur_context, n, k] = to_parse.back();
@@ -1408,7 +1410,11 @@ inline NodeRef<Key> DecodeScript(I& in, I last, const Ctx& ctx) {
         }
     }
     if (constructed.size() != 1) return {};
-    return constructed.front();
+    const NodeRef<Key> tl_node = std::move(constructed.front());
+    // Note that due to how ComputeType works (only assign the type to the node if the
+    // subs' types are valid) this would fail if any node of tree is badly typed.
+    if (!tl_node->IsValidTopLevel()) return {};
+    return tl_node;
 }
 
 } // namespace internal

Original file line number	Diff line number	Diff line change
`@@ -1089,6 +1089,8 @@ inline NodeRef<Key> DecodeScript(I& in, I last, const Ctx& ctx) {`
`1089`	`1089`	`to_parse.emplace_back(DecodeContext::BKV_EXPR, -1, -1);`
`1090`	`1090`
`1091`	`1091`	`while (!to_parse.empty()) {`
	`1092`	`+ // Exit early if the Miniscript is not going to be valid.`
	`1093`	`+ if (!constructed.empty() && !constructed.back()->IsValid()) return {};`
`1092`	`1094`
`1093`	`1095`	`// Get the current context we are decoding within`
`1094`	`1096`	`auto [cur_context, n, k] = to_parse.back();`
`@@ -1408,7 +1410,11 @@ inline NodeRef<Key> DecodeScript(I& in, I last, const Ctx& ctx) {`
`1408`	`1410`	`}`
`1409`	`1411`	`}`
`1410`	`1412`	`if (constructed.size() != 1) return {};`
`1411`		`- return constructed.front();`
	`1413`	`+ const NodeRef<Key> tl_node = std::move(constructed.front());`
	`1414`	`+ // Note that due to how ComputeType works (only assign the type to the node if the`
	`1415`	`+ // subs' types are valid) this would fail if any node of tree is badly typed.`
	`1416`	`+ if (!tl_node->IsValidTopLevel()) return {};`
	`1417`	`+ return tl_node;`
`1412`	`1418`	`}`
`1413`	`1419`
`1414`	`1420`	`} // namespace internal`