From f389a0ec39da9535e0c0659ccf8e61af1f1afd7e Mon Sep 17 00:00:00 2001 From: Duncan Dean Date: Tue, 9 May 2023 08:19:32 +0200 Subject: [PATCH 1/2] Move TransactionU16LenLimited to ser.rs --- lightning/src/ln/msgs.rs | 50 ++++----------------------------------- lightning/src/util/ser.rs | 39 ++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 45 deletions(-) diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index 27f8544e3b5..39cb9454db8 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -26,7 +26,7 @@ use bitcoin::secp256k1::PublicKey; use bitcoin::secp256k1::ecdsa::Signature; -use bitcoin::{secp256k1, Witness, Transaction}; +use bitcoin::{secp256k1, Witness}; use bitcoin::blockdata::script::Script; use bitcoin::hash_types::{Txid, BlockHash}; @@ -42,7 +42,7 @@ use crate::io_extras::read_to_end; use crate::events::{MessageSendEventsProvider, OnionMessageProvider}; use crate::util::logger; -use crate::util::ser::{LengthReadable, Readable, ReadableArgs, Writeable, Writer, WithoutLength, FixedLengthReader, HighZeroBytesDroppedBigSize, Hostname}; +use crate::util::ser::{LengthReadable, Readable, ReadableArgs, Writeable, Writer, WithoutLength, FixedLengthReader, HighZeroBytesDroppedBigSize, Hostname, TransactionU16LenLimited}; use crate::ln::{PaymentPreimage, PaymentHash, PaymentSecret}; @@ -425,45 +425,6 @@ pub struct ChannelReady { pub short_channel_id_alias: Option, } -/// A wrapper for a `Transaction` which can only be constructed with [`TransactionU16LenLimited::new`] -/// if the `Transaction`'s consensus-serialized length is <= u16::MAX. -/// -/// Use [`TransactionU16LenLimited::into_transaction`] to convert into the contained `Transaction`. -#[derive(Clone, Debug, PartialEq, Eq)] -pub struct TransactionU16LenLimited(Transaction); - -impl TransactionU16LenLimited { - /// Constructs a new `TransactionU16LenLimited` from a `Transaction` only if it's consensus- - /// serialized length is <= u16::MAX. - pub fn new(transaction: Transaction) -> Result { - if transaction.serialized_length() > (u16::MAX as usize) { - Err(()) - } else { - Ok(Self(transaction)) - } - } - - /// Consumes this `TransactionU16LenLimited` and returns its contained `Transaction`. - pub fn into_transaction(self) -> Transaction { - self.0 - } -} - -impl Writeable for TransactionU16LenLimited { - fn write(&self, w: &mut W) -> Result<(), io::Error> { - (self.0.serialized_length() as u16).write(w)?; - self.0.write(w) - } -} - -impl Readable for TransactionU16LenLimited { - fn read(r: &mut R) -> Result { - let len = ::read(r)?; - let mut tx_reader = FixedLengthReader::new(r, len as u64); - Ok(Self(Readable::read(&mut tx_reader)?)) - } -} - /// A tx_add_input message for adding an input during interactive transaction construction /// // TODO(dual_funding): Add spec link for `tx_add_input`. @@ -850,7 +811,7 @@ impl NetAddress { } impl Writeable for NetAddress { -fn write(&self, writer: &mut W) -> Result<(), io::Error> { + fn write(&self, writer: &mut W) -> Result<(), io::Error> { match self { &NetAddress::IPv4 { ref addr, ref port } => { 1u8.write(writer)?; @@ -2454,10 +2415,9 @@ mod tests { use hex; use crate::ln::{PaymentPreimage, PaymentHash, PaymentSecret}; use crate::ln::features::{ChannelFeatures, ChannelTypeFeatures, InitFeatures, NodeFeatures}; - use crate::ln::msgs::{self, TransactionU16LenLimited}; - use crate::ln::msgs::{FinalOnionHopData, OnionErrorPacket, OnionHopDataFormat}; + use crate::ln::msgs::{self, FinalOnionHopData, OnionErrorPacket, OnionHopDataFormat}; use crate::routing::gossip::{NodeAlias, NodeId}; - use crate::util::ser::{Writeable, Readable, Hostname}; + use crate::util::ser::{Writeable, Readable, Hostname, TransactionU16LenLimited}; use bitcoin::hashes::hex::FromHex; use bitcoin::util::address::Address; diff --git a/lightning/src/util/ser.rs b/lightning/src/util/ser.rs index 707ca2b326d..1570e5aeea4 100644 --- a/lightning/src/util/ser.rs +++ b/lightning/src/util/ser.rs @@ -1349,6 +1349,45 @@ impl Readable for Duration { } } +/// A wrapper for a `Transaction` which can only be constructed with [`TransactionU16LenLimited::new`] +/// if the `Transaction`'s consensus-serialized length is <= u16::MAX. +/// +/// Use [`TransactionU16LenLimited::into_transaction`] to convert into the contained `Transaction`. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct TransactionU16LenLimited(Transaction); + +impl TransactionU16LenLimited { + /// Constructs a new `TransactionU16LenLimited` from a `Transaction` only if it's consensus- + /// serialized length is <= u16::MAX. + pub fn new(transaction: Transaction) -> Result { + if transaction.serialized_length() > (u16::MAX as usize) { + Err(()) + } else { + Ok(Self(transaction)) + } + } + + /// Consumes this `TransactionU16LenLimited` and returns its contained `Transaction`. + pub fn into_transaction(self) -> Transaction { + self.0 + } +} + +impl Writeable for TransactionU16LenLimited { + fn write(&self, w: &mut W) -> Result<(), io::Error> { + (self.0.serialized_length() as u16).write(w)?; + self.0.write(w) + } +} + +impl Readable for TransactionU16LenLimited { + fn read(r: &mut R) -> Result { + let len = ::read(r)?; + let mut tx_reader = FixedLengthReader::new(r, len as u64); + Ok(Self(Readable::read(&mut tx_reader)?)) + } +} + #[cfg(test)] mod tests { use core::convert::TryFrom; From 33e901a88f43fd6426b14b934fcdd15d5f6bc04e Mon Sep 17 00:00:00 2001 From: Duncan Dean Date: Tue, 9 May 2023 08:37:58 +0200 Subject: [PATCH 2/2] Enforce that no bytes remain for TransactionU16LenLimited read --- lightning/src/util/ser.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lightning/src/util/ser.rs b/lightning/src/util/ser.rs index 1570e5aeea4..cbdb5485e7b 100644 --- a/lightning/src/util/ser.rs +++ b/lightning/src/util/ser.rs @@ -1384,7 +1384,12 @@ impl Readable for TransactionU16LenLimited { fn read(r: &mut R) -> Result { let len = ::read(r)?; let mut tx_reader = FixedLengthReader::new(r, len as u64); - Ok(Self(Readable::read(&mut tx_reader)?)) + let tx: Transaction = Readable::read(&mut tx_reader)?; + if tx_reader.bytes_remain() { + Err(DecodeError::BadLengthDescriptor) + } else { + Ok(Self(tx)) + } } }