Skip to content

Commit

Permalink
sign: don't assume we are parsing a sane Miniscript
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
darosior committed Apr 22, 2024
1 parent bdb33ec commit 4d8d213
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/script/sign.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
24 changes: 24 additions & 0 deletions src/test/script_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 4d8d213

Please sign in to comment.