Merge pull request #854 from TheBlueMatt/2021-03-fix-lens

Fix serialization expected lengths and check them in test/fuzzing

diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs
index 647fc323880dc62fddde8a94730d549f0e6d9d97..da7ba15a1ace67de30ca0e8a01b1173e9253f6ac 100644 (file)
--- 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<u32>,
 }
 
-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 01a4378456498d7c20706784da60ea13790e3d26..8a7713dc74ec9ceea3e427aab41995cc7fa8e995 100644 (file)
--- 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<W: Writer>(&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<W: Writer>(&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 4a9f300638a4c4c3b2ad7d243b58c2f569557841..de83f227b2365af938539a62c5688d25d7d7be37 100644 (file)
--- 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 ceabcc1ddbeb385666c909943842d01d61887b91..5062bae769cf9d9d752791226875a131ae58895e 100644 (file)
--- 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,24 +145,36 @@ macro_rules! impl_writeable {
        }
 }
 macro_rules! impl_writeable_len_match {
-       ($st:ident, {$({$m: pat, $l: expr}),*}, {$($field:ident),*}) => {
-               impl Writeable for $st {
+       ($struct: ident, $cmp: tt, {$({$match: pat, $length: expr}),*}, {$($field:ident),*}) => {
+               impl Writeable for $struct {
                        fn write<W: Writer>(&self, w: &mut W) -> Result<(), ::std::io::Error> {
-                               w.size_hint(match *self {
-                                       $($m => $l,)*
-                               });
+                               let len = match *self {
+                                       $($match => $length,)*
+                               };
+                               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(())
                        }
                }
 
-               impl ::util::ser::Readable for $st {
+               impl ::util::ser::Readable for $struct {
                        fn read<R: ::std::io::Read>(r: &mut R) -> Result<Self, DecodeError> {
                                Ok(Self {
                                        $($field: Readable::read(r)?),*
                                })
                        }
                }
+       };
+       ($struct: ident, {$({$match: pat, $length: expr}),*}, {$($field:ident),*}) => {
+               impl_writeable_len_match!($struct, ==, { $({ $match, $length }),* }, { $($field),* });
        }
 }