From 3e9727b808814e6a6656a071a247695d9d95c572 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 28 Nov 2022 01:00:38 +0000 Subject: [PATCH] Use a variable-length integer for many serialization wrappers The lightning protocol uses u16s for lengths in most cases. As our serialization framework primarily targets that, we must as well. However, because we may serialize objects that have more than 65K entries, we want to be able to store larger values. Thus, we define a variable length integer here which is backwards-compatible but treats 0xffff as "read eight more bytes". This doesn't address any specific known issue, but feels like good practice just in case. --- lightning/src/util/ser.rs | 75 +++++++++++++++++++++++++++++---------- 1 file changed, 57 insertions(+), 18 deletions(-) diff --git a/lightning/src/util/ser.rs b/lightning/src/util/ser.rs index 44725e72..84d1a2e0 100644 --- a/lightning/src/util/ser.rs +++ b/lightning/src/util/ser.rs @@ -383,6 +383,40 @@ impl Readable for BigSize { } } +/// The lightning protocol uses u16s for lengths in most cases. As our serialization framework +/// primarily targets that, we must as well. However, because we may serialize objects that have +/// more than 65K entries, we need to be able to store larger values. Thus, we define a variable +/// length integer here that is backwards-compatible for values < 0xffff. We treat 0xffff as +/// "read eight more bytes". +/// +/// To ensure we only have one valid encoding per value, we add 0xffff to values written as eight +/// bytes. Thus, 0xfffe is serialized as 0xfffe, whereas 0xffff is serialized as +/// 0xffff0000000000000000 (i.e. read-eight-bytes then zero). +struct CollectionLength(pub u64); +impl Writeable for CollectionLength { + #[inline] + fn write(&self, writer: &mut W) -> Result<(), io::Error> { + if self.0 < 0xffff { + (self.0 as u16).write(writer) + } else { + 0xffffu16.write(writer)?; + (self.0 - 0xffff).write(writer) + } + } +} + +impl Readable for CollectionLength { + #[inline] + fn read(r: &mut R) -> Result { + let mut val: u64 = ::read(r)? as u64; + if val == 0xffff { + val = ::read(r)? + .checked_add(0xffff).ok_or(DecodeError::InvalidValue)?; + } + Ok(CollectionLength(val)) + } +} + /// In TLV we occasionally send fields which only consist of, or potentially end with, a /// variable-length integer which is simply truncated by skipping high zero bytes. This type /// encapsulates such integers implementing [`Readable`]/[`Writeable`] for them. @@ -597,7 +631,7 @@ macro_rules! impl_for_map { { #[inline] fn write(&self, w: &mut W) -> Result<(), io::Error> { - (self.len() as u16).write(w)?; + CollectionLength(self.len() as u64).write(w)?; for (key, value) in self.iter() { key.write(w)?; value.write(w)?; @@ -611,9 +645,9 @@ macro_rules! impl_for_map { { #[inline] fn read(r: &mut R) -> Result { - let len: u16 = Readable::read(r)?; - let mut ret = $constr(len as usize); - for _ in 0..len { + let len: CollectionLength = Readable::read(r)?; + let mut ret = $constr(len.0 as usize); + for _ in 0..len.0 { let k = K::read(r)?; let v_opt = V::read(r)?; if let Some(v) = v_opt { @@ -637,7 +671,7 @@ where T: Writeable + Eq + Hash { #[inline] fn write(&self, w: &mut W) -> Result<(), io::Error> { - (self.len() as u16).write(w)?; + CollectionLength(self.len() as u64).write(w)?; for item in self.iter() { item.write(w)?; } @@ -650,9 +684,9 @@ where T: Readable + Eq + Hash { #[inline] fn read(r: &mut R) -> Result { - let len: u16 = Readable::read(r)?; - let mut ret = HashSet::with_capacity(len as usize); - for _ in 0..len { + let len: CollectionLength = Readable::read(r)?; + let mut ret = HashSet::with_capacity(cmp::min(len.0 as usize, MAX_BUF_SIZE / core::mem::size_of::())); + for _ in 0..len.0 { if !ret.insert(T::read(r)?) { return Err(DecodeError::InvalidValue) } @@ -667,7 +701,7 @@ macro_rules! impl_for_vec { impl<$($name : Writeable),*> Writeable for Vec<$ty> { #[inline] fn write(&self, w: &mut W) -> Result<(), io::Error> { - (self.len() as u16).write(w)?; + CollectionLength(self.len() as u64).write(w)?; for elem in self.iter() { elem.write(w)?; } @@ -678,9 +712,9 @@ macro_rules! impl_for_vec { impl<$($name : Readable),*> Readable for Vec<$ty> { #[inline] fn read(r: &mut R) -> Result { - let len: u16 = Readable::read(r)?; - let mut ret = Vec::with_capacity(cmp::min(len as usize, MAX_BUF_SIZE / core::mem::size_of::<$ty>())); - for _ in 0..len { + let len: CollectionLength = Readable::read(r)?; + let mut ret = Vec::with_capacity(cmp::min(len.0 as usize, MAX_BUF_SIZE / core::mem::size_of::<$ty>())); + for _ in 0..len.0 { if let Some(val) = MaybeReadable::read(r)? { ret.push(val); } @@ -694,7 +728,7 @@ macro_rules! impl_for_vec { impl Writeable for Vec { #[inline] fn write(&self, w: &mut W) -> Result<(), io::Error> { - (self.len() as u16).write(w)?; + CollectionLength(self.len() as u64).write(w)?; w.write_all(&self) } } @@ -702,10 +736,15 @@ impl Writeable for Vec { impl Readable for Vec { #[inline] fn read(r: &mut R) -> Result { - let len: u16 = Readable::read(r)?; - let mut ret = Vec::with_capacity(len as usize); - ret.resize(len as usize, 0); - r.read_exact(&mut ret)?; + let mut len: CollectionLength = Readable::read(r)?; + let mut ret = Vec::new(); + while len.0 > 0 { + let readamt = cmp::min(len.0 as usize, MAX_BUF_SIZE); + let readstart = ret.len(); + ret.resize(readstart + readamt, 0); + r.read_exact(&mut ret[readstart..])?; + len.0 -= readamt as u64; + } Ok(ret) } } @@ -1040,7 +1079,7 @@ impl Readable for () { impl Writeable for String { #[inline] fn write(&self, w: &mut W) -> Result<(), io::Error> { - (self.len() as u16).write(w)?; + CollectionLength(self.len() as u64).write(w)?; w.write_all(self.as_bytes()) } } -- 2.30.2