Skip to content

Commit c355ea4

Browse files
TheBlueMattTheBlueMatt
authored
Merge pull request #3640 from valentinewallace/2025-03-fixed-len-readable
Require `LengthLimitedRead` for structs that always drain reader
2 parents aaef672 + 94f8952 commit c355ea4

File tree

20 files changed

+418
-266
lines changed

20 files changed

+418
-266
lines changed

‎fuzz/src/chanmon_consistency.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ use lightning::types::payment::{PaymentHash, PaymentPreimage, PaymentSecret};
6767
use lightning::util::config::UserConfig;
6868
use lightning::util::hash_tables::*;
6969
use lightning::util::logger::Logger;
70-
use lightning::util::ser::{Readable, ReadableArgs, Writeable, Writer};
70+
use lightning::util::ser::{LengthReadable, ReadableArgs, Writeable, Writer};
7171
use lightning::util::test_channel_signer::{EnforcementState, TestChannelSigner};
7272

7373
use lightning_invoice::RawBolt11Invoice;
@@ -1103,7 +1103,7 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
11031103
// update_fail_htlc as we do when we reject a payment.
11041104
let mut msg_ser = update_add.encode();
11051105
msg_ser[1000] ^= 0xff;
1106-
let new_msg = UpdateAddHTLC::read(&mut Cursor::new(&msg_ser)).unwrap();
1106+
let new_msg = UpdateAddHTLC::read_from_fixed_length_buffer(&mut &msg_ser[..]).unwrap();
11071107
dest.handle_update_add_htlc(nodes[$node].get_our_node_id(), &new_msg);
11081108
}
11091109
}

‎fuzz/src/msg_targets/utils.rs

Lines changed: 21 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -30,16 +30,16 @@ impl Writer for VecWriter {
3030
#[macro_export]
3131
macro_rules! test_msg {
3232
($MsgType: path, $data: ident) => {{
33-
use lightning::util::ser::{Readable, Writeable};
34-
let mut r = ::lightning::io::Cursor::new($data);
35-
if let Ok(msg) = <$MsgType as Readable>::read(&mut r) {
36-
let p = r.position() as usize;
33+
use lightning::util::ser::{LengthReadable, Writeable};
34+
let mut r = &$data[..];
35+
if let Ok(msg) = <$MsgType as LengthReadable>::read_from_fixed_length_buffer(&mut r) {
36+
let p = $data.len() - r.len() as usize;
3737
let mut w = VecWriter(Vec::new());
3838
msg.write(&mut w).unwrap();
3939

4040
assert_eq!(w.0.len(), p);
4141
assert_eq!(msg.serialized_length(), p);
42-
assert_eq!(&r.into_inner()[..p], &w.0[..p]);
42+
assert_eq!(&$data[..p], &w.0[..p]);
4343
}
4444
}};
4545
}
@@ -49,15 +49,16 @@ macro_rules! test_msg {
4949
#[macro_export]
5050
macro_rules! test_msg_simple {
5151
($MsgType: path, $data: ident) => {{
52-
use lightning::util::ser::{Readable, Writeable};
53-
let mut r = ::lightning::io::Cursor::new($data);
54-
if let Ok(msg) = <$MsgType as Readable>::read(&mut r) {
52+
use lightning::util::ser::{LengthReadable, Writeable};
53+
if let Ok(msg) =
54+
<$MsgType as LengthReadable>::read_from_fixed_length_buffer(&mut &$data[..])
55+
{
5556
let mut w = VecWriter(Vec::new());
5657
msg.write(&mut w).unwrap();
5758
assert_eq!(msg.serialized_length(), w.0.len());
5859

5960
let msg =
60-
<$MsgType as Readable>::read(&mut ::lightning::io::Cursor::new(&w.0)).unwrap();
61+
<$MsgType as LengthReadable>::read_from_fixed_length_buffer(&mut &w.0[..]).unwrap();
6162
let mut w_two = VecWriter(Vec::new());
6263
msg.write(&mut w_two).unwrap();
6364
assert_eq!(&w.0[..], &w_two.0[..]);
@@ -70,12 +71,13 @@ macro_rules! test_msg_simple {
7071
#[macro_export]
7172
macro_rules! test_msg_exact {
7273
($MsgType: path, $data: ident) => {{
73-
use lightning::util::ser::{Readable, Writeable};
74-
let mut r = ::lightning::io::Cursor::new($data);
75-
if let Ok(msg) = <$MsgType as Readable>::read(&mut r) {
74+
use lightning::util::ser::{LengthReadable, Writeable};
75+
if let Ok(msg) =
76+
<$MsgType as LengthReadable>::read_from_fixed_length_buffer(&mut &$data[..])
77+
{
7678
let mut w = VecWriter(Vec::new());
7779
msg.write(&mut w).unwrap();
78-
assert_eq!(&r.into_inner()[..], &w.0[..]);
80+
assert_eq!(&$data[..], &w.0[..]);
7981
assert_eq!(msg.serialized_length(), w.0.len());
8082
}
8183
}};
@@ -86,17 +88,18 @@ macro_rules! test_msg_exact {
8688
#[macro_export]
8789
macro_rules! test_msg_hole {
8890
($MsgType: path, $data: ident, $hole: expr, $hole_len: expr) => {{
89-
use lightning::util::ser::{Readable, Writeable};
90-
let mut r = ::lightning::io::Cursor::new($data);
91-
if let Ok(msg) = <$MsgType as Readable>::read(&mut r) {
91+
use lightning::util::ser::{LengthReadable, Writeable};
92+
if let Ok(msg) =
93+
<$MsgType as LengthReadable>::read_from_fixed_length_buffer(&mut &$data[..])
94+
{
9295
let mut w = VecWriter(Vec::new());
9396
msg.write(&mut w).unwrap();
9497
let p = w.0.len() as usize;
9598
assert_eq!(msg.serialized_length(), p);
9699

97100
assert_eq!(w.0.len(), p);
98-
assert_eq!(&r.get_ref()[..$hole], &w.0[..$hole]);
99-
assert_eq!(&r.get_ref()[$hole + $hole_len..p], &w.0[$hole + $hole_len..]);
101+
assert_eq!(&$data[..$hole], &w.0[..$hole]);
102+
assert_eq!(&$data[$hole + $hole_len..p], &w.0[$hole + $hole_len..]);
100103
}
101104
}};
102105
}

‎fuzz/src/onion_message.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,20 +26,23 @@ use lightning::onion_message::packet::OnionMessageContents;
2626
use lightning::sign::{EntropySource, NodeSigner, Recipient, SignerProvider};
2727
use lightning::types::features::InitFeatures;
2828
use lightning::util::logger::Logger;
29-
use lightning::util::ser::{Readable, Writeable, Writer};
29+
use lightning::util::ser::{LengthReadable, Writeable, Writer};
3030
use lightning::util::test_channel_signer::TestChannelSigner;
3131

3232
use lightning_invoice::RawBolt11Invoice;
3333

3434
use crate::utils::test_logger;
3535

36-
use lightning::io::{self, Cursor};
36+
use lightning::io;
3737
use std::sync::atomic::{AtomicU64, Ordering};
3838

3939
#[inline]
4040
/// Actual fuzz test, method signature and name are fixed
4141
pub fn do_test<L: Logger>(data: &[u8], logger: &L) {
42-
if let Ok(msg) = <msgs::OnionMessage as Readable>::read(&mut Cursor::new(data)) {
42+
let mut reader = &data[..];
43+
if let Ok(msg) =
44+
<msgs::OnionMessage as LengthReadable>::read_from_fixed_length_buffer(&mut reader)
45+
{
4346
let mut secret_bytes = [1; 32];
4447
secret_bytes[31] = 2;
4548
let secret = SecretKey::from_slice(&secret_bytes).unwrap();

‎fuzz/src/router.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ use lightning::routing::utxo::{UtxoFuture, UtxoLookup, UtxoLookupError, UtxoResu
3030
use lightning::types::features::{BlindedHopFeatures, Bolt12InvoiceFeatures};
3131
use lightning::util::config::UserConfig;
3232
use lightning::util::hash_tables::*;
33-
use lightning::util::ser::Readable;
33+
use lightning::util::ser::LengthReadable;
3434

3535
use bitcoin::hashes::Hash;
3636
use bitcoin::network::Network;
@@ -146,10 +146,11 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
146146

147147
macro_rules! decode_msg {
148148
($MsgType: path, $len: expr) => {{
149-
let mut reader = ::lightning::io::Cursor::new(get_slice!($len));
150-
match <$MsgType>::read(&mut reader) {
149+
let data = get_slice!($len);
150+
let mut reader = &data[..];
151+
match <$MsgType>::read_from_fixed_length_buffer(&mut reader) {
151152
Ok(msg) => {
152-
assert_eq!(reader.position(), $len as u64);
153+
assert_eq!(reader.len(), $len);
153154
msg
154155
},
155156
Err(e) => match e {

‎lightning-custom-message/src/lib.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
//! use lightning::ln::peer_handler::CustomMessageHandler;
2525
//! use lightning::ln::wire::{CustomMessageReader, self};
2626
//! # use lightning::types::features::{InitFeatures, NodeFeatures};
27-
//! use lightning::util::ser::Writeable;
27+
//! use lightning::util::ser::{LengthLimitedRead, Writeable};
2828
//! # use lightning::util::ser::Writer;
2929
//!
3030
//! // Assume that `FooHandler` and `BarHandler` are defined in one crate and `BazHandler` is
@@ -52,7 +52,7 @@
5252
//! impl CustomMessageReader for FooHandler {
5353
//! // ...
5454
//! # type CustomMessage = Foo;
55-
//! # fn read<R: io::Read>(
55+
//! # fn read<R: LengthLimitedRead>(
5656
//! # &self, _message_type: u16, _buffer: &mut R
5757
//! # ) -> Result<Option<Self::CustomMessage>, DecodeError> {
5858
//! # unimplemented!()
@@ -104,7 +104,7 @@
104104
//! impl CustomMessageReader for BarHandler {
105105
//! // ...
106106
//! # type CustomMessage = Bar;
107-
//! # fn read<R: io::Read>(
107+
//! # fn read<R: LengthLimitedRead>(
108108
//! # &self, _message_type: u16, _buffer: &mut R
109109
//! # ) -> Result<Option<Self::CustomMessage>, DecodeError> {
110110
//! # unimplemented!()
@@ -156,7 +156,7 @@
156156
//! impl CustomMessageReader for BazHandler {
157157
//! // ...
158158
//! # type CustomMessage = Baz;
159-
//! # fn read<R: io::Read>(
159+
//! # fn read<R: LengthLimitedRead>(
160160
//! # &self, _message_type: u16, _buffer: &mut R
161161
//! # ) -> Result<Option<Self::CustomMessage>, DecodeError> {
162162
//! # unimplemented!()
@@ -340,7 +340,7 @@ macro_rules! composite_custom_message_handler {
340340

341341
impl $crate::lightning::ln::wire::CustomMessageReader for $handler {
342342
type CustomMessage = $message;
343-
fn read<R: $crate::lightning::io::Read>(
343+
fn read<R: $crate::lightning::util::ser::LengthLimitedRead>(
344344
&self, message_type: u16, buffer: &mut R
345345
) -> Result<Option<Self::CustomMessage>, $crate::lightning::ln::msgs::DecodeError> {
346346
match message_type {

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

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

21-
use lightning::ln::msgs::LightningError;
21+
use lightning::ln::msgs::{DecodeError, LightningError};
2222
use lightning::ln::wire;
23-
use lightning::util::ser::WithoutLength;
23+
use lightning::util::ser::{LengthLimitedRead, LengthReadable, WithoutLength};
2424

2525
use bitcoin::secp256k1::PublicKey;
2626

@@ -168,9 +168,10 @@ impl lightning::util::ser::Writeable for RawLSPSMessage {
168168
}
169169
}
170170

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)?;
171+
impl LengthReadable for RawLSPSMessage {
172+
fn read_from_fixed_length_buffer<R: LengthLimitedRead>(r: &mut R) -> Result<Self, DecodeError> {
173+
let payload_without_length: WithoutLength<String> =
174+
LengthReadable::read_from_fixed_length_buffer(r)?;
174175
Ok(Self { payload: payload_without_length.0 })
175176
}
176177
}

‎lightning-liquidity/src/manager.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ use lightning::ln::peer_handler::CustomMessageHandler;
2727
use lightning::ln::wire::CustomMessageReader;
2828
use lightning::sign::EntropySource;
2929
use lightning::util::logger::Level;
30-
use lightning::util::ser::Readable;
30+
use lightning::util::ser::{LengthLimitedRead, LengthReadable};
3131

3232
use lightning_types::features::{InitFeatures, NodeFeatures};
3333

@@ -449,11 +449,13 @@ where
449449
{
450450
type CustomMessage = RawLSPSMessage;
451451

452-
fn read<RD: lightning::io::Read>(
452+
fn read<RD: LengthLimitedRead>(
453453
&self, message_type: u16, buffer: &mut RD,
454454
) -> Result<Option<Self::CustomMessage>, lightning::ln::msgs::DecodeError> {
455455
match message_type {
456-
LSPS_MESSAGE_TYPE_ID => Ok(Some(RawLSPSMessage::read(buffer)?)),
456+
LSPS_MESSAGE_TYPE_ID => {
457+
Ok(Some(RawLSPSMessage::read_from_fixed_length_buffer(buffer)?))
458+
},
457459
_ => Ok(None),
458460
}
459461
}

‎lightning/src/crypto/streams.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use crate::crypto::chacha20poly1305rfc::ChaCha20Poly1305RFC;
44
use crate::io::{self, Read, Write};
55
use crate::ln::msgs::DecodeError;
66
use crate::util::ser::{
7-
FixedLengthReader, LengthRead, LengthReadableArgs, Readable, Writeable, Writer,
7+
FixedLengthReader, LengthLimitedRead, LengthReadableArgs, Readable, Writeable, Writer,
88
};
99

1010
pub(crate) struct ChaChaReader<'a, R: io::Read> {
@@ -56,16 +56,16 @@ pub(crate) struct ChaChaPolyReadAdapter<R: Readable> {
5656
}
5757

5858
impl<T: Readable> LengthReadableArgs<[u8; 32]> for ChaChaPolyReadAdapter<T> {
59-
// Simultaneously read and decrypt an object from a LengthRead, storing it in Self::readable.
60-
// LengthRead must be used instead of std::io::Read because we need the total length to separate
61-
// out the tag at the end.
62-
fn read<R: LengthRead>(r: &mut R, secret: [u8; 32]) -> Result<Self, DecodeError> {
63-
if r.total_bytes() < 16 {
59+
// Simultaneously read and decrypt an object from a LengthLimitedRead storing it in
60+
// Self::readable. LengthLimitedRead must be used instead of std::io::Read because we need the
61+
// total length to separate out the tag at the end.
62+
fn read<R: LengthLimitedRead>(r: &mut R, secret: [u8; 32]) -> Result<Self, DecodeError> {
63+
if r.remaining_bytes() < 16 {
6464
return Err(DecodeError::InvalidValue);
6565
}
6666

6767
let mut chacha = ChaCha20Poly1305RFC::new(&secret, &[0; 12], &[]);
68-
let decrypted_len = r.total_bytes() - 16;
68+
let decrypted_len = r.remaining_bytes() - 16;
6969
let s = FixedLengthReader::new(r, decrypted_len);
7070
let mut chacha_stream = ChaChaPolyReader { chacha: &mut chacha, read: s };
7171
let readable: T = Readable::read(&mut chacha_stream)?;

‎lightning/src/ln/channelmanager.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ use crate::util::config::{ChannelConfig, ChannelConfigUpdate, ChannelConfigOverr
8383
use crate::util::wakers::{Future, Notifier};
8484
use crate::util::scid_utils::fake_scid;
8585
use crate::util::string::UntrustedString;
86-
use crate::util::ser::{BigSize, FixedLengthReader, Readable, ReadableArgs, MaybeReadable, Writeable, Writer, VecWriter};
86+
use crate::util::ser::{BigSize, FixedLengthReader, LengthReadable, Readable, ReadableArgs, MaybeReadable, Writeable, Writer, VecWriter};
8787
use crate::util::logger::{Level, Logger, WithContext};
8888
use crate::util::errors::APIError;
8989
#[cfg(async_payments)] use {
@@ -13016,14 +13016,14 @@ impl Readable for HTLCFailureMsg {
1301613016
2 => {
1301713017
let length: BigSize = Readable::read(reader)?;
1301813018
let mut s = FixedLengthReader::new(reader, length.0);
13019-
let res = Readable::read(&mut s)?;
13019+
let res = LengthReadable::read_from_fixed_length_buffer(&mut s)?;
1302013020
s.eat_remaining()?; // Return ShortRead if there's actually not enough bytes
1302113021
Ok(HTLCFailureMsg::Relay(res))
1302213022
},
1302313023
3 => {
1302413024
let length: BigSize = Readable::read(reader)?;
1302513025
let mut s = FixedLengthReader::new(reader, length.0);
13026-
let res = Readable::read(&mut s)?;
13026+
let res = LengthReadable::read_from_fixed_length_buffer(&mut s)?;
1302713027
s.eat_remaining()?; // Return ShortRead if there's actually not enough bytes
1302813028
Ok(HTLCFailureMsg::Malformed(res))
1302913029
},

0 commit comments

Comments
 (0)