Limit TLV stream decoding to type ranges

BOLT 12 messages are limited to a range of TLV record types. Refactor
decode_tlv_stream into a decode_tlv_stream_range macro for limiting
which types are parsed. Requires a SeekReadable trait for rewinding when
a type outside of the range is seen. This allows for composing TLV
streams of different ranges.

Updates offer parsing accordingly and adds a test demonstrating failure
if a type outside of the range is included.

diff --git a/lightning/src/offers/offer.rs b/lightning/src/offers/offer.rs
index e57334f316c02745f2d53127ef3c548b3383af47..6b2dfe2aa5359770f18da167732a013a64857d3c 100644 (file)
--- a/lightning/src/offers/offer.rs
+++ b/lightning/src/offers/offer.rs
@@ -76,9 +76,9 @@ use core::time::Duration;
 use crate::io;
 use crate::ln::features::OfferFeatures;
 use crate::ln::msgs::MAX_VALUE_MSAT;
-use crate::offers::parse::{Bech32Encode, ParseError, SemanticError};
+use crate::offers::parse::{Bech32Encode, ParseError, ParsedMessage, SemanticError};
 use crate::onion_message::BlindedPath;
-use crate::util::ser::{HighZeroBytesDroppedBigSize, Readable, WithoutLength, Writeable, Writer};
+use crate::util::ser::{HighZeroBytesDroppedBigSize, WithoutLength, Writeable, Writer};
 use crate::util::string::PrintableString;
 
 use crate::prelude::*;
@@ -394,15 +394,6 @@ impl Writeable for OfferContents {
        }
 }
 
-impl TryFrom<Vec<u8>> for Offer {
-       type Error = ParseError;
-
-       fn try_from(bytes: Vec<u8>) -> Result<Self, Self::Error> {
-               let tlv_stream: OfferTlvStream = Readable::read(&mut &bytes[..])?;
-               Offer::try_from((bytes, tlv_stream))
-       }
-}
-
 /// The minimum amount required for an item in an [`Offer`], denominated in either bitcoin or
 /// another currency.
 #[derive(Clone, Debug, PartialEq)]
@@ -449,7 +440,7 @@ impl Quantity {
        }
 }
 
-tlv_stream!(OfferTlvStream, OfferTlvStreamRef, {
+tlv_stream!(OfferTlvStream, OfferTlvStreamRef, 1..80, {
        (2, chains: (Vec<ChainHash>, WithoutLength)),
        (4, metadata: (Vec<u8>, WithoutLength)),
        (6, currency: CurrencyCode),
@@ -467,8 +458,6 @@ impl Bech32Encode for Offer {
        const BECH32_HRP: &'static str = "lno";
 }
 
-type ParsedOffer = (Vec<u8>, OfferTlvStream);
-
 impl FromStr for Offer {
        type Err = ParseError;
 
@@ -477,11 +466,12 @@ impl FromStr for Offer {
        }
 }
 
-impl TryFrom<ParsedOffer> for Offer {
+impl TryFrom<Vec<u8>> for Offer {
        type Error = ParseError;
 
-       fn try_from(offer: ParsedOffer) -> Result<Self, Self::Error> {
-               let (bytes, tlv_stream) = offer;
+       fn try_from(bytes: Vec<u8>) -> Result<Self, Self::Error> {
+               let offer = ParsedMessage::<OfferTlvStream>::try_from(bytes)?;
+               let ParsedMessage { bytes, tlv_stream } = offer;
                let contents = OfferContents::try_from(tlv_stream)?;
                Ok(Offer { bytes, contents })
        }
@@ -551,10 +541,10 @@ mod tests {
        use core::num::NonZeroU64;
        use core::time::Duration;
        use crate::ln::features::OfferFeatures;
-       use crate::ln::msgs::MAX_VALUE_MSAT;
+       use crate::ln::msgs::{DecodeError, MAX_VALUE_MSAT};
        use crate::offers::parse::{ParseError, SemanticError};
        use crate::onion_message::{BlindedHop, BlindedPath};
-       use crate::util::ser::Writeable;
+       use crate::util::ser::{BigSize, Writeable};
        use crate::util::string::PrintableString;
 
        fn pubkey(byte: u8) -> PublicKey {
@@ -1003,6 +993,22 @@ mod tests {
                        },
                }
        }
+
+       #[test]
+       fn fails_parsing_offer_with_extra_tlv_records() {
+               let offer = OfferBuilder::new("foo".into(), pubkey(42)).build().unwrap();
+
+               let mut encoded_offer = Vec::new();
+               offer.write(&mut encoded_offer).unwrap();
+               BigSize(80).write(&mut encoded_offer).unwrap();
+               BigSize(32).write(&mut encoded_offer).unwrap();
+               [42u8; 32].write(&mut encoded_offer).unwrap();
+
+               match Offer::try_from(encoded_offer) {
+                       Ok(_) => panic!("expected error"),
+                       Err(e) => assert_eq!(e, ParseError::Decode(DecodeError::InvalidValue)),
+               }
+       }
 }
 
 #[cfg(test)]

diff --git a/lightning/src/offers/parse.rs b/lightning/src/offers/parse.rs
index 032e95b3cc52199a095c29a78c064abed2b6c40f..19d7d74ed6111fd6ddc9fef9d7410a8a47b43354 100644 (file)
--- a/lightning/src/offers/parse.rs
+++ b/lightning/src/offers/parse.rs
@@ -13,7 +13,9 @@ use bitcoin::bech32;
 use bitcoin::bech32::{FromBase32, ToBase32};
 use core::convert::TryFrom;
 use core::fmt;
+use crate::io;
 use crate::ln::msgs::DecodeError;
+use crate::util::ser::SeekReadable;
 
 use crate::prelude::*;
 
@@ -74,6 +76,30 @@ impl<'a> AsRef<str> for Bech32String<'a> {
        }
 }
 
+/// A wrapper for reading a message as a TLV stream `T` from a byte sequence, while still
+/// maintaining ownership of the bytes for later use.
+pub(crate) struct ParsedMessage<T: SeekReadable> {
+       pub bytes: Vec<u8>,
+       pub tlv_stream: T,
+}
+
+impl<T: SeekReadable> TryFrom<Vec<u8>> for ParsedMessage<T> {
+       type Error = DecodeError;
+
+       fn try_from(bytes: Vec<u8>) -> Result<Self, Self::Error> {
+               let mut cursor = io::Cursor::new(bytes);
+               let tlv_stream: T = SeekReadable::read(&mut cursor)?;
+
+               // Ensure that there are no more TLV records left to parse.
+               if cursor.position() < cursor.get_ref().len() as u64 {
+                       return Err(DecodeError::InvalidValue);
+               }
+
+               let bytes = cursor.into_inner();
+               Ok(Self { bytes, tlv_stream })
+       }
+}
+
 /// Error when parsing a bech32 encoded message using [`str::parse`].
 #[derive(Debug, PartialEq)]
 pub enum ParseError {

diff --git a/lightning/src/util/ser.rs b/lightning/src/util/ser.rs
index 7c4ba7fd50f99888745a6944971c10f3f00f77a9..04dc04e55b025f937f542b27779feae467e064fb 100644 (file)
--- a/lightning/src/util/ser.rs
+++ b/lightning/src/util/ser.rs
@@ -11,7 +11,7 @@
 //! as ChannelsManagers and ChannelMonitors.
 
 use crate::prelude::*;
-use crate::io::{self, Read, Write};
+use crate::io::{self, Read, Seek, Write};
 use crate::io_extras::{copy, sink};
 use core::hash::Hash;
 use crate::sync::Mutex;
@@ -219,6 +219,13 @@ pub trait Readable
        fn read<R: Read>(reader: &mut R) -> Result<Self, DecodeError>;
 }
 
+/// A trait that various rust-lightning types implement allowing them to be read in from a
+/// `Read + Seek`.
+pub(crate) trait SeekReadable where Self: Sized {
+       /// Reads a Self in from the given Read
+       fn read<R: Read + Seek>(reader: &mut R) -> Result<Self, DecodeError>;
+}
+
 /// A trait that various higher-level rust-lightning types implement allowing them to be read in
 /// from a Read given some additional set of arguments which is required to deserialize.
 ///

diff --git a/lightning/src/util/ser_macros.rs b/lightning/src/util/ser_macros.rs
index 129fb8f407b088de53aa7a94439d68fb478ed0c4..db7553d5ff8e3dea88b206bc462011e1c8d96f22 100644 (file)
--- a/lightning/src/util/ser_macros.rs
+++ b/lightning/src/util/ser_macros.rs
@@ -201,6 +201,17 @@ macro_rules! decode_tlv {
 // `Ok(false)` if the message type is unknown, and `Err(DecodeError)` if parsing fails.
 macro_rules! decode_tlv_stream {
        ($stream: expr, {$(($type: expr, $field: ident, $fieldty: tt)),* $(,)*}
+        $(, $decode_custom_tlv: expr)?) => { {
+               let rewind = |_, _| { unreachable!() };
+               use core::ops::RangeBounds;
+               decode_tlv_stream_range!(
+                       $stream, .., rewind, {$(($type, $field, $fieldty)),*} $(, $decode_custom_tlv)?
+               );
+       } }
+}
+
+macro_rules! decode_tlv_stream_range {
+       ($stream: expr, $range: expr, $rewind: ident, {$(($type: expr, $field: ident, $fieldty: tt)),* $(,)*}
         $(, $decode_custom_tlv: expr)?) => { {
                use $crate::ln::msgs::DecodeError;
                let mut last_seen_type: Option<u64> = None;
@@ -215,7 +226,7 @@ macro_rules! decode_tlv_stream {
                                // UnexpectedEof. This should in every case be largely cosmetic, but its nice to
                                // pass the TLV test vectors exactly, which requre this distinction.
                                let mut tracking_reader = ser::ReadTrackingReader::new(&mut stream_ref);
-                               match $crate::util::ser::Readable::read(&mut tracking_reader) {
+                               match <$crate::util::ser::BigSize as $crate::util::ser::Readable>::read(&mut tracking_reader) {
                                        Err(DecodeError::ShortRead) => {
                                                if !tracking_reader.have_read {
                                                        break 'tlv_read;
@@ -224,7 +235,15 @@ macro_rules! decode_tlv_stream {
                                                }
                                        },
                                        Err(e) => return Err(e),
-                                       Ok(t) => t,
+                                       Ok(t) => if $range.contains(&t.0) { t } else {
+                                               drop(tracking_reader);
+
+                                               // Assumes the type id is minimally encoded, which is enforced on read.
+                                               use $crate::util::ser::Writeable;
+                                               let bytes_read = t.serialized_length();
+                                               $rewind(stream_ref, bytes_read);
+                                               break 'tlv_read;
+                                       },
                                }
                        };
 
@@ -472,7 +491,7 @@ macro_rules! impl_writeable_tlv_based {
 /// [`Readable`]: crate::util::ser::Readable
 /// [`Writeable`]: crate::util::ser::Writeable
 macro_rules! tlv_stream {
-       ($name:ident, $nameref:ident, {
+       ($name:ident, $nameref:ident, $range:expr, {
                $(($type:expr, $field:ident : $fieldty:tt)),* $(,)*
        }) => {
                #[derive(Debug)]
@@ -497,12 +516,15 @@ macro_rules! tlv_stream {
                        }
                }
 
-               impl $crate::util::ser::Readable for $name {
-                       fn read<R: $crate::io::Read>(reader: &mut R) -> Result<Self, $crate::ln::msgs::DecodeError> {
+               impl $crate::util::ser::SeekReadable for $name {
+                       fn read<R: $crate::io::Read + $crate::io::Seek>(reader: &mut R) -> Result<Self, $crate::ln::msgs::DecodeError> {
                                $(
                                        init_tlv_field_var!($field, option);
                                )*
-                               decode_tlv_stream!(reader, {
+                               let rewind = |cursor: &mut R, offset: usize| {
+                                       cursor.seek($crate::io::SeekFrom::Current(-(offset as i64))).expect("");
+                               };
+                               decode_tlv_stream_range!(reader, $range, rewind, {
                                        $(($type, $field, (option, encoding: $fieldty))),*
                                });