From 6804533e144a318557ec9d4cfbf94089a0d6f699 Mon Sep 17 00:00:00 2001 From: Mehrdad Afshari Date: Sun, 17 May 2026 05:02:17 +0000 Subject: [PATCH] fix: remove padded_len() u8 overflow in DATA frame padding release padded_len() returned Option and computed pad_len + 1 to account for the pad length field byte. When pad_len=255 (the maximum per RFC 7540 Section 6.1), 255u8 + 1 overflows to 0, causing the auto-release in recv_data() to release 0 bytes instead of 256. This leaks 256 bytes of flow control capacity per frame for both the stream and connection windows. Remove padded_len() entirely (it had a single call site) and compute padding overhead inline as flow_controlled_len() - payload().len(), which does not rely on details of the payload construction at all, making it more robust, and uses usize arithmetic and cannot overflow. --- src/frame/data.rs | 44 ++++++++++++++++++++++++++++++++++++--- src/proto/streams/recv.rs | 12 ++++++----- 2 files changed, 48 insertions(+), 8 deletions(-) diff --git a/src/frame/data.rs b/src/frame/data.rs index 3af0af11..918c55b7 100644 --- a/src/frame/data.rs +++ b/src/frame/data.rs @@ -149,9 +149,47 @@ impl Data { } } - /// If this frame is PADDED, it returns the pad len + 1 (length field). - pub(crate) fn padded_len(&self) -> Option { - self.pad_len.map(|n| n + 1) +} + +#[cfg(test)] +mod tests { + use super::*; + use bytes::Bytes; + + fn data_frame(data: &[u8], pad_len: Option) -> Data { + Data { + stream_id: StreamId::from(1), + data: Bytes::copy_from_slice(data), + flags: DataFlags::default(), + pad_len, + } + } + + #[test] + fn padding_overhead_no_padding() { + let frame = data_frame(b"hello", None); + assert_eq!(frame.flow_controlled_len() - frame.payload().len(), 0); + } + + #[test] + fn padding_overhead_small() { + let frame = data_frame(b"hello", Some(10)); + assert_eq!(frame.flow_controlled_len() - frame.payload().len(), 11); + } + + #[test] + fn padding_overhead_max_does_not_overflow() { + // Regression: the old padded_len() returned u8, so pad_len=255 + // caused 255u8 + 1 = 0. The correct overhead is 256. + let frame = data_frame(b"", Some(255)); + assert_eq!(frame.flow_controlled_len() - frame.payload().len(), 256); + } + + #[test] + fn padding_overhead_max_with_data() { + let frame = data_frame(b"hello", Some(255)); + assert_eq!(frame.flow_controlled_len() - frame.payload().len(), 256); + assert_eq!(frame.flow_controlled_len(), 261); } } diff --git a/src/proto/streams/recv.rs b/src/proto/streams/recv.rs index 7ab76887..a21b74cf 100644 --- a/src/proto/streams/recv.rs +++ b/src/proto/streams/recv.rs @@ -736,14 +736,16 @@ impl Recv { // Track the data as in-flight stream.in_flight_recv_data += sz; - // We auto-release the padded length, since the user cannot. - if let Some(padded_len) = frame.padded_len() { + // Auto-release padding overhead (pad_len field + padding bytes), + // since the user only sees the data payload via `payload()`. + let padding = (frame.flow_controlled_len() - frame.payload().len()) as WindowSize; + if padding > 0 { tracing::trace!( - "recv_data; auto-releasing padded length of {:?} for {:?}", - padded_len, + "recv_data; auto-releasing padding of {:?} for {:?}", + padding, stream.id, ); - let _res = self.release_capacity(padded_len.into(), stream, &mut None); + let _res = self.release_capacity(padding, stream, &mut None); // cannot fail, we JUST added more in_flight data above. debug_assert!(_res.is_ok()); }