diff options
| author | Andrew Chow <github@achow101.com> | 2023-07-17 19:08:56 -0400 |
|---|---|---|
| committer | Andrew Chow <github@achow101.com> | 2023-07-17 19:16:09 -0400 |
| commit | bc88f3ab903f8a748a7570a6e17579dc4e9ba791 (patch) | |
| tree | 690ff2f9c0a2e7e5096a10a1b422ab64b4a314c7 /src/script/descriptor.cpp | |
| parent | 306157ae92f47b36a7ad438cf76969a1ab6ef401 (diff) | |
| parent | c7db88af71b3204171f33399aa4f33b40a4f7cd9 (diff) | |
Merge bitcoin/bitcoin#27997: Descriptors: rule out unspendable miniscript descriptors
c7db88af71b3204171f33399aa4f33b40a4f7cd9 descriptor: assert we never parse a sane miniscript with no pubkey (Antoine Poinsot)
a49402a9ec7431c286139b76f8759719a99a8551 qa: make sure we don't let unspendable Miniscript descriptors be imported (Antoine Poinsot)
639e3b6c9759a7a582c5c86fdbfa5ea99cb7bb16 descriptor: refuse to parse unspendable miniscript descriptors (Antoine Poinsot)
e3280eae1b53006d74d11f3cf9d7a9dc7ff2c39e 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 c7db88af71b3204171f33399aa4f33b40a4f7cd9
achow101:
ACK c7db88af71b3204171f33399aa4f33b40a4f7cd9
Tree-SHA512: e79bc9f7842e98a4e8f358f05811fca51b15b4b80a171c0d2b17cf4bb1f578a18e4397bc2ece9817d392e0de0196ee6a054b7318441fd3566dd22e1f03eb64a5
Diffstat (limited to 'src/script/descriptor.cpp')
| -rw-r--r-- | src/script/descriptor.cpp | 10 |
1 files changed, 8 insertions, 2 deletions
diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index dff6429259..f57f167e5d 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -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)); } } |