highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2026-02-11 09:42:17 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#27997: Descriptors: rule out unspendable miniscript descriptors

Browse Source 
c7db88af71 descriptor: assert we never parse a sane miniscript with no pubkey (Antoine Poinsot)
a49402a9ec qa: make sure we don't let unspendable Miniscript descriptors be imported (Antoine Poinsot)
639e3b6c97 descriptor: refuse to parse unspendable miniscript descriptors (Antoine Poinsot)
e3280eae1b miniscript: make GetStackSize() and GetOps() return optionals (Antoine Poinsot)

Pull request description:

  `IsSane()` in Miniscript does not ensure a Script is actually spendable. This is an issue as we would accept any sane Miniscript when parsing a descriptor. Fix this by explicitly checking a Miniscript descriptor is both sane and spendable when parsing it.

  This bug was exposed due to a check added in #22838 (https://github.com/bitcoin/bitcoin/pull/22838#discussion_r1226859880) that triggered a fuzz crash (https://github.com/bitcoin/bitcoin/pull/22838#issuecomment-1612510057).

ACKs for top commit:
  sipa:
    utACK c7db88af71
  achow101:
    ACK c7db88af71

Tree-SHA512: e79bc9f7842e98a4e8f358f05811fca51b15b4b80a171c0d2b17cf4bb1f578a18e4397bc2ece9817d392e0de0196ee6a054b7318441fd3566dd22e1f03eb64a5
This commit is contained in:
Andrew Chow
2023-07-17 19:08:56 -04:00
parent 306157ae92 c7db88af71
commit bc88f3ab90
5 changed files with 48 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
src/script/descriptor.cpp
View File

@@ -15,6 +15,7 @@
#include <common/args.h>
#include <span.h>
#include <util/bip32.h>
#include <util/check.h>
#include <util/spanparsing.h>
#include <util/strencodings.h>
#include <util/vector.h>
@@ -1553,14 +1554,14 @@ std::unique_ptr<DescriptorImpl> ParseScript(uint32_t& key_exp_index, Span<const
error = std::move(parser.m_key_parsing_error);
return nullptr;
}
if (!node->IsSane()) {
if (!node->IsSane() || node->IsNotSatisfiable()) {
// Try to find the first insane sub for better error reporting.
auto insane_node = node.get();
if (const auto sub = node->FindInsaneSub()) insane_node = sub;
if (const auto str = insane_node->ToString(parser)) error = *str;
if (!insane_node->IsValid()) {
error += " is invalid";
} else {
} else if (!node->IsSane()) {
error += " is not sane";
if (!insane_node->IsNonMalleable()) {
error += ": malleable witnesses exist";
@@ -1573,9 +1574,14 @@ std::unique_ptr<DescriptorImpl> ParseScript(uint32_t& key_exp_index, Span<const
} else if (!insane_node->ValidSatisfactions()) {
error += ": needs witnesses that may exceed resource limits";
}
} else {
error += " is not satisfiable";
}
return nullptr;
}
// A signature check is required for a miniscript to be sane. Therefore no sane miniscript
// may have an empty list of public keys.
CHECK_NONFATAL(!parser.m_keys.empty());
return std::make_unique<MiniscriptDescriptor>(std::move(parser.m_keys), std::move(node));
}
}