Use LengthReadable in impl_writeable_msg

See previous commit.

When deserializing objects via this macro, there is no length prefix so the
deser code will read the provided reader until it runs out of bytes.

Readable is not an appropriate trait for this situation because it should only
be used for structs that are prefixed with a length and know when to stop
reading. LengthReadable instead requires that the caller supply only the bytes
that are reserved for this struct.

Author: valentinewallace

fuzz/src/chanmon_consistency.rs

@@ -65,7 +65,7 @@ use lightning::types::payment::{PaymentHash, PaymentPreimage, PaymentSecret};
 use lightning::util::config::UserConfig;
 use lightning::util::hash_tables::*;
 use lightning::util::logger::Logger;
-use lightning::util::ser::{Readable, ReadableArgs, Writeable, Writer};
+use lightning::util::ser::{FixedLengthReader, LengthReadable, ReadableArgs, Writeable, Writer};
 use lightning::util::test_channel_signer::{EnforcementState, TestChannelSigner};
 
 use lightning_invoice::RawBolt11Invoice;
@@ -1101,7 +1101,9 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
 											// update_fail_htlc as we do when we reject a payment.
 											let mut msg_ser = update_add.encode();
 											msg_ser[1000] ^= 0xff;
-											let new_msg = UpdateAddHTLC::read(&mut Cursor::new(&msg_ser)).unwrap();
+											let mut cursor = Cursor::new(&msg_ser);
+											let mut reader = FixedLengthReader::new(&mut cursor, msg_ser.len() as u64);
+											let new_msg = UpdateAddHTLC::read_from_fixed_length_buffer(&mut reader).unwrap();
 											dest.handle_update_add_htlc(nodes[$node].get_our_node_id(), &new_msg);
 										}
 									}

fuzz/src/msg_targets/utils.rs

@@ -49,15 +49,17 @@ macro_rules! test_msg {
 #[macro_export]
 macro_rules! test_msg_simple {
 	($MsgType: path, $data: ident) => {{
-		use lightning::util::ser::{Readable, Writeable};
-		let mut r = ::lightning::io::Cursor::new($data);
-		if let Ok(msg) = <$MsgType as Readable>::read(&mut r) {
+		use lightning::util::ser::{LengthReadable, Writeable};
+		let mut s = ::lightning::io::Cursor::new($data);
+		let mut r = ::lightning::util::ser::FixedLengthReader::new(&mut s, $data.len() as u64);
+		if let Ok(msg) = <$MsgType as LengthReadable>::read_from_fixed_length_buffer(&mut r) {
 			let mut w = VecWriter(Vec::new());
 			msg.write(&mut w).unwrap();
 			assert_eq!(msg.serialized_length(), w.0.len());
 
-			let msg =
-				<$MsgType as Readable>::read(&mut ::lightning::io::Cursor::new(&w.0)).unwrap();
+			let mut s = ::lightning::io::Cursor::new(&w.0);
+			let mut r = ::lightning::util::ser::FixedLengthReader::new(&mut s, w.0.len() as u64);
+			let msg = <$MsgType as LengthReadable>::read_from_fixed_length_buffer(&mut r).unwrap();
 			let mut w_two = VecWriter(Vec::new());
 			msg.write(&mut w_two).unwrap();
 			assert_eq!(&w.0[..], &w_two.0[..]);

lightning/src/ln/channelmanager.rs

@@ -83,7 +83,7 @@ use crate::util::config::{ChannelConfig, ChannelConfigUpdate, ChannelConfigOverr
 use crate::util::wakers::{Future, Notifier};
 use crate::util::scid_utils::fake_scid;
 use crate::util::string::UntrustedString;
-use crate::util::ser::{BigSize, FixedLengthReader, Readable, ReadableArgs, MaybeReadable, Writeable, Writer, VecWriter};
+use crate::util::ser::{BigSize, FixedLengthReader, LengthReadable, Readable, ReadableArgs, MaybeReadable, Writeable, Writer, VecWriter};
 use crate::util::logger::{Level, Logger, WithContext};
 use crate::util::errors::APIError;
 #[cfg(async_payments)] use {
@@ -12882,14 +12882,14 @@ impl Readable for HTLCFailureMsg {
 			2 => {
 				let length: BigSize = Readable::read(reader)?;
 				let mut s = FixedLengthReader::new(reader, length.0);
-				let res = Readable::read(&mut s)?;
+				let res = LengthReadable::read_from_fixed_length_buffer(&mut s)?;
 				s.eat_remaining()?; // Return ShortRead if there's actually not enough bytes
 				Ok(HTLCFailureMsg::Relay(res))
 			},
 			3 => {
 				let length: BigSize = Readable::read(reader)?;
 				let mut s = FixedLengthReader::new(reader, length.0);
-				let res = Readable::read(&mut s)?;
+				let res = LengthReadable::read_from_fixed_length_buffer(&mut s)?;
 				s.eat_remaining()?; // Return ShortRead if there's actually not enough bytes
 				Ok(HTLCFailureMsg::Malformed(res))
 			},

lightning/src/ln/msgs.rs

@@ -2012,7 +2012,7 @@ impl LengthReadable for TrampolineOnionPacket {
 
 		let hop_data_len = r.total_bytes().saturating_sub(66); // 1 (version) + 33 (pubkey) + 32 (HMAC) = 66
 		let mut rd = FixedLengthReader::new(r, hop_data_len);
-		let hop_data = WithoutLength::<Vec<u8>>::read(&mut rd)?.0;
+		let hop_data = WithoutLength::<Vec<u8>>::read_from_fixed_length_buffer(&mut rd)?.0;
 
 		let hmac = Readable::read(r)?;
 
@@ -4379,7 +4379,9 @@ mod tests {
 		let encoded_value = closing_signed.encode();
 		let target_value = <Vec<u8>>::from_hex("020202020202020202020202020202020202020202020202020202020202020200083a840000034dd977cb9b53d93a6ff64bb5f1e158b4094b66e798fb12911168a3ccdf80a83096340a6a95da0ae8d9f776528eecdbb747eb6b545495a4319ed5378e35b21e073a").unwrap();
 		assert_eq!(encoded_value, target_value);
-		assert_eq!(msgs::ClosingSigned::read(&mut Cursor::new(&target_value)).unwrap(), closing_signed);
+		let mut msg_stream = Cursor::new(&target_value);
+		let mut msg_reader = FixedLengthReader::new(&mut msg_stream, target_value.len() as u64);
+		assert_eq!(msgs::ClosingSigned::read_from_fixed_length_buffer(&mut msg_reader).unwrap(), closing_signed);
 
 		let closing_signed_with_range = msgs::ClosingSigned {
 			channel_id: ChannelId::from_bytes([2; 32]),
@@ -4393,7 +4395,9 @@ mod tests {
 		let encoded_value_with_range = closing_signed_with_range.encode();
 		let target_value_with_range = <Vec<u8>>::from_hex("020202020202020202020202020202020202020202020202020202020202020200083a840000034dd977cb9b53d93a6ff64bb5f1e158b4094b66e798fb12911168a3ccdf80a83096340a6a95da0ae8d9f776528eecdbb747eb6b545495a4319ed5378e35b21e073a011000000000deadbeef1badcafe01234567").unwrap();
 		assert_eq!(encoded_value_with_range, target_value_with_range);
-		assert_eq!(msgs::ClosingSigned::read(&mut Cursor::new(&target_value_with_range)).unwrap(),
+		let mut msg_stream = Cursor::new(&target_value_with_range);
+		let mut msg_reader = FixedLengthReader::new(&mut msg_stream, target_value_with_range.len() as u64);
+		assert_eq!(msgs::ClosingSigned::read_from_fixed_length_buffer(&mut msg_reader).unwrap(),
 			closing_signed_with_range);
 	}
 
@@ -4557,7 +4561,9 @@ mod tests {
 		let encoded_value = init_msg.encode();
 		let target_value = <Vec<u8>>::from_hex("0000000001206fe28c0ab6f1b372c1a6a246ae63f74f931e8365e15a089c68d61900000000000307017f00000103e8").unwrap();
 		assert_eq!(encoded_value, target_value);
-		assert_eq!(msgs::Init::read(&mut Cursor::new(&target_value)).unwrap(), init_msg);
+		let mut msg_stream = Cursor::new(&target_value);
+		let mut msg_reader = FixedLengthReader::new(&mut msg_stream, target_value.len() as u64);
+		assert_eq!(msgs::Init::read_from_fixed_length_buffer(&mut msg_reader).unwrap(), init_msg);
 	}
 
 	#[test]
@@ -4794,7 +4800,8 @@ mod tests {
 		{ // verify that a codec round trip works
 			let mut reader = Cursor::new(&encoded_trampoline_packet);
 			let mut trampoline_packet_reader = FixedLengthReader::new(&mut reader, encoded_trampoline_packet.len() as u64);
-			let decoded_trampoline_packet: TrampolineOnionPacket = <TrampolineOnionPacket as LengthReadable>::read_from_fixed_length_buffer(&mut trampoline_packet_reader).unwrap();
+			let decoded_trampoline_packet: TrampolineOnionPacket =
+				<TrampolineOnionPacket as LengthReadable>::read_from_fixed_length_buffer(&mut trampoline_packet_reader).unwrap();
 			assert_eq!(decoded_trampoline_packet.encode(), encoded_trampoline_packet);
 		}
 
@@ -4920,7 +4927,9 @@ mod tests {
 		let target_value = <Vec<u8>>::from_hex("06226e46111a0b59caaf126043eb5bbf28c34f3a5e332a1fc7b2b73cf188910f000186a0000005dc").unwrap();
 		assert_eq!(encoded_value, target_value);
 
-		query_channel_range = Readable::read(&mut Cursor::new(&target_value[..])).unwrap();
+		let mut msg_stream = Cursor::new(&target_value[..]);
+		let mut msg_reader = FixedLengthReader::new(&mut msg_stream, target_value.len() as u64);
+		query_channel_range = LengthReadable::read_from_fixed_length_buffer(&mut msg_reader).unwrap();
 		assert_eq!(query_channel_range.first_blocknum, 100000);
 		assert_eq!(query_channel_range.number_of_blocks, 1500);
 	}
@@ -5004,7 +5013,9 @@ mod tests {
 		let target_value = <Vec<u8>>::from_hex("06226e46111a0b59caaf126043eb5bbf28c34f3a5e332a1fc7b2b73cf188910f01").unwrap();
 		assert_eq!(encoded_value, target_value);
 
-		reply_short_channel_ids_end = Readable::read(&mut Cursor::new(&target_value[..])).unwrap();
+		let mut msg_stream = Cursor::new(&target_value[..]);
+		let mut msg_reader = FixedLengthReader::new(&mut msg_stream, target_value.len() as u64);
+		reply_short_channel_ids_end = LengthReadable::read_from_fixed_length_buffer(&mut msg_reader).unwrap();
 		assert_eq!(reply_short_channel_ids_end.chain_hash, expected_chain_hash);
 		assert_eq!(reply_short_channel_ids_end.full_information, true);
 	}
@@ -5021,7 +5032,10 @@ mod tests {
 		let target_value = <Vec<u8>>::from_hex("06226e46111a0b59caaf126043eb5bbf28c34f3a5e332a1fc7b2b73cf188910f5ec57980ffffffff").unwrap();
 		assert_eq!(encoded_value, target_value);
 
-		gossip_timestamp_filter = Readable::read(&mut Cursor::new(&target_value[..])).unwrap();
+		let mut stream = Cursor::new(&target_value[..]);
+		gossip_timestamp_filter = LengthReadable::read_from_fixed_length_buffer(
+			&mut FixedLengthReader::new(&mut stream, target_value.len() as u64)
+		).unwrap();
 		assert_eq!(gossip_timestamp_filter.chain_hash, expected_chain_hash);
 		assert_eq!(gossip_timestamp_filter.first_timestamp, 1590000000);
 		assert_eq!(gossip_timestamp_filter.timestamp_range, 0xffff_ffff);

lightning/src/ln/peer_handler.rs

@@ -1544,7 +1544,8 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, OM: Deref, L: Deref, CM
 									try_potential_handleerror!(peer,
 										peer.channel_encryptor.decrypt_message(&mut peer.pending_read_buffer[..]));
 
-									let mut reader = io::Cursor::new(&peer.pending_read_buffer[..peer.pending_read_buffer.len() - 16]);
+									let mut cursor = io::Cursor::new(&peer.pending_read_buffer[..peer.pending_read_buffer.len() - 16]);
+									let mut reader = crate::util::ser::FixedLengthReader::new(&mut cursor, (peer.pending_read_buffer.len() - 16) as u64);
 									let message_result = wire::read(&mut reader, &*self.message_handler.custom_message_handler);
 
 									// Reset read buffer