foster/bitcoin-core
0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-03-06 14:19:59 -05:00
Code Issues Activity

Merge bitcoin/bitcoin#29853: sign: don't assume we are parsing a sane TapMiniscript

Browse source 
4d8d21320e sign: don't assume we are parsing a sane Miniscript (Antoine Poinsot)

Pull request description:

  The script provided for signature might be externally provided, for instance by way of 'finalizepsbt'. Therefore the script might be ill-crafted, so don't assume pubkeys are always 32 bytes.

  Thanks to Niklas for finding this.

  FIxes https://github.com/bitcoin/bitcoin/issues/29851.

ACKs for top commit:
  achow101:
    ACK 4d8d21320e
  furszy:
    ACK 4d8d21320e with a small nuance that could be tackled in a follow-up by someone else (or never).

Tree-SHA512: 29b7948b56e6dc05eac1014d684f2129ab1d19cb1e5d304216c826b7057c0e1d84ceb18731b91124b680e17d90e38de9f9a5526e4f6ecc3ea816881a6599bb47
This commit is contained in:
merge-script 2024-04-24 21:14:26 +08:00
commit c143244ce3
No known key found for this signature in database
GPG key ID: 2EEB9F5CC09526C1
2 changed files with 25 additions and 1 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
src/script/sign.cpp
View file

@ -295,7 +295,7 @@ struct TapSatisfier: Satisfier<XOnlyPubKey> {
//! Conversion from a raw xonly public key.
template <typename I>
std::optional<XOnlyPubKey> FromPKBytes(I first, I last) const {
CHECK_NONFATAL(last - first == 32);
if (last - first != 32) return {};
XOnlyPubKey pubkey;
std::copy(first, last, pubkey.begin());
return pubkey;

24
src/test/script_tests.cpp
View file

@ -1254,6 +1254,30 @@ BOOST_AUTO_TEST_CASE(script_combineSigs)
BOOST_CHECK(combined.scriptSig == partial3c);
}
/**
* Reproduction of an exception incorrectly raised when parsing a public key inside a TapMiniscript.
*/
BOOST_AUTO_TEST_CASE(sign_invalid_miniscript)
{
FillableSigningProvider keystore;
SignatureData sig_data;
CMutableTransaction prev, curr;
// Create a Taproot output which contains a leaf in which a non-32 bytes push is used where a public key is expected
// by the Miniscript parser. This offending Script was found by the RPC fuzzer.
const auto invalid_pubkey{ParseHex("173d36c8c9c9c9ffffffffffff0200000000021e1e37373721361818181818181e1e1e1e19000000000000000000b19292929292926b006c9b9b9292")};
TaprootBuilder builder;
builder.Add(0, {invalid_pubkey}, 0xc0);
XOnlyPubKey nums{ParseHex("50929b74c1a04954b78b4b6035e97a5e078a5a0f28ec96d547bfee9ace803ac0")};
builder.Finalize(nums);
prev.vout.emplace_back(0, GetScriptForDestination(builder.GetOutput()));
curr.vin.emplace_back(COutPoint{prev.GetHash(), 0});
sig_data.tr_spenddata = builder.GetSpendData();
// SignSignature can fail but it shouldn't raise an exception (nor crash).
BOOST_CHECK(!SignSignature(keystore, CTransaction(prev), curr, 0, SIGHASH_ALL, sig_data));
}
BOOST_AUTO_TEST_CASE(script_standard_push)
{
ScriptError err;