Use LengthLimitedReadable for all read-to-end Readables

When deserializing these structs, they will consume the reader until it is out
of bytes. Therefore, it's safer if they are only provided with readers that
limit how much of the byte stream will be read.

Most of the relevant structs were updated in the previous commit when we
transitioned impl_writeable_msg to use LengthReadable, here we finish the job.

Author: valentinewallace

lightning-liquidity/src/lsps0/ser.rs

@@ -18,9 +18,10 @@ use crate::lsps2::msgs::{
 };
 use crate::prelude::{HashMap, String};
 
-use lightning::ln::msgs::LightningError;
+use lightning::io;
+use lightning::ln::msgs::{DecodeError, LightningError};
 use lightning::ln::wire;
-use lightning::util::ser::WithoutLength;
+use lightning::util::ser::{Readable, WithoutLength};
 
 use bitcoin::secp256k1::PublicKey;
 
@@ -168,9 +169,21 @@ impl lightning::util::ser::Writeable for RawLSPSMessage {
 	}
 }
 
-impl lightning::util::ser::Readable for RawLSPSMessage {
-	fn read<R: lightning::io::Read>(r: &mut R) -> Result<Self, lightning::ln::msgs::DecodeError> {
-		let payload_without_length = WithoutLength::read(r)?;
+impl Readable for RawLSPSMessage {
+	fn read<R: io::Read>(r: &mut R) -> Result<Self, DecodeError> {
+		// Because strings will consume the entire reader when read, they are intended to be read from a
+		// length-limiting reader. However, there's no way of knowing the length ahead of time in this
+		// case so use a wrapper.
+		struct UnknownLengthString(String);
+		impl Readable for UnknownLengthString {
+			fn read<R: io::Read>(r: &mut R) -> Result<Self, DecodeError> {
+				let v: alloc::vec::Vec<u8> = Readable::read(r)?;
+				let ret = String::from_utf8(v).map_err(|_| DecodeError::InvalidValue)?;
+				Ok(Self(ret))
+			}
+		}
+
+		let payload_without_length: UnknownLengthString = Readable::read(r)?;
 		Ok(Self { payload: payload_without_length.0 })
 	}
 }

lightning/src/ln/msgs.rs

@@ -2412,8 +2412,8 @@ impl Writeable for AcceptChannel {
 	}
 }
 
-impl Readable for AcceptChannel {
-	fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
+impl LengthReadable for AcceptChannel {
+	fn read_from_fixed_length_buffer<R: LengthLimitedRead>(r: &mut R) -> Result<Self, DecodeError> {
 		let temporary_channel_id: ChannelId = Readable::read(r)?;
 		let dust_limit_satoshis: u64 = Readable::read(r)?;
 		let max_htlc_value_in_flight_msat: u64 = Readable::read(r)?;
@@ -2497,8 +2497,8 @@ impl Writeable for AcceptChannelV2 {
 	}
 }
 
-impl Readable for AcceptChannelV2 {
-	fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
+impl LengthReadable for AcceptChannelV2 {
+	fn read_from_fixed_length_buffer<R: LengthLimitedRead>(r: &mut R) -> Result<Self, DecodeError> {
 		let temporary_channel_id: ChannelId = Readable::read(r)?;
 		let funding_satoshis: u64 = Readable::read(r)?;
 		let dust_limit_satoshis: u64 = Readable::read(r)?;
@@ -2760,8 +2760,8 @@ impl Writeable for Init {
 	}
 }
 
-impl Readable for Init {
-	fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
+impl LengthReadable for Init {
+	fn read_from_fixed_length_buffer<R: LengthLimitedRead>(r: &mut R) -> Result<Self, DecodeError> {
 		let global_features: InitFeatures = Readable::read(r)?;
 		let features: InitFeatures = Readable::read(r)?;
 		let mut remote_network_address: Option<SocketAddress> = None;
@@ -2806,8 +2806,8 @@ impl Writeable for OpenChannel {
 	}
 }
 
-impl Readable for OpenChannel {
-	fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
+impl LengthReadable for OpenChannel {
+	fn read_from_fixed_length_buffer<R: LengthLimitedRead>(r: &mut R) -> Result<Self, DecodeError> {
 		let chain_hash: ChainHash = Readable::read(r)?;
 		let temporary_channel_id: ChannelId = Readable::read(r)?;
 		let funding_satoshis: u64 = Readable::read(r)?;
@@ -2890,8 +2890,8 @@ impl Writeable for OpenChannelV2 {
 	}
 }
 
-impl Readable for OpenChannelV2 {
-	fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
+impl LengthReadable for OpenChannelV2 {
+	fn read_from_fixed_length_buffer<R: LengthLimitedRead>(r: &mut R) -> Result<Self, DecodeError> {
 		let chain_hash: ChainHash = Readable::read(r)?;
 		let temporary_channel_id: ChannelId = Readable::read(r)?;
 		let funding_feerate_sat_per_1000_weight: u32 = Readable::read(r)?;
@@ -3526,8 +3526,8 @@ impl Writeable for Ping {
 	}
 }
 
-impl Readable for Ping {
-	fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
+impl LengthReadable for Ping {
+	fn read_from_fixed_length_buffer<R: LengthLimitedRead>(r: &mut R) -> Result<Self, DecodeError> {
 		Ok(Ping {
 			ponglen: Readable::read(r)?,
 			byteslen: {
@@ -3546,8 +3546,8 @@ impl Writeable for Pong {
 	}
 }
 
-impl Readable for Pong {
-	fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
+impl LengthReadable for Pong {
+	fn read_from_fixed_length_buffer<R: LengthLimitedRead>(r: &mut R) -> Result<Self, DecodeError> {
 		Ok(Pong {
 			byteslen: {
 				let byteslen = Readable::read(r)?;
@@ -3653,8 +3653,8 @@ impl Writeable for ErrorMessage {
 	}
 }
 
-impl Readable for ErrorMessage {
-	fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
+impl LengthReadable for ErrorMessage {
+	fn read_from_fixed_length_buffer<R: LengthLimitedRead>(r: &mut R) -> Result<Self, DecodeError> {
 		Ok(Self {
 			channel_id: Readable::read(r)?,
 			data: {
@@ -3680,8 +3680,8 @@ impl Writeable for WarningMessage {
 	}
 }
 
-impl Readable for WarningMessage {
-	fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
+impl LengthReadable for WarningMessage {
+	fn read_from_fixed_length_buffer<R: LengthLimitedRead>(r: &mut R) -> Result<Self, DecodeError> {
 		Ok(Self {
 			channel_id: Readable::read(r)?,
 			data: {
@@ -3787,8 +3787,8 @@ impl_writeable!(NodeAnnouncement, {
 	contents
 });
 
-impl Readable for QueryShortChannelIds {
-	fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
+impl LengthReadable for QueryShortChannelIds {
+	fn read_from_fixed_length_buffer<R: LengthLimitedRead>(r: &mut R) -> Result<Self, DecodeError> {
 		let chain_hash: ChainHash = Readable::read(r)?;
 
 		let encoding_len: u16 = Readable::read(r)?;
@@ -3863,8 +3863,8 @@ impl_writeable_msg!(QueryChannelRange, {
 	number_of_blocks
 }, {});
 
-impl Readable for ReplyChannelRange {
-	fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
+impl LengthReadable for ReplyChannelRange {
+	fn read_from_fixed_length_buffer<R: LengthLimitedRead>(r: &mut R) -> Result<Self, DecodeError> {
 		let chain_hash: ChainHash = Readable::read(r)?;
 		let first_blocknum: u32 = Readable::read(r)?;
 		let number_of_blocks: u32 = Readable::read(r)?;
@@ -5445,7 +5445,7 @@ mod tests {
 			let encoded_value = reply_channel_range.encode();
 			assert_eq!(encoded_value, target_value);
 
-			reply_channel_range = Readable::read(&mut Cursor::new(&target_value[..])).unwrap();
+			reply_channel_range = LengthReadable::read_from_fixed_length_buffer(&mut &target_value[..]).unwrap();
 			assert_eq!(reply_channel_range.chain_hash, expected_chain_hash);
 			assert_eq!(reply_channel_range.first_blocknum, 756230);
 			assert_eq!(reply_channel_range.number_of_blocks, 1500);
@@ -5455,7 +5455,7 @@ mod tests {
 			assert_eq!(reply_channel_range.short_channel_ids[2], 0x000000000045a6c4);
 		} else {
 			target_value.append(&mut <Vec<u8>>::from_hex("001601789c636000833e08659309a65878be010010a9023a").unwrap());
-			let result: Result<msgs::ReplyChannelRange, msgs::DecodeError> = Readable::read(&mut Cursor::new(&target_value[..]));
+			let result: Result<msgs::ReplyChannelRange, msgs::DecodeError> = LengthReadable::read_from_fixed_length_buffer(&mut &target_value[..]);
 			assert!(result.is_err(), "Expected decode failure with unsupported zlib encoding");
 		}
 	}
@@ -5479,14 +5479,14 @@ mod tests {
 			let encoded_value = query_short_channel_ids.encode();
 			assert_eq!(encoded_value, target_value);
 
-			query_short_channel_ids = Readable::read(&mut Cursor::new(&target_value[..])).unwrap();
+			query_short_channel_ids = LengthReadable::read_from_fixed_length_buffer(&mut &target_value[..]).unwrap();
 			assert_eq!(query_short_channel_ids.chain_hash, expected_chain_hash);
 			assert_eq!(query_short_channel_ids.short_channel_ids[0], 0x000000000000008e);
 			assert_eq!(query_short_channel_ids.short_channel_ids[1], 0x0000000000003c69);
 			assert_eq!(query_short_channel_ids.short_channel_ids[2], 0x000000000045a6c4);
 		} else {
 			target_value.append(&mut <Vec<u8>>::from_hex("001601789c636000833e08659309a65878be010010a9023a").unwrap());
-			let result: Result<msgs::QueryShortChannelIds, msgs::DecodeError> = Readable::read(&mut Cursor::new(&target_value[..]));
+			let result: Result<msgs::QueryShortChannelIds, msgs::DecodeError> = LengthReadable::read_from_fixed_length_buffer(&mut &target_value[..]);
 			assert!(result.is_err(), "Expected decode failure with unsupported zlib encoding");
 		}
 	}

lightning/src/ln/wire.rs

@@ -257,21 +257,39 @@ where
 	H::Target: CustomMessageReader<CustomMessage = T>,
 {
 	match message_type {
-		msgs::Init::TYPE => Ok(Message::Init(Readable::read(buffer)?)),
-		msgs::ErrorMessage::TYPE => Ok(Message::Error(Readable::read(buffer)?)),
-		msgs::WarningMessage::TYPE => Ok(Message::Warning(Readable::read(buffer)?)),
-		msgs::Ping::TYPE => Ok(Message::Ping(Readable::read(buffer)?)),
-		msgs::Pong::TYPE => Ok(Message::Pong(Readable::read(buffer)?)),
+		msgs::Init::TYPE => {
+			Ok(Message::Init(LengthReadable::read_from_fixed_length_buffer(buffer)?))
+		},
+		msgs::ErrorMessage::TYPE => {
+			Ok(Message::Error(LengthReadable::read_from_fixed_length_buffer(buffer)?))
+		},
+		msgs::WarningMessage::TYPE => {
+			Ok(Message::Warning(LengthReadable::read_from_fixed_length_buffer(buffer)?))
+		},
+		msgs::Ping::TYPE => {
+			Ok(Message::Ping(LengthReadable::read_from_fixed_length_buffer(buffer)?))
+		},
+		msgs::Pong::TYPE => {
+			Ok(Message::Pong(LengthReadable::read_from_fixed_length_buffer(buffer)?))
+		},
 		msgs::PeerStorage::TYPE => {
 			Ok(Message::PeerStorage(LengthReadable::read_from_fixed_length_buffer(buffer)?))
 		},
 		msgs::PeerStorageRetrieval::TYPE => Ok(Message::PeerStorageRetrieval(
 			LengthReadable::read_from_fixed_length_buffer(buffer)?,
 		)),
-		msgs::OpenChannel::TYPE => Ok(Message::OpenChannel(Readable::read(buffer)?)),
-		msgs::OpenChannelV2::TYPE => Ok(Message::OpenChannelV2(Readable::read(buffer)?)),
-		msgs::AcceptChannel::TYPE => Ok(Message::AcceptChannel(Readable::read(buffer)?)),
-		msgs::AcceptChannelV2::TYPE => Ok(Message::AcceptChannelV2(Readable::read(buffer)?)),
+		msgs::OpenChannel::TYPE => {
+			Ok(Message::OpenChannel(LengthReadable::read_from_fixed_length_buffer(buffer)?))
+		},
+		msgs::OpenChannelV2::TYPE => {
+			Ok(Message::OpenChannelV2(LengthReadable::read_from_fixed_length_buffer(buffer)?))
+		},
+		msgs::AcceptChannel::TYPE => {
+			Ok(Message::AcceptChannel(LengthReadable::read_from_fixed_length_buffer(buffer)?))
+		},
+		msgs::AcceptChannelV2::TYPE => {
+			Ok(Message::AcceptChannelV2(LengthReadable::read_from_fixed_length_buffer(buffer)?))
+		},
 		msgs::FundingCreated::TYPE => {
 			Ok(Message::FundingCreated(LengthReadable::read_from_fixed_length_buffer(buffer)?))
 		},
@@ -364,9 +382,9 @@ where
 		},
 		msgs::NodeAnnouncement::TYPE => Ok(Message::NodeAnnouncement(Readable::read(buffer)?)),
 		msgs::ChannelUpdate::TYPE => Ok(Message::ChannelUpdate(Readable::read(buffer)?)),
-		msgs::QueryShortChannelIds::TYPE => {
-			Ok(Message::QueryShortChannelIds(Readable::read(buffer)?))
-		},
+		msgs::QueryShortChannelIds::TYPE => Ok(Message::QueryShortChannelIds(
+			LengthReadable::read_from_fixed_length_buffer(buffer)?,
+		)),
 		msgs::ReplyShortChannelIdsEnd::TYPE => Ok(Message::ReplyShortChannelIdsEnd(
 			LengthReadable::read_from_fixed_length_buffer(buffer)?,
 		)),

lightning/src/offers/invoice.rs

@@ -148,8 +148,8 @@ use crate::offers::signer::{self, Metadata};
 use crate::types::features::{Bolt12InvoiceFeatures, InvoiceRequestFeatures, OfferFeatures};
 use crate::types::payment::PaymentHash;
 use crate::util::ser::{
-	CursorReadable, HighZeroBytesDroppedBigSize, Iterable, Readable, WithoutLength, Writeable,
-	Writer,
+	CursorReadable, HighZeroBytesDroppedBigSize, Iterable, LengthLimitedRead, LengthReadable,
+	WithoutLength, Writeable, Writer,
 };
 use crate::util::string::PrintableString;
 use bitcoin::address::Address;
@@ -1398,9 +1398,11 @@ impl Writeable for Bolt12Invoice {
 	}
 }
 
-impl Readable for Bolt12Invoice {
-	fn read<R: io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
-		let bytes: WithoutLength<Vec<u8>> = Readable::read(reader)?;
+impl LengthReadable for Bolt12Invoice {
+	fn read_from_fixed_length_buffer<R: LengthLimitedRead>(
+		reader: &mut R,
+	) -> Result<Self, DecodeError> {
+		let bytes: WithoutLength<Vec<u8>> = LengthReadable::read_from_fixed_length_buffer(reader)?;
 		Self::try_from(bytes.0).map_err(|_| DecodeError::InvalidValue)
 	}
 }

lightning/src/offers/invoice_request.rs

@@ -86,7 +86,8 @@ use crate::onion_message::dns_resolution::HumanReadableName;
 use crate::types::features::InvoiceRequestFeatures;
 use crate::types::payment::PaymentHash;
 use crate::util::ser::{
-	CursorReadable, HighZeroBytesDroppedBigSize, Readable, WithoutLength, Writeable, Writer,
+	CursorReadable, HighZeroBytesDroppedBigSize, LengthLimitedRead, LengthReadable, Readable,
+	WithoutLength, Writeable, Writer,
 };
 use crate::util::string::{PrintableString, UntrustedString};
 use bitcoin::constants::ChainHash;
@@ -1119,9 +1120,9 @@ impl Writeable for InvoiceRequestContents {
 	}
 }
 
-impl Readable for InvoiceRequest {
-	fn read<R: io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
-		let bytes: WithoutLength<Vec<u8>> = Readable::read(reader)?;
+impl LengthReadable for InvoiceRequest {
+	fn read_from_fixed_length_buffer<R: LengthLimitedRead>(r: &mut R) -> Result<Self, DecodeError> {
+		let bytes: WithoutLength<Vec<u8>> = LengthReadable::read_from_fixed_length_buffer(r)?;
 		Self::try_from(bytes.0).map_err(|_| DecodeError::InvalidValue)
 	}
 }

lightning/src/offers/offer.rs

@@ -88,7 +88,8 @@ use crate::offers::parse::{Bech32Encode, Bolt12ParseError, Bolt12SemanticError,
 use crate::offers::signer::{self, Metadata, MetadataMaterial};
 use crate::types::features::OfferFeatures;
 use crate::util::ser::{
-	CursorReadable, HighZeroBytesDroppedBigSize, Readable, WithoutLength, Writeable, Writer,
+	CursorReadable, HighZeroBytesDroppedBigSize, LengthLimitedRead, LengthReadable, Readable,
+	WithoutLength, Writeable, Writer,
 };
 use crate::util::string::PrintableString;
 use bitcoin::constants::ChainHash;
@@ -1033,9 +1034,11 @@ impl OfferContents {
 	}
 }
 
-impl Readable for Offer {
-	fn read<R: io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
-		let bytes: WithoutLength<Vec<u8>> = Readable::read(reader)?;
+impl LengthReadable for Offer {
+	fn read_from_fixed_length_buffer<R: LengthLimitedRead>(
+		reader: &mut R,
+	) -> Result<Self, DecodeError> {
+		let bytes: WithoutLength<Vec<u8>> = LengthReadable::read_from_fixed_length_buffer(reader)?;
 		Self::try_from(bytes.0).map_err(|_| DecodeError::InvalidValue)
 	}
 }

lightning/src/offers/refund.rs

@@ -102,7 +102,9 @@ use crate::offers::signer::{self, Metadata, MetadataMaterial};
 use crate::sign::EntropySource;
 use crate::types::features::InvoiceRequestFeatures;
 use crate::types::payment::PaymentHash;
-use crate::util::ser::{CursorReadable, Readable, WithoutLength, Writeable, Writer};
+use crate::util::ser::{
+	CursorReadable, LengthLimitedRead, LengthReadable, WithoutLength, Writeable, Writer,
+};
 use crate::util::string::PrintableString;
 use bitcoin::constants::ChainHash;
 use bitcoin::network::Network;
@@ -822,9 +824,11 @@ impl RefundContents {
 	}
 }
 
-impl Readable for Refund {
-	fn read<R: io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
-		let bytes: WithoutLength<Vec<u8>> = Readable::read(reader)?;
+impl LengthReadable for Refund {
+	fn read_from_fixed_length_buffer<R: LengthLimitedRead>(
+		reader: &mut R,
+	) -> Result<Self, DecodeError> {
+		let bytes: WithoutLength<Vec<u8>> = LengthReadable::read_from_fixed_length_buffer(reader)?;
 		Self::try_from(bytes.0).map_err(|_| DecodeError::InvalidValue)
 	}
 }