Account for message type prefix len in prevtx length enforcement
What the developer wrote
Authored by Wilmer Paulino
Account for message type prefix len in prevtx length enforcement
Lightning wire messages have a maximum size of 65535 bytes, which
already accounts for the required two-byte message type prefix. Our
`TxAddInput::prevtx` length enforcement relied solely on the
`serialized_length` of the message, which does not account for the
prefix.
What changed, and why it matters
This commit fixes a small but real accounting bug in how rust-lightning checks whether a previous transaction (prevtx) attached to a funding input will make the resulting Lightning wire message too large. The code previously compared the message body length against the 65,535-byte wire limit, but forgot to add the 2-byte message type prefix. That meant a prevtx could be accepted even though the final on-the-wire message would exceed the protocol's maximum size, which would likely cause the peer to reject or fail to parse the message. The fix adds those 2 bytes to the calculation and updates the test to hit the real boundary.
Review whether any other message-size validations in the codebase also omit the 2-byte message type prefix. Backport to maintained release branches if dual-funding is enabled there. No immediate emergency response is indicated, but the fix should ride along with the next maintenance release.
Security signals we found
Off-by-constant length check in protocol message size enforcement
Potential acceptance of a prevtx that produces an oversized Lightning wire message
Denial-of-service / protocol-interop risk from peer message rejection
Test updated to exercise the corrected boundary
Evidence from the diff
In lightning/src/ln/funding.rs, validate_inputs enforces that a constructed TxAddInput message fits within LN_MAX_MSG_LEN (65535). The old code computed MESSAGE_TEMPLATE.serialized_length() + input.prevtx.serialized_length() and compared it to LN_MAX_MSG_LEN. However, serialized_length() on an msgs::TxAddInput returns only the payload length; the 2-byte big-endian message type prefix is added by the wire encoder. The corrected code adds MESSAGE_TYPE_PREFIX_LEN (2 bytes). The unit test was also updated: instead of a prevtx whose body alone exceeds the limit, it now crafts a prevtx where msg.serialized_length() + 2 == LN_MAX_MSG_LEN + 1, i.e. exactly one byte over the true wire limit, confirming the boundary is now enforced correctly.
Changed components
lightning/src/ln/funding.rsDual-funded channel input validationTxAddInput message constructionInspect captured patch +21 / −4
diff --git a/lightning/src/ln/funding.rs b/lightning/src/ln/funding.rs
index bcc5c66..df7bc2f 100644
--- a/lightning/src/ln/funding.rs
+++ b/lightning/src/ln/funding.rs
@@ -526,6 +526,7 @@ fn validate_inputs(inputs: &[ConfirmedUtxo]) -> Result<(), FundingContributionEr
}
use crate::util::ser::Writeable;
+ const MESSAGE_TYPE_PREFIX_LEN: usize = 2;
const MESSAGE_TEMPLATE: msgs::TxAddInput = msgs::TxAddInput {
channel_id: ChannelId([0; 32]),
serial_id: 0,
@@ -535,7 +536,9 @@ fn validate_inputs(inputs: &[ConfirmedUtxo]) -> Result<(), FundingContributionEr
// Mutually exclusive with prevtx, which is accounted for below.
shared_input_txid: None,
};
- let message_len = MESSAGE_TEMPLATE.serialized_length() + input.prevtx.serialized_length();
+ let message_len = MESSAGE_TYPE_PREFIX_LEN
+ + MESSAGE_TEMPLATE.serialized_length()
+ + input.prevtx.serialized_length();
(message_len <= LN_MAX_MSG_LEN)
.then(|| ())
.ok_or(FundingContributionError::PrevTxTooLarge)?;
@@ -2788,17 +2791,31 @@ mod tests {
}
#[test]
- fn test_build_funding_contribution_rejects_oversized_prevtx() {
+ fn test_build_funding_contribution_rejects_prevtx_exceeding_wire_message_limit() {
use crate::util::ser::Writeable;
let feerate = FeeRate::from_sat_per_kwu(2000);
let prevtx = Transaction {
input: vec![],
- output: vec![funding_output_sats(50_000); 2_200],
+ output: vec![
+ funding_output_sats(50_000),
+ TxOut {
+ value: Amount::ZERO,
+ script_pubkey: ScriptBuf::from_bytes(vec![0; 65_430]),
+ },
+ ],
version: Version::TWO,
lock_time: bitcoin::absolute::LockTime::ZERO,
};
- assert!(prevtx.serialized_length() > crate::ln::LN_MAX_MSG_LEN);
+ let msg = crate::ln::msgs::TxAddInput {
+ channel_id: crate::ln::types::ChannelId([0; 32]),
+ serial_id: 0,
+ prevtx: Some(prevtx.clone()),
+ prevtx_out: 0,
+ sequence: 0,
+ shared_input_txid: None,
+ };
+ assert_eq!(msg.serialized_length() + 2, crate::ln::LN_MAX_MSG_LEN + 1);
let wallet = SingleUtxoWallet {
utxo: ConfirmedUtxo::new_p2wpkh(prevtx, 0).unwrap(),
Why this scored 45/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.