From 0e5a56f25f51d07290613a9363bd8c0c9fc8e0be Mon Sep 17 00:00:00 2001 From: Lorenz Leutgeb Date: Thu, 10 Sep 2026 10:13:51 +0200 Subject: [PATCH] protocol: Limit allocations when decoding frames `VarInt` can get large enough to exhaust available memory on many machines. Instead of allocating directly, check the size of the allocation against some limit. The limit chosen for frames is 128 KiB, which is roughly twice as much as actual gossip and Git protocol frames are expected to be. However, 128 KiB is arguably short enough to not burden a standard consumer machine too much. --- crates/radicle-protocol/src/deserializer.rs | 13 ++++++++++++- crates/radicle-protocol/src/wire.rs | 5 ++++- crates/radicle-protocol/src/wire/frame.rs | 7 +++++-- crates/radicle-protocol/src/wire/varint.rs | 17 ++++++++++++++--- 4 files changed, 35 insertions(+), 7 deletions(-) diff --git a/crates/radicle-protocol/src/deserializer.rs b/crates/radicle-protocol/src/deserializer.rs index 90575c711..9b9c755a9 100644 --- a/crates/radicle-protocol/src/deserializer.rs +++ b/crates/radicle-protocol/src/deserializer.rs @@ -58,7 +58,18 @@ impl Deserializer { Ok(Some(msg)) } - Err(wire::Error::UnexpectedEnd { .. }) => Ok(None), + Err(wire::Error::UnexpectedEnd { .. }) => { + log::debug!("Dropping incomplete frame: Expected more bytes."); + Ok(None) + } + Err(wire::Error::FrameTooLong { length, limit }) => { + log::debug!( + "Dropping frame that is too large: Expected at most {} bytes, got {} bytes.", + limit, + length + ); + Ok(None) + } Err(wire::Error::Invalid(err)) => Err(err), } } diff --git a/crates/radicle-protocol/src/wire.rs b/crates/radicle-protocol/src/wire.rs index 232755f08..ffde93600 100644 --- a/crates/radicle-protocol/src/wire.rs +++ b/crates/radicle-protocol/src/wire.rs @@ -93,6 +93,9 @@ pub enum Error { #[error(transparent)] Invalid(#[from] Invalid), + #[error("frame too long: expected at most {limit}, got {length}")] + FrameTooLong { length: usize, limit: usize }, + #[error( "unexpected end of buffer, requested {requested} more bytes but only {available} are available" )] @@ -147,7 +150,7 @@ pub trait Decode: Sized { } Ok(value) } - Err(err @ Error::UnexpectedEnd { .. }) => { + Err(err @ (Error::UnexpectedEnd { .. } | Error::FrameTooLong { .. })) => { panic!("{}", err); } Err(Error::Invalid(e)) => Err(e), diff --git a/crates/radicle-protocol/src/wire/frame.rs b/crates/radicle-protocol/src/wire/frame.rs index 50c90dd99..5972cd72a 100644 --- a/crates/radicle-protocol/src/wire/frame.rs +++ b/crates/radicle-protocol/src/wire/frame.rs @@ -210,6 +210,9 @@ pub struct Frame { } impl Frame { + /// The maximum length of a frame, in bytes (128 KiB). + pub const LENGTH_LIMIT: usize = 1 << 17; + /// Create a 'git' protocol frame. pub fn git(stream: StreamId, data: Vec) -> Self { Self { @@ -360,7 +363,7 @@ impl wire::Decode for Frame { Ok(frame) } Ok(StreamType::Gossip) => { - let data = varint::payload::decode(buf)?; + let data = varint::payload::decode(buf, Frame::::LENGTH_LIMIT)?; let mut cursor = io::Cursor::new(data); let msg = M::decode(&mut cursor)?; let frame = Frame { @@ -375,7 +378,7 @@ impl wire::Decode for Frame { Ok(frame) } Ok(StreamType::Git) => { - let data = varint::payload::decode(buf)?; + let data = varint::payload::decode(buf, Frame::::LENGTH_LIMIT)?; Ok(Frame::git(stream, data)) } Err(n) => Err(wire::Invalid::StreamType { actual: n }.into()), diff --git a/crates/radicle-protocol/src/wire/varint.rs b/crates/radicle-protocol/src/wire/varint.rs index 884f5efa4..a2ef22fb5 100644 --- a/crates/radicle-protocol/src/wire/varint.rs +++ b/crates/radicle-protocol/src/wire/varint.rs @@ -163,10 +163,21 @@ pub mod payload { buf.put_slice(payload); } - /// Decode varint-prefixed data payload. - pub fn decode(buf: &mut impl Buf) -> Result, wire::Error> { + /// Decode varint-prefixed data payload, if the size of the payload + /// is at most `limit` bytes. + pub fn decode(buf: &mut impl Buf, limit: usize) -> Result, wire::Error> { let size = VarInt::decode(buf)?; - let mut data = vec![0; *size as usize]; + + let size = *size as usize; + + if size > limit { + return Err(wire::Error::FrameTooLong { + length: size, + limit, + }); + } + + let mut data = vec![0; size]; buf.try_copy_to_slice(&mut data[..])?; Ok(data)