From a8426dfdacbc4e896adf7cd5d97be79d976ccf5d Mon Sep 17 00:00:00 2001 From: Lorenz Leutgeb Date: Wed, 20 Aug 2025 15:34:02 +0200 Subject: [PATCH] protocol: Fix panic when serializing large frames In 3c5668edd22ae4b9a085220d6be552f944ccb038 I mistakenly moved the check the length of an encoded `wire::Message` from its `impl wire::Encode` to the `fn serialize`, so that it would not apply only to messages (as originally intended), but to *any* argument of `serialize`. This is not a problem for gossip messages, but catastrophic for Git streams, which tend to send a lot of data. The fix is simple: Move the check back into `impl Encode for Message`. --- crates/radicle-protocol/src/wire.rs | 8 ++------ crates/radicle-protocol/src/wire/frame.rs | 21 +++++++++++++++++++++ crates/radicle-protocol/src/wire/message.rs | 2 ++ 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/crates/radicle-protocol/src/wire.rs b/crates/radicle-protocol/src/wire.rs index d3d9f78e..5674a0ab 100644 --- a/crates/radicle-protocol/src/wire.rs +++ b/crates/radicle-protocol/src/wire.rs @@ -103,14 +103,10 @@ pub trait Decode: Sized { } /// Encode an object into a byte vector. -/// -/// # Panics -/// -/// If the encoded object exceeds [`Size::MAX`]. pub fn serialize(data: &E) -> Vec { - let mut buffer = Vec::new().limit(Size::MAX as usize); + let mut buffer = Vec::new(); data.encode(&mut buffer); - buffer.into_inner() + buffer } /// Decode an object from a slice. diff --git a/crates/radicle-protocol/src/wire/frame.rs b/crates/radicle-protocol/src/wire/frame.rs index 0a2fd0c8..d2254c1e 100644 --- a/crates/radicle-protocol/src/wire/frame.rs +++ b/crates/radicle-protocol/src/wire/frame.rs @@ -390,4 +390,25 @@ mod test { assert_eq!(StreamId::control(Link::Inbound), StreamId(VarInt(0b001))); assert_eq!(StreamId::gossip(Link::Inbound), StreamId(VarInt(0b011))); } + + #[test] + fn test_encode_git_large() { + let size = u16::MAX as usize * 3; + assert!( + size > (wire::Size::MAX as usize * 2), + "we want to test sizes that are way larger than any gossip message" + ); + + let a_lot_of_data = vec![0u8; size]; + + let frame: Frame = Frame::git(StreamId(0u8.into()), a_lot_of_data); + + // In previous versions since 3c5668e this would panic. + let bytes = wire::serialize(&frame); + + assert!( + bytes.len() > wire::Size::MAX as usize * 2, + "just making sure that whatever was encoded is still quite large" + ); + } } diff --git a/crates/radicle-protocol/src/wire/message.rs b/crates/radicle-protocol/src/wire/message.rs index 864bb0f3..23244615 100644 --- a/crates/radicle-protocol/src/wire/message.rs +++ b/crates/radicle-protocol/src/wire/message.rs @@ -234,6 +234,8 @@ impl wire::Decode for Info { impl wire::Encode for Message { fn encode(&self, buf: &mut impl BufMut) { + let buf = &mut buf.limit(wire::Size::MAX as usize); + self.type_id().encode(buf); match self {