Skip to content

Commit 6e79392

Browse files
valentinewallacevalentinewallace
committed
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.
1 parent ebe10ad commit 6e79392

File tree

9 files changed

+91
-56
lines changed

9 files changed

+91
-56
lines changed

‎lightning-liquidity/src/lsps0/ser.rs‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,10 @@ use crate::lsps2::msgs::{
1818
};
1919
use crate::prelude::{HashMap, String};
2020

21-
use lightning::ln::msgs::LightningError;
21+
use lightning::io;
22+
use lightning::ln::msgs::{DecodeError, LightningError};
2223
use lightning::ln::wire;
23-
use lightning::util::ser::WithoutLength;
24+
use lightning::util::ser::{Readable, WithoutLength};
2425

2526
use bitcoin::secp256k1::PublicKey;
2627

@@ -168,9 +169,21 @@ impl lightning::util::ser::Writeable for RawLSPSMessage {
168169
}
169170
}
170171

171-
impl lightning::util::ser::Readable for RawLSPSMessage {
172-
fn read<R: lightning::io::Read>(r: &mut R) -> Result<Self, lightning::ln::msgs::DecodeError> {
173-
let payload_without_length = WithoutLength::read(r)?;
172+
impl Readable for RawLSPSMessage {
173+
fn read<R: io::Read>(r: &mut R) -> Result<Self, DecodeError> {
174+
// Because strings will consume the entire reader when read, they are intended to be read from a
175+
// length-limiting reader. However, there's no way of knowing the length ahead of time in this
176+
// case so use a wrapper.
177+
struct UnknownLengthString(String);
178+
impl Readable for UnknownLengthString {
179+
fn read<R: io::Read>(r: &mut R) -> Result<Self, DecodeError> {
180+
let v: Vec<u8> = Readable::read(r)?;
181+
let ret = String::from_utf8(v).map_err(|_| DecodeError::InvalidValue)?;
182+
Ok(Self(ret))
183+
}
184+
}
185+
186+
let payload_without_length: UnknownLengthString = Readable::read(r)?;
174187
Ok(Self { payload: payload_without_length.0 })
175188
}
176189
}

‎lightning/src/ln/msgs.rs‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2412,8 +2412,8 @@ impl Writeable for AcceptChannel {
24122412
}
24132413
}
24142414

2415-
impl Readable for AcceptChannel {
2416-
fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
2415+
impl LengthReadable for AcceptChannel {
2416+
fn read_from_fixed_length_buffer<R: LengthLimitedRead>(r: &mut R) -> Result<Self, DecodeError> {
24172417
let temporary_channel_id: ChannelId = Readable::read(r)?;
24182418
let dust_limit_satoshis: u64 = Readable::read(r)?;
24192419
let max_htlc_value_in_flight_msat: u64 = Readable::read(r)?;
@@ -2497,8 +2497,8 @@ impl Writeable for AcceptChannelV2 {
24972497
}
24982498
}
24992499

2500-
impl Readable for AcceptChannelV2 {
2501-
fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
2500+
impl LengthReadable for AcceptChannelV2 {
2501+
fn read_from_fixed_length_buffer<R: LengthLimitedRead>(r: &mut R) -> Result<Self, DecodeError> {
25022502
let temporary_channel_id: ChannelId = Readable::read(r)?;
25032503
let funding_satoshis: u64 = Readable::read(r)?;
25042504
let dust_limit_satoshis: u64 = Readable::read(r)?;
@@ -2760,8 +2760,8 @@ impl Writeable for Init {
27602760
}
27612761
}
27622762

2763-
impl Readable for Init {
2764-
fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
2763+
impl LengthReadable for Init {
2764+
fn read_from_fixed_length_buffer<R: LengthLimitedRead>(r: &mut R) -> Result<Self, DecodeError> {
27652765
let global_features: InitFeatures = Readable::read(r)?;
27662766
let features: InitFeatures = Readable::read(r)?;
27672767
let mut remote_network_address: Option<SocketAddress> = None;
@@ -2806,8 +2806,8 @@ impl Writeable for OpenChannel {
28062806
}
28072807
}
28082808

2809-
impl Readable for OpenChannel {
2810-
fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
2809+
impl LengthReadable for OpenChannel {
2810+
fn read_from_fixed_length_buffer<R: LengthLimitedRead>(r: &mut R) -> Result<Self, DecodeError> {
28112811
let chain_hash: ChainHash = Readable::read(r)?;
28122812
let temporary_channel_id: ChannelId = Readable::read(r)?;
28132813
let funding_satoshis: u64 = Readable::read(r)?;
@@ -2890,8 +2890,8 @@ impl Writeable for OpenChannelV2 {
28902890
}
28912891
}
28922892

2893-
impl Readable for OpenChannelV2 {
2894-
fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
2893+
impl LengthReadable for OpenChannelV2 {
2894+
fn read_from_fixed_length_buffer<R: LengthLimitedRead>(r: &mut R) -> Result<Self, DecodeError> {
28952895
let chain_hash: ChainHash = Readable::read(r)?;
28962896
let temporary_channel_id: ChannelId = Readable::read(r)?;
28972897
let funding_feerate_sat_per_1000_weight: u32 = Readable::read(r)?;

‎lightning/src/ln/wire.rs‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,9 @@ where
257257
H::Target: CustomMessageReader<CustomMessage = T>,
258258
{
259259
match message_type {
260-
msgs::Init::TYPE => Ok(Message::Init(Readable::read(buffer)?)),
260+
msgs::Init::TYPE => {
261+
Ok(Message::Init(LengthReadable::read_from_fixed_length_buffer(buffer)?))
262+
},
261263
msgs::ErrorMessage::TYPE => Ok(Message::Error(Readable::read(buffer)?)),
262264
msgs::WarningMessage::TYPE => Ok(Message::Warning(Readable::read(buffer)?)),
263265
msgs::Ping::TYPE => Ok(Message::Ping(Readable::read(buffer)?)),
@@ -268,10 +270,18 @@ where
268270
msgs::PeerStorageRetrieval::TYPE => Ok(Message::PeerStorageRetrieval(
269271
LengthReadable::read_from_fixed_length_buffer(buffer)?,
270272
)),
271-
msgs::OpenChannel::TYPE => Ok(Message::OpenChannel(Readable::read(buffer)?)),
272-
msgs::OpenChannelV2::TYPE => Ok(Message::OpenChannelV2(Readable::read(buffer)?)),
273-
msgs::AcceptChannel::TYPE => Ok(Message::AcceptChannel(Readable::read(buffer)?)),
274-
msgs::AcceptChannelV2::TYPE => Ok(Message::AcceptChannelV2(Readable::read(buffer)?)),
273+
msgs::OpenChannel::TYPE => {
274+
Ok(Message::OpenChannel(LengthReadable::read_from_fixed_length_buffer(buffer)?))
275+
},
276+
msgs::OpenChannelV2::TYPE => {
277+
Ok(Message::OpenChannelV2(LengthReadable::read_from_fixed_length_buffer(buffer)?))
278+
},
279+
msgs::AcceptChannel::TYPE => {
280+
Ok(Message::AcceptChannel(LengthReadable::read_from_fixed_length_buffer(buffer)?))
281+
},
282+
msgs::AcceptChannelV2::TYPE => {
283+
Ok(Message::AcceptChannelV2(LengthReadable::read_from_fixed_length_buffer(buffer)?))
284+
},
275285
msgs::FundingCreated::TYPE => {
276286
Ok(Message::FundingCreated(LengthReadable::read_from_fixed_length_buffer(buffer)?))
277287
},

‎lightning/src/offers/invoice.rs‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -148,8 +148,8 @@ use crate::offers::signer::{self, Metadata};
148148
use crate::types::features::{Bolt12InvoiceFeatures, InvoiceRequestFeatures, OfferFeatures};
149149
use crate::types::payment::PaymentHash;
150150
use crate::util::ser::{
151-
CursorReadable, HighZeroBytesDroppedBigSize, Iterable, Readable, WithoutLength, Writeable,
152-
Writer,
151+
CursorReadable, HighZeroBytesDroppedBigSize, Iterable, LengthLimitedRead, LengthReadable,
152+
WithoutLength, Writeable, Writer,
153153
};
154154
use crate::util::string::PrintableString;
155155
use bitcoin::address::Address;
@@ -1398,9 +1398,11 @@ impl Writeable for Bolt12Invoice {
13981398
}
13991399
}
14001400

1401-
impl Readable for Bolt12Invoice {
1402-
fn read<R: io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
1403-
let bytes: WithoutLength<Vec<u8>> = Readable::read(reader)?;
1401+
impl LengthReadable for Bolt12Invoice {
1402+
fn read_from_fixed_length_buffer<R: LengthLimitedRead>(
1403+
reader: &mut R,
1404+
) -> Result<Self, DecodeError> {
1405+
let bytes: WithoutLength<Vec<u8>> = LengthReadable::read_from_fixed_length_buffer(reader)?;
14041406
Self::try_from(bytes.0).map_err(|_| DecodeError::InvalidValue)
14051407
}
14061408
}

‎lightning/src/offers/invoice_request.rs‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,8 @@ use crate::onion_message::dns_resolution::HumanReadableName;
8686
use crate::types::features::InvoiceRequestFeatures;
8787
use crate::types::payment::PaymentHash;
8888
use crate::util::ser::{
89-
CursorReadable, HighZeroBytesDroppedBigSize, Readable, WithoutLength, Writeable, Writer,
89+
CursorReadable, HighZeroBytesDroppedBigSize, LengthLimitedRead, LengthReadable, Readable,
90+
WithoutLength, Writeable, Writer,
9091
};
9192
use crate::util::string::{PrintableString, UntrustedString};
9293
use bitcoin::constants::ChainHash;
@@ -1119,9 +1120,9 @@ impl Writeable for InvoiceRequestContents {
11191120
}
11201121
}
11211122

1122-
impl Readable for InvoiceRequest {
1123-
fn read<R: io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
1124-
let bytes: WithoutLength<Vec<u8>> = Readable::read(reader)?;
1123+
impl LengthReadable for InvoiceRequest {
1124+
fn read_from_fixed_length_buffer<R: LengthLimitedRead>(r: &mut R) -> Result<Self, DecodeError> {
1125+
let bytes: WithoutLength<Vec<u8>> = LengthReadable::read_from_fixed_length_buffer(r)?;
11251126
Self::try_from(bytes.0).map_err(|_| DecodeError::InvalidValue)
11261127
}
11271128
}

‎lightning/src/offers/offer.rs‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,8 @@ use crate::offers::parse::{Bech32Encode, Bolt12ParseError, Bolt12SemanticError,
8888
use crate::offers::signer::{self, Metadata, MetadataMaterial};
8989
use crate::types::features::OfferFeatures;
9090
use crate::util::ser::{
91-
CursorReadable, HighZeroBytesDroppedBigSize, Readable, WithoutLength, Writeable, Writer,
91+
CursorReadable, HighZeroBytesDroppedBigSize, LengthLimitedRead, LengthReadable, Readable,
92+
WithoutLength, Writeable, Writer,
9293
};
9394
use crate::util::string::PrintableString;
9495
use bitcoin::constants::ChainHash;
@@ -1033,9 +1034,11 @@ impl OfferContents {
10331034
}
10341035
}
10351036

1036-
impl Readable for Offer {
1037-
fn read<R: io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
1038-
let bytes: WithoutLength<Vec<u8>> = Readable::read(reader)?;
1037+
impl LengthReadable for Offer {
1038+
fn read_from_fixed_length_buffer<R: LengthLimitedRead>(
1039+
reader: &mut R,
1040+
) -> Result<Self, DecodeError> {
1041+
let bytes: WithoutLength<Vec<u8>> = LengthReadable::read_from_fixed_length_buffer(reader)?;
10391042
Self::try_from(bytes.0).map_err(|_| DecodeError::InvalidValue)
10401043
}
10411044
}

‎lightning/src/offers/refund.rs‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,9 @@ use crate::offers::signer::{self, Metadata, MetadataMaterial};
102102
use crate::sign::EntropySource;
103103
use crate::types::features::InvoiceRequestFeatures;
104104
use crate::types::payment::PaymentHash;
105-
use crate::util::ser::{CursorReadable, Readable, WithoutLength, Writeable, Writer};
105+
use crate::util::ser::{
106+
CursorReadable, LengthLimitedRead, LengthReadable, WithoutLength, Writeable, Writer,
107+
};
106108
use crate::util::string::PrintableString;
107109
use bitcoin::constants::ChainHash;
108110
use bitcoin::network::Network;
@@ -822,9 +824,11 @@ impl RefundContents {
822824
}
823825
}
824826

825-
impl Readable for Refund {
826-
fn read<R: io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
827-
let bytes: WithoutLength<Vec<u8>> = Readable::read(reader)?;
827+
impl LengthReadable for Refund {
828+
fn read_from_fixed_length_buffer<R: LengthLimitedRead>(
829+
reader: &mut R,
830+
) -> Result<Self, DecodeError> {
831+
let bytes: WithoutLength<Vec<u8>> = LengthReadable::read_from_fixed_length_buffer(reader)?;
828832
Self::try_from(bytes.0).map_err(|_| DecodeError::InvalidValue)
829833
}
830834
}

‎lightning/src/util/ser.rs‎

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -740,10 +740,10 @@ impl Writeable for WithoutLength<&String> {
740740
w.write_all(self.0.as_bytes())
741741
}
742742
}
743-
impl Readable for WithoutLength<String> {
743+
impl LengthReadable for WithoutLength<String> {
744744
#[inline]
745-
fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
746-
let v: WithoutLength<Vec<u8>> = Readable::read(r)?;
745+
fn read_from_fixed_length_buffer<R: LengthLimitedRead>(r: &mut R) -> Result<Self, DecodeError> {
746+
let v: WithoutLength<Vec<u8>> = LengthReadable::read_from_fixed_length_buffer(r)?;
747747
Ok(Self(String::from_utf8(v.0).map_err(|_| DecodeError::InvalidValue)?))
748748
}
749749
}
@@ -759,9 +759,9 @@ impl Writeable for UntrustedString {
759759
}
760760
}
761761

762-
impl Readable for UntrustedString {
763-
fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
764-
let s: String = Readable::read(r)?;
762+
impl LengthReadable for UntrustedString {
763+
fn read_from_fixed_length_buffer<R: LengthLimitedRead>(r: &mut R) -> Result<Self, DecodeError> {
764+
let s: String = LengthReadable::read_from_fixed_length_buffer(r)?;
765765
Ok(Self(s))
766766
}
767767
}
@@ -772,10 +772,10 @@ impl Writeable for WithoutLength<&UntrustedString> {
772772
WithoutLength(&self.0 .0).write(w)
773773
}
774774
}
775-
impl Readable for WithoutLength<UntrustedString> {
775+
impl LengthReadable for WithoutLength<UntrustedString> {
776776
#[inline]
777-
fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
778-
let s: WithoutLength<String> = Readable::read(r)?;
777+
fn read_from_fixed_length_buffer<R: LengthLimitedRead>(r: &mut R) -> Result<Self, DecodeError> {
778+
let s: WithoutLength<String> = LengthReadable::read_from_fixed_length_buffer(r)?;
779779
Ok(Self(UntrustedString(s.0)))
780780
}
781781
}
@@ -808,9 +808,11 @@ impl<S: AsWriteableSlice> Writeable for WithoutLength<S> {
808808
}
809809
}
810810

811-
impl<T: MaybeReadable> Readable for WithoutLength<Vec<T>> {
811+
impl<T: MaybeReadable> LengthReadable for WithoutLength<Vec<T>> {
812812
#[inline]
813-
fn read<R: Read>(reader: &mut R) -> Result<Self, DecodeError> {
813+
fn read_from_fixed_length_buffer<R: LengthLimitedRead>(
814+
reader: &mut R,
815+
) -> Result<Self, DecodeError> {
814816
let mut values = Vec::new();
815817
loop {
816818
let mut track_read = ReadTrackingReader::new(reader);
@@ -841,10 +843,10 @@ impl Writeable for WithoutLength<&ScriptBuf> {
841843
}
842844
}
843845

844-
impl Readable for WithoutLength<ScriptBuf> {
846+
impl LengthReadable for WithoutLength<ScriptBuf> {
845847
#[inline]
846-
fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
847-
let v: WithoutLength<Vec<u8>> = Readable::read(r)?;
848+
fn read_from_fixed_length_buffer<R: LengthLimitedRead>(r: &mut R) -> Result<Self, DecodeError> {
849+
let v: WithoutLength<Vec<u8>> = LengthReadable::read_from_fixed_length_buffer(r)?;
848850
Ok(WithoutLength(script::Builder::from(v.0).into_script()))
849851
}
850852
}
@@ -1522,9 +1524,9 @@ impl Writeable for String {
15221524
w.write_all(self.as_bytes())
15231525
}
15241526
}
1525-
impl Readable for String {
1527+
impl LengthReadable for String {
15261528
#[inline]
1527-
fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
1529+
fn read_from_fixed_length_buffer<R: LengthLimitedRead>(r: &mut R) -> Result<Self, DecodeError> {
15281530
let v: Vec<u8> = Readable::read(r)?;
15291531
let ret = String::from_utf8(v).map_err(|_| DecodeError::InvalidValue)?;
15301532
Ok(ret)

‎lightning/src/util/ser_macros.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -409,7 +409,7 @@ macro_rules! _decode_tlv {
409409
$field = $trait::read(&mut $reader $(, $read_arg)*)?;
410410
}};
411411
($outer_reader: expr, $reader: expr, $field: ident, required_vec) => {{
412-
let f: $crate::util::ser::WithoutLength<Vec<_>> = $crate::util::ser::Readable::read(&mut $reader)?;
412+
let f: $crate::util::ser::WithoutLength<Vec<_>> = $crate::util::ser::LengthReadable::read_from_fixed_length_buffer(&mut $reader)?;
413413
$field = f.0;
414414
}};
415415
($outer_reader: expr, $reader: expr, $field: ident, option) => {{
@@ -427,7 +427,7 @@ macro_rules! _decode_tlv {
427427
_decode_tlv!($outer_reader, $reader, $field, required);
428428
}};
429429
($outer_reader: expr, $reader: expr, $field: ident, optional_vec) => {{
430-
let f: $crate::util::ser::WithoutLength<Vec<_>> = $crate::util::ser::Readable::read(&mut $reader)?;
430+
let f: $crate::util::ser::WithoutLength<Vec<_>> = $crate::util::ser::LengthReadable::read_from_fixed_length_buffer(&mut $reader)?;
431431
$field = Some(f.0);
432432
}};
433433
// `upgradable_required` indicates we're reading a required TLV that may have been upgraded
@@ -659,7 +659,7 @@ macro_rules! _decode_tlv_stream_range {
659659
/// struct MyCustomMessage {
660660
/// pub field_1: u32,
661661
/// pub field_2: bool,
662-
/// pub field_3: String,
662+
/// pub field_3: Vec<u8>,
663663
/// pub tlv_optional_integer: Option<u32>,
664664
/// }
665665
///

0 commit comments

Comments
 (0)