Skip to content

Commit 9cd4091

Browse files
committed
Verify the holder provided valid witnesses and uses SIGHASH_ALL
LDK checks the following: * Each input spends an output that is one of P2WPKH, P2WSH, or P2TR. These were already checked by LDK when the inputs to be contributed were provided. * All signatures use the `SIGHASH_ALL` sighash type. * P2WPKH and P2TR key path spends are valid (verifies signatures) NOTE: * When checking P2WSH spends, LDK tries to decode 70-72 byte witness elements as ECDSA signatures with a sighash flag. If the internal DER-decoding fails, then LDK just assumes it wasn't a signature and carries with checks. If the element can be decoded as an ECDSA signature, the the sighash flag must be `SIGHASH_ALL`. * When checking P2TR script-path spends, LDK assumes all elements of exactly 65 bytes with the last byte matching any valid sighash flag byte are schnorr signatures and checks that the sighash type is `SIGHASH_ALL`. If the last byte is not any valid sighash flag, the element is assumed not to be a signature and is ignored. Elements of 64 bytes are not checked because if they were schnorr signatures then they would implicitly be `SIGHASH_DEFAULT` which is an alias of `SIGHASH_ALL`.
1 parent c2b293c commit 9cd4091

File tree

3 files changed

+537
-27
lines changed

3 files changed

+537
-27
lines changed

lightning/src/ln/channel.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7991,7 +7991,11 @@ where
79917991
})
79927992
.and_then(|signing_session| {
79937993
signing_session
7994-
.provide_holder_witnesses(self.context.channel_id, witnesses)
7994+
.provide_holder_witnesses(
7995+
&self.context.secp_ctx,
7996+
self.context.channel_id,
7997+
witnesses,
7998+
)
79957999
.map_err(|err| APIError::APIMisuseError { err })
79968000
})?;
79978001

lightning/src/ln/channelmanager.rs

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5933,19 +5933,32 @@ where
59335933
/// counterparty's signature(s) the funding transaction will automatically be broadcast via the
59345934
/// [`BroadcasterInterface`] provided when this `ChannelManager` was constructed.
59355935
///
5936-
/// `SIGHASH_ALL` MUST be used for all signatures when providing signatures.
5937-
///
5938-
/// <div class="warning">
5939-
/// WARNING: LDK makes no attempt to prevent the counterparty from using non-standard inputs which
5940-
/// will prevent the funding transaction from being relayed on the bitcoin network and hence being
5941-
/// confirmed.
5942-
/// </div>
5936+
/// `SIGHASH_ALL` MUST be used for all signatures when providing signatures, otherwise your
5937+
/// funds can be held hostage!
5938+
///
5939+
/// LDK checks the following:
5940+
/// * Each input spends an output that is one of P2WPKH, P2WSH, or P2TR.
5941+
/// These were already checked by LDK when the inputs to be contributed were provided.
5942+
/// * All signatures use the `SIGHASH_ALL` sighash type.
5943+
/// * P2WPKH and P2TR key path spends are valid (verifies signatures)
5944+
///
5945+
/// NOTE:
5946+
/// * When checking P2WSH spends, LDK tries to decode 70-72 byte witness elements as ECDSA
5947+
/// signatures with a sighash flag. If the internal DER-decoding fails, then LDK just
5948+
/// assumes it wasn't a signature and carries with checks. If the element can be decoded
5949+
/// as an ECDSA signature, the the sighash flag must be `SIGHASH_ALL`.
5950+
/// * When checking P2TR script-path spends, LDK assumes all elements of exactly 65 bytes
5951+
/// with the last byte matching any valid sighash flag byte are schnorr signatures and checks
5952+
/// that the sighash type is `SIGHASH_ALL`. If the last byte is not any valid sighash flag, the
5953+
/// element is assumed not to be a signature and is ignored. Elements of 64 bytes are not
5954+
/// checked because if they were schnorr signatures then they would implicitly be `SIGHASH_DEFAULT`
5955+
/// which is an alias of `SIGHASH_ALL`.
59435956
///
59445957
/// Returns [`ChannelUnavailable`] when a channel is not found or an incorrect
59455958
/// `counterparty_node_id` is provided.
59465959
///
59475960
/// Returns [`APIMisuseError`] when a channel is not in a state where it is expecting funding
5948-
/// signatures.
5961+
/// signatures or if any of the checks described above fail.
59495962
///
59505963
/// [`FundingTransactionReadyForSigning`]: events::Event::FundingTransactionReadyForSigning
59515964
/// [`ChannelUnavailable`]: APIError::ChannelUnavailable

0 commit comments

Comments
 (0)