Require LengthLimitedRead for structs that always drain reader

fuzz/src/chanmon_consistency.rs

@@ -67,7 +67,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::{LengthReadable, ReadableArgs, Writeable, Writer};
 use lightning::util::test_channel_signer::{EnforcementState, TestChannelSigner};
 
 use lightning_invoice::RawBolt11Invoice;
@@ -1103,7 +1103,7 @@ 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 new_msg = UpdateAddHTLC::read_from_fixed_length_buffer(&mut &msg_ser[..]).unwrap();
 											dest.handle_update_add_htlc(nodes[$node].get_our_node_id(), &new_msg);
 										}
 									}

fuzz/src/msg_targets/utils.rs

@@ -49,15 +49,16 @@ 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};
+		if let Ok(msg) =
+			<$MsgType as LengthReadable>::read_from_fixed_length_buffer(&mut &$data[..])
+		{
 			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();
+				<$MsgType as LengthReadable>::read_from_fixed_length_buffer(&mut &w.0[..]).unwrap();
 			let mut w_two = VecWriter(Vec::new());
 			msg.write(&mut w_two).unwrap();
 			assert_eq!(&w.0[..], &w_two.0[..]);

lightning/src/crypto/streams.rs

@@ -4,7 +4,7 @@ use crate::crypto::chacha20poly1305rfc::ChaCha20Poly1305RFC;
 use crate::io::{self, Read, Write};
 use crate::ln::msgs::DecodeError;
 use crate::util::ser::{
-	FixedLengthReader, LengthRead, LengthReadableArgs, Readable, Writeable, Writer,
+	FixedLengthReader, LengthLimitedRead, LengthReadableArgs, Readable, Writeable, Writer,
 };
 
 pub(crate) struct ChaChaReader<'a, R: io::Read> {
@@ -56,16 +56,16 @@ pub(crate) struct ChaChaPolyReadAdapter<R: Readable> {
 }
 
 impl<T: Readable> LengthReadableArgs<[u8; 32]> for ChaChaPolyReadAdapter<T> {
-	// Simultaneously read and decrypt an object from a LengthRead, storing it in Self::readable.
-	// LengthRead must be used instead of std::io::Read because we need the total length to separate
-	// out the tag at the end.
-	fn read<R: LengthRead>(r: &mut R, secret: [u8; 32]) -> Result<Self, DecodeError> {
-		if r.total_bytes() < 16 {
+	// Simultaneously read and decrypt an object from a LengthLimitedRead storing it in
+	// Self::readable. LengthLimitedRead must be used instead of std::io::Read because we need the
+	// total length to separate out the tag at the end.
+	fn read<R: LengthLimitedRead>(r: &mut R, secret: [u8; 32]) -> Result<Self, DecodeError> {
+		if r.remaining_bytes() < 16 {
 			return Err(DecodeError::InvalidValue);
 		}
 
 		let mut chacha = ChaCha20Poly1305RFC::new(&secret, &[0; 12], &[]);
-		let decrypted_len = r.total_bytes() - 16;
+		let decrypted_len = r.remaining_bytes() - 16;
 		let s = FixedLengthReader::new(r, decrypted_len);
 		let mut chacha_stream = ChaChaPolyReader { chacha: &mut chacha, read: s };
 		let readable: T = Readable::read(&mut chacha_stream)?;

lightning/src/ln/channelmanager.rs

@@ -81,7 +81,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 {
@@ -12843,14 +12843,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

@@ -59,7 +59,7 @@ use crate::io_extras::read_to_end;
 
 use crate::crypto::streams::ChaChaPolyReadAdapter;
 use crate::util::logger;
-use crate::util::ser::{BigSize, FixedLengthReader, HighZeroBytesDroppedBigSize, Hostname, LengthRead, LengthReadable, LengthReadableArgs, Readable, ReadableArgs, TransactionU16LenLimited, WithoutLength, Writeable, Writer};
+use crate::util::ser::{BigSize, FixedLengthReader, HighZeroBytesDroppedBigSize, Hostname, LengthLimitedRead, LengthReadable, LengthReadableArgs, Readable, ReadableArgs, TransactionU16LenLimited, WithoutLength, Writeable, Writer};
 use crate::util::base32;
 
 use crate::routing::gossip::{NodeAlias, NodeId};
@@ -2323,13 +2323,14 @@ impl Writeable for TrampolineOnionPacket {
 }
 
 impl LengthReadable for TrampolineOnionPacket {
-	fn read_from_fixed_length_buffer<R: LengthRead>(r: &mut R) -> Result<Self, DecodeError> {
+	fn read_from_fixed_length_buffer<R: LengthLimitedRead>(r: &mut R) -> Result<Self, DecodeError> {
+		let hop_data_len = r.remaining_bytes().saturating_sub(66); // 1 (version) + 33 (pubkey) + 32 (HMAC) = 66
+
 		let version = Readable::read(r)?;
 		let public_key = Readable::read(r)?;
 
-		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)?;
 
@@ -3936,7 +3937,7 @@ mod tests {
 	use crate::ln::msgs::{self, FinalOnionHopData, OnionErrorPacket, CommonOpenChannelFields, CommonAcceptChannelFields, OutboundTrampolinePayload, TrampolineOnionPacket, InboundOnionForwardPayload, InboundOnionReceivePayload};
 	use crate::ln::msgs::SocketAddress;
 	use crate::routing::gossip::{NodeAlias, NodeId};
-	use crate::util::ser::{BigSize, FixedLengthReader, Hostname, LengthReadable, Readable, ReadableArgs, TransactionU16LenLimited, Writeable};
+	use crate::util::ser::{BigSize, Hostname, LengthReadable, Readable, ReadableArgs, TransactionU16LenLimited, Writeable};
 	use crate::util::test_utils;
 
 	use bitcoin::hex::FromHex;
@@ -4875,7 +4876,7 @@ 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);
+		assert_eq!(msgs::ClosingSigned::read_from_fixed_length_buffer(&mut &target_value[..]).unwrap(), closing_signed);
 
 		let closing_signed_with_range = msgs::ClosingSigned {
 			channel_id: ChannelId::from_bytes([2; 32]),
@@ -4889,7 +4890,7 @@ 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(),
+		assert_eq!(msgs::ClosingSigned::read_from_fixed_length_buffer(&mut &target_value_with_range[..]).unwrap(),
 			closing_signed_with_range);
 	}
 
@@ -5053,7 +5054,7 @@ 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);
+		assert_eq!(msgs::Init::read_from_fixed_length_buffer(&mut &target_value[..]).unwrap(), init_msg);
 	}
 
 	#[test]
@@ -5288,9 +5289,10 @@ mod tests {
 		assert_eq!(encoded_trampoline_packet.len(), 716);
 
 		{ // 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 &encoded_trampoline_packet[..]
+				).unwrap();
 			assert_eq!(decoded_trampoline_packet.encode(), encoded_trampoline_packet);
 		}
 
@@ -5416,7 +5418,7 @@ 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();
+		query_channel_range = LengthReadable::read_from_fixed_length_buffer(&mut &target_value[..]).unwrap();
 		assert_eq!(query_channel_range.first_blocknum, 100000);
 		assert_eq!(query_channel_range.number_of_blocks, 1500);
 	}
@@ -5500,7 +5502,7 @@ 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();
+		reply_short_channel_ids_end = LengthReadable::read_from_fixed_length_buffer(&mut &target_value[..]).unwrap();
 		assert_eq!(reply_short_channel_ids_end.chain_hash, expected_chain_hash);
 		assert_eq!(reply_short_channel_ids_end.full_information, true);
 	}
@@ -5517,7 +5519,9 @@ 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();
+		gossip_timestamp_filter = LengthReadable::read_from_fixed_length_buffer(
+			&mut &target_value[..]
+		).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

@@ -1541,8 +1541,10 @@ 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 message_result = wire::read(&mut reader, &*self.message_handler.custom_message_handler);
+									let message_result = wire::read(
+										&mut &peer.pending_read_buffer[..peer.pending_read_buffer.len() - 16],
+										&*self.message_handler.custom_message_handler
+									);
 
 									// Reset read buffer
 									if peer.pending_read_buffer.capacity() > 8192 { peer.pending_read_buffer = Vec::new(); }