From 25b9fd8079fe9e8bf1f61c26921614abe7b5ff84 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 22 Mar 2021 18:27:34 -0400 Subject: [PATCH] Fix serialization expected lengths and check them in test/fuzzing --- lightning/src/ln/chan_utils.rs | 9 +++++++-- lightning/src/ln/msgs.rs | 12 ++++++------ lightning/src/util/config.rs | 2 +- lightning/src/util/ser_macros.rs | 28 +++++++++++++++++++++++++--- 4 files changed, 39 insertions(+), 12 deletions(-) diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs index 647fc323..da7ba15a 100644 --- a/lightning/src/ln/chan_utils.rs +++ b/lightning/src/ln/chan_utils.rs @@ -39,6 +39,7 @@ use std::io::Read; use std::ops::Deref; use chain; +// Maximum size of a serialized HTLCOutputInCommitment const HTLC_OUTPUT_IN_COMMITMENT_SIZE: usize = 1 + 8 + 4 + 32 + 5; pub(crate) const MAX_HTLCS: u16 = 483; @@ -320,7 +321,8 @@ pub struct TxCreationKeys { /// Broadcaster's Payment Key (which isn't allowed to be spent from for some delay) pub broadcaster_delayed_payment_key: PublicKey, } -impl_writeable!(TxCreationKeys, 33*6, + +impl_writeable!(TxCreationKeys, 33*5, { per_commitment_point, revocation_key, broadcaster_htlc_key, countersignatory_htlc_key, broadcaster_delayed_payment_key }); /// One counterparty's public keys which do not change over the life of a channel. @@ -427,7 +429,10 @@ pub struct HTLCOutputInCommitment { pub transaction_output_index: Option, } -impl_writeable!(HTLCOutputInCommitment, HTLC_OUTPUT_IN_COMMITMENT_SIZE, { +impl_writeable_len_match!(HTLCOutputInCommitment, { + { HTLCOutputInCommitment { transaction_output_index: None, .. }, HTLC_OUTPUT_IN_COMMITMENT_SIZE - 4 }, + { _, HTLC_OUTPUT_IN_COMMITMENT_SIZE } + }, { offered, amount_msat, cltv_expiry, diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index 01a43784..8a7713dc 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -1396,7 +1396,7 @@ impl Readable for Pong { impl Writeable for UnsignedChannelAnnouncement { fn write(&self, w: &mut W) -> Result<(), ::std::io::Error> { - w.size_hint(2 + 2*32 + 4*33 + self.features.byte_count() + self.excess_data.len()); + w.size_hint(2 + 32 + 8 + 4*33 + self.features.byte_count() + self.excess_data.len()); self.features.write(w)?; self.chain_hash.write(w)?; self.short_channel_id.write(w)?; @@ -1430,7 +1430,7 @@ impl Readable for UnsignedChannelAnnouncement { impl_writeable_len_match!(ChannelAnnouncement, { { ChannelAnnouncement { contents: UnsignedChannelAnnouncement {ref features, ref excess_data, ..}, .. }, - 2 + 2*32 + 4*33 + features.byte_count() + excess_data.len() + 4*64 } + 2 + 32 + 8 + 4*33 + features.byte_count() + excess_data.len() + 4*64 } }, { node_signature_1, node_signature_2, @@ -1491,8 +1491,8 @@ impl Readable for UnsignedChannelUpdate { } impl_writeable_len_match!(ChannelUpdate, { - { ChannelUpdate { contents: UnsignedChannelUpdate {ref excess_data, ..}, .. }, - 64 + excess_data.len() + 64 } + { ChannelUpdate { contents: UnsignedChannelUpdate {ref excess_data, ref htlc_maximum_msat, ..}, .. }, + 64 + 64 + excess_data.len() + if let OptionalField::Present(_) = htlc_maximum_msat { 8 } else { 0 } } }, { signature, contents @@ -1528,7 +1528,7 @@ impl Readable for ErrorMessage { impl Writeable for UnsignedNodeAnnouncement { fn write(&self, w: &mut W) -> Result<(), ::std::io::Error> { - w.size_hint(64 + 76 + self.features.byte_count() + self.addresses.len()*38 + self.excess_address_data.len() + self.excess_data.len()); + w.size_hint(76 + self.features.byte_count() + self.addresses.len()*38 + self.excess_address_data.len() + self.excess_data.len()); self.features.write(w)?; self.timestamp.write(w)?; self.node_id.write(w)?; @@ -1611,7 +1611,7 @@ impl Readable for UnsignedNodeAnnouncement { } } -impl_writeable_len_match!(NodeAnnouncement, { +impl_writeable_len_match!(NodeAnnouncement, <=, { { NodeAnnouncement { contents: UnsignedNodeAnnouncement { ref features, ref addresses, ref excess_address_data, ref excess_data, ..}, .. }, 64 + 76 + features.byte_count() + addresses.len()*(NetAddress::MAX_LEN as usize + 1) + excess_address_data.len() + excess_data.len() } }, { diff --git a/lightning/src/util/config.rs b/lightning/src/util/config.rs index 4a9f3006..de83f227 100644 --- a/lightning/src/util/config.rs +++ b/lightning/src/util/config.rs @@ -223,7 +223,7 @@ impl Default for ChannelConfig { } //Add write and readable traits to channelconfig -impl_writeable!(ChannelConfig, 8+2+1+1, { +impl_writeable!(ChannelConfig, 4+2+1+1, { fee_proportional_millionths, cltv_expiry_delta, announced_channel, diff --git a/lightning/src/util/ser_macros.rs b/lightning/src/util/ser_macros.rs index ceabcc1d..e99c4e0e 100644 --- a/lightning/src/util/ser_macros.rs +++ b/lightning/src/util/ser_macros.rs @@ -120,6 +120,16 @@ macro_rules! impl_writeable { if $len != 0 { w.size_hint($len); } + #[cfg(any(test, feature = "fuzztarget"))] + { + // In tests, assert that the hard-coded length matches the actual one + if $len != 0 { + use util::ser::LengthCalculatingWriter; + let mut len_calc = LengthCalculatingWriter(0); + $( self.$field.write(&mut len_calc)?; )* + assert_eq!(len_calc.0, $len); + } + } $( self.$field.write(w)?; )* Ok(()) } @@ -135,12 +145,21 @@ macro_rules! impl_writeable { } } macro_rules! impl_writeable_len_match { - ($st:ident, {$({$m: pat, $l: expr}),*}, {$($field:ident),*}) => { + ($st:ident, $cmp: tt, {$({$m: pat, $l: expr}),*}, {$($field:ident),*}) => { impl Writeable for $st { fn write(&self, w: &mut W) -> Result<(), ::std::io::Error> { - w.size_hint(match *self { + let len = match *self { $($m => $l,)* - }); + }; + w.size_hint(len); + #[cfg(any(test, feature = "fuzztarget"))] + { + // In tests, assert that the hard-coded length matches the actual one + use util::ser::LengthCalculatingWriter; + let mut len_calc = LengthCalculatingWriter(0); + $( self.$field.write(&mut len_calc)?; )* + assert!(len_calc.0 $cmp len); + } $( self.$field.write(w)?; )* Ok(()) } @@ -153,6 +172,9 @@ macro_rules! impl_writeable_len_match { }) } } + }; + ($st:ident, {$({$m: pat, $l: expr}),*}, {$($field:ident),*}) => { + impl_writeable_len_match!($st, ==, { $({ $m, $l }),* }, { $($field),* }); } } -- 2.30.2