From fe532cba40349ade2c6d0510c95a0c98e05ba900 Mon Sep 17 00:00:00 2001 From: Harshvardhan Shrivastava Date: Wed, 5 Aug 2026 14:00:59 +0530 Subject: [PATCH 1/6] fix(sources): cap compressed and decompressed payload size to prevent OOM --- Cargo.lock | 2 + Cargo.toml | 2 +- .../src/decoding/framing/chunked_gelf.rs | 108 +++- lib/vector-common/Cargo.toml | 2 + lib/vector-common/src/decompression.rs | 503 ++++++++++++++++++ lib/vector-common/src/lib.rs | 2 + src/cli.rs | 89 ++++ src/sources/datadog_agent/mod.rs | 40 +- src/sources/datadog_agent/tests.rs | 136 +++++ src/sources/fluent/mod.rs | 54 +- src/sources/splunk_hec/mod.rs | 163 +++++- src/sources/util/http/encoding.rs | 422 ++++++++++++++- src/sources/util/http/mod.rs | 2 + 13 files changed, 1436 insertions(+), 89 deletions(-) create mode 100644 lib/vector-common/src/decompression.rs diff --git a/Cargo.lock b/Cargo.lock index fb653b03b5..7764c8c232 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -12671,6 +12671,7 @@ dependencies = [ "crossbeam-utils", "derivative", "dyn-clone", + "flate2", "futures 0.3.31", "hostname 0.4.0", "indexmap 2.7.0", @@ -12694,6 +12695,7 @@ dependencies = [ "tracing-test", "vector-config", "vrl", + "zstd 0.13.2", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index aa9ab71c86..db82ff14ac 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -694,7 +694,7 @@ sources-prometheus-pushgateway = ["sinks-prometheus", "sources-utils-http", "vec sources-pulsar = ["dep:apache-avro", "dep:pulsar"] sources-redis = ["dep:redis"] sources-socket = ["sources-utils-net", "tokio-util/net"] -sources-splunk_hec = ["dep:roaring"] +sources-splunk_hec = ["dep:roaring", "sources-utils-http-encoding"] sources-statsd = ["sources-utils-net", "tokio-util/net"] sources-stdin = ["tokio-util/io"] sources-syslog = ["codecs-syslog", "sources-utils-net", "tokio-util/net"] diff --git a/lib/codecs/src/decoding/framing/chunked_gelf.rs b/lib/codecs/src/decoding/framing/chunked_gelf.rs index f8fcc8da44..d39afcb9aa 100644 --- a/lib/codecs/src/decoding/framing/chunked_gelf.rs +++ b/lib/codecs/src/decoding/framing/chunked_gelf.rs @@ -2,11 +2,9 @@ use super::{BoxedFramingError, FramingError}; use crate::{BytesDecoder, StreamDecodingError}; use bytes::{Buf, Bytes, BytesMut}; use derivative::Derivative; -use flate2::read::{MultiGzDecoder, ZlibDecoder}; use snafu::{ensure, ResultExt, Snafu}; use std::any::Any; use std::collections::HashMap; -use std::io::Read; use std::sync::{Arc, Mutex}; use std::time::Duration; use tokio; @@ -14,6 +12,7 @@ use tokio::task::JoinHandle; use tokio_util::codec::Decoder; use tracing::{debug, trace, warn}; use vector_common::constants::{GZIP_MAGIC, ZLIB_MAGIC}; +use vector_common::decompression::CappedDecoder; use vector_config::configurable_component; const GELF_MAGIC: &[u8] = &[0x1e, 0x0f]; @@ -210,22 +209,14 @@ impl ChunkedGelfDecompression { pub fn decompress(&self, data: Bytes) -> Result { let decompressed = match self { - Self::Gzip => { - let mut decoder = MultiGzDecoder::new(data.reader()); - let mut decompressed = Vec::new(); - decoder - .read_to_end(&mut decompressed) - .context(GzipDecompressionSnafu)?; - Bytes::from(decompressed) - } - Self::Zlib => { - let mut decoder = ZlibDecoder::new(data.reader()); - let mut decompressed = Vec::new(); - decoder - .read_to_end(&mut decompressed) - .context(ZlibDecompressionSnafu)?; - Bytes::from(decompressed) - } + Self::Gzip => CappedDecoder::gzip(data.reader()) + .decompress() + .map(Bytes::from) + .context(GzipDecompressionSnafu)?, + Self::Zlib => CappedDecoder::zlib(data.reader()) + .decompress() + .map(Bytes::from) + .context(ZlibDecompressionSnafu)?, Self::None => data, }; Ok(decompressed) @@ -1278,4 +1269,85 @@ mod tests { assert_eq!(detected_compression, ChunkedGelfDecompression::None); } + + /// OBE-10706: a GELF payload used to be inflated with an unbounded `read_to_end`, so a small + /// datagram could drive an arbitrarily large allocation. + /// + /// `MultiGzDecoder` walks every concatenated member, so one cheap member repeated past the cap + /// is enough to exceed it — no single oversized member required. + #[test] + fn gzip_decompression_is_capped() { + use vector_common::decompression::DEFAULT_MAX_DECOMPRESSED_SIZE_BYTES; + + let member = Compression::Gzip.compress(&vec![0u8; 1024 * 1024]); + let members = DEFAULT_MAX_DECOMPRESSED_SIZE_BYTES / (1024 * 1024) + 1; + let mut bomb = BytesMut::new(); + for _ in 0..members { + bomb.put_slice(&member); + } + let bomb = bomb.freeze(); + + assert!( + bomb.len() < 1024 * 1024, + "the bomb must stay small on the wire to be a meaningful test, got {} bytes", + bomb.len() + ); + + let error = ChunkedGelfDecompression::Gzip + .decompress(bomb) + .expect_err("a payload inflating past the cap must be rejected"); + + assert!(matches!( + error, + ChunkedGelfDecompressionError::GzipDecompression { .. } + )); + } + + /// The zlib arm needs its own bomb: unlike gzip, zlib has no concatenated-stream form, so the + /// payload must be a single oversized stream. Fed to the encoder in chunks to keep the test's + /// own memory bounded. + #[test] + fn zlib_decompression_is_capped() { + use std::io::Write as IoWrite; + + use vector_common::decompression::DEFAULT_MAX_DECOMPRESSED_SIZE_BYTES; + + let mut encoder = ZlibEncoder::new(Vec::new(), flate2::Compression::best()); + let chunk = vec![0u8; 1024 * 1024]; + for _ in 0..(DEFAULT_MAX_DECOMPRESSED_SIZE_BYTES / (1024 * 1024) + 1) { + encoder.write_all(&chunk).unwrap(); + } + let bomb = Bytes::from(encoder.finish().unwrap()); + + assert!( + bomb.len() < 1024 * 1024, + "the bomb must stay small on the wire to be a meaningful test, got {} bytes", + bomb.len() + ); + + let error = ChunkedGelfDecompression::Zlib + .decompress(bomb) + .expect_err("a payload inflating past the cap must be rejected"); + + assert!(matches!( + error, + ChunkedGelfDecompressionError::ZlibDecompression { .. } + )); + } + + /// The cap must not disturb ordinary traffic. Zlib shares the same `CappedDecoder` wrapper as + /// gzip, so exercising both here covers the wiring of each arm. + #[rstest] + #[case(Compression::Gzip)] + #[case(Compression::Zlib)] + fn decompression_under_the_cap_is_unaffected(#[case] compression: Compression) { + let payload = "the quick brown fox".repeat(1024); + let compressed = compression.compress(&payload); + + let decompressed = ChunkedGelfDecompression::from_magic(&compressed) + .decompress(compressed) + .expect("a payload within the cap must decompress"); + + assert_eq!(decompressed, Bytes::from(payload)); + } } diff --git a/lib/vector-common/Cargo.toml b/lib/vector-common/Cargo.toml index fb865ddc96..359dece03b 100644 --- a/lib/vector-common/Cargo.toml +++ b/lib/vector-common/Cargo.toml @@ -41,6 +41,7 @@ bytes = { version = "1.9.0", default-features = false } chrono.workspace = true crossbeam-utils = { version = "0.8.20", default-features = false } derivative = { version = "2.2.0", default-features = false } +flate2.workspace = true futures.workspace = true indexmap.workspace = true metrics.workspace = true @@ -60,6 +61,7 @@ snafu.workspace = true regex.workspace = true tokio-util.workspace = true serde_with.workspace = true +zstd.workspace = true [dev-dependencies] futures = { version = "0.3.31", default-features = false, features = ["async-await"] } diff --git a/lib/vector-common/src/decompression.rs b/lib/vector-common/src/decompression.rs new file mode 100644 index 0000000000..e0fe9ea452 --- /dev/null +++ b/lib/vector-common/src/decompression.rs @@ -0,0 +1,503 @@ +//! Shared decompression limits used to prevent decompression-bomb (`DoS`) attacks. +//! +//! A length or compressed payload read from an untrusted peer must never drive an unbounded +//! in-memory allocation. This module owns the global decompressed-size cap and the helpers that +//! enforce it, so every source and codec that decompresses untrusted input shares a single, +//! consistently-configured limit. +//! +//! # Usage +//! +//! Wrap any decompression at an untrusted boundary with the appropriate [`CappedDecoder`] +//! constructor and call [`CappedDecoder::decompress`]: +//! +//! ```rust,ignore +//! let data = CappedDecoder::gzip(reader).decompress()?; +//! let data = CappedDecoder::zlib(reader).decompress()?; +//! let data = CappedDecoder::zstd(reader)?.decompress()?; +//! ``` +//! +//! The constructors enforce the global decompressed-size cap so that a compression bomb cannot +//! drive unbounded allocation. + +// Raw decoder types (flate2 / zstd) should only be constructed here, in the module that wraps +// them safely. Once every source is migrated off the raw types this can be enforced with a +// `clippy.toml` `disallowed-types` entry. + +use std::{ + fmt, + io::{self, Read}, + sync::OnceLock, +}; + +use flate2::read::{MultiGzDecoder, ZlibDecoder}; + +/// Default cap on the size of any decompressed payload. +/// +/// Prevents a compressed "bomb" from causing unbounded memory growth. +pub const DEFAULT_MAX_DECOMPRESSED_SIZE_BYTES: usize = 100 * 1024 * 1024; + +static MAX_DECOMPRESSED_SIZE_BYTES: OnceLock = OnceLock::new(); +static MAX_ZLIB_COMPRESSED_FRAME_SIZE_BYTES: OnceLock = OnceLock::new(); +static MAX_ZSTD_WINDOW_LOG: OnceLock> = OnceLock::new(); + +/// Maps a decompressed cap to the largest compressed frame that can legitimately produce output +/// within it, using zlib's worst-case expansion of 13.5% + 11 bytes. This lets us reject an +/// oversized declared payload before buffering it, without rejecting a valid frame whose +/// decompressed content stays within the decompressed cap. +/// +/// See ("the worst case ... can result in an expansion of at +/// most 13.5%, plus eleven bytes"). +#[allow(clippy::cast_possible_truncation)] // limit derives from a usize; saturating math keeps it in range +const fn zlib_compressed_frame_limit(decompressed_limit: usize) -> usize { + (decompressed_limit as u64) + .saturating_mul(1135) + .saturating_div(1000) + .saturating_add(11) as usize +} + +const DEFAULT_MAX_ZLIB_COMPRESSED_FRAME_SIZE_BYTES: usize = + zlib_compressed_frame_limit(DEFAULT_MAX_DECOMPRESSED_SIZE_BYTES); + +const DEFAULT_MAX_ZSTD_WINDOW_LOG: Option = + zstd_window_log_max(DEFAULT_MAX_DECOMPRESSED_SIZE_BYTES); + +/// Override the global decompressed payload size cap. Must be called before any sources start. +/// +/// # Panics +/// +/// Panics if called more than once, as the global cap may only be initialized a single time. +pub fn set_max_decompressed_size_bytes(size: usize) { + MAX_DECOMPRESSED_SIZE_BYTES + .set(size) + .expect("max_decompressed_size_bytes already set"); + MAX_ZLIB_COMPRESSED_FRAME_SIZE_BYTES + .set(zlib_compressed_frame_limit(size)) + .expect("max_zlib_compressed_frame_size_bytes already set"); + MAX_ZSTD_WINDOW_LOG + .set(zstd_window_log_max(size)) + .expect("max_zstd_window_log already set"); +} + +/// Returns the currently configured decompressed payload size cap. +pub fn max_decompressed_size_bytes() -> usize { + *MAX_DECOMPRESSED_SIZE_BYTES + .get() + .unwrap_or(&DEFAULT_MAX_DECOMPRESSED_SIZE_BYTES) +} + +/// Returns the maximum compressed frame wire size we are willing to buffer, derived from the +/// decompressed cap plus zlib's worst-case expansion. See `zlib_compressed_frame_limit`. +pub fn max_zlib_compressed_frame_size_bytes() -> usize { + *MAX_ZLIB_COMPRESSED_FRAME_SIZE_BYTES + .get() + .unwrap_or(&DEFAULT_MAX_ZLIB_COMPRESSED_FRAME_SIZE_BYTES) +} + +/// Smallest zstd `window_log_max` capable of representing `max_decompressed_size` bytes. +/// +/// zstd frames declare a window size that the decoder must allocate up front; a crafted frame can +/// request a multi-gigabyte window even though its output would later trip the decompressed-size +/// cap. Clamping the decoder's `window_log_max` to the smallest power-of-two window that can still +/// hold a legitimate payload bounds that allocation. A zero cap maps to the minimum window log +/// (not `None`) so the guard stays at its strictest rather than being disabled. +/// +/// This is protocol-neutral: the ceiling is derived from the decompressed cap so any transport's +/// frames decode as long as their window fits the cap. Transports that impose a tighter, +/// spec-mandated window (HTTP `Content-Encoding: zstd`, see [`http_zstd_window_log_max`]) apply +/// that on top. +#[must_use] +#[allow(clippy::manual_clamp)] // `usize::clamp` is not a const fn; the manual form keeps this const +pub const fn zstd_window_log_max(max_decompressed_size: usize) -> Option { + const MIN_ZSTD_WINDOW_LOG: u32 = 10; + const MAX_ZSTD_WINDOW_LOG: u32 = 31; + + // `window_log_max` is expressed as a power-of-two log. Use the smallest zstd window capable of + // representing the configured byte budget. + match max_decompressed_size.checked_sub(1) { + // A zero cap has no representable window; fall back to the smallest window rather than + // leaving the allocation guard unset. + None => Some(MIN_ZSTD_WINDOW_LOG), + Some(max_index) => { + let window_log = usize::BITS - max_index.leading_zeros(); + let clamped = if window_log < MIN_ZSTD_WINDOW_LOG { + MIN_ZSTD_WINDOW_LOG + } else if window_log > MAX_ZSTD_WINDOW_LOG { + MAX_ZSTD_WINDOW_LOG + } else { + window_log + }; + Some(clamped) + } + } +} + +/// RFC 9659 window ceiling for zstd under HTTP `Content-Encoding: zstd`: conformant senders require +/// a `Window_Size` of at most 8 MB (2^23) and decoders need only support up to that. This bounds +/// the decoder's window allocation to 8 MB regardless of the (much larger) decompressed cap. It +/// governs HTTP content coding only; other transports (e.g. gRPC/OTLP, whose clients are not bound +/// by RFC 9659 and may legitimately use larger windows) are not clamped to it. +/// See . +pub const HTTP_ZSTD_WINDOW_LOG_MAX: u32 = 23; + +/// Like [`zstd_window_log_max`] but additionally clamped to the RFC 9659 HTTP window ceiling +/// ([`HTTP_ZSTD_WINDOW_LOG_MAX`]). Use for HTTP `Content-Encoding: zstd`; use the protocol-neutral +/// [`zstd_window_log_max`] for transports RFC 9659 does not govern. +#[must_use] +pub fn http_zstd_window_log_max(max_decompressed_size: usize) -> Option { + zstd_window_log_max(max_decompressed_size).map(|w| w.min(HTTP_ZSTD_WINDOW_LOG_MAX)) +} + +/// Returns the zstd `window_log_max` derived from the global decompressed cap +/// ([`max_decompressed_size_bytes`]). +/// +/// Convenience getter for the common case where the decoder window should track the global cap; +/// use [`zstd_window_log_max`] directly when enforcing an explicit, non-global limit (e.g. the +/// HTTP body decompressor's per-call limit). +#[must_use] +pub fn max_zstd_window_log() -> Option { + MAX_ZSTD_WINDOW_LOG + .get() + .copied() + .unwrap_or(DEFAULT_MAX_ZSTD_WINDOW_LOG) +} + +/// Error raised when a decompressed payload would exceed the configured size cap. +/// +/// Surfaced (wrapped in [`io::Error`]) by [`CappedDecoder::decompress`] and the [`CappedReader`] +/// returned by [`CappedDecoder::into_reader`]. Use [`is_decompressed_size_limit_error`] to detect +/// it and distinguish an oversized-input fault from an unrelated I/O error. +#[derive(Debug)] +pub struct DecompressedSizeLimitExceeded; + +impl fmt::Display for DecompressedSizeLimitExceeded { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str("decompressed size exceeds the configured limit") + } +} + +impl std::error::Error for DecompressedSizeLimitExceeded {} + +/// Returns whether `error` was raised because decompression hit the size cap (see +/// [`DecompressedSizeLimitExceeded`]). +#[must_use] +pub fn is_decompressed_size_limit_error(error: &io::Error) -> bool { + fn is_marker(source: &(dyn std::error::Error + Send + Sync + 'static)) -> bool { + source.is::() + } + + error.get_ref().is_some_and(is_marker) +} + +/// A size-capped decompression reader. +/// +/// Wraps any `R: Read` (typically a raw decoder like `MultiGzDecoder` or `ZlibDecoder`) and +/// enforces the configured decompressed-size cap so that a compression bomb cannot drive +/// unbounded memory allocation. +/// +/// Construct via the typed class methods ([`CappedDecoder::gzip`], [`CappedDecoder::zlib`], +/// [`CappedDecoder::zstd`]) rather than by wrapping a raw decoder directly. Read the whole payload +/// into memory with [`CappedDecoder::decompress`], or stream it through [`CappedDecoder::into_reader`]. +pub struct CappedDecoder { + inner: io::Take, + limit: usize, +} + +impl CappedDecoder { + fn with_limit(reader: R, limit: usize) -> Self { + Self { + inner: reader.take((limit as u64).saturating_add(1)), + limit, + } + } + + /// Reads all decompressed bytes into a `Vec`, returning an error if the output exceeds the + /// configured cap. + /// + /// # Errors + /// + /// Returns an error if reading from the underlying decoder fails, or + /// [`DecompressedSizeLimitExceeded`] if the decompressed output exceeds the cap. + pub fn decompress(self) -> io::Result> { + let mut buf = Vec::new(); + self.into_reader().read_to_end(&mut buf)?; + Ok(buf) + } + + /// Converts the decoder into a streaming [`CappedReader`] that enforces the cap as bytes are + /// read, rather than buffering the whole payload up front. + /// + /// Prefer this over consuming a raw decoder directly: the returned reader errors out (instead + /// of silently truncating) the moment the decompressed output would exceed the cap, so a + /// streaming consumer such as [`io::copy`], `serde_json::from_reader`, or `BufReader` cannot + /// process a truncated-but-valid-looking payload. + pub fn into_reader(self) -> CappedReader { + CappedReader { + inner: self.inner, + limit: self.limit, + consumed: 0, + } + } +} + +/// A streaming, size-capped decompression reader returned by [`CappedDecoder::into_reader`]. +/// +/// Yields decompressed bytes incrementally and returns a [`DecompressedSizeLimitExceeded`] error +/// (wrapped in [`io::Error`]) as soon as the cumulative output would exceed the cap. +pub struct CappedReader { + inner: io::Take, + limit: usize, + consumed: usize, +} + +impl Read for CappedReader { + fn read(&mut self, buf: &mut [u8]) -> io::Result { + // The underlying reader is bounded one byte past the cap, so reading beyond `limit` is the + // unambiguous signal that the payload is oversized. + let n = self.inner.read(buf)?; + self.consumed = self.consumed.saturating_add(n); + if self.consumed > self.limit { + return Err(io::Error::other(DecompressedSizeLimitExceeded)); + } + Ok(n) + } +} + +impl CappedDecoder> { + /// Creates a capped gzip decoder using the global decompressed-size cap. + pub fn gzip(reader: S) -> Self { + Self::gzip_with_limit(reader, max_decompressed_size_bytes()) + } + + /// Creates a capped gzip decoder using an explicit decompressed-size cap. + pub fn gzip_with_limit(reader: S, limit: usize) -> Self { + Self::with_limit(MultiGzDecoder::new(reader), limit) + } +} + +impl CappedDecoder> { + /// Creates a capped zlib/deflate decoder using the global decompressed-size cap. + pub fn zlib(reader: S) -> Self { + Self::zlib_with_limit(reader, max_decompressed_size_bytes()) + } + + /// Creates a capped zlib/deflate decoder using an explicit decompressed-size cap. + pub fn zlib_with_limit(reader: S, limit: usize) -> Self { + Self::with_limit(ZlibDecoder::new(reader), limit) + } +} + +impl CappedDecoder>> { + /// Creates a capped zstd decoder using the global decompressed-size cap. + /// + /// Also constrains the decoder's internal window allocation via `window_log_max` so a crafted + /// frame cannot request a large window before the decompressed-size cap trips. The window is + /// derived from the cap only ([`zstd_window_log_max`]); for HTTP `Content-Encoding: zstd` use + /// [`zstd_http`](Self::zstd_http), which applies the tighter RFC 9659 ceiling. + /// + /// # Errors + /// + /// Returns an error if the zstd decoder cannot be initialized (e.g. invalid header). + pub fn zstd(reader: S) -> io::Result { + Self::zstd_with_limit(reader, max_decompressed_size_bytes()) + } + + /// Creates a capped zstd decoder using an explicit decompressed-size cap, with the window + /// derived from that cap only ([`zstd_window_log_max`]). + /// + /// # Errors + /// + /// Returns an error if the zstd decoder cannot be initialized (e.g. invalid header). + pub fn zstd_with_limit(reader: S, limit: usize) -> io::Result { + Self::zstd_with_window_log(reader, limit, zstd_window_log_max(limit)) + } + + /// Creates a capped zstd decoder for HTTP `Content-Encoding: zstd` using the global + /// decompressed-size cap, clamping the decoder window to the RFC 9659 8 MB ceiling + /// ([`http_zstd_window_log_max`]). + /// + /// # Errors + /// + /// Returns an error if the zstd decoder cannot be initialized (e.g. invalid header). + pub fn zstd_http(reader: S) -> io::Result { + Self::zstd_http_with_limit(reader, max_decompressed_size_bytes()) + } + + /// Creates a capped zstd decoder for HTTP `Content-Encoding: zstd` using an explicit + /// decompressed-size cap, clamping the decoder window to the RFC 9659 8 MB ceiling + /// ([`http_zstd_window_log_max`]). + /// + /// # Errors + /// + /// Returns an error if the zstd decoder cannot be initialized (e.g. invalid header). + pub fn zstd_http_with_limit(reader: S, limit: usize) -> io::Result { + Self::zstd_with_window_log(reader, limit, http_zstd_window_log_max(limit)) + } + + fn zstd_with_window_log( + reader: S, + limit: usize, + window_log_max: Option, + ) -> io::Result { + let mut decoder = zstd::stream::read::Decoder::new(reader)?; + if let Some(window_log_max) = window_log_max { + decoder.window_log_max(window_log_max)?; + } + Ok(Self::with_limit(decoder, limit)) + } +} + +#[cfg(test)] +mod tests { + use std::io::Write; + + use flate2::{write::GzEncoder, write::ZlibEncoder, Compression}; + + use super::*; + + /// Compresses `len` zero bytes with gzip. Highly compressible, so the wire form is tiny + /// relative to the output — the shape of a decompression bomb. + fn gzip_bomb(len: usize) -> Vec { + let mut encoder = GzEncoder::new(Vec::new(), Compression::best()); + encoder.write_all(&vec![0u8; len]).unwrap(); + encoder.finish().unwrap() + } + + fn zlib_bomb(len: usize) -> Vec { + let mut encoder = ZlibEncoder::new(Vec::new(), Compression::best()); + encoder.write_all(&vec![0u8; len]).unwrap(); + encoder.finish().unwrap() + } + + #[test] + fn gzip_within_limit_decompresses() { + let payload = gzip_bomb(1024); + let out = CappedDecoder::gzip_with_limit(payload.as_slice(), 1024) + .decompress() + .expect("payload exactly at the limit must decompress"); + assert_eq!(out.len(), 1024); + } + + #[test] + fn gzip_over_limit_is_rejected() { + let payload = gzip_bomb(64 * 1024); + let error = CappedDecoder::gzip_with_limit(payload.as_slice(), 1024) + .decompress() + .expect_err("payload over the limit must be rejected"); + assert!( + is_decompressed_size_limit_error(&error), + "expected the size-limit marker, got {error}" + ); + } + + /// The load-bearing case for `MultiGzDecoder`: a single member's size does not bound the + /// attack, because the decoder walks every concatenated member. The cap must apply to the + /// summed output, not per member. + #[test] + fn gzip_concatenated_members_are_capped_in_aggregate() { + let member = gzip_bomb(1024); + let mut payload = Vec::new(); + for _ in 0..8 { + payload.extend_from_slice(&member); + } + + // Each member on its own is within the limit; together they are not. + let error = CappedDecoder::gzip_with_limit(payload.as_slice(), 4096) + .decompress() + .expect_err("concatenated members must be capped in aggregate"); + assert!( + is_decompressed_size_limit_error(&error), + "expected the size-limit marker, got {error}" + ); + } + + #[test] + fn zlib_over_limit_is_rejected() { + let payload = zlib_bomb(64 * 1024); + let error = CappedDecoder::zlib_with_limit(payload.as_slice(), 1024) + .decompress() + .expect_err("payload over the limit must be rejected"); + assert!(is_decompressed_size_limit_error(&error)); + } + + /// A single frame declaring a window larger than the cap is refused by the `window_log_max` + /// clamp, before any output buffer is allocated. + #[test] + fn zstd_oversized_window_is_rejected() { + let payload = zstd::encode_all(vec![0u8; 64 * 1024].as_slice(), 19).unwrap(); + let result = CappedDecoder::zstd_with_limit(payload.as_slice(), 1024) + .expect("decoder init") + .decompress(); + assert!( + result.is_err(), + "a frame whose window exceeds the cap must not decode" + ); + } + + /// Concatenated frames each fit the window clamp, so the aggregate output is what the size cap + /// has to catch. + #[test] + fn zstd_over_limit_is_rejected() { + // Level 1 keeps each frame's declared window under the 1 MiB cap's 2^20 ceiling, so the + // window clamp stays out of the way and the size cap is what rejects the payload. + let frame = zstd::encode_all(vec![0u8; 256 * 1024].as_slice(), 1).unwrap(); + let mut payload = Vec::new(); + for _ in 0..8 { + payload.extend_from_slice(&frame); + } + + let error = CappedDecoder::zstd_with_limit(payload.as_slice(), 1024 * 1024) + .expect("decoder init") + .decompress() + .expect_err("payload over the limit must be rejected"); + assert!( + is_decompressed_size_limit_error(&error), + "expected the size-limit marker, got {error}" + ); + } + + /// A streaming consumer must see an error rather than a truncated-but-plausible payload. + #[test] + fn streaming_reader_errors_instead_of_truncating() { + let payload = gzip_bomb(64 * 1024); + let mut reader = CappedDecoder::gzip_with_limit(payload.as_slice(), 1024).into_reader(); + + let mut sink = Vec::new(); + let error = std::io::copy(&mut reader, &mut sink) + .expect_err("streaming past the limit must error, not silently truncate"); + assert!(is_decompressed_size_limit_error(&error)); + assert!( + sink.len() <= 1024, + "must not hand more than the limit to the consumer, got {}", + sink.len() + ); + } + + /// An unrelated I/O failure must not be mistaken for the size cap. + #[test] + fn unrelated_io_error_is_not_a_limit_error() { + let error = CappedDecoder::gzip_with_limit(b"not gzip at all".as_slice(), 1024) + .decompress() + .expect_err("invalid gzip must fail"); + assert!(!is_decompressed_size_limit_error(&error)); + } + + #[test] + fn zstd_window_log_tracks_the_cap() { + // 100 MiB needs a 2^27 window; the HTTP variant is clamped to RFC 9659's 2^23. + assert_eq!(zstd_window_log_max(100 * 1024 * 1024), Some(27)); + assert_eq!( + http_zstd_window_log_max(100 * 1024 * 1024), + Some(HTTP_ZSTD_WINDOW_LOG_MAX) + ); + // A zero cap clamps to the tightest window rather than disabling the guard. + assert_eq!(zstd_window_log_max(0), Some(10)); + } + + #[test] + fn default_cap_is_used_when_unset() { + assert_eq!( + max_decompressed_size_bytes(), + DEFAULT_MAX_DECOMPRESSED_SIZE_BYTES + ); + } +} diff --git a/lib/vector-common/src/lib.rs b/lib/vector-common/src/lib.rs index 9264895f84..756b55ae4b 100644 --- a/lib/vector-common/src/lib.rs +++ b/lib/vector-common/src/lib.rs @@ -24,6 +24,8 @@ pub mod config; pub mod constants; +pub mod decompression; + #[cfg(feature = "conversion")] pub use vrl::compiler::TimeZone; diff --git a/src/cli.rs b/src/cli.rs index 6ab179ddfa..ca54f3b4ea 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -236,6 +236,45 @@ pub struct RootOpts { /// `--watch-config`. #[arg(long, env = "VECTOR_ALLOW_EMPTY_CONFIG", default_value = "false")] pub allow_empty_config: bool, + + /// Maximum number of bytes allowed after decompressing a payload. + /// + /// Sources that decompress incoming payloads (gzip, deflate, zstd) enforce this cap to + /// prevent a compressed "bomb" from exhausting memory. Payloads whose decompressed size + /// exceeds the limit are rejected. + /// + /// Defaults to 104857600 (100 MiB). Raise this only when sources routinely receive + /// legitimately large compressed payloads. + /// + /// Must be at least 1024; a cap below that would reject essentially all traffic rather than + /// just bombs. + #[arg( + long, + env = "VECTOR_MAX_DECOMPRESSED_SIZE_BYTES", + default_value_t = vector_common::decompression::DEFAULT_MAX_DECOMPRESSED_SIZE_BYTES, + value_parser = parse_max_decompressed_size_bytes, + )] + pub max_decompressed_size_bytes: usize, +} + +/// Lower bound for `--max-decompressed-size-bytes`. +/// +/// Guards against a value (notably `0`) that would silently reject all compressed ingestion, which +/// looks identical to a broken pipeline from the outside. +const MIN_MAX_DECOMPRESSED_SIZE_BYTES: usize = 1024; + +fn parse_max_decompressed_size_bytes(raw: &str) -> Result { + let value: usize = raw + .parse() + .map_err(|_| format!("`{raw}` is not a valid number of bytes"))?; + + if value < MIN_MAX_DECOMPRESSED_SIZE_BYTES { + return Err(format!( + "must be at least {MIN_MAX_DECOMPRESSED_SIZE_BYTES} bytes, got {value}" + )); + } + + Ok(value) } impl RootOpts { @@ -262,6 +301,10 @@ impl RootOpts { } crate::metrics::init_global().expect("metrics initialization failed"); + + vector_common::decompression::set_max_decompressed_size_bytes( + self.max_decompressed_size_bytes, + ); } } @@ -395,3 +438,49 @@ pub fn handle_config_errors(errors: Vec) -> exitcode::ExitCode { exitcode::CONFIG } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn max_decompressed_size_accepts_reasonable_values() { + assert_eq!( + parse_max_decompressed_size_bytes("104857600"), + Ok(104_857_600) + ); + assert_eq!( + parse_max_decompressed_size_bytes("1024"), + Ok(MIN_MAX_DECOMPRESSED_SIZE_BYTES) + ); + } + + /// A zero (or near-zero) cap would reject essentially all compressed ingestion while looking + /// like a silently broken pipeline, so it must fail loudly at startup instead. + #[test] + fn max_decompressed_size_rejects_values_below_the_floor() { + for raw in ["0", "1", "512", "1023"] { + assert!( + parse_max_decompressed_size_bytes(raw).is_err(), + "{raw} should have been rejected" + ); + } + } + + #[test] + fn max_decompressed_size_rejects_non_numeric() { + assert!(parse_max_decompressed_size_bytes("100MiB").is_err()); + assert!(parse_max_decompressed_size_bytes("-1").is_err()); + assert!(parse_max_decompressed_size_bytes("").is_err()); + } + + /// The clap default must track the constant the decompression module actually enforces. + #[test] + fn cli_default_matches_the_enforced_default() { + let opts = RootOpts::parse_from(["vector"]); + assert_eq!( + opts.max_decompressed_size_bytes, + vector_common::decompression::DEFAULT_MAX_DECOMPRESSED_SIZE_BYTES + ); + } +} diff --git a/src/sources/datadog_agent/mod.rs b/src/sources/datadog_agent/mod.rs index 7b3b3f85c7..f49b3c6ae1 100644 --- a/src/sources/datadog_agent/mod.rs +++ b/src/sources/datadog_agent/mod.rs @@ -19,11 +19,10 @@ pub(crate) mod ddtrace_proto { use std::convert::Infallible; use std::time::Duration; -use std::{fmt::Debug, io::Read, net::SocketAddr, sync::Arc}; +use std::{fmt::Debug, net::SocketAddr, sync::Arc}; use bytes::{Buf, Bytes}; use chrono::{serde::ts_milliseconds, DateTime, Utc}; -use flate2::read::{MultiGzDecoder, ZlibDecoder}; use futures::{FutureExt, StreamExt}; use http::StatusCode; use hyper::service::make_service_fn; @@ -34,6 +33,7 @@ use snafu::Snafu; use tokio::net::TcpStream; use tower::ServiceBuilder; use tracing::Span; +use vector_common::decompression::CappedDecoder; use vector_lib::codecs::decoding::{DeserializerConfig, FramingConfig}; use vector_lib::config::{LegacyKey, LogNamespace}; use vector_lib::configurable::configurable_component; @@ -467,26 +467,22 @@ impl DatadogAgentSource { for encoding in encodings.rsplit(',').map(str::trim) { body = match encoding { "identity" => body, - "gzip" | "x-gzip" => { - let mut decoded = Vec::new(); - MultiGzDecoder::new(body.reader()) - .read_to_end(&mut decoded) - .map_err(|error| handle_decode_error(encoding, error))?; - decoded.into() - } - "zstd" => { - let mut decoded = Vec::new(); - zstd::stream::copy_decode(body.reader(), &mut decoded) - .map_err(|error| handle_decode_error(encoding, error))?; - decoded.into() - } - "deflate" | "x-deflate" => { - let mut decoded = Vec::new(); - ZlibDecoder::new(body.reader()) - .read_to_end(&mut decoded) - .map_err(|error| handle_decode_error(encoding, error))?; - decoded.into() - } + // Cap each decompressed payload so a compression bomb cannot drive unbounded + // allocation on this unauthenticated HTTP listener. Capping every round also + // bounds a stacked `Content-Encoding: gzip,gzip,...` chain, since each round's + // output is the next round's input. + "gzip" | "x-gzip" => CappedDecoder::gzip(body.reader()) + .decompress() + .map_err(|error| handle_decode_error(encoding, error))? + .into(), + "zstd" => CappedDecoder::zstd_http(body.reader()) + .and_then(CappedDecoder::decompress) + .map_err(|error| handle_decode_error(encoding, error))? + .into(), + "deflate" | "x-deflate" => CappedDecoder::zlib(body.reader()) + .decompress() + .map_err(|error| handle_decode_error(encoding, error))? + .into(), encoding => { return Err(ErrorMessage::new( StatusCode::UNSUPPORTED_MEDIA_TYPE, diff --git a/src/sources/datadog_agent/tests.rs b/src/sources/datadog_agent/tests.rs index c8c0803f27..84fb90c144 100644 --- a/src/sources/datadog_agent/tests.rs +++ b/src/sources/datadog_agent/tests.rs @@ -2635,3 +2635,139 @@ async fn permit_origin_blocks_non_fatal_emits_bad_peer_metric() { } register_validatable_component!(DatadogAgentConfig); + +/// OBE-11237: every `Content-Encoding` branch inflated the request body with an unbounded +/// `read_to_end`, so a small body on this unauthenticated listener could exhaust memory. +mod decompression_caps { + use std::io::Write as _; + + use similar_asserts::assert_eq; + use vector_common::decompression::DEFAULT_MAX_DECOMPRESSED_SIZE_BYTES; + + use super::*; + use crate::sources::datadog_agent::DatadogAgentSource; + + fn test_source() -> DatadogAgentSource { + let decoder = crate::codecs::Decoder::new( + Framer::Bytes(BytesDecoder::new()), + Deserializer::Bytes(BytesDeserializer), + ); + DatadogAgentSource::new(true, decoder, "http", None, LogNamespace::Legacy, false) + } + + /// One cheap gzip member repeated past the cap. `MultiGzDecoder` walks every concatenated + /// member, so no single oversized member is required. + fn gzip_bomb() -> Bytes { + let mut encoder = flate2::write::GzEncoder::new(Vec::new(), flate2::Compression::best()); + encoder.write_all(&vec![0u8; 1024 * 1024]).unwrap(); + let member = encoder.finish().unwrap(); + + let mut bomb = Vec::new(); + for _ in 0..(DEFAULT_MAX_DECOMPRESSED_SIZE_BYTES / (1024 * 1024) + 1) { + bomb.extend_from_slice(&member); + } + assert!( + bomb.len() < 1024 * 1024, + "the bomb must stay small on the wire to be a meaningful test, got {} bytes", + bomb.len() + ); + Bytes::from(bomb) + } + + fn zlib_bomb() -> Bytes { + let mut encoder = flate2::write::ZlibEncoder::new(Vec::new(), flate2::Compression::best()); + encoder + .write_all(&vec![0u8; DEFAULT_MAX_DECOMPRESSED_SIZE_BYTES + 1]) + .unwrap(); + Bytes::from(encoder.finish().unwrap()) + } + + #[test] + fn gzip_body_over_the_cap_is_rejected() { + let error = test_source() + .decode(&Some("gzip".to_owned()), gzip_bomb(), "/api/v2/logs") + .expect_err("a body inflating past the cap must be rejected"); + + assert_eq!(error.status_code(), http::StatusCode::UNPROCESSABLE_ENTITY); + } + + #[test] + fn deflate_body_over_the_cap_is_rejected() { + let error = test_source() + .decode(&Some("deflate".to_owned()), zlib_bomb(), "/api/v2/logs") + .expect_err("a body inflating past the cap must be rejected"); + + assert_eq!(error.status_code(), http::StatusCode::UNPROCESSABLE_ENTITY); + } + + /// Stacking encodings used to multiply the amplification. Capping each round bounds the chain, + /// because every round's output is the next round's input. + /// + /// The payload must be genuinely double-gzipped: handing a singly-gzipped body a `gzip,gzip` + /// header would fail the second round as malformed regardless of the cap, and pass for the + /// wrong reason. Here the outer round yields the (small) bomb and the inner round is what + /// exceeds the cap. + #[test] + fn stacked_encodings_are_capped_at_every_round() { + let mut encoder = flate2::write::GzEncoder::new(Vec::new(), flate2::Compression::best()); + encoder.write_all(&gzip_bomb()).unwrap(); + let doubly_compressed = Bytes::from(encoder.finish().unwrap()); + + let error = test_source() + .decode( + &Some("gzip,gzip".to_owned()), + doubly_compressed, + "/api/v2/logs", + ) + .expect_err("a stacked chain inflating past the cap must be rejected"); + + assert_eq!(error.status_code(), http::StatusCode::UNPROCESSABLE_ENTITY); + } + + /// Level 1 keeps each frame's declared window well under the RFC 9659 8 MiB ceiling that + /// `zstd_http` applies, so the window clamp stays out of the way and the size cap is what + /// rejects the payload. Concatenated frames are what push the aggregate past the cap. + #[test] + fn zstd_body_over_the_cap_is_rejected() { + let frame = zstd::encode_all(vec![0u8; 1024 * 1024].as_slice(), 1).unwrap(); + let mut bomb = Vec::new(); + for _ in 0..(DEFAULT_MAX_DECOMPRESSED_SIZE_BYTES / (1024 * 1024) + 1) { + bomb.extend_from_slice(&frame); + } + + let error = test_source() + .decode(&Some("zstd".to_owned()), Bytes::from(bomb), "/api/v2/logs") + .expect_err("a body inflating past the cap must be rejected"); + + assert_eq!(error.status_code(), http::StatusCode::UNPROCESSABLE_ENTITY); + } + + /// A realistic agent frame must still decode: the 8 MiB window clamp must not reject ordinary + /// zstd traffic. + #[test] + fn zstd_body_under_the_cap_is_unaffected() { + let payload = b"{\"message\":\"hello\"}"; + let compressed = Bytes::from(zstd::encode_all(&payload[..], 3).unwrap()); + + let decoded = test_source() + .decode(&Some("zstd".to_owned()), compressed, "/api/v2/logs") + .expect("a body within the cap must decode"); + + assert_eq!(decoded, Bytes::from_static(payload)); + } + + /// The cap must not disturb ordinary traffic. + #[test] + fn body_under_the_cap_is_unaffected() { + let payload = b"{\"message\":\"hello\"}"; + let mut encoder = flate2::write::GzEncoder::new(Vec::new(), flate2::Compression::default()); + encoder.write_all(payload).unwrap(); + let compressed = Bytes::from(encoder.finish().unwrap()); + + let decoded = test_source() + .decode(&Some("gzip".to_owned()), compressed, "/api/v2/logs") + .expect("a body within the cap must decode"); + + assert_eq!(decoded, Bytes::from_static(payload)); + } +} diff --git a/src/sources/fluent/mod.rs b/src/sources/fluent/mod.rs index 47afb99ded..d5495405e9 100644 --- a/src/sources/fluent/mod.rs +++ b/src/sources/fluent/mod.rs @@ -1,16 +1,16 @@ use std::collections::HashMap; -use std::io::{self, Read}; +use std::io; use std::net::SocketAddr; use std::time::Duration; use base64::prelude::{Engine as _, BASE64_STANDARD}; use bytes::{Buf, Bytes, BytesMut}; use chrono::Utc; -use flate2::read::MultiGzDecoder; use rmp_serde::{decode, Deserializer, Serializer}; use serde::{Deserialize, Serialize}; use smallvec::{smallvec, SmallVec}; use tokio_util::codec::Decoder; +use vector_common::decompression::CappedDecoder; use vector_lib::codecs::{BytesDeserializerConfig, StreamDecodingError}; use vector_lib::config::{LegacyKey, LogNamespace}; use vector_lib::configurable::configurable_component; @@ -406,13 +406,9 @@ impl FluentDecoder { } FluentMessage::PackedForwardWithOptions(tag, bin, options) => { let buf = match options.compressed.as_deref() { - Some("gzip") => { - let mut buf = Vec::new(); - MultiGzDecoder::new(io::Cursor::new(bin.into_vec())) - .read_to_end(&mut buf) - .map(|_| buf) - .map_err(Into::into) - } + Some("gzip") => CappedDecoder::gzip(io::Cursor::new(bin.into_vec())) + .decompress() + .map_err(Into::into), Some("text") | None => Ok(bin.into_vec()), Some(s) => Err(DecodeError::UnknownCompression(s.to_owned())), }?; @@ -851,6 +847,46 @@ mod tests { assert_event_data_eq!(got.0[2], expected[2]); } + /// OBE-11233 / OBE-10708: `CompressedPackedForward` inflated the client's gzip payload with an + /// unbounded `read_to_end`, so a small frame could drive an arbitrarily large allocation. + /// + /// `MultiGzDecoder` walks every concatenated member, so repeating one cheap member past the cap + /// is enough to exceed it — no single oversized member required. + #[test] + fn compressed_packed_forward_decompression_is_capped() { + use std::collections::BTreeMap; + use std::io::Write as _; + + use vector_common::decompression::DEFAULT_MAX_DECOMPRESSED_SIZE_BYTES; + + let mut encoder = flate2::write::GzEncoder::new(Vec::new(), flate2::Compression::best()); + encoder.write_all(&vec![0u8; 1024 * 1024]).unwrap(); + let member = encoder.finish().unwrap(); + + let mut bomb = Vec::new(); + for _ in 0..(DEFAULT_MAX_DECOMPRESSED_SIZE_BYTES / (1024 * 1024) + 1) { + bomb.extend_from_slice(&member); + } + assert!( + bomb.len() < 1024 * 1024, + "the bomb must stay small on the wire to be a meaningful test, got {} bytes", + bomb.len() + ); + + let options = BTreeMap::from([("compressed", "gzip")]); + let message = rmp_serde::to_vec(&("tag.name", serde_bytes::ByteBuf::from(bomb), options)) + .expect("failed to build the fluent frame"); + + let error = + decode_all(message).expect_err("a payload inflating past the cap must be rejected"); + + assert!(matches!(error, DecodeError::IO(_)), "got {error:?}"); + assert!( + !error.can_continue(), + "an oversized frame must drop the connection rather than be retried" + ); + } + fn decode_all(message: Vec) -> Result<(SmallVec<[Event; 1]>, usize), DecodeError> { let mut buf = BytesMut::from(&message[..]); diff --git a/src/sources/splunk_hec/mod.rs b/src/sources/splunk_hec/mod.rs index 1d6d2b6dff..75f5d894e3 100644 --- a/src/sources/splunk_hec/mod.rs +++ b/src/sources/splunk_hec/mod.rs @@ -1,7 +1,6 @@ use std::{ collections::{BTreeSet, HashMap}, convert::Infallible, - io::Read, net::{Ipv4Addr, SocketAddr}, sync::Arc, time::Duration, @@ -9,7 +8,6 @@ use std::{ use bytes::{Buf, Bytes}; use chrono::{DateTime, TimeZone, Utc}; -use flate2::read::MultiGzDecoder; use futures::{FutureExt, StreamExt}; use http::StatusCode; use hyper::{service::make_service_fn, Server}; @@ -22,6 +20,7 @@ use snafu::Snafu; use tokio::net::TcpStream; use tower::ServiceBuilder; use tracing::Span; +use vector_common::decompression::CappedDecoder; use vector_lib::internal_event::{CountByteSize, InternalEventHandle as _, Registered}; use vector_lib::lookup::lookup_v2::OptionalValuePath; use vector_lib::lookup::{self, event_path, owned_value_path}; @@ -54,7 +53,7 @@ use crate::{ EventsReceived, HttpBytesReceived, SplunkHecRequestBodyInvalidError, SplunkHecRequestError, }, serde::bool_or_struct, - sources::util::handle_accept_error, + sources::util::{handle_accept_error, http::capped_body, http::ErrorMessage}, source_sender::ClosedError, tls::{MaybeTlsSettings, TlsEnableableConfig}, SourceSender, @@ -355,7 +354,7 @@ impl SplunkSource { .and(warp::addr::remote()) .and(warp::header::optional::("X-Forwarded-For")) .and(self.gzip()) - .and(warp::body::bytes()) + .and(capped_body()) .and(warp::path::full()) .and_then( move |_, @@ -375,10 +374,10 @@ impl SplunkSource { return Err(Rejection::from(ApiError::MissingChannel)); } - let mut data = Vec::new(); + let data; let (byte_size, body) = if gzip { - MultiGzDecoder::new(body.reader()) - .read_to_end(&mut data) + data = CappedDecoder::gzip(body.reader()) + .decompress() .map_err(|_| Rejection::from(ApiError::BadRequest))?; (data.len(), String::from_utf8_lossy(data.as_slice())) } else { @@ -459,7 +458,7 @@ impl SplunkSource { .and(warp::addr::remote()) .and(warp::header::optional::("X-Forwarded-For")) .and(self.gzip()) - .and(warp::body::bytes()) + .and(capped_body()) .and(warp::path::full()) .and_then( move |_, @@ -1045,10 +1044,9 @@ fn raw_event( ) -> Result { // Process gzip let message: Value = if gzip { - let mut data = Vec::new(); - match MultiGzDecoder::new(bytes.reader()).read_to_end(&mut data) { - Ok(0) => return Err(ApiError::NoData.into()), - Ok(_) => Value::from(Bytes::from(data)), + match CappedDecoder::gzip(bytes.reader()).decompress() { + Ok(data) if data.is_empty() => return Err(ApiError::NoData.into()), + Ok(data) => Value::from(Bytes::from(data)), Err(error) => { emit!(SplunkHecRequestBodyInvalidError { error }); return Err(ApiError::InvalidDataFormat { event: 0 }.into()); @@ -1249,6 +1247,11 @@ async fn finish_err(rejection: Rejection) -> Result<(Response,), Rejection> { response_json(StatusCode::BAD_REQUEST, splunk_response::ACK_IS_DISABLED) } },)) + } else if let Some(error) = rejection.find::() { + // `capped_body()` rejects an oversized request body with an `ErrorMessage` carrying a + // 413. Without this arm warp would fall through to a generic 500, which would misreport + // a client error as a server fault. + Ok((empty_response(error.status_code()),)) } else { Err(rejection) } @@ -2914,5 +2917,141 @@ mod tests { ); } + /// OBE-11554: both HEC handlers inflated a client-supplied gzip body with an unbounded + /// `read_to_end`. Amplification was measured at 1029:1 on a listener that accepts + /// unauthenticated requests by default, so ~4 MiB of upload exceeded a 4Gi pod limit. + mod gzip_bomb { + use super::*; + + /// One cheap gzip member repeated past the cap. `MultiGzDecoder` walks every concatenated + /// member, so a single member's size does not bound the attack. + fn gzip_bomb() -> Vec { + use std::io::Write as _; + + use vector_common::decompression::DEFAULT_MAX_DECOMPRESSED_SIZE_BYTES; + + let mut encoder = + flate2::write::GzEncoder::new(Vec::new(), flate2::Compression::best()); + encoder.write_all(&vec![0u8; 1024 * 1024]).unwrap(); + let member = encoder.finish().unwrap(); + + let mut bomb = Vec::new(); + for _ in 0..(DEFAULT_MAX_DECOMPRESSED_SIZE_BYTES / (1024 * 1024) + 1) { + bomb.extend_from_slice(&member); + } + assert!( + bomb.len() < 1024 * 1024, + "the bomb must stay small on the wire to be a meaningful test, got {} bytes", + bomb.len() + ); + bomb + } + + fn gzip(payload: &[u8]) -> Vec { + use std::io::Write as _; + + let mut encoder = + flate2::write::GzEncoder::new(Vec::new(), flate2::Compression::default()); + encoder.write_all(payload).unwrap(); + encoder.finish().unwrap() + } + + async fn post_gzip(address: SocketAddr, api: &str, body: Vec) -> Response { + reqwest::Client::new() + .post(format!("http://{}/{}", address, api)) + .header("Authorization", format!("Splunk {}", TOKEN)) + .header("Content-Encoding", "gzip") + .header("x-splunk-request-channel", "channel") + .body(body) + .send() + .await + .unwrap() + } + + #[tokio::test] + async fn event_endpoint_rejects_gzip_bomb() { + let (_source, address) = source(None).await; + + let response = post_gzip(address, "services/collector/event", gzip_bomb()).await; + + assert_eq!(400, response.status().as_u16()); + // Both the cap trip and a merely unparseable body answer 400, so the status alone + // would pass even with the cap removed. They differ in the body: `ApiError::BadRequest` + // (the cap trip) is empty, while `InvalidDataFormat` carries a JSON document. + assert!( + response.bytes().await.unwrap().is_empty(), + "expected the empty-bodied BadRequest raised by the cap, not a parse failure" + ); + } + + #[tokio::test] + async fn raw_endpoint_rejects_gzip_bomb() { + let (_source, address) = source(None).await; + + let response = post_gzip(address, "services/collector/raw", gzip_bomb()).await; + + assert_eq!(400, response.status().as_u16()); + } + + /// The compressed body itself is now bounded, not just the decompressed output. + /// + /// Sends a handcrafted request declaring an enormous `Content-Length` with no body, so + /// `capped_body()`'s declared-length guard fires before a single body byte is read. This + /// keeps the test free of a real multi-gigabyte upload while still exercising the filter + /// and the `ErrorMessage` arm of `finish_err`. + #[tokio::test] + async fn oversized_declared_body_is_rejected_with_413() { + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + + let (_source, address) = source(None).await; + + let mut stream = tokio::net::TcpStream::connect(address).await.unwrap(); + let request = format!( + "POST /services/collector/raw HTTP/1.1\r\n\ + Host: {address}\r\n\ + Authorization: Splunk {TOKEN}\r\n\ + x-splunk-request-channel: channel\r\n\ + Content-Length: 999999999999\r\n\ + \r\n" + ); + stream.write_all(request.as_bytes()).await.unwrap(); + stream.flush().await.unwrap(); + + // Without the declared-length guard the server waits for the body that never comes, + // so bound the read: a regression must fail here rather than hang the suite. + let mut response = vec![0u8; 128]; + let n = tokio::time::timeout( + tokio::time::Duration::from_secs(10), + stream.read(&mut response), + ) + .await + .expect("server must answer without waiting for the declared body") + .unwrap(); + let status_line = String::from_utf8_lossy(&response[..n]); + + assert!( + status_line.starts_with("HTTP/1.1 413"), + "expected 413 Payload Too Large, got: {status_line}" + ); + } + + /// The cap must not disturb ordinary gzip traffic on either handler. + #[tokio::test] + async fn ordinary_gzip_body_is_accepted() { + let (_source, address) = source(None).await; + + let event = post_gzip( + address, + "services/collector/event", + gzip(br#"{"event":"hello"}"#), + ) + .await; + assert_eq!(200, event.status().as_u16()); + + let raw = post_gzip(address, "services/collector/raw", gzip(b"hello")).await; + assert_eq!(200, raw.status().as_u16()); + } + } + register_validatable_component!(SplunkConfig); } diff --git a/src/sources/util/http/encoding.rs b/src/sources/util/http/encoding.rs index 39051f67ac..b446107a70 100644 --- a/src/sources/util/http/encoding.rs +++ b/src/sources/util/http/encoding.rs @@ -1,39 +1,85 @@ -use std::io::Read; - -use bytes::{Buf, Bytes}; -use flate2::read::{MultiGzDecoder, ZlibDecoder}; +use bytes::{Buf, BufMut, Bytes, BytesMut}; +use futures_util::StreamExt; use snap::raw::Decoder as SnappyDecoder; use warp::http::StatusCode; +use warp::{filters::BoxedFilter, Filter}; use super::error::ErrorMessage; use crate::internal_events::HttpDecompressError; +use vector_common::decompression::{ + is_decompressed_size_limit_error, max_decompressed_size_bytes, CappedDecoder, +}; + +/// Collects a request body into [`Bytes`] while enforcing an in-memory size cap. +/// +/// The cap is the global decompressed-size limit ([`max_decompressed_size_bytes`]): it bounds the +/// raw (still-compressed) body a source buffers before decompression, so a large upload cannot +/// drive unbounded allocation independently of the decompressed-size cap. +pub(crate) fn capped_body() -> BoxedFilter<(Bytes,)> { + let max_body_size = max_decompressed_size_bytes(); + let max_body_size_header = u64::try_from(max_body_size).unwrap_or(u64::MAX); + + warp::header::optional::("content-length") + .and_then(move |declared: Option| async move { + if declared.is_some_and(|len| len > max_body_size_header) { + Err(warp::reject::custom(request_body_too_large_error( + max_body_size, + ))) + } else { + Ok(()) + } + }) + .untuple_one() + .and(warp::body::stream()) + .and_then(move |body| async move { + collect_body_with_limit(body, max_body_size) + .await + .map_err(warp::reject::custom) + }) + .boxed() +} -pub fn decode(header: Option<&str>, mut body: Bytes) -> Result { +/// Decompresses the body based on the Content-Encoding header. +/// +/// Supports gzip, deflate, snappy, zstd, and identity (no compression). +/// +/// Caps the decompressed output at the global limit to mitigate decompression-bomb DoS attacks. +pub fn decode(header: Option<&str>, body: Bytes) -> Result { + decode_with_limit(header, body, max_decompressed_size_bytes()) +} + +/// Like [`decode`], but allows the caller to control the decompressed size cap. +fn decode_with_limit( + header: Option<&str>, + mut body: Bytes, + max_decompressed_size: usize, +) -> Result { if let Some(encodings) = header { + // Each round is capped, which also bounds a stacked `Content-Encoding: gzip,gzip,...` + // chain, since every round's output is the next round's input. for encoding in encodings.rsplit(',').map(str::trim) { body = match encoding { "identity" => body, - "gzip" => { - let mut decoded = Vec::new(); - MultiGzDecoder::new(body.reader()) - .read_to_end(&mut decoded) - .map_err(|error| handle_decode_error(encoding, error))?; - decoded.into() - } - "deflate" => { - let mut decoded = Vec::new(); - ZlibDecoder::new(body.reader()) - .read_to_end(&mut decoded) - .map_err(|error| handle_decode_error(encoding, error))?; - decoded.into() - } - "snappy" => SnappyDecoder::new() - .decompress_vec(&body) - .map_err(|error| handle_decode_error(encoding, error))? - .into(), - "zstd" => zstd::decode_all(body.reader()) - .map_err(|error| handle_decode_error(encoding, error))? - .into(), + "gzip" => CappedDecoder::gzip_with_limit(body.reader(), max_decompressed_size) + .decompress() + .map(Bytes::from) + .map_err(|error| { + emit_decompress_error(encoding, error, max_decompressed_size) + })?, + "deflate" => CappedDecoder::zlib_with_limit(body.reader(), max_decompressed_size) + .decompress() + .map(Bytes::from) + .map_err(|error| { + emit_decompress_error(encoding, error, max_decompressed_size) + })?, + "snappy" => decompress_snappy(&body, max_decompressed_size)?, + "zstd" => CappedDecoder::zstd_http_with_limit(body.reader(), max_decompressed_size) + .map_err(|error| emit_decompress_error(encoding, error, max_decompressed_size))? + .decompress() + .map(Bytes::from) + .map_err(|error| { + emit_decompress_error(encoding, error, max_decompressed_size) + })?, encoding => { return Err(ErrorMessage::new( StatusCode::UNSUPPORTED_MEDIA_TYPE, @@ -44,10 +90,138 @@ pub fn decode(header: Option<&str>, mut body: Bytes) -> Result ErrorMessage { +fn decompress_snappy(body: &Bytes, max_decompressed_size: usize) -> Result { + // Snappy stores the decompressed length in the frame header, so reject oversized + // payloads before allocating the output buffer. + let len = snap::raw::decompress_len(body).map_err(|error| { + emit_decompress_error( + "snappy", + std::io::Error::other(error), + max_decompressed_size, + ) + })?; + if len > max_decompressed_size { + return Err(decompressed_too_large_error( + "snappy", + max_decompressed_size, + )); + } + let decoded = SnappyDecoder::new().decompress_vec(body).map_err(|error| { + emit_decompress_error( + "snappy", + std::io::Error::other(error), + max_decompressed_size, + ) + })?; + Ok(decoded.into()) +} + +/// Spare capacity added to the initial buffer so a third or later chunk can be appended without +/// reallocating right away. +const ADDITIONAL_CAPACITY_FOR_CHUNKS_BEYOND_FIRST_TWO: usize = 16 * 1024; + +/// Collects the body into [`Bytes`] under `max_body_size`, mirroring the fast paths of hyper's +/// `to_bytes`. Single-chunk bodies avoid the `BytesMut` allocation; a buffer sized for both chunks +/// plus an arbitrary 16 KiB (to try to avoid having to reallocate multiple times once other chunks +/// arrive) is only allocated once a second chunk arrives. +async fn collect_body_with_limit(body: S, max_body_size: usize) -> Result +where + S: futures_util::Stream>, + B: Buf, +{ + futures_util::pin_mut!(body); + + let mut total_body_size: usize = 0; + let mut admit_chunk_within_limit = |chunk: Result| -> Result { + let chunk = chunk.map_err(|error| { + ErrorMessage::new( + StatusCode::BAD_REQUEST, + format!("Failed reading request body: {}", error), + ) + })?; + + total_body_size = total_body_size.saturating_add(chunk.remaining()); + if total_body_size > max_body_size { + return Err(request_body_too_large_error(max_body_size)); + } + + Ok(chunk) + }; + + let Some(chunk) = body.next().await else { + return Ok(Bytes::new()); + }; + let mut first = admit_chunk_within_limit(chunk)?; + + let Some(chunk) = body.next().await else { + return Ok(first.copy_to_bytes(first.remaining())); + }; + let second = admit_chunk_within_limit(chunk)?; + + let mut bytes = BytesMut::with_capacity( + first.remaining() + second.remaining() + ADDITIONAL_CAPACITY_FOR_CHUNKS_BEYOND_FIRST_TWO, + ); + bytes.put(first); + bytes.put(second); + + while let Some(chunk) = body.next().await { + bytes.put(admit_chunk_within_limit(chunk)?); + } + + Ok(bytes.freeze()) +} + +fn ensure_body_within_limit( + body: &Bytes, + encoding: &str, + max_decompressed_size: usize, +) -> Result<(), ErrorMessage> { + if body.len() > max_decompressed_size { + return Err(decompressed_too_large_error( + encoding, + max_decompressed_size, + )); + } + Ok(()) +} + +fn request_body_too_large_error(max: usize) -> ErrorMessage { + ErrorMessage::new( + StatusCode::PAYLOAD_TOO_LARGE, + format!("Request body exceeds limit of {} bytes.", max), + ) +} + +fn decompressed_too_large_error(encoding: &str, max: usize) -> ErrorMessage { + ErrorMessage::new( + StatusCode::PAYLOAD_TOO_LARGE, + format!( + "Decompressed {} body exceeds limit of {} bytes.", + encoding, max + ), + ) +} + +/// Maps a decompression failure to a response. If `error` is a `DecompressedSizeLimitExceeded` +/// (the decompressed output exceeded the configured size cap), it becomes a `413 Payload Too +/// Large` reporting the cap that was actually enforced, matching the request-body and snappy size +/// errors. Any other decode failure emits an `HttpDecompressError` event and becomes a +/// `422 Unprocessable Entity`. +/// +/// Callers whose error is not already an [`std::io::Error`] (e.g. snappy) wrap it via +/// [`std::io::Error::other`]. +fn emit_decompress_error( + encoding: &str, + error: std::io::Error, + max_decompressed_size: usize, +) -> ErrorMessage { + if is_decompressed_size_limit_error(&error) { + return decompressed_too_large_error(encoding, max_decompressed_size); + } emit!(HttpDecompressError { encoding, error: &error @@ -57,3 +231,197 @@ fn handle_decode_error(encoding: &str, error: impl std::error::Error) -> ErrorMe format!("Failed decompressing payload with {} decoder.", encoding), ) } + +#[cfg(test)] +mod tests { + use std::io::Write; + + use flate2::{write::GzEncoder, write::ZlibEncoder, Compression}; + use futures_util::stream; + + use super::*; + + const LIMIT: usize = 64 * 1024; + + /// Asserts the rejection came from the guard that stops the allocation *before* it happens + /// (the per-encoding streaming cap, or snappy's declared-length pre-check), rather than from + /// the `ensure_body_within_limit` backstop, which reports "identity" and only fires once the + /// whole payload has already been materialised in memory. + fn assert_rejected_by_streaming_cap(error: &ErrorMessage, encoding: &str) { + let rendered = error.to_string(); + assert!( + rendered.contains(&format!("Decompressed {encoding} body")), + "expected rejection by the {encoding} cap before allocating, got: {rendered}" + ); + } + + fn gzip(plaintext: &[u8]) -> Bytes { + let mut encoder = GzEncoder::new(Vec::new(), Compression::best()); + encoder.write_all(plaintext).unwrap(); + Bytes::from(encoder.finish().unwrap()) + } + + fn deflate(plaintext: &[u8]) -> Bytes { + let mut encoder = ZlibEncoder::new(Vec::new(), Compression::best()); + encoder.write_all(plaintext).unwrap(); + Bytes::from(encoder.finish().unwrap()) + } + + // ---- positive cases: ordinary traffic must be unaffected ---- + + #[test] + fn gzip_within_limit_is_decoded() { + let decoded = decode_with_limit(Some("gzip"), gzip(b"hello"), LIMIT).expect("must decode"); + assert_eq!(decoded, Bytes::from_static(b"hello")); + } + + #[test] + fn deflate_within_limit_is_decoded() { + let decoded = + decode_with_limit(Some("deflate"), deflate(b"hello"), LIMIT).expect("must decode"); + assert_eq!(decoded, Bytes::from_static(b"hello")); + } + + #[test] + fn snappy_within_limit_is_decoded() { + let body = Bytes::from(snap::raw::Encoder::new().compress_vec(b"hello").unwrap()); + let decoded = decode_with_limit(Some("snappy"), body, LIMIT).expect("must decode"); + assert_eq!(decoded, Bytes::from_static(b"hello")); + } + + /// zstd is exercised at a production-scale limit on purpose. `zstd_http` derives the decoder + /// window from the limit (clamped to RFC 9659's 8 MiB), so at a small limit the window clamp + /// binds tighter than the size cap and would refuse even a legitimate frame. At the real + /// 100 MiB default the clamp sits at 8 MiB, which is what this models. + const ZSTD_LIMIT: usize = 8 * 1024 * 1024; + + #[test] + fn zstd_within_limit_is_decoded() { + let body = Bytes::from(zstd::encode_all(&b"hello"[..], 1).unwrap()); + let decoded = decode_with_limit(Some("zstd"), body, ZSTD_LIMIT).expect("must decode"); + assert_eq!(decoded, Bytes::from_static(b"hello")); + } + + #[test] + fn identity_within_limit_passes_through() { + let decoded = + decode_with_limit(Some("identity"), Bytes::from_static(b"hello"), LIMIT).unwrap(); + assert_eq!(decoded, Bytes::from_static(b"hello")); + } + + // ---- negative cases: oversized payloads must be rejected, not buffered ---- + + #[test] + fn gzip_exceeding_limit_returns_413() { + let body = gzip(&vec![0u8; LIMIT + 1]); + let error = decode_with_limit(Some("gzip"), body, LIMIT).expect_err("must be rejected"); + assert_eq!(error.status_code(), StatusCode::PAYLOAD_TOO_LARGE); + assert_rejected_by_streaming_cap(&error, "gzip"); + } + + #[test] + fn deflate_exceeding_limit_returns_413() { + let body = deflate(&vec![0u8; LIMIT + 1]); + let error = decode_with_limit(Some("deflate"), body, LIMIT).expect_err("must be rejected"); + assert_eq!(error.status_code(), StatusCode::PAYLOAD_TOO_LARGE); + assert_rejected_by_streaming_cap(&error, "deflate"); + } + + /// Snappy declares its output length in the frame header, so this must be rejected without + /// ever allocating the output buffer. + #[test] + fn snappy_exceeding_limit_returns_413_before_allocating() { + let body = Bytes::from( + snap::raw::Encoder::new() + .compress_vec(&vec![0u8; LIMIT + 1]) + .unwrap(), + ); + let error = decode_with_limit(Some("snappy"), body, LIMIT).expect_err("must be rejected"); + assert_eq!(error.status_code(), StatusCode::PAYLOAD_TOO_LARGE); + assert_rejected_by_streaming_cap(&error, "snappy"); + } + + /// Concatenated level-1 frames each fit the 8 MiB window clamp, so the aggregate output is + /// what the size cap has to catch — not the window guard. + #[test] + fn zstd_exceeding_limit_returns_413() { + let frame = zstd::encode_all(vec![0u8; 1024 * 1024].as_slice(), 1).unwrap(); + let mut bomb = Vec::new(); + for _ in 0..(ZSTD_LIMIT / (1024 * 1024) + 1) { + bomb.extend_from_slice(&frame); + } + + let error = decode_with_limit(Some("zstd"), Bytes::from(bomb), ZSTD_LIMIT) + .expect_err("must be rejected"); + assert_eq!(error.status_code(), StatusCode::PAYLOAD_TOO_LARGE); + assert_rejected_by_streaming_cap(&error, "zstd"); + } + + /// An uncompressed body over the cap must be rejected too, otherwise `identity` would be a + /// trivial bypass of the whole mechanism. + #[test] + fn identity_exceeding_limit_returns_413() { + let body = Bytes::from(vec![0u8; LIMIT + 1]); + let error = decode_with_limit(Some("identity"), body, LIMIT).expect_err("must be rejected"); + assert_eq!(error.status_code(), StatusCode::PAYLOAD_TOO_LARGE); + } + + #[test] + fn missing_content_encoding_exceeding_limit_returns_413() { + let body = Bytes::from(vec![0u8; LIMIT + 1]); + let error = decode_with_limit(None, body, LIMIT).expect_err("must be rejected"); + assert_eq!(error.status_code(), StatusCode::PAYLOAD_TOO_LARGE); + } + + /// Stacking encodings must not multiply the amplification: every round is capped. + #[test] + fn stacked_encodings_are_capped_at_every_round() { + let outer = gzip(&gzip(&vec![0u8; LIMIT + 1])); + let error = + decode_with_limit(Some("gzip,gzip"), outer, LIMIT).expect_err("must be rejected"); + assert_eq!(error.status_code(), StatusCode::PAYLOAD_TOO_LARGE); + assert_rejected_by_streaming_cap(&error, "gzip"); + } + + /// A malformed payload must stay a 422, distinct from the 413 the cap raises — otherwise the + /// size tests above could be passing for the wrong reason. + #[test] + fn malformed_payload_is_422_not_413() { + let error = decode_with_limit(Some("gzip"), Bytes::from_static(b"not gzip"), LIMIT) + .expect_err("must be rejected"); + assert_eq!(error.status_code(), StatusCode::UNPROCESSABLE_ENTITY); + } + + #[test] + fn unsupported_encoding_is_415() { + let error = decode_with_limit(Some("br"), Bytes::from_static(b"x"), LIMIT) + .expect_err("must be rejected"); + assert_eq!(error.status_code(), StatusCode::UNSUPPORTED_MEDIA_TYPE); + } + + // ---- body collection ---- + + #[tokio::test] + async fn collect_body_within_limit_succeeds() { + let chunks: Vec> = vec![ + Ok(Bytes::from_static(b"foo")), + Ok(Bytes::from_static(b"bar")), + ]; + let collected = collect_body_with_limit(stream::iter(chunks), LIMIT) + .await + .expect("must collect"); + assert_eq!(collected, Bytes::from_static(b"foobar")); + } + + /// The running total must trip mid-stream, so no single chunk needs to exceed the cap. + #[tokio::test] + async fn collect_body_rejects_oversized_stream() { + let chunk = Bytes::from(vec![0u8; LIMIT / 2]); + let chunks: Vec> = + vec![Ok(chunk.clone()), Ok(chunk.clone()), Ok(chunk)]; + let error = collect_body_with_limit(stream::iter(chunks), LIMIT) + .await + .expect_err("must be rejected"); + assert_eq!(error.status_code(), StatusCode::PAYLOAD_TOO_LARGE); + } +} diff --git a/src/sources/util/http/mod.rs b/src/sources/util/http/mod.rs index ae01187b78..6419a92ae8 100644 --- a/src/sources/util/http/mod.rs +++ b/src/sources/util/http/mod.rs @@ -22,6 +22,8 @@ mod query; #[cfg(feature = "sources-utils-http-auth")] pub use auth::{HttpSourceAuth, HttpSourceAuthConfig}; +#[cfg(feature = "sources-splunk_hec")] +pub(crate) use encoding::capped_body; #[cfg(feature = "sources-utils-http-encoding")] pub use encoding::decode; #[cfg(feature = "sources-utils-http-error")] From 809ebd7bd52448f02e82b7847d155f86d44d8892 Mon Sep 17 00:00:00 2001 From: Harshvardhan Shrivastava Date: Wed, 5 Aug 2026 17:51:51 +0530 Subject: [PATCH 2/6] fix(sources): extend decompression caps to the remaining network sources --- Cargo.toml | 2 +- src/sources/aws_kinesis_firehose/filters.rs | 56 ++-- src/sources/aws_kinesis_firehose/handlers.rs | 107 +++++++- src/sources/aws_kinesis_firehose/mod.rs | 131 ++++++++++ src/sources/datadog_agent/logs.rs | 3 +- src/sources/datadog_agent/metrics.rs | 7 +- src/sources/datadog_agent/mod.rs | 18 +- src/sources/datadog_agent/tests.rs | 23 +- src/sources/datadog_agent/traces.rs | 3 +- src/sources/fluent/mod.rs | 86 +++++- src/sources/http_server.rs | 55 ++++ src/sources/logstash.rs | 114 +++++++- src/sources/opentelemetry/http.rs | 7 +- src/sources/opentelemetry/mod.rs | 11 +- src/sources/splunk_hec/mod.rs | 63 ++++- src/sources/util/grpc/decompression.rs | 259 ++++++++++++++++++- src/sources/util/http/encoding.rs | 2 +- src/sources/util/http/mod.rs | 7 +- src/sources/util/http/prelude.rs | 4 +- src/sources/util/mod.rs | 1 + src/sources/vector/mod.rs | 10 +- 21 files changed, 883 insertions(+), 86 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index db82ff14ac..b9d5ce0af4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -655,7 +655,7 @@ sources-metrics = [ sources-amqp = ["lapin"] sources-apache_metrics = ["sources-utils-http-client"] sources-aws_ecs_metrics = ["sources-utils-http-client"] -sources-aws_kinesis_firehose = ["dep:base64"] +sources-aws_kinesis_firehose = ["dep:base64", "sources-utils-http-encoding"] sources-aws_s3 = ["aws-core", "dep:aws-sdk-sqs", "dep:aws-sdk-s3", "dep:semver", "dep:async-compression", "sources-aws_sqs", "tokio-util/io"] sources-aws_sqs = ["aws-core", "dep:aws-sdk-sqs"] sources-datadog_agent = ["sources-utils-http-error", "protobuf-build", "dep:prost"] diff --git a/src/sources/aws_kinesis_firehose/filters.rs b/src/sources/aws_kinesis_firehose/filters.rs index afd3ec507f..1755c7812c 100644 --- a/src/sources/aws_kinesis_firehose/filters.rs +++ b/src/sources/aws_kinesis_firehose/filters.rs @@ -1,15 +1,14 @@ -use std::{convert::Infallible, io}; +use std::convert::Infallible; use bytes::{Buf, Bytes}; use chrono::Utc; -use flate2::read::MultiGzDecoder; use snafu::ResultExt; use vector_lib::config::LogNamespace; use vector_lib::internal_event::{BytesReceived, Protocol}; use warp::{http::StatusCode, Filter}; use super::{ - errors::{ParseSnafu, RequestError}, + errors::{DecodeSnafu, ParseSnafu, RequestError}, handlers, models::{FirehoseRequest, FirehoseResponse}, Compression, @@ -17,6 +16,7 @@ use super::{ use crate::{ codecs, internal_events::{AwsKinesisFirehoseRequestError, AwsKinesisFirehoseRequestReceived}, + sources::util::{decompression::CappedDecoder, http::capped_body, http::ErrorMessage}, SourceSender, }; @@ -71,28 +71,35 @@ fn parse_body() -> impl Filter("Content-Encoding")) .and(warp::header("X-Amz-Firehose-Request-Id")) - .and(warp::body::bytes()) + .and(capped_body()) .and_then( |encoding: Option, request_id: String, body: Bytes| async move { - match encoding { - Some(s) if s == "gzip" => { - Ok(Box::new(MultiGzDecoder::new(body.reader())) as Box) - } - Some(s) => Err(warp::reject::Rejection::from( - RequestError::UnsupportedEncoding { - encoding: s, - request_id: request_id.clone(), - }, - )), - None => Ok(Box::new(body.reader()) as Box), - } - .and_then(|r| { - serde_json::from_reader(r) - .context(ParseSnafu { + // Decompress (if needed) into a buffer capped by the global decompressed-size + // limit so a gzip bomb cannot drive unbounded allocation. + let decoded: Bytes = match encoding { + Some(s) if s == "gzip" => CappedDecoder::gzip(body.reader()) + .decompress() + .map(Bytes::from) + .with_context(|_| DecodeSnafu { request_id: request_id.clone(), }) - .map_err(warp::reject::custom) - }) + .map_err(warp::reject::custom)?, + Some(s) => { + return Err(warp::reject::Rejection::from( + RequestError::UnsupportedEncoding { + encoding: s, + request_id, + }, + )); + } + None => body, + }; + + serde_json::from_slice(&decoded) + .context(ParseSnafu { + request_id: request_id.clone(), + }) + .map_err(warp::reject::custom) }, ) } @@ -156,6 +163,13 @@ async fn handle_firehose_rejection(err: warp::Rejection) -> Result() { + // `capped_body()` rejects an oversized request body with an `ErrorMessage` carrying a 413. + // Without this arm warp falls through to a generic 500, which would misreport a client + // error as a server fault. + code = e.status_code(); + message = e.to_string(); + request_id = None; } else { code = StatusCode::INTERNAL_SERVER_ERROR; message = format!("{:?}", err); diff --git a/src/sources/aws_kinesis_firehose/handlers.rs b/src/sources/aws_kinesis_firehose/handlers.rs index cd424d97a3..f58818c3c3 100644 --- a/src/sources/aws_kinesis_firehose/handlers.rs +++ b/src/sources/aws_kinesis_firehose/handlers.rs @@ -1,9 +1,6 @@ -use std::io::Read; - use base64::prelude::{Engine as _, BASE64_STANDARD}; use bytes::Bytes; use chrono::Utc; -use flate2::read::MultiGzDecoder; use futures::StreamExt; use snafu::{ResultExt, Snafu}; use tokio_util::codec::FramedRead; @@ -36,7 +33,10 @@ use crate::{ internal_events::{ AwsKinesisFirehoseAutomaticRecordDecodeError, EventsReceived, StreamClosedError, }, - sources::aws_kinesis_firehose::AwsKinesisFirehoseConfig, + sources::{ + aws_kinesis_firehose::AwsKinesisFirehoseConfig, + util::decompression::{is_decompressed_size_limit_error, CappedDecoder}, + }, SourceSender, }; @@ -222,6 +222,14 @@ fn decode_record( Compression::Auto => { if is_gzip(&buf) { decode_gzip(&buf[..]).or_else(|error| { + // An exceeded size cap means the magic bytes really were gzip and the payload + // is oversized, so reject it. Only fall back to forwarding the raw bytes when + // auto-detection guessed wrong (valid-looking magic, but not actually gzip). + if is_decompressed_size_limit_error(&error) { + return Err(error).with_context(|_| DecompressionSnafu { + compression: Compression::Gzip, + }); + } emit!(AwsKinesisFirehoseAutomaticRecordDecodeError { compression: Compression::Gzip, error @@ -247,12 +255,8 @@ fn is_gzip(data: &[u8]) -> bool { } fn decode_gzip(data: &[u8]) -> std::io::Result { - let mut decoded = Vec::new(); - - let mut gz = MultiGzDecoder::new(data); - gz.read_to_end(&mut decoded)?; - - Ok(Bytes::from(decoded)) + // Cap the decompressed output so a gzip-bomb record cannot drive unbounded allocation. + CappedDecoder::gzip(data).decompress().map(Bytes::from) } #[cfg(test)] @@ -272,4 +276,87 @@ mod tests { let compressed = encoder.finish().unwrap(); assert!(is_gzip(&compressed)); } + + /// One cheap gzip member repeated past the cap. `MultiGzDecoder` walks every concatenated + /// member, so no single oversized member is required. + fn gzip_bomb() -> Vec { + use crate::sources::util::decompression::DEFAULT_MAX_DECOMPRESSED_SIZE_BYTES; + + let mut encoder = GzEncoder::new(Vec::new(), Compression::best()); + encoder.write_all(&vec![0u8; 1024 * 1024]).unwrap(); + let member = encoder.finish().unwrap(); + + let mut bomb = Vec::new(); + for _ in 0..(DEFAULT_MAX_DECOMPRESSED_SIZE_BYTES / (1024 * 1024) + 1) { + bomb.extend_from_slice(&member); + } + assert!( + bomb.len() < 1024 * 1024, + "the bomb must stay small on the wire to be a meaningful test, got {} bytes", + bomb.len() + ); + bomb + } + + fn record(data: &[u8]) -> EncodedFirehoseRecord { + EncodedFirehoseRecord { + data: BASE64_STANDARD.encode(data), + } + } + + /// A gzip-bomb record must be refused rather than inflated. + #[test] + fn explicit_gzip_record_over_the_cap_is_rejected() { + let error = decode_record(&record(&gzip_bomb()), super::Compression::Gzip) + .expect_err("a record inflating past the cap must be rejected"); + + assert!(matches!( + error, + RecordDecodeError::Decompression { .. } + )); + } + + /// Under `Auto`, an oversized payload whose magic bytes really are gzip must be rejected, not + /// silently forwarded as raw bytes. The raw-bytes fallback exists only for a mis-detection. + #[test] + fn auto_detected_gzip_record_over_the_cap_is_rejected_not_forwarded() { + let error = decode_record(&record(&gzip_bomb()), super::Compression::Auto) + .expect_err("an oversized auto-detected gzip record must not fall back to raw bytes"); + + assert!(matches!( + error, + RecordDecodeError::Decompression { .. } + )); + } + + /// The cap must not disturb ordinary records, on either the explicit or the auto path. + #[test] + fn records_under_the_cap_are_unaffected() { + let mut encoder = GzEncoder::new(Vec::new(), Compression::fast()); + encoder.write_all(CONTENT).unwrap(); + let compressed = encoder.finish().unwrap(); + + for compression in [super::Compression::Gzip, super::Compression::Auto] { + let decoded = decode_record(&record(&compressed), compression) + .expect("a record within the cap must decode"); + assert_eq!(decoded, Bytes::from_static(CONTENT)); + } + + let plain = decode_record(&record(CONTENT), super::Compression::None) + .expect("an uncompressed record must decode"); + assert_eq!(plain, Bytes::from_static(CONTENT)); + } + + /// Auto-detection guessing wrong (gzip magic, not actually gzip) must still fall back to + /// forwarding the raw bytes -- the size cap must not turn that into a hard failure. + #[test] + fn auto_detection_mistake_still_falls_back_to_raw_bytes() { + let mut not_gzip = vector_common::constants::GZIP_MAGIC.to_vec(); + not_gzip.extend_from_slice(b"definitely not a gzip stream"); + + let decoded = decode_record(&record(¬_gzip), super::Compression::Auto) + .expect("a mis-detected record must fall back to raw bytes"); + + assert_eq!(decoded, Bytes::from(not_gzip)); + } } diff --git a/src/sources/aws_kinesis_firehose/mod.rs b/src/sources/aws_kinesis_firehose/mod.rs index 4788692219..b4601c3724 100644 --- a/src/sources/aws_kinesis_firehose/mod.rs +++ b/src/sources/aws_kinesis_firehose/mod.rs @@ -431,6 +431,137 @@ mod tests { builder.send().await } + /// Request-level caps, distinct from the per-record caps covered in `handlers::tests`. + mod request_body_caps { + use futures::StreamExt; + use similar_asserts::assert_eq; + + use super::*; + + /// The source is built with `acknowledgements: true`, and `new_test_finalize` only marks + /// an event acknowledged once it is dropped. So a test that awaits the HTTP response must + /// drain the pipeline concurrently, or the handler waits forever for an ack that cannot + /// arrive. + fn drain(rx: impl Stream + Unpin + Send + 'static) -> tokio::task::JoinHandle<()> { + tokio::spawn(async move { + let mut rx = rx; + while rx.next().await.is_some() {} + }) + } + + fn post(address: SocketAddr) -> reqwest::RequestBuilder { + reqwest::Client::new() + .post(format!("http://{}", address)) + .header("host", address.to_string()) + .header("x-amz-firehose-protocol-version", "1.0") + .header("x-amz-firehose-request-id", REQUEST_ID.to_string()) + .header("x-amz-firehose-source-arn", SOURCE_ARN.to_string()) + .header("content-type", "application/json") + } + + /// A `Content-Encoding: gzip` bomb on the request body must be refused rather than + /// inflated. `MultiGzDecoder` walks every concatenated member, so one cheap member + /// repeated past the cap suffices. + #[tokio::test] + async fn gzip_encoded_request_body_over_the_cap_is_rejected() { + use crate::sources::util::decompression::DEFAULT_MAX_DECOMPRESSED_SIZE_BYTES; + + let (rx, address) = source(None, None, false, Compression::None, true, false).await; + let draining = drain(rx); + + let mut encoder = + GzEncoder::new(Cursor::new(vec![0u8; 1024 * 1024]), flate2::Compression::best()); + let mut member = Vec::new(); + encoder.read_to_end(&mut member).unwrap(); + + let mut bomb = Vec::new(); + for _ in 0..(DEFAULT_MAX_DECOMPRESSED_SIZE_BYTES / (1024 * 1024) + 1) { + bomb.extend_from_slice(&member); + } + assert!(bomb.len() < 1024 * 1024, "bomb must stay small on the wire"); + + let response = post(address) + .header("content-encoding", "gzip") + .body(bomb) + .send() + .await + .unwrap(); + + // Asserting only `!= 200` would pass for the wrong reason: with the cap removed the + // bomb inflates to ~101 MiB, `serde_json` then fails to parse it and the handler + // answers 401 (`RequestError::Parse`), which is also non-200. Pin the decode failure + // specifically, which only the cap can produce. + assert_eq!(400, response.status().as_u16()); + let body = response.text().await.unwrap(); + assert!( + body.contains("Could not decode record"), + "expected the capped-decompression error, got: {body}" + ); + draining.abort(); + } + + /// `capped_body()` refuses an oversized declared `Content-Length` before reading any body + /// bytes. Sent over a raw socket so the test does not have to upload gigabytes. + #[tokio::test] + async fn oversized_declared_content_length_is_rejected() { + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + + let (_rx, address) = source(None, None, false, Compression::None, true, false).await; + + let mut stream = tokio::net::TcpStream::connect(address).await.unwrap(); + let request = format!( + "POST / HTTP/1.1\r\n\ + Host: {address}\r\n\ + x-amz-firehose-protocol-version: 1.0\r\n\ + x-amz-firehose-request-id: {REQUEST_ID}\r\n\ + x-amz-firehose-source-arn: {SOURCE_ARN}\r\n\ + Content-Type: application/json\r\n\ + Content-Length: 999999999999\r\n\ + \r\n" + ); + stream.write_all(request.as_bytes()).await.unwrap(); + stream.flush().await.unwrap(); + + // Without the declared-length guard the server waits for a body that never arrives, + // so bound the read: a regression must fail here rather than hang the suite. + let mut response = vec![0u8; 128]; + let n = tokio::time::timeout( + std::time::Duration::from_secs(10), + stream.read(&mut response), + ) + .await + .expect("server must answer without waiting for the declared body") + .unwrap(); + let status_line = String::from_utf8_lossy(&response[..n]); + + assert!( + status_line.starts_with("HTTP/1.1 413"), + "expected 413 Payload Too Large, got: {status_line}" + ); + } + + /// An ordinary gzip-encoded request must still be accepted. + #[tokio::test] + async fn ordinary_gzip_encoded_request_is_accepted() { + let (rx, address) = source(None, None, false, Compression::None, true, false).await; + let draining = drain(rx); + + let response = send( + address, + Utc::now(), + vec![RECORD.as_bytes()], + None, + true, + Compression::None, + ) + .await + .unwrap(); + + assert_eq!(200, response.status().as_u16()); + draining.abort(); + } + } + async fn spawn_send( address: SocketAddr, timestamp: DateTime, diff --git a/src/sources/datadog_agent/logs.rs b/src/sources/datadog_agent/logs.rs index 50ee4aebb2..d282c79a17 100644 --- a/src/sources/datadog_agent/logs.rs +++ b/src/sources/datadog_agent/logs.rs @@ -12,6 +12,7 @@ use vector_lib::{config::LegacyKey, EstimatedJsonEncodedSizeOf}; use vrl::core::Value; use warp::{filters::BoxedFilter, path as warp_path, path::FullPath, reply::Response, Filter}; +use crate::sources::util::http::capped_body; use crate::common::datadog::DDTAGS; use crate::{ event::Event, @@ -37,7 +38,7 @@ pub(crate) fn build_warp_filter( .and(warp::header::optional::("content-encoding")) .and(warp::header::optional::("dd-api-key")) .and(warp::query::()) - .and(warp::body::bytes()) + .and(capped_body()) .and_then( move |_, path: FullPath, diff --git a/src/sources/datadog_agent/metrics.rs b/src/sources/datadog_agent/metrics.rs index fd3bcdfe66..65fe6c8c5c 100644 --- a/src/sources/datadog_agent/metrics.rs +++ b/src/sources/datadog_agent/metrics.rs @@ -14,6 +14,7 @@ use vector_lib::{ EstimatedJsonEncodedSizeOf, }; +use crate::sources::util::http::capped_body; use crate::{ common::datadog::{DatadogMetricType, DatadogSeriesMetric}, config::log_schema, @@ -69,7 +70,7 @@ fn sketches_service( .and(warp::header::optional::("content-encoding")) .and(warp::header::optional::("dd-api-key")) .and(warp::query::()) - .and(warp::body::bytes()) + .and(capped_body()) .and_then( move |path: FullPath, encoding_header: Option, @@ -107,7 +108,7 @@ fn series_v1_service( .and(warp::header::optional::("content-encoding")) .and(warp::header::optional::("dd-api-key")) .and(warp::query::()) - .and(warp::body::bytes()) + .and(capped_body()) .and_then( move |path: FullPath, encoding_header: Option, @@ -148,7 +149,7 @@ fn series_v2_service( .and(warp::header::optional::("content-encoding")) .and(warp::header::optional::("dd-api-key")) .and(warp::query::()) - .and(warp::body::bytes()) + .and(capped_body()) .and_then( move |path: FullPath, encoding_header: Option, diff --git a/src/sources/datadog_agent/mod.rs b/src/sources/datadog_agent/mod.rs index f49b3c6ae1..7b19199e66 100644 --- a/src/sources/datadog_agent/mod.rs +++ b/src/sources/datadog_agent/mod.rs @@ -33,7 +33,9 @@ use snafu::Snafu; use tokio::net::TcpStream; use tower::ServiceBuilder; use tracing::Span; -use vector_common::decompression::CappedDecoder; +use crate::sources::util::decompression::{ + is_decompressed_size_limit_error, max_decompressed_size_bytes, CappedDecoder, +}; use vector_lib::codecs::decoding::{DeserializerConfig, FramingConfig}; use vector_lib::config::{LegacyKey, LogNamespace}; use vector_lib::configurable::configurable_component; @@ -540,7 +542,19 @@ pub(crate) async fn handle_request( } } -fn handle_decode_error(encoding: &str, error: impl std::error::Error) -> ErrorMessage { +fn handle_decode_error(encoding: &str, error: std::io::Error) -> ErrorMessage { + // A size-cap trip is an oversized-request client fault, so report it as 413 with the limit + // that was enforced, matching the shared HTTP decoder. Anything else is malformed input (422). + if is_decompressed_size_limit_error(&error) { + return ErrorMessage::new( + StatusCode::PAYLOAD_TOO_LARGE, + format!( + "Decompressed {} body exceeds limit of {} bytes.", + encoding, + max_decompressed_size_bytes() + ), + ); + } emit!(HttpDecompressError { encoding, error: &error diff --git a/src/sources/datadog_agent/tests.rs b/src/sources/datadog_agent/tests.rs index 84fb90c144..3d8a098388 100644 --- a/src/sources/datadog_agent/tests.rs +++ b/src/sources/datadog_agent/tests.rs @@ -2688,7 +2688,7 @@ mod decompression_caps { .decode(&Some("gzip".to_owned()), gzip_bomb(), "/api/v2/logs") .expect_err("a body inflating past the cap must be rejected"); - assert_eq!(error.status_code(), http::StatusCode::UNPROCESSABLE_ENTITY); + assert_eq!(error.status_code(), http::StatusCode::PAYLOAD_TOO_LARGE); } #[test] @@ -2697,7 +2697,7 @@ mod decompression_caps { .decode(&Some("deflate".to_owned()), zlib_bomb(), "/api/v2/logs") .expect_err("a body inflating past the cap must be rejected"); - assert_eq!(error.status_code(), http::StatusCode::UNPROCESSABLE_ENTITY); + assert_eq!(error.status_code(), http::StatusCode::PAYLOAD_TOO_LARGE); } /// Stacking encodings used to multiply the amplification. Capping each round bounds the chain, @@ -2721,7 +2721,7 @@ mod decompression_caps { ) .expect_err("a stacked chain inflating past the cap must be rejected"); - assert_eq!(error.status_code(), http::StatusCode::UNPROCESSABLE_ENTITY); + assert_eq!(error.status_code(), http::StatusCode::PAYLOAD_TOO_LARGE); } /// Level 1 keeps each frame's declared window well under the RFC 9659 8 MiB ceiling that @@ -2739,7 +2739,7 @@ mod decompression_caps { .decode(&Some("zstd".to_owned()), Bytes::from(bomb), "/api/v2/logs") .expect_err("a body inflating past the cap must be rejected"); - assert_eq!(error.status_code(), http::StatusCode::UNPROCESSABLE_ENTITY); + assert_eq!(error.status_code(), http::StatusCode::PAYLOAD_TOO_LARGE); } /// A realistic agent frame must still decode: the 8 MiB window clamp must not reject ordinary @@ -2756,6 +2756,21 @@ mod decompression_caps { assert_eq!(decoded, Bytes::from_static(payload)); } + /// Malformed input must stay a 422, distinct from the 413 the cap raises — otherwise the + /// size tests above could be passing for the wrong reason. + #[test] + fn malformed_payload_is_422_not_413() { + let error = test_source() + .decode( + &Some("gzip".to_owned()), + Bytes::from_static(b"not gzip at all"), + "/api/v2/logs", + ) + .expect_err("malformed input must be rejected"); + + assert_eq!(error.status_code(), http::StatusCode::UNPROCESSABLE_ENTITY); + } + /// The cap must not disturb ordinary traffic. #[test] fn body_under_the_cap_is_unaffected() { diff --git a/src/sources/datadog_agent/traces.rs b/src/sources/datadog_agent/traces.rs index a9bef10865..28cc86076c 100644 --- a/src/sources/datadog_agent/traces.rs +++ b/src/sources/datadog_agent/traces.rs @@ -12,6 +12,7 @@ use warp::{filters::BoxedFilter, path, path::FullPath, reply::Response, Filter, use vector_lib::internal_event::{CountByteSize, InternalEventHandle as _}; use vector_lib::EstimatedJsonEncodedSizeOf; +use crate::sources::util::http::capped_body; use crate::{ event::{Event, ObjectMap, TraceEvent, Value}, sources::{ @@ -48,7 +49,7 @@ fn build_trace_filter( "X-Datadog-Reported-Languages", )) .and(warp::query::()) - .and(warp::body::bytes()) + .and(capped_body()) .and_then( move |path: FullPath, encoding_header: Option, diff --git a/src/sources/fluent/mod.rs b/src/sources/fluent/mod.rs index d5495405e9..887ebc3ab3 100644 --- a/src/sources/fluent/mod.rs +++ b/src/sources/fluent/mod.rs @@ -10,7 +10,6 @@ use rmp_serde::{decode, Deserializer, Serializer}; use serde::{Deserialize, Serialize}; use smallvec::{smallvec, SmallVec}; use tokio_util::codec::Decoder; -use vector_common::decompression::CappedDecoder; use vector_lib::codecs::{BytesDeserializerConfig, StreamDecodingError}; use vector_lib::config::{LegacyKey, LogNamespace}; use vector_lib::configurable::configurable_component; @@ -21,6 +20,7 @@ use vector_lib::schema::Definition; use vrl::value::kind::Collection; use vrl::value::{Kind, Value}; +use super::util::decompression::{max_decompressed_size_bytes, CappedDecoder}; use super::util::net::{SocketListenAddr, TcpSource, TcpSourceAck, TcpSourceAcker}; use crate::{ config::{ @@ -264,6 +264,13 @@ pub enum DecodeError { Decode(decode::Error), UnknownCompression(String), UnexpectedValue(rmpv::Value), + /// The buffered frame grew past the maximum allowed size before a complete message could be + /// decoded. Bounds memory when a peer declares an oversized msgpack array/map/string and + /// streams the bytes to force unbounded buffering. + FrameTooLarge { + size: usize, + max: usize, + }, } impl std::fmt::Display for DecodeError { @@ -277,6 +284,13 @@ impl std::fmt::Display for DecodeError { DecodeError::UnexpectedValue(value) => { write!(f, "unexpected msgpack value, ignoring: {}", value) } + DecodeError::FrameTooLarge { size, max } => { + write!( + f, + "fluent frame exceeds maximum size before decoding: {} bytes buffered, limit is {} bytes", + size, max + ) + } } } } @@ -288,6 +302,9 @@ impl StreamDecodingError for DecodeError { DecodeError::Decode(_) => true, DecodeError::UnknownCompression(_) => true, DecodeError::UnexpectedValue(_) => true, + // An oversized partial frame has no framing boundary to resync on, so the connection + // must be dropped rather than re-decoded in a loop. + DecodeError::FrameTooLarge { .. } => false, } } } @@ -307,11 +324,18 @@ impl From for DecodeError { #[derive(Debug)] struct FluentDecoder { log_namespace: LogNamespace, + /// Maximum number of bytes that may be buffered while waiting for a complete frame. Bounds + /// memory against a peer that declares an oversized msgpack structure and streams the bytes to + /// force unbounded buffering. + max_frame_size: usize, } impl FluentDecoder { - const fn new(log_namespace: LogNamespace) -> Self { - Self { log_namespace } + fn new(log_namespace: LogNamespace) -> Self { + Self { + log_namespace, + max_frame_size: max_decompressed_size_bytes(), + } } fn handle_message( @@ -460,6 +484,17 @@ impl Decoder for FluentDecoder { )) = res { if custom.kind() == io::ErrorKind::UnexpectedEof { + // We need more bytes before a full message can be decoded. Bound the + // buffer so a peer cannot force unbounded memory growth by declaring a + // huge msgpack array/map/string and streaming the bytes: if the frame has + // already grown past the limit without yielding a complete message, drop + // the connection. + if src.len() > self.max_frame_size { + return Err(DecodeError::FrameTooLarge { + size: src.len(), + max: self.max_frame_size, + }); + } return Ok(None); } } @@ -847,6 +882,51 @@ mod tests { assert_event_data_eq!(got.0[2], expected[2]); } + /// A valid but incomplete frame must ask for more data rather than erroring — otherwise the + /// frame cap would break ordinary streaming reads. + #[test] + fn decode_incomplete_frame_requests_more_data() { + // An array of 2 elements (`0x92`) with a tag string declaring 16 bytes (`0xb0`) but only + // 4 bytes provided: a valid, incomplete frame. + let partial: Vec = vec![0x92, 0xb0, b't', b'a', b'g']; + let mut buf = BytesMut::from(&partial[..]); + let mut decoder = FluentDecoder::new(LogNamespace::default()); + + assert!(matches!(decoder.decode(&mut buf), Ok(None))); + // The buffer is retained so more bytes can complete the frame. + assert_eq!(buf.len(), partial.len()); + } + + /// OBE-11557: a peer declaring an oversized msgpack structure could stream bytes forever and + /// grow the connection's frame buffer without bound, since an incomplete frame simply asked + /// for more data. + #[test] + fn decode_oversized_frame_is_rejected() { + // Same shape as above (a 2-element array whose string is declared far larger than what has + // arrived), but with a decoder whose frame cap is tiny. + let max_frame_size = 8; + let partial: Vec = vec![0x92, 0xb0, b't', b'a', b'g', b'.', b'n', b'a', b'm', b'e']; + assert!(partial.len() > max_frame_size); + + let mut buf = BytesMut::from(&partial[..]); + let mut decoder = FluentDecoder { + log_namespace: LogNamespace::default(), + max_frame_size, + }; + + let error = match decoder.decode(&mut buf) { + Err(error) => error, + Ok(_) => panic!("expected FrameTooLarge, got Ok"), + }; + + assert!( + matches!(error, DecodeError::FrameTooLarge { size, max } if size == partial.len() && max == max_frame_size), + "unexpected error: {error:?}" + ); + // A frame-too-large error must terminate the connection. + assert!(!error.can_continue()); + } + /// OBE-11233 / OBE-10708: `CompressedPackedForward` inflated the client's gzip payload with an /// unbounded `read_to_end`, so a small frame could drive an arbitrarily large allocation. /// diff --git a/src/sources/http_server.rs b/src/sources/http_server.rs index 59599a17c5..7c475f06ff 100644 --- a/src/sources/http_server.rs +++ b/src/sources/http_server.rs @@ -1838,6 +1838,61 @@ mod tests { spawn_simple_http_source(address, permit_origin, context).await; } + /// The shared `HttpSource` filter now collects the body through `capped_body()`, which + /// refuses an oversized declared `Content-Length` before reading any body bytes. Sent over a + /// raw socket so the test does not have to upload gigabytes. + /// + /// This covers `http_server` and, through the same prelude filter, `heroku_logs`. + #[tokio::test] + async fn oversized_declared_body_is_rejected_with_413() { + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + + let (sender, _recv) = SourceSender::new_test_finalize(EventStatus::Delivered); + let address = next_addr(); + spawn_simple_http_source(address, None, SourceContext::new_test(sender, None)).await; + wait_for_tcp(address).await; + + let mut stream = tokio::net::TcpStream::connect(address).await.unwrap(); + let request = format!( + "POST / HTTP/1.1\r\n\ + Host: {address}\r\n\ + Content-Length: 999999999999\r\n\ + \r\n" + ); + stream.write_all(request.as_bytes()).await.unwrap(); + stream.flush().await.unwrap(); + + // Without the declared-length guard the server waits for a body that never arrives, so + // bound the read: a regression must fail here rather than hang the suite. + let mut response = vec![0u8; 128]; + let n = tokio::time::timeout( + std::time::Duration::from_secs(10), + stream.read(&mut response), + ) + .await + .expect("server must answer without waiting for the declared body") + .unwrap(); + let status_line = String::from_utf8_lossy(&response[..n]); + + assert!( + status_line.starts_with("HTTP/1.1 413"), + "expected 413 Payload Too Large, got: {status_line}" + ); + } + + /// An ordinary body must still be accepted through the same filter. + #[tokio::test] + async fn ordinary_body_is_accepted() { + let (sender, _recv) = SourceSender::new_test_finalize(EventStatus::Delivered); + let address = next_addr(); + spawn_simple_http_source(address, None, SourceContext::new_test(sender, None)).await; + wait_for_tcp(address).await; + + let response = send_http_event(address, "hello").await.unwrap(); + + assert_eq!(200, response.status().as_u16()); + } + async fn send_http_event( address: std::net::SocketAddr, body: &'static str, diff --git a/src/sources/logstash.rs b/src/sources/logstash.rs index f5682f4464..ceb41c8e41 100644 --- a/src/sources/logstash.rs +++ b/src/sources/logstash.rs @@ -3,12 +3,11 @@ use std::time::Duration; use std::{ collections::{BTreeMap, VecDeque}, convert::TryFrom, - io::{self, Read}, + io, }; use vector_lib::ipallowlist::IpAllowlistConfig; use bytes::{Buf, Bytes, BytesMut}; -use flate2::read::ZlibDecoder; use smallvec::{smallvec, SmallVec}; use snafu::{ResultExt, Snafu}; use tokio_util::codec::Decoder; @@ -22,6 +21,9 @@ use vector_lib::{ use vrl::value::kind::Collection; use vrl::value::{KeyString, Kind}; +use super::util::decompression::{ + max_decompressed_size_bytes, max_zlib_compressed_frame_size_bytes, CappedDecoder, +}; use super::util::net::{SocketListenAddr, TcpSource, TcpSourceAck, TcpSourceAcker}; use crate::{ config::{ @@ -654,21 +656,33 @@ fn decode_compressed_frame( return Ok(None); } let payload_size = rest.get_u32() as usize; + let limit = max_decompressed_size_bytes(); + + // Reject an oversized declared payload before buffering it, so a peer cannot force multi-GB + // buffering by advertising a huge length and slow-streaming its bytes. The bound includes + // zlib's worst-case expansion so a valid frame whose decompressed content is within `limit` + // is never rejected here; the decompressed cap itself is still enforced below. + let compressed_limit = max_zlib_compressed_frame_size_bytes(); + if payload_size > compressed_limit { + return Err(DecodeError::DecompressionFailed { + source: io::Error::other(format!( + "compressed frame payload size {} exceeds limit of {} bytes", + payload_size, compressed_limit + )), + }); + } if rest.remaining() < payload_size { - src.reserve(payload_size); return Ok(None); } let (slice, right) = rest.split_at(payload_size); rest = right; - let mut buf = Vec::new(); - - let res = ZlibDecoder::new(io::Cursor::new(slice)) - .read_to_end(&mut buf) - .context(DecompressionFailedSnafu) - .map(|_| BytesMut::from(&buf[..])); + let res = CappedDecoder::zlib_with_limit(io::Cursor::new(slice), limit) + .decompress() + .map(|decompressed| BytesMut::from(decompressed.as_slice())) + .context(DecompressionFailedSnafu); let byte_size = bytes_remaining(src, rest); src.advance(byte_size); @@ -904,6 +918,88 @@ mod test { assert_eq!(definitions, Some(expected_definition)) } + + /// OBE-10711: a compressed frame's 4-byte length header was fed straight to `src.reserve()`, + /// so six bytes on the wire could commit a multi-gigabyte allocation. The declared length is + /// now checked against zlib's worst-case expansion of the decompressed cap first. + #[test] + fn oversized_declared_frame_is_rejected_before_reserving() { + let mut src = BytesMut::new(); + src.put_u32(u32::MAX); + src.put_slice(b"partial"); + + let error = decode_compressed_frame(&mut src) + .expect_err("a frame declaring more than the cap must be rejected"); + + assert!( + error.to_string().contains("exceeds limit"), + "expected the declared-size guard, got: {error}" + ); + } + + /// A frame whose *compressed* length is legitimate but which inflates past the decompressed + /// cap must still be refused — the declared-length guard alone is not enough. + #[test] + fn decompressed_bomb_is_rejected() { + use std::io::Write as _; + + use vector_common::decompression::DEFAULT_MAX_DECOMPRESSED_SIZE_BYTES; + + let mut encoder = flate2::write::ZlibEncoder::new(Vec::new(), flate2::Compression::best()); + let chunk = vec![0u8; 1024 * 1024]; + for _ in 0..(DEFAULT_MAX_DECOMPRESSED_SIZE_BYTES / (1024 * 1024) + 1) { + encoder.write_all(&chunk).unwrap(); + } + let compressed = encoder.finish().unwrap(); + + assert!( + compressed.len() < max_zlib_compressed_frame_size_bytes(), + "the bomb must pass the declared-length guard so the decompressed cap is what fires" + ); + + let mut src = BytesMut::new(); + src.put_u32(compressed.len() as u32); + src.put_slice(&compressed); + + let error = decode_compressed_frame(&mut src) + .expect_err("a frame inflating past the cap must be rejected"); + + assert!(matches!(error, DecodeError::DecompressionFailed { .. })); + } + + /// The caps must not disturb an ordinary compressed frame. + #[test] + fn ordinary_compressed_frame_is_accepted() { + use std::io::Write as _; + + let mut encoder = + flate2::write::ZlibEncoder::new(Vec::new(), flate2::Compression::default()); + encoder.write_all(b"").unwrap(); + let compressed = encoder.finish().unwrap(); + + let mut src = BytesMut::new(); + src.put_u32(compressed.len() as u32); + src.put_slice(&compressed); + + let frames = decode_compressed_frame(&mut src) + .expect("a well-formed frame within the caps must decode"); + + assert!(frames.is_some_and(|frames| frames.is_empty())); + } + + /// An incomplete frame whose declared length is legitimate must still be treated as "need more + /// bytes", not rejected — otherwise the caps would break normal streaming reads. + #[test] + fn incomplete_frame_within_the_cap_waits_for_more_bytes() { + let mut src = BytesMut::new(); + src.put_u32(64); + src.put_slice(b"only a few bytes so far"); + + let result = decode_compressed_frame(&mut src) + .expect("an incomplete but legitimate frame must not error"); + + assert!(result.is_none(), "expected the decoder to await more bytes"); + } } #[cfg(all(test, feature = "logstash-integration-tests"))] diff --git a/src/sources/opentelemetry/http.rs b/src/sources/opentelemetry/http.rs index d658bdc50b..b2c3b63cda 100644 --- a/src/sources/opentelemetry/http.rs +++ b/src/sources/opentelemetry/http.rs @@ -33,6 +33,7 @@ use vector_lib::ipallowlist::IpAllowlistConfig; use crate::http::{KeepaliveConfig, MaxConnectionAgeLayer}; use crate::sources::http_server::HttpConfigParamKind; use crate::sources::util::add_headers; +use crate::sources::util::http::capped_body; use crate::sources::util::handle_accept_error; use crate::{ event::Event, @@ -161,7 +162,7 @@ fn build_warp_log_filter( )) .and(warp::header::optional::("content-encoding")) .and(warp::header::headers_cloned()) - .and(warp::body::bytes()) + .and(capped_body()) .and_then( move |encoding_header: Option, headers_config: HeaderMap, body: Bytes| { let events = decode(encoding_header.as_deref(), body) @@ -199,7 +200,7 @@ fn build_warp_metrics_filter( "application/x-protobuf", )) .and(warp::header::optional::("content-encoding")) - .and(warp::body::bytes()) + .and(capped_body()) .and_then(move |encoding_header: Option, body: Bytes| { let events = decode(encoding_header.as_deref(), body).and_then(|body| { bytes_received.emit(ByteSize(body.len())); @@ -230,7 +231,7 @@ fn build_warp_trace_filter( "application/x-protobuf", )) .and(warp::header::optional::("content-encoding")) - .and(warp::body::bytes()) + .and(capped_body()) .and_then(move |encoding_header: Option, body: Bytes| { let events = decode(encoding_header.as_deref(), body).and_then(|body| { bytes_received.emit(ByteSize(body.len())); diff --git a/src/sources/opentelemetry/mod.rs b/src/sources/opentelemetry/mod.rs index ff750040a8..73c87abd88 100644 --- a/src/sources/opentelemetry/mod.rs +++ b/src/sources/opentelemetry/mod.rs @@ -44,7 +44,10 @@ use crate::{ }, http::KeepaliveConfig, serde::bool_or_struct, - sources::{util::grpc::run_grpc_server_with_routes, Source}, + sources::{ + util::{decompression::max_decompressed_size_bytes, grpc::run_grpc_server_with_routes}, + Source, + }, tls::{MaybeTlsSettings, TlsEnableableConfig}, }; @@ -182,7 +185,7 @@ impl SourceConfig for OpentelemetryConfig { events_received: events_received.clone(), }) .accept_compressed(CompressionEncoding::Gzip) - .max_decoding_message_size(usize::MAX); + .max_decoding_message_size(max_decompressed_size_bytes()); let trace_service = TraceServiceServer::new(Service { pipeline: cx.out.clone(), @@ -191,7 +194,7 @@ impl SourceConfig for OpentelemetryConfig { events_received: events_received.clone(), }) .accept_compressed(CompressionEncoding::Gzip) - .max_decoding_message_size(usize::MAX); + .max_decoding_message_size(max_decompressed_size_bytes()); let metrics_service = MetricsServiceServer::new(Service { pipeline: cx.out.clone(), @@ -200,7 +203,7 @@ impl SourceConfig for OpentelemetryConfig { events_received: events_received.clone(), }) .accept_compressed(CompressionEncoding::Gzip) - .max_decoding_message_size(usize::MAX); + .max_decoding_message_size(max_decompressed_size_bytes()); let mut builder = RoutesBuilder::default(); builder diff --git a/src/sources/splunk_hec/mod.rs b/src/sources/splunk_hec/mod.rs index 75f5d894e3..9fa073c8c4 100644 --- a/src/sources/splunk_hec/mod.rs +++ b/src/sources/splunk_hec/mod.rs @@ -20,7 +20,6 @@ use snafu::Snafu; use tokio::net::TcpStream; use tower::ServiceBuilder; use tracing::Span; -use vector_common::decompression::CappedDecoder; use vector_lib::internal_event::{CountByteSize, InternalEventHandle as _, Registered}; use vector_lib::lookup::lookup_v2::OptionalValuePath; use vector_lib::lookup::{self, event_path, owned_value_path}; @@ -53,8 +52,10 @@ use crate::{ EventsReceived, HttpBytesReceived, SplunkHecRequestBodyInvalidError, SplunkHecRequestError, }, serde::bool_or_struct, - sources::util::{handle_accept_error, http::capped_body, http::ErrorMessage}, source_sender::ClosedError, + sources::util::{ + decompression::CappedDecoder, handle_accept_error, http::capped_body, http::ErrorMessage, + }, tls::{MaybeTlsSettings, TlsEnableableConfig}, SourceSender, }; @@ -536,10 +537,15 @@ impl SplunkSource { .and(path!("ack")) .and(self.authorization()) .and(SplunkSource::required_channel()) - .and(warp::body::json()) - .and_then(move |_, channel_id: String, body: HecAckStatusRequest| { + // `warp::body::json()` aggregates the whole body unbounded; cap it first and parse the + // bytes ourselves. Token auth is optional in config, so this endpoint can be reached + // unauthenticated. + .and(capped_body()) + .and_then(move |_, channel_id: String, body: Bytes| { let idx_ack = idx_ack.clone(); async move { + let body: HecAckStatusRequest = serde_json::from_slice(&body) + .map_err(|_| Rejection::from(ApiError::BadRequest))?; if let Some(idx_ack) = idx_ack { let ack_statuses = idx_ack .get_acks_status_from_channel(channel_id, &body.acks) @@ -3053,5 +3059,54 @@ mod tests { } } + /// `capped_body()` replaced `warp::body::bytes()` on both HEC handlers. It collects the body + /// by streaming chunks rather than letting hyper buffer it in one shot, so it is worth pinning + /// that this does not add per-request latency. + /// + /// Measured at 0-3 ms when this was written; the 100 ms bound is deliberately loose so the + /// test catches a systematic regression (a stall waiting on end-of-stream would cost hundreds + /// of ms) without tripping on CI scheduling noise. The median is used so one stalled sample + /// cannot fail the run. + #[tokio::test] + async fn capped_body_does_not_add_request_latency() { + const SAMPLES: usize = 9; + const MAX_MEDIAN: Duration = Duration::from_millis(100); + + let (_source, address) = source(None).await; + let client = reqwest::Client::new(); + + let post = |client: reqwest::Client, address: SocketAddr| async move { + client + .post(format!("http://{}/services/collector/event", address)) + .header("Authorization", format!("Splunk {}", TOKEN)) + .header("x-splunk-request-channel", "channel") + .body(r#"{"event":"hello"}"#) + .send() + .await + .unwrap() + }; + + // Warm the connection pool so we measure the filter, not TCP setup. + assert_eq!(200, post(client.clone(), address).await.status().as_u16()); + + let mut samples = Vec::with_capacity(SAMPLES); + for _ in 0..SAMPLES { + let started = std::time::Instant::now(); + let response = post(client.clone(), address).await; + let elapsed = started.elapsed(); + assert_eq!(200, response.status().as_u16()); + samples.push(elapsed); + } + + samples.sort_unstable(); + let median = samples[SAMPLES / 2]; + + assert!( + median < MAX_MEDIAN, + "capped_body() added per-request latency: median {median:?} over {SAMPLES} requests \ + exceeds {MAX_MEDIAN:?} (samples: {samples:?})" + ); + } + register_validatable_component!(SplunkConfig); } diff --git a/src/sources/util/grpc/decompression.rs b/src/sources/util/grpc/decompression.rs index 16293df4b0..6b1a28a8e8 100644 --- a/src/sources/util/grpc/decompression.rs +++ b/src/sources/util/grpc/decompression.rs @@ -1,6 +1,6 @@ use std::{ cmp, - io::Write, + io::{self, Write}, mem, pin::Pin, task::{Context, Poll}, @@ -23,11 +23,25 @@ use vector_lib::internal_event::{ }; use crate::internal_events::{GrpcError, GrpcInvalidCompressionSchemeError}; +use crate::sources::util::decompression::{ + is_decompressed_size_limit_error, max_decompressed_size_bytes, + max_zlib_compressed_frame_size_bytes, DecompressedSizeLimitExceeded, +}; // Every gRPC message has a five byte header: // - a compressed flag (u8, 0/1 for compressed/decompressed) // - a length prefix, indicating the number of remaining bytes to read (u32) const GRPC_MESSAGE_HEADER_LEN: usize = mem::size_of::() + mem::size_of::(); +// Fixed container framing a valid frame adds on top of zlib's worst-case expansion. Added to the +// compressed-frame pre-filter so a small cap does not reject a typical gzip frame whose +// decompressed size is within the cap. +// +// gzip's mandatory framing is 18 bytes (10 header + 8 trailer), but its optional FNAME, FCOMMENT +// and FEXTRA fields are unbounded (RFC 1952 section 2.3.1): a gzip frame carrying more than this +// slack in those optional fields could still be rejected here. Encoders don't emit them in +// practice, so 22 covers the realistic case; the prefilter is only a cheap wire-size guard and the +// authoritative per-output cap is still enforced during decompression. +const GRPC_COMPRESSED_FRAME_OVERHEAD_SLACK: usize = 22; const GRPC_ENCODING_HEADER: &str = "grpc-encoding"; const GRPC_ACCEPT_ENCODING_HEADER: &str = "grpc-accept-encoding"; @@ -80,12 +94,61 @@ impl Default for State { } } -fn new_decompressor() -> GzDecoder> { +/// Maps a decompressor `io::Error` to a gRPC [`Status`]: an oversized payload becomes +/// `out_of_range` (a client fault, matching the existing >4GB handling) while anything else falls +/// back to `internal` with `internal_msg`. +fn decompressor_error_to_status(error: &io::Error, internal_msg: &'static str) -> Status { + if is_decompressed_size_limit_error(error) { + Status::out_of_range("decompressed message exceeds the maximum allowed size") + } else { + Status::internal(internal_msg) + } +} + +/// A `Write` sink that appends into a `Vec` but refuses to grow past `max_len`, so a streaming +/// decompressor errors out *during* decompression rather than first materializing an oversized +/// output and only then having its size checked. +struct LimitedWriter { + buf: Vec, + max_len: usize, +} + +impl LimitedWriter { + const fn new(buf: Vec, max_len: usize) -> Self { + Self { buf, max_len } + } + + fn into_inner(self) -> Vec { + self.buf + } +} + +impl Write for LimitedWriter { + fn write(&mut self, data: &[u8]) -> io::Result { + if self.buf.len().saturating_add(data.len()) > self.max_len { + return Err(io::Error::other(DecompressedSizeLimitExceeded)); + } + self.buf.extend_from_slice(data); + Ok(data.len()) + } + + fn flush(&mut self) -> io::Result<()> { + Ok(()) + } +} + +fn new_decompressor() -> GzDecoder { // Create the backing buffer for the decompressor and set the compression flag to false (0) and pre-allocate // the space for the length prefix, which we'll fill out once we've finalized the decompressor. let buf = vec![0; GRPC_MESSAGE_HEADER_LEN]; - GzDecoder::new(buf) + // Cap the decompressed output so a compression bomb on this unauthenticated gRPC listener + // cannot drive unbounded allocation. The buffer already holds the 5-byte header, so the sink + // may grow to the header plus the decompressed cap; anything larger errors mid-decompression. + GzDecoder::new(LimitedWriter::new( + buf, + GRPC_MESSAGE_HEADER_LEN.saturating_add(max_decompressed_size_bytes()), + )) } async fn drive_body_decompression( @@ -133,6 +196,19 @@ async fn drive_body_decompression( // decompressor incrementally because there's no good reason to make both the internal buffer and // the decompressor buffer expand if we don't have to. if is_compressed { + // Reject a compressed payload whose declared wire size could not + // legitimately decompress within the cap, before we buffer any of it. The + // bound (decompressed cap plus zlib's worst-case expansion, shared with the + // logstash source) keeps a peer from advertising a huge length and + // slow-streaming bytes to grow the decompressor's input buffer unbounded. + let compressed_frame_limit = max_zlib_compressed_frame_size_bytes() + .saturating_add(GRPC_COMPRESSED_FRAME_OVERHEAD_SLACK); + if message_len > compressed_frame_limit { + return Err(Status::out_of_range( + "compressed message length exceeds the maximum allowed size", + )); + } + // We skip the header in the buffer because it doesn't matter to the decompressor and we // recreate it anyways. buf.advance(GRPC_MESSAGE_HEADER_LEN); @@ -141,6 +217,15 @@ async fn drive_body_decompression( remaining: message_len, }; } else { + // Reject an identity (uncompressed) message larger than the cap before + // buffering it to `overall_len`, so a large declared length cannot drive + // unbounded buffering here ahead of tonic's own decode-size limit. + if message_len > max_decompressed_size_bytes() { + return Err(Status::out_of_range( + "message length exceeds the maximum allowed size", + )); + } + let overall_len = GRPC_MESSAGE_HEADER_LEN + message_len; state = State::Forward { overall_len }; } @@ -173,8 +258,11 @@ async fn drive_body_decompression( // asynchronously since we already have the data, and that's the only asynchronous part. let to_take = cmp::min(available, *remaining); let decompressor = decompressor.get_or_insert_with(new_decompressor); - if decompressor.write_all(&buf[..to_take]).is_err() { - return Err(Status::internal("failed to write to decompressor")); + if let Err(error) = decompressor.write_all(&buf[..to_take]) { + return Err(decompressor_error_to_status( + &error, + "failed to write to decompressor", + )); } *remaining -= to_take; @@ -188,13 +276,16 @@ async fn drive_body_decompression( let result = decompressor .take() .expect("consumed decompressor when no decompressor was present") - .finish(); - - // The only I/O errors that occur during `finish` should be I/O errors from writing to the internal - // buffer, but `Vec` is infallible in this regard, so this should be impossible without having - // first panicked due to memory exhaustion. - let mut buf = result.map_err(|_| { - Status::internal( + .finish() + .map(LimitedWriter::into_inner); + + // Decompression can fail here either because the payload exceeded the size + // cap (an oversized-request client fault) or, for malformed input, during + // finalization; map the former to `out_of_range` and treat anything else as + // an internal error. + let mut buf = result.map_err(|error| { + decompressor_error_to_status( + &error, "reached impossible error during decompressor finalization", ) })?; @@ -375,3 +466,147 @@ impl Layer for DecompressionAndMetricsLayer { } } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::sources::util::decompression::DEFAULT_MAX_DECOMPRESSED_SIZE_BYTES; + + fn gzip(payload: &[u8]) -> Vec { + use flate2::write::GzEncoder; + let mut encoder = GzEncoder::new(Vec::new(), flate2::Compression::best()); + encoder.write_all(payload).unwrap(); + encoder.finish().unwrap() + } + + #[test] + fn limited_writer_accepts_within_limit() { + let mut writer = LimitedWriter::new(Vec::new(), 8); + writer + .write_all(b"12345678") + .expect("exactly at the limit must be accepted"); + assert_eq!(writer.into_inner(), b"12345678"); + } + + #[test] + fn limited_writer_rejects_past_limit() { + let mut writer = LimitedWriter::new(Vec::new(), 8); + let error = writer + .write_all(b"123456789") + .expect_err("one byte past the limit must be rejected"); + + assert!( + is_decompressed_size_limit_error(&error), + "expected the size-limit marker, got {error}" + ); + } + + /// The cap must fire *during* decompression rather than after materialising the whole output, + /// which is the entire point of the `LimitedWriter` sink. + #[test] + fn gzip_decompressor_rejects_bomb_mid_stream() { + let bomb = gzip(&vec![0u8; 1024 * 1024]); + let mut decoder = GzDecoder::new(LimitedWriter::new(Vec::new(), 4096)); + + let error = decoder + .write_all(&bomb) + .expect_err("a payload inflating past the cap must be rejected"); + + assert!(is_decompressed_size_limit_error(&error)); + } + + #[test] + fn gzip_decompressor_passes_ordinary_payload() { + let payload = b"hello grpc"; + let mut decoder = GzDecoder::new(LimitedWriter::new(Vec::new(), 4096)); + decoder.write_all(&gzip(payload)).expect("must decompress"); + let out = decoder.finish().map(LimitedWriter::into_inner).unwrap(); + + assert_eq!(out, payload); + } + + /// An oversized payload is a client fault, so it must surface as `out_of_range` rather than + /// being reported as an internal server error. + #[test] + fn size_limit_maps_to_out_of_range_other_errors_to_internal() { + let limit_error = io::Error::other(DecompressedSizeLimitExceeded); + assert_eq!( + decompressor_error_to_status(&limit_error, "internal").code(), + tonic::Code::OutOfRange + ); + + let other = io::Error::other("some unrelated failure"); + assert_eq!( + decompressor_error_to_status(&other, "internal").code(), + tonic::Code::Internal + ); + } + + fn grpc_frame(compressed: bool, declared_len: u32) -> Body { + let mut frame = vec![u8::from(compressed)]; + frame.extend_from_slice(&declared_len.to_be_bytes()); + Body::from(frame) + } + + async fn drive(frame: Body) -> Result { + let (sender, _receiver) = Body::channel(); + drive_body_decompression(frame, sender).await + } + + /// A compressed frame declaring more bytes than could legitimately decompress within the cap + /// must be refused from its header alone, before any of the payload is buffered. + #[tokio::test] + async fn oversized_compressed_frame_length_is_rejected_from_the_header() { + let declared = u32::MAX; + assert!( + declared as usize + > max_zlib_compressed_frame_size_bytes() + .saturating_add(GRPC_COMPRESSED_FRAME_OVERHEAD_SLACK), + "the declared length must exceed the prefilter for this test to mean anything" + ); + + let status = drive(grpc_frame(true, declared)) + .await + .expect_err("an oversized declared length must be rejected"); + + assert_eq!(status.code(), tonic::Code::OutOfRange); + } + + /// The same guard is needed on the identity path: an uncompressed message declaring a huge + /// length would otherwise be buffered to `overall_len` before tonic's own limit applied. + #[tokio::test] + async fn oversized_identity_frame_length_is_rejected_from_the_header() { + let declared = u32::MAX; + assert!(declared as usize > max_decompressed_size_bytes()); + + let status = drive(grpc_frame(false, declared)) + .await + .expect_err("an oversized identity length must be rejected"); + + assert_eq!(status.code(), tonic::Code::OutOfRange); + } + + /// A legitimate declared length must not be refused by either guard — the frame simply waits + /// for its payload. + #[tokio::test] + async fn ordinary_frame_length_is_accepted() { + for compressed in [true, false] { + let result = drive(grpc_frame(compressed, 64)).await; + assert!( + result.is_ok(), + "a small declared length must pass the guards (compressed={compressed}), got {:?}", + result.err() + ); + } + } + + /// The new decompressor must carry the global cap, not an unbounded sink. + #[test] + fn new_decompressor_is_capped_at_the_global_limit() { + let decoder = new_decompressor(); + assert_eq!( + decoder.get_ref().max_len, + GRPC_MESSAGE_HEADER_LEN + DEFAULT_MAX_DECOMPRESSED_SIZE_BYTES + ); + } +} diff --git a/src/sources/util/http/encoding.rs b/src/sources/util/http/encoding.rs index b446107a70..1588976823 100644 --- a/src/sources/util/http/encoding.rs +++ b/src/sources/util/http/encoding.rs @@ -6,7 +6,7 @@ use warp::{filters::BoxedFilter, Filter}; use super::error::ErrorMessage; use crate::internal_events::HttpDecompressError; -use vector_common::decompression::{ +use crate::sources::util::decompression::{ is_decompressed_size_limit_error, max_decompressed_size_bytes, CappedDecoder, }; diff --git a/src/sources/util/http/mod.rs b/src/sources/util/http/mod.rs index 6419a92ae8..7d11f939d7 100644 --- a/src/sources/util/http/mod.rs +++ b/src/sources/util/http/mod.rs @@ -22,7 +22,12 @@ mod query; #[cfg(feature = "sources-utils-http-auth")] pub use auth::{HttpSourceAuth, HttpSourceAuthConfig}; -#[cfg(feature = "sources-splunk_hec")] +#[cfg(any( + feature = "sources-aws_kinesis_firehose", + feature = "sources-opentelemetry", + feature = "sources-splunk_hec", + feature = "sources-utils-http-prelude", +))] pub(crate) use encoding::capped_body; #[cfg(feature = "sources-utils-http-encoding")] pub use encoding::decode; diff --git a/src/sources/util/http/prelude.rs b/src/sources/util/http/prelude.rs index 50713b417b..376a594938 100644 --- a/src/sources/util/http/prelude.rs +++ b/src/sources/util/http/prelude.rs @@ -44,7 +44,7 @@ use crate::{ use super::{ auth::{HttpSourceAuth, HttpSourceAuthConfig}, - encoding::decode, + encoding::{capped_body, decode}, error::ErrorMessage, }; @@ -132,7 +132,7 @@ pub trait HttpSource: Clone + Send + Sync + 'static { .and(warp::header::optional::("authorization")) .and(warp::header::optional::("content-encoding")) .and(warp::header::headers_cloned()) - .and(warp::body::bytes()) + .and(capped_body()) .and(warp::query::>()) .and(warp::filters::ext::optional()) .and_then( diff --git a/src/sources/util/mod.rs b/src/sources/util/mod.rs index 155bdbf568..1ee5da3c6b 100644 --- a/src/sources/util/mod.rs +++ b/src/sources/util/mod.rs @@ -3,6 +3,7 @@ mod body_decoding; #[cfg(feature = "sources-vector")] pub mod jwt_auth; +pub mod decompression; mod encoding_config; #[cfg(all(unix, feature = "sources-dnstap"))] pub mod framestream; diff --git a/src/sources/vector/mod.rs b/src/sources/vector/mod.rs index 85e7fc6953..e0ff44d617 100644 --- a/src/sources/vector/mod.rs +++ b/src/sources/vector/mod.rs @@ -24,8 +24,8 @@ use crate::{ serde::bool_or_struct, sources::{ util::{ - add_auth_metadata, grpc::run_grpc_server, Auth, AuthConfig, AuthContext, AuthError, - AuthEventError, EventValidator, + add_auth_metadata, decompression::max_decompressed_size_bytes, grpc::run_grpc_server, + Auth, AuthConfig, AuthContext, AuthError, AuthEventError, EventValidator, }, Source, }, @@ -366,8 +366,10 @@ impl SourceConfig for VectorConfig { auth_metrics, }) .accept_compressed(tonic::codec::CompressionEncoding::Gzip) - // Tonic added a default of 4MB in 0.9. This replaces the old behavior. - .max_decoding_message_size(usize::MAX); + // Tonic added a default of 4MB in 0.9. Bound this by the global decompressed-size cap + // rather than `usize::MAX` so a single oversized message cannot drive unbounded + // allocation on this unauthenticated listener. + .max_decoding_message_size(max_decompressed_size_bytes()); let source = run_grpc_server(self.address, tls_settings, service, cx.shutdown).map_err(|error| { From 02a775c0f4c13df5b1acda8437778a107b49c8d7 Mon Sep 17 00:00:00 2001 From: Harshvardhan Shrivastava Date: Wed, 5 Aug 2026 21:26:36 +0530 Subject: [PATCH 3/6] Adding missed file --- src/sources/util/decompression.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 src/sources/util/decompression.rs diff --git a/src/sources/util/decompression.rs b/src/sources/util/decompression.rs new file mode 100644 index 0000000000..e8c96d17d3 --- /dev/null +++ b/src/sources/util/decompression.rs @@ -0,0 +1,11 @@ +//! Re-export of the shared decompression limits. +//! +//! The implementation lives in [`vector_common::decompression`] so that both the source crate and +//! `lib/codecs` can enforce the same global decompressed-size cap without duplicating it. This +//! module mirrors upstream's `crate::sources::util::decompression` import path. +pub use vector_common::decompression::{ + http_zstd_window_log_max, is_decompressed_size_limit_error, max_decompressed_size_bytes, + max_zlib_compressed_frame_size_bytes, max_zstd_window_log, set_max_decompressed_size_bytes, + zstd_window_log_max, CappedDecoder, CappedReader, DecompressedSizeLimitExceeded, + DEFAULT_MAX_DECOMPRESSED_SIZE_BYTES, HTTP_ZSTD_WINDOW_LOG_MAX, +}; From f4acdaa5c199ce404652196e1f989c18818104d9 Mon Sep 17 00:00:00 2001 From: Harshvardhan Shrivastava Date: Thu, 6 Aug 2026 16:52:21 +0530 Subject: [PATCH 4/6] OBE-10708,OBE-11233,fluent source: bound msgpack structure, entry count and connections --- src/sources/fluent/message.rs | 8 +- src/sources/fluent/mod.rs | 252 ++++++++++++++++++++++++-- src/sources/fluent/scan.rs | 323 ++++++++++++++++++++++++++++++++++ 3 files changed, 563 insertions(+), 20 deletions(-) create mode 100644 src/sources/fluent/scan.rs diff --git a/src/sources/fluent/message.rs b/src/sources/fluent/message.rs index 25c3c05890..597a4ef944 100644 --- a/src/sources/fluent/message.rs +++ b/src/sources/fluent/message.rs @@ -32,8 +32,12 @@ pub(super) enum FluentMessage { PackedForward(FluentTag, serde_bytes::ByteBuf), PackedForwardWithOptions(FluentTag, serde_bytes::ByteBuf, FluentMessageOptions), - // should be last as it'll match any other message - Heartbeat(rmpv::Value), // should be Nil if heartbeat + // Should be last, as an untagged variant matches whatever the earlier ones reject. + // + // Deliberately `()` rather than `rmpv::Value`: the Forward spec sends nil for a heartbeat, and + // typing it as nil means an unrecognised message is refused by serde instead of being + // materialised into an arbitrary, arbitrarily-nested value. + Heartbeat(()), } /// Server options sent by client. diff --git a/src/sources/fluent/mod.rs b/src/sources/fluent/mod.rs index 887ebc3ab3..1d8e50e096 100644 --- a/src/sources/fluent/mod.rs +++ b/src/sources/fluent/mod.rs @@ -35,7 +35,20 @@ use crate::{ }; mod message; +mod scan; use self::message::{FluentEntry, FluentMessage, FluentRecord, FluentTag, FluentTimestamp}; +use self::scan::{scan_msgpack_frame, MAX_MSGPACK_DEPTH}; + +/// Default ceiling on concurrent connections to the (unauthenticated) fluent listener. +const fn default_connection_limit() -> Option { + Some(1024) +} + +/// Maximum number of entries decoded from a single frame. +/// +/// A frame within the byte cap can still carry a very large number of tiny entries, each of which +/// becomes an `Event`; this bounds the burst that one frame can turn into. +const MAX_ENTRIES_PER_FRAME: usize = 100_000; /// Configuration for the `fluent` source. #[configurable_component(source("fluent", "Collect logs from a Fluentd or Fluent Bit agent."))] @@ -45,9 +58,21 @@ pub struct FluentConfig { address: SocketListenAddr, /// The maximum number of TCP connections that are allowed at any given time. + /// + /// Defaults to a finite value: the source is unauthenticated, so an unlimited connection + /// count lets a peer multiply any per-connection memory cost without bound. #[configurable(metadata(docs::type_unit = "connections"))] + #[serde(default = "default_connection_limit")] connection_limit: Option, + /// The maximum size, in bytes, of a single MessagePack frame buffered while waiting for a + /// complete message. + /// + /// Defaults to the global `--max-decompressed-size-bytes` limit. + #[configurable(metadata(docs::type_unit = "bytes"))] + #[serde(default, skip_serializing_if = "vector_lib::serde::is_default")] + max_frame_bytes: Option, + #[configurable(derived)] keepalive: Option, @@ -83,7 +108,8 @@ impl GenerateConfig for FluentConfig { tls: None, receive_buffer_bytes: None, acknowledgements: Default::default(), - connection_limit: Some(2), + connection_limit: default_connection_limit(), + max_frame_bytes: None, log_namespace: None, }) .unwrap() @@ -95,7 +121,7 @@ impl GenerateConfig for FluentConfig { impl SourceConfig for FluentConfig { async fn build(&self, cx: SourceContext) -> crate::Result { let log_namespace = cx.log_namespace(self.log_namespace); - let source = FluentSource::new(log_namespace); + let source = FluentSource::new(log_namespace, self.max_frame_bytes); let shutdown_secs = Duration::from_secs(30); let tls_config = self.tls.as_ref().map(|tls| tls.tls_config.clone()); let tls_client_metadata_key = self @@ -213,13 +239,15 @@ impl FluentConfig { struct FluentSource { log_namespace: LogNamespace, legacy_host_key_path: Option, + max_frame_bytes: Option, } impl FluentSource { - fn new(log_namespace: LogNamespace) -> Self { + fn new(log_namespace: LogNamespace, max_frame_bytes: Option) -> Self { Self { log_namespace, legacy_host_key_path: log_schema().host_key().cloned(), + max_frame_bytes, } } } @@ -231,7 +259,7 @@ impl TcpSource for FluentSource { type Acker = FluentAcker; fn decoder(&self) -> Self::Decoder { - FluentDecoder::new(self.log_namespace) + FluentDecoder::new(self.log_namespace, self.max_frame_bytes) } fn handle_events(&self, events: &mut [Event], host: SocketAddr) { @@ -263,7 +291,6 @@ pub enum DecodeError { IO(io::Error), Decode(decode::Error), UnknownCompression(String), - UnexpectedValue(rmpv::Value), /// The buffered frame grew past the maximum allowed size before a complete message could be /// decoded. Bounds memory when a peer declares an oversized msgpack array/map/string and /// streams the bytes to force unbounded buffering. @@ -271,6 +298,27 @@ pub enum DecodeError { size: usize, max: usize, }, + /// The frame nests deeper than `rmp_serde` can safely recurse over. Nesting costs one byte per + /// level, so a byte-size cap cannot bound it. + FrameTooDeep { + depth: usize, + max: usize, + }, + /// The frame declares a string/binary length or element count that no frame within the size + /// cap could satisfy. + DeclaredLengthTooLarge { + len: usize, + max: usize, + }, + /// A marker byte that is never valid MessagePack. + InvalidMarker { + marker: u8, + }, + /// One frame decoded into more entries than a single frame is allowed to produce. + TooManyEntries { + count: usize, + max: usize, + }, } impl std::fmt::Display for DecodeError { @@ -281,9 +329,6 @@ impl std::fmt::Display for DecodeError { DecodeError::UnknownCompression(compression) => { write!(f, "unknown compression: {}", compression) } - DecodeError::UnexpectedValue(value) => { - write!(f, "unexpected msgpack value, ignoring: {}", value) - } DecodeError::FrameTooLarge { size, max } => { write!( f, @@ -291,6 +336,30 @@ impl std::fmt::Display for DecodeError { size, max ) } + DecodeError::FrameTooDeep { depth, max } => { + write!( + f, + "fluent frame nests too deeply: depth {} exceeds limit of {}", + depth, max + ) + } + DecodeError::DeclaredLengthTooLarge { len, max } => { + write!( + f, + "fluent frame declares a length of {} bytes, beyond the {} byte limit", + len, max + ) + } + DecodeError::InvalidMarker { marker } => { + write!(f, "invalid msgpack marker byte {:#04x}", marker) + } + DecodeError::TooManyEntries { count, max } => { + write!( + f, + "fluent frame decodes to {} entries, beyond the limit of {}", + count, max + ) + } } } } @@ -301,10 +370,13 @@ impl StreamDecodingError for DecodeError { DecodeError::IO(_) => false, DecodeError::Decode(_) => true, DecodeError::UnknownCompression(_) => true, - DecodeError::UnexpectedValue(_) => true, - // An oversized partial frame has no framing boundary to resync on, so the connection - // must be dropped rather than re-decoded in a loop. - DecodeError::FrameTooLarge { .. } => false, + // A structurally hostile or oversized partial frame has no framing boundary to + // resync on, so the connection must be dropped rather than re-decoded in a loop. + DecodeError::FrameTooLarge { .. } + | DecodeError::FrameTooDeep { .. } + | DecodeError::DeclaredLengthTooLarge { .. } + | DecodeError::InvalidMarker { .. } + | DecodeError::TooManyEntries { .. } => false, } } } @@ -331,13 +403,25 @@ struct FluentDecoder { } impl FluentDecoder { - fn new(log_namespace: LogNamespace) -> Self { + fn new(log_namespace: LogNamespace, max_frame_bytes: Option) -> Self { Self { log_namespace, - max_frame_size: max_decompressed_size_bytes(), + max_frame_size: max_frame_bytes.unwrap_or_else(max_decompressed_size_bytes), } } + /// Bounds how many events one frame may expand into. A frame within the byte cap can still + /// carry a very large number of tiny entries. + fn ensure_entry_count(count: usize) -> Result<(), DecodeError> { + if count > MAX_ENTRIES_PER_FRAME { + return Err(DecodeError::TooManyEntries { + count, + max: MAX_ENTRIES_PER_FRAME, + }); + } + Ok(()) + } + fn handle_message( &mut self, message: Result, @@ -373,6 +457,7 @@ impl FluentDecoder { Ok(Some((frame, byte_size))) } FluentMessage::Forward(tag, entries) => { + Self::ensure_entry_count(entries.len())?; let events = entries .into_iter() .map(|FluentEntry(timestamp, record)| { @@ -391,6 +476,7 @@ impl FluentDecoder { Ok(Some((frame, byte_size))) } FluentMessage::ForwardWithOptions(tag, entries, options) => { + Self::ensure_entry_count(entries.len())?; let events = entries .into_iter() .map(|FluentEntry(timestamp, record)| { @@ -415,6 +501,7 @@ impl FluentDecoder { while let Some(FluentEntry(timestamp, record)) = FluentEntryStreamDecoder.decode(&mut buf)? { + Self::ensure_entry_count(events.len() + 1)?; events.push(Event::from(FluentEvent { tag: tag.clone(), timestamp, @@ -443,6 +530,7 @@ impl FluentDecoder { while let Some(FluentEntry(timestamp, record)) = FluentEntryStreamDecoder.decode(&mut buf)? { + Self::ensure_entry_count(events.len() + 1)?; events.push(Event::from(FluentEvent { tag: tag.clone(), timestamp, @@ -456,8 +544,7 @@ impl FluentDecoder { }; Ok(Some((frame, byte_size))) } - FluentMessage::Heartbeat(rmpv::Value::Nil) => Ok(None), - FluentMessage::Heartbeat(value) => Err(DecodeError::UnexpectedValue(value)), + FluentMessage::Heartbeat(()) => Ok(None), } } } @@ -472,6 +559,12 @@ impl Decoder for FluentDecoder { return Ok(None); } + // Reject structurally hostile frames before `rmp_serde` recurses over them. Nesting + // costs one byte per level on the wire, so `max_frame_size` cannot bound recursion + // depth on its own. Truncation is not an error here: the `UnexpectedEof` path below + // still asks for more bytes. + scan_msgpack_frame(&src[..], MAX_MSGPACK_DEPTH, self.max_frame_size)?; + let (byte_size, res) = { let mut des = Deserializer::new(io::Cursor::new(&src[..])); @@ -530,6 +623,10 @@ impl Decoder for FluentEntryStreamDecoder { if src.is_empty() { return Ok(None); } + + // The entries inside a `PackedForward` payload are attacker-controlled too — the gzip cap + // bounds their size but not their nesting depth. + scan_msgpack_frame(&src[..], MAX_MSGPACK_DEPTH, max_decompressed_size_bytes())?; let (byte_size, res) = { let mut des = Deserializer::new(io::Cursor::new(&src[..])); @@ -890,7 +987,7 @@ mod tests { // 4 bytes provided: a valid, incomplete frame. let partial: Vec = vec![0x92, 0xb0, b't', b'a', b'g']; let mut buf = BytesMut::from(&partial[..]); - let mut decoder = FluentDecoder::new(LogNamespace::default()); + let mut decoder = FluentDecoder::new(LogNamespace::default(), None); assert!(matches!(decoder.decode(&mut buf), Ok(None))); // The buffer is retained so more bytes can complete the frame. @@ -927,6 +1024,121 @@ mod tests { assert!(!error.can_continue()); } + /// OBE-11233: the report asks for a per-source frame cap rather than only a global one. + #[test] + fn max_frame_bytes_config_overrides_the_global_cap() { + let decoder = FluentDecoder::new(LogNamespace::default(), Some(4096)); + assert_eq!(decoder.max_frame_size, 4096); + + let default = FluentDecoder::new(LogNamespace::default(), None); + assert_eq!(default.max_frame_size, max_decompressed_size_bytes()); + } + + /// OBE-11233: the listener is unauthenticated, so an unlimited connection count multiplies + /// every per-connection cost. + #[test] + fn connection_limit_defaults_to_a_finite_value() { + let config: FluentConfig = toml::from_str(r#"address = "0.0.0.0:24224""#).unwrap(); + assert_eq!(config.connection_limit, default_connection_limit()); + assert!(config.connection_limit.is_some()); + } + + /// OBE-11233: a frame within the byte cap can still carry a huge number of tiny entries. + #[test] + fn entry_count_beyond_the_limit_is_rejected() { + let error = FluentDecoder::ensure_entry_count(MAX_ENTRIES_PER_FRAME + 1) + .expect_err("a frame decoding to too many entries must be rejected"); + + assert!(matches!(error, DecodeError::TooManyEntries { .. })); + assert!(!error.can_continue()); + } + + #[test] + fn entry_count_within_the_limit_is_accepted() { + FluentDecoder::ensure_entry_count(MAX_ENTRIES_PER_FRAME) + .expect("a frame at the limit must be accepted"); + } + + /// A nil heartbeat is the documented Forward-protocol keepalive and must still be accepted. + #[test] + fn nil_heartbeat_is_accepted() { + let mut buf = BytesMut::from(&[0xc0u8][..]); // msgpack nil + let mut decoder = FluentDecoder::new(LogNamespace::default(), None); + + assert!( + matches!(decoder.decode(&mut buf), Ok(None)), + "a nil heartbeat must be consumed without producing an event" + ); + assert!(buf.is_empty(), "the heartbeat byte must be consumed"); + } + + /// OBE-11233: the catch-all used to be `rmpv::Value`, so any unrecognised message was + /// materialised into an arbitrary value. It is now typed as nil, so serde refuses the message + /// instead — and the failure must stay recoverable, since an unknown message shape from an + /// otherwise well-behaved client is not a reason to drop the connection. + #[test] + fn unrecognised_message_is_refused_without_materialising_it() { + // A bare integer matches no variant: not a heartbeat, not a tagged message. + let mut buf = BytesMut::from(&[0x2au8][..]); + let mut decoder = FluentDecoder::new(LogNamespace::default(), None); + + let error = match decoder.decode(&mut buf) { + Err(error) => error, + Ok(_) => panic!("expected a decode error, got Ok"), + }; + + assert!( + error.can_continue(), + "an unknown message shape must not drop the connection" + ); + } + + /// OBE-10708: a deeply nested frame must be refused by our pre-scan *before* `rmp_serde` + /// sees it. + /// + /// This cannot be written as a test of the library's own behaviour: OBE-11233 claims + /// rmp-serde/rmpv "provide a recursion-depth guard (MAX_DEPTH=128)", but that is wrong on two + /// counts. rmp-serde 1.3.0 defaults to 1024, not 128, and measurement shows the guard does not + /// fire on the `rmpv::Value` path at all — deserialising 2,000 nesting levels directly + /// overflows the stack and aborts the process rather than returning `DepthLimitExceeded`. + /// Our scan is therefore the only thing standing between this input and a crash, and the + /// assertion below is safe precisely because the scan runs first. + #[test] + fn deeply_nested_frame_is_rejected_before_rmp_serde_recurses() { + // 0x91 is a one-element array, so each byte adds a nesting level. 2,000 levels is enough + // to abort the process if it ever reaches `rmp_serde`. + let mut buf = BytesMut::from(&vec![0x91u8; 2_000][..]); + let mut decoder = FluentDecoder::new(LogNamespace::default(), None); + + let error = match decoder.decode(&mut buf) { + Err(error) => error, + Ok(_) => panic!("expected FrameTooDeep, got Ok"), + }; + + assert!( + matches!(error, DecodeError::FrameTooDeep { .. }), + "unexpected error: {error:?}" + ); + assert!( + !error.can_continue(), + "an over-deep frame must drop the connection" + ); + } + + /// The same guard must protect the inner entry stream, whose contents come from a decompressed + /// `PackedForward` payload and are equally untrusted. + #[test] + fn deeply_nested_inner_entry_is_rejected() { + let mut buf = BytesMut::from(&vec![0x91u8; 2_000][..]); + + let error = match FluentEntryStreamDecoder.decode(&mut buf) { + Err(error) => error, + Ok(_) => panic!("expected FrameTooDeep, got Ok"), + }; + + assert!(matches!(error, DecodeError::FrameTooDeep { .. })); + } + /// OBE-11233 / OBE-10708: `CompressedPackedForward` inflated the client's gzip payload with an /// unbounded `read_to_end`, so a small frame could drive an arbitrarily large allocation. /// @@ -970,7 +1182,7 @@ mod tests { fn decode_all(message: Vec) -> Result<(SmallVec<[Event; 1]>, usize), DecodeError> { let mut buf = BytesMut::from(&message[..]); - let mut decoder = FluentDecoder::new(LogNamespace::default()); + let mut decoder = FluentDecoder::new(LogNamespace::default(), None); let (frame, byte_size) = decoder.decode(&mut buf)?.unwrap(); Ok((frame.into(), byte_size)) @@ -1022,6 +1234,7 @@ mod tests { receive_buffer_bytes: None, acknowledgements: true.into(), connection_limit: None, + max_frame_bytes: None, log_namespace: None, } .build(SourceContext::new_test(sender, None)) @@ -1087,6 +1300,7 @@ mod tests { receive_buffer_bytes: None, acknowledgements: false.into(), connection_limit: None, + max_frame_bytes: None, log_namespace: Some(true), }; @@ -1143,6 +1357,7 @@ mod tests { receive_buffer_bytes: None, acknowledgements: false.into(), connection_limit: None, + max_frame_bytes: None, log_namespace: None, }; @@ -1365,6 +1580,7 @@ mod integration_tests { receive_buffer_bytes: None, acknowledgements: false.into(), connection_limit: None, + max_frame_bytes: None, log_namespace: None, } .build(SourceContext::new_test(sender, None)) diff --git a/src/sources/fluent/scan.rs b/src/sources/fluent/scan.rs new file mode 100644 index 0000000000..4fa7b73983 --- /dev/null +++ b/src/sources/fluent/scan.rs @@ -0,0 +1,323 @@ +//! Structural pre-scan of an untrusted MessagePack frame. +//! +//! `rmp_serde` deserialises nested MessagePack by recursing, and the `fluent` source hands it +//! attacker-controlled bytes. Nesting costs one byte per level on the wire (`0x91` for a +//! one-element array), so a small frame can drive hundreds of thousands of stack frames and +//! overflow the stack — a byte-size cap such as +//! [`FluentDecoder::max_frame_size`](super::FluentDecoder) cannot defend against it. +//! +//! Recursion is reachable through more than one path: `FluentRecord` values are `rmpv::Value`, +//! the `Heartbeat` variant is a bare `rmpv::Value`, `#[serde(untagged)]` buffers input into +//! serde's own recursive `Content` type before any variant is chosen, and converting the result +//! into a VRL `Value` (and later dropping it) recurses again. Bounding depth here, at admission, +//! bounds all of them at once. +//! +//! The scan is deliberately **iterative**: a recursive scanner would reintroduce the very bug it +//! exists to prevent. + +use super::DecodeError; + +/// Maximum MessagePack nesting depth accepted from a peer. +/// +/// Fluent records are shallow in practice — a tag, a timestamp and a flat map of fields. This +/// leaves generous headroom for nested objects while keeping recursion far below any stack limit. +pub(super) const MAX_MSGPACK_DEPTH: usize = 128; + +/// Walks the MessagePack structure in `buf` without recursing, rejecting frames that are nested +/// too deeply or that declare a length no legitimate frame could satisfy. +/// +/// A truncated buffer is **not** an error: the caller is a streaming decoder, so an incomplete +/// prefix simply means more bytes are needed and the scan stops early. Only structural violations +/// are reported. +/// +/// `max_len` bounds any single declared length (string, binary, ext) and any declared element +/// count. A container needs at least one byte per element, so a count beyond `max_len` can never +/// be satisfied within a frame that size — rejecting it up front also keeps this scan cheap. +pub(super) fn scan_msgpack_frame( + buf: &[u8], + max_depth: usize, + max_len: usize, +) -> Result<(), DecodeError> { + // Remaining element count at each open container level; the initial entry is the single + // top-level value. + let mut stack: Vec = vec![1]; + let mut pos: usize = 0; + + // Reads `n` bytes as a big-endian length, or signals truncation. + fn read_len(buf: &[u8], pos: usize, n: usize) -> Option { + let bytes = buf.get(pos..pos + n)?; + let mut value: u64 = 0; + for byte in bytes { + value = (value << 8) | u64::from(*byte); + } + usize::try_from(value).ok() + } + + let too_large = |len: usize| DecodeError::DeclaredLengthTooLarge { len, max: max_len }; + + while let Some(remaining) = stack.last_mut() { + if *remaining == 0 { + stack.pop(); + continue; + } + *remaining -= 1; + + let Some(&marker) = buf.get(pos) else { + // Truncated: the caller needs more bytes. + return Ok(()); + }; + pos += 1; + + // `payload` is a byte count to skip; `children` is a count of nested values to expect. + let (payload, children) = match marker { + // fixint (positive and negative), nil, false, true + 0x00..=0x7f | 0xc0 | 0xc2 | 0xc3 | 0xe0..=0xff => (0, 0), + // never used + 0xc1 => return Err(DecodeError::InvalidMarker { marker }), + 0x80..=0x8f => (0, 2 * usize::from(marker & 0x0f)), // fixmap + 0x90..=0x9f => (0, usize::from(marker & 0x0f)), // fixarray + 0xa0..=0xbf => (usize::from(marker & 0x1f), 0), // fixstr + 0xcc | 0xd0 => (1, 0), + 0xcd | 0xd1 => (2, 0), + 0xca | 0xce | 0xd2 => (4, 0), + 0xcb | 0xcf | 0xd3 => (8, 0), + 0xd4 => (2, 0), // fixext1 (type + 1) + 0xd5 => (3, 0), // fixext2 + 0xd6 => (5, 0), // fixext4 + 0xd7 => (9, 0), // fixext8 + 0xd8 => (17, 0), // fixext16 + // bin / str with an explicit length + 0xc4 | 0xd9 | 0xc5 | 0xda | 0xc6 | 0xdb => { + let width = match marker { + 0xc4 | 0xd9 => 1, + 0xc5 | 0xda => 2, + _ => 4, + }; + let Some(len) = read_len(buf, pos, width) else { + return Ok(()); + }; + if len > max_len { + return Err(too_large(len)); + } + pos += width; + (len, 0) + } + // ext with an explicit length (payload carries a one-byte type tag) + 0xc7 | 0xc8 | 0xc9 => { + let width = match marker { + 0xc7 => 1, + 0xc8 => 2, + _ => 4, + }; + let Some(len) = read_len(buf, pos, width) else { + return Ok(()); + }; + if len > max_len { + return Err(too_large(len)); + } + pos += width; + (len.saturating_add(1), 0) + } + // array / map with an explicit element count + 0xdc | 0xdd | 0xde | 0xdf => { + let width = if matches!(marker, 0xdc | 0xde) { 2 } else { 4 }; + let Some(count) = read_len(buf, pos, width) else { + return Ok(()); + }; + if count > max_len { + return Err(too_large(count)); + } + pos += width; + let children = if matches!(marker, 0xde | 0xdf) { + count.saturating_mul(2) + } else { + count + }; + (0, children) + } + }; + + if payload > 0 { + match pos.checked_add(payload) { + Some(next) if next <= buf.len() => pos = next, + // Truncated, or a length that overflows the buffer: need more bytes. + _ => return Ok(()), + } + } + + if children > 0 { + if stack.len() >= max_depth { + return Err(DecodeError::FrameTooDeep { + depth: stack.len() + 1, + max: max_depth, + }); + } + stack.push(children); + } + } + + Ok(()) +} + +#[cfg(test)] +mod tests { + use vector_lib::codecs::StreamDecodingError; + + use super::*; + + const MAX_LEN: usize = 1024 * 1024; + + fn scan(buf: &[u8]) -> Result<(), DecodeError> { + scan_msgpack_frame(buf, MAX_MSGPACK_DEPTH, MAX_LEN) + } + + /// `0x91` is a one-element array, so each byte adds a nesting level. + fn nested(levels: usize) -> Vec { + let mut buf = vec![0x91; levels]; + buf.push(0xc0); // nil at the centre + buf + } + + // ---- accept: ordinary frames must be unaffected ---- + + #[test] + fn accepts_a_typical_fluent_frame() { + // ["tag", 1441588984, {"message": "foo"}] + let frame = rmp_serde::to_vec(&( + "tag.name", + 1_441_588_984u32, + std::collections::BTreeMap::from([("message", "foo")]), + )) + .unwrap(); + + scan(&frame).expect("an ordinary fluent frame must be accepted"); + } + + #[test] + fn accepts_nesting_just_below_the_limit() { + scan(&nested(MAX_MSGPACK_DEPTH - 1)).expect("nesting within the limit must be accepted"); + } + + #[test] + fn accepts_every_scalar_marker() { + for frame in [ + vec![0xc0], // nil + vec![0xc2], // false + vec![0xc3], // true + vec![0x7f], // positive fixint + vec![0xff], // negative fixint + vec![0xcc, 0x01], // uint8 + vec![0xcd, 0x00, 0x01], // uint16 + vec![0xce, 0, 0, 0, 1], // uint32 + vec![0xcf, 0, 0, 0, 0, 0, 0, 0, 1], // uint64 + vec![0xcb, 0, 0, 0, 0, 0, 0, 0, 0], // float64 + vec![0xa3, b'f', b'o', b'o'], // fixstr + vec![0xc4, 0x02, 0xaa, 0xbb], // bin8 + vec![0xd4, 0x00, 0x01], // fixext1 + vec![0xc7, 0x01, 0x00, 0xaa], // ext8 + ] { + scan(&frame).unwrap_or_else(|e| panic!("marker {:#04x} rejected: {e}", frame[0])); + } + } + + /// A streaming decoder feeds partial frames constantly; truncation must never be an error. + #[test] + fn truncation_is_not_an_error() { + let frame = rmp_serde::to_vec(&("tag.name", 1u32, "payload")).unwrap(); + for cut in 0..frame.len() { + scan(&frame[..cut]) + .unwrap_or_else(|e| panic!("truncation at {cut} must not error, got {e}")); + } + } + + /// A declared length larger than the buffer is truncation, not a violation, so long as it + /// stays within the cap — the rest of the frame may still be in flight. + #[test] + fn declared_length_within_the_cap_but_not_yet_arrived_is_truncation() { + // bin32 declaring 4096 bytes, none of which have arrived. + let frame = vec![0xc6, 0x00, 0x00, 0x10, 0x00]; + scan(&frame).expect("a legitimate declared length awaiting bytes must not error"); + } + + // ---- reject: the two vectors this scan exists for ---- + + /// OBE-10708: one byte per nesting level, so a byte-size cap cannot bound recursion depth. + #[test] + fn rejects_nesting_past_the_limit() { + let error = scan(&nested(MAX_MSGPACK_DEPTH + 1)) + .expect_err("nesting past the limit must be rejected"); + + assert!( + matches!(error, DecodeError::FrameTooDeep { max, .. } if max == MAX_MSGPACK_DEPTH), + "unexpected error: {error:?}" + ); + assert!( + !error.can_continue(), + "an over-deep frame must drop the connection" + ); + } + + /// The depth guard must hold for maps as well as arrays. + #[test] + fn rejects_deep_map_nesting() { + // 0x81 is a one-pair fixmap: key, then a nested map as the value. + let mut frame = Vec::new(); + for _ in 0..=MAX_MSGPACK_DEPTH { + frame.push(0x81); + frame.push(0xc0); // nil key + } + frame.push(0xc0); + + let error = scan(&frame).expect_err("deep map nesting must be rejected"); + assert!(matches!(error, DecodeError::FrameTooDeep { .. })); + } + + /// OBE-11233: a declared length no frame could satisfy is refused up front, so the claim + /// cannot resurface if a dependency bump changes how `rmp_serde` pre-allocates. + #[test] + fn rejects_declared_length_beyond_the_cap() { + // bin32 declaring ~4 GiB. + let frame = vec![0xc6, 0xff, 0xff, 0xff, 0xff]; + + let error = scan(&frame).expect_err("an impossible declared length must be rejected"); + assert!( + matches!(error, DecodeError::DeclaredLengthTooLarge { max, .. } if max == MAX_LEN), + "unexpected error: {error:?}" + ); + assert!(!error.can_continue()); + } + + #[test] + fn rejects_declared_element_count_beyond_the_cap() { + // array32 declaring ~4 billion elements. + let frame = vec![0xdd, 0xff, 0xff, 0xff, 0xff]; + + let error = scan(&frame).expect_err("an impossible element count must be rejected"); + assert!(matches!(error, DecodeError::DeclaredLengthTooLarge { .. })); + } + + #[test] + fn rejects_str32_and_map32_beyond_the_cap() { + for frame in [ + vec![0xdb, 0xff, 0xff, 0xff, 0xff], // str32 + vec![0xdf, 0xff, 0xff, 0xff, 0xff], // map32 + ] { + let error = scan(&frame).expect_err("an impossible declared length must be rejected"); + assert!(matches!(error, DecodeError::DeclaredLengthTooLarge { .. })); + } + } + + #[test] + fn rejects_the_never_used_marker() { + let error = scan(&[0xc1]).expect_err("0xc1 is never valid msgpack"); + assert!(matches!(error, DecodeError::InvalidMarker { marker: 0xc1 })); + } + + /// The scan must not itself recurse, or it reintroduces the bug it prevents. A frame far + /// deeper than any stack could handle must return an error rather than crash the process. + #[test] + fn scanning_is_iterative_and_survives_pathological_depth() { + let error = scan(&nested(5_000_000)).expect_err("must be rejected, not overflow the stack"); + assert!(matches!(error, DecodeError::FrameTooDeep { .. })); + } +} From 1aad6b314ae5e9a53fd77a8eab3ed53dd8a50272 Mon Sep 17 00:00:00 2001 From: Harshvardhan Shrivastava Date: Fri, 7 Aug 2026 00:20:31 +0530 Subject: [PATCH 5/6] sinks: route remaining decompression through the CappedDecoder --- Cargo.toml | 4 ++-- clippy.toml | 9 +++++++ lib/vector-common/src/decompression.rs | 16 ++++++++++--- lib/vector-core/src/sink/compressor.rs | 18 ++++---------- lib/vector-core/src/sink/zstd.rs | 13 ++++++---- lib/vector-core/src/test_util.rs | 24 +++++++++---------- src/sinks/aws_s3/integration_tests.rs | 11 +++++---- src/sinks/azure_blob/integration_tests.rs | 4 ++-- src/sinks/datadog/metrics/encoder.rs | 17 +++++-------- .../datadog/metrics/integration_tests.rs | 7 ++---- .../traces/apm_stats/integration_tests.rs | 9 ++++--- src/sinks/http/tests.rs | 9 +++---- src/sinks/prometheus/exporter.rs | 8 +++---- src/sinks/util/buffer/mod.rs | 17 ++++--------- src/sinks/util/test.rs | 12 +++++++--- src/sources/util/http/mod.rs | 1 + tests/e2e/datadog/metrics/mod.rs | 8 ++----- 17 files changed, 96 insertions(+), 91 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index b9d5ce0af4..377ae09766 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -494,7 +494,7 @@ vrl.workspace = true rustls = "0.23" tokio-rustls = "0.26" wiremock = "0.6.2" -zstd = { version = "0.13.0", default-features = false } +zstd.workspace = true [patch.crates-io] # The upgrade for `tokio-util` >= 0.6.9 is blocked on https://github.com/vectordotdev/vector/issues/11257. @@ -658,7 +658,7 @@ sources-aws_ecs_metrics = ["sources-utils-http-client"] sources-aws_kinesis_firehose = ["dep:base64", "sources-utils-http-encoding"] sources-aws_s3 = ["aws-core", "dep:aws-sdk-sqs", "dep:aws-sdk-s3", "dep:semver", "dep:async-compression", "sources-aws_sqs", "tokio-util/io"] sources-aws_sqs = ["aws-core", "dep:aws-sdk-sqs"] -sources-datadog_agent = ["sources-utils-http-error", "protobuf-build", "dep:prost"] +sources-datadog_agent = ["sources-utils-http-encoding", "sources-utils-http-error", "protobuf-build", "dep:prost"] sources-demo_logs = ["dep:fakedata"] sources-dnstap = ["sources-utils-net-tcp", "dep:base64", "dep:hickory-proto", "dep:dnsmsg-parser", "dep:dnstap-parser", "protobuf-build", "dep:prost"] sources-docker_logs = ["docker"] diff --git a/clippy.toml b/clippy.toml index 25937ac641..13140480d1 100644 --- a/clippy.toml +++ b/clippy.toml @@ -4,6 +4,10 @@ cognitive-complexity-threshold = 75 # https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_method disallowed-methods = [ { path = "std::io::Write::write", reason = "This doesn't handle short writes, use `write_all` instead." }, + { path = "zstd::stream::copy_decode", reason = "Use `vector_common::decompression::CappedDecoder::zstd` (or `CappedDecoder::zstd_http` in HTTP contexts) to enforce the decompression size cap." }, + { path = "zstd::stream::decode_all", reason = "Use `vector_common::decompression::CappedDecoder::zstd` (or `CappedDecoder::zstd_http` in HTTP contexts) to enforce the decompression size cap." }, + { path = "zstd::bulk::decompress", reason = "Use `vector_common::decompression::CappedDecoder::zstd` (or `CappedDecoder::zstd_http` in HTTP contexts) to enforce the decompression size cap." }, + { path = "warp::body::bytes", reason = "Reads the whole request body into memory unbounded. Use `crate::sources::util::http::capped_body()` to cap the compressed body size at the global decompressed-size limit." }, ] disallowed-types = [ @@ -11,4 +15,9 @@ disallowed-types = [ { path = "once_cell::unsync::OnceCell", reason = "Use `std::cell::OnceCell` instead." }, { path = "once_cell::sync::Lazy", reason = "Use `std::sync::LazyLock` instead." }, { path = "once_cell::unsync::Lazy", reason = "Use `std::sync::LazyCell` instead." }, + { path = "flate2::read::GzDecoder", reason = "Use `vector_common::decompression::CappedDecoder::gzip` to enforce the decompression size cap." }, + { path = "flate2::read::MultiGzDecoder", reason = "Use `vector_common::decompression::CappedDecoder::gzip` to enforce the decompression size cap." }, + { path = "flate2::read::ZlibDecoder", reason = "Use `vector_common::decompression::CappedDecoder::zlib` to enforce the decompression size cap." }, + { path = "flate2::read::DeflateDecoder", reason = "Use `vector_common::decompression::CappedDecoder` for decompression with the size cap." }, + { path = "zstd::stream::read::Decoder", reason = "Use `vector_common::decompression::CappedDecoder::zstd` (or `CappedDecoder::zstd_http` in HTTP contexts) to enforce the decompression size cap." }, ] diff --git a/lib/vector-common/src/decompression.rs b/lib/vector-common/src/decompression.rs index e0fe9ea452..bf1873bf51 100644 --- a/lib/vector-common/src/decompression.rs +++ b/lib/vector-common/src/decompression.rs @@ -19,9 +19,11 @@ //! The constructors enforce the global decompressed-size cap so that a compression bomb cannot //! drive unbounded allocation. -// Raw decoder types (flate2 / zstd) should only be constructed here, in the module that wraps -// them safely. Once every source is migrated off the raw types this can be enforced with a -// `clippy.toml` `disallowed-types` entry. +// Raw decoder types (flate2 / zstd) are only allowed in this module, which wraps them safely. +#![expect( + clippy::disallowed_types, + reason = "this module implements CappedDecoder, the safe wrapper around raw decoders; raw types may only appear here" +)] use std::{ fmt, @@ -249,6 +251,14 @@ pub struct CappedReader { consumed: usize, } +/// The reader type produced by [`CappedDecoder::zstd`] and friends via +/// [`CappedDecoder::into_reader`]. +/// +/// Naming this type otherwise requires spelling the raw `zstd` decoder, which the +/// `disallowed-types` lint forbids outside this module. Store this alias instead of the raw type +/// when a struct needs to hold a capped zstd reader. +pub type CappedZstdReader = CappedReader>>; + impl Read for CappedReader { fn read(&mut self, buf: &mut [u8]) -> io::Result { // The underlying reader is bounded one byte past the cap, so reading beyond `limit` is the diff --git a/lib/vector-core/src/sink/compressor.rs b/lib/vector-core/src/sink/compressor.rs index 71e45a0cc0..f9f82f55da 100644 --- a/lib/vector-core/src/sink/compressor.rs +++ b/lib/vector-core/src/sink/compressor.rs @@ -4,7 +4,7 @@ use std::{io, io::Read, io::BufWriter}; use bytes::{BufMut, BytesMut, Bytes, Buf}; use flate2::write::{GzEncoder, ZlibEncoder}; -use flate2::read::{GzDecoder, ZlibDecoder}; +use vector_common::decompression::CappedDecoder; use crate::sink::compression::Compression; use super::{ snappy::SnappyEncoder, snappy::SnappyDecoder, @@ -206,18 +206,10 @@ impl Decompressor { pub fn decompress(&self, bytes: Bytes) -> io::Result { match self { Decompressor::Plain => Ok(bytes), - Decompressor::Gzip => { - let mut decoder = GzDecoder::new(io::Cursor::new(bytes)); - let mut buff: Vec = Vec::with_capacity(OUTPUT_BUFFER_CAPACITY); - Read::read_to_end(&mut decoder, &mut buff)?; - Ok(buff.into()) - }, - Decompressor::Zlib => { - let mut decoder = ZlibDecoder::new(bytes.reader()); - let mut buff: Vec = Vec::with_capacity(OUTPUT_BUFFER_CAPACITY); - Read::read_to_end(&mut decoder, &mut buff)?; - Ok(buff.into()) - }, + Decompressor::Gzip => Ok(CappedDecoder::gzip(io::Cursor::new(bytes)) + .decompress()? + .into()), + Decompressor::Zlib => Ok(CappedDecoder::zlib(bytes.reader()).decompress()?.into()), Decompressor::Zstd => { let mut decoder = ZstdDecoder::new(bytes.reader())?; let mut buff: Vec = Vec::with_capacity(OUTPUT_BUFFER_CAPACITY); diff --git a/lib/vector-core/src/sink/zstd.rs b/lib/vector-core/src/sink/zstd.rs index c5459f3c52..edeadf2798 100644 --- a/lib/vector-core/src/sink/zstd.rs +++ b/lib/vector-core/src/sink/zstd.rs @@ -1,6 +1,7 @@ use std::fmt::Display; use std::io; use std::io::{Read, Write}; +use vector_common::decompression::{CappedDecoder, CappedZstdReader}; use crate::sink::compression::CompressionLevel; #[derive(Debug)] @@ -68,20 +69,24 @@ impl std::fmt::Debug for ZstdEncoder { /// 2. Sharing only internal writer, which implements `Sync` unsafe impl Sync for ZstdEncoder {} +/// Streaming zstd decoder bounded by the global decompressed-size cap. +/// +/// Delegates to [`CappedDecoder`], so a frame that expands past the cap fails with an error +/// instead of being silently truncated or driving an unbounded allocation. pub struct ZstdDecoder { - inner: zstd::Decoder<'static, io::BufReader>, + inner: CappedZstdReader, } impl ZstdDecoder { pub fn new(reader: R) -> io::Result { - let decoder = zstd::Decoder::new(reader)?; - Ok(Self { inner: decoder }) + Ok(Self { + inner: CappedDecoder::zstd(reader)?.into_reader(), + }) } } impl Read for ZstdDecoder { fn read(&mut self, buf: &mut [u8]) -> io::Result { - #[allow(clippy::disallowed_methods)] // Caller handles the result of `read`. self.inner.read(buf) } } diff --git a/lib/vector-core/src/test_util.rs b/lib/vector-core/src/test_util.rs index edfa61ab82..1321c71e5a 100644 --- a/lib/vector-core/src/test_util.rs +++ b/lib/vector-core/src/test_util.rs @@ -21,7 +21,6 @@ use crate::event::EventContainer; use crate::event::{BatchNotifier, Event, EventArray, LogEvent, MetricTags, MetricValue}; use crate::event::{Metric, MetricKind}; use chrono::{DateTime, SubsecRound, Utc}; -use flate2::read::MultiGzDecoder; use futures::{stream, task::noop_waker_ref, FutureExt, SinkExt, Stream, StreamExt, TryStreamExt}; use openssl::ssl::{SslConnector, SslFiletype, SslMethod, SslVerifyMode}; use rand::{thread_rng, Rng}; @@ -39,8 +38,8 @@ use tokio_stream::wrappers::TcpListenerStream; use tokio_stream::wrappers::UnixListenerStream; use tokio_util::codec::{Encoder, FramedRead, FramedWrite, LinesCodec}; use vector_buffers::topology::channel::LimitedReceiver; +use vector_common::decompression::CappedDecoder; use vector_config::component::GenerateConfig; -use zstd::Decoder as ZstdDecoder; #[cfg(test)] pub(crate) fn open_fixture(path: impl AsRef) -> crate::Result { @@ -434,22 +433,23 @@ pub fn lines_from_gzip_file>(path: P) -> Vec { let mut file = File::open(path).unwrap(); let mut gzip_bytes = Vec::new(); file.read_to_end(&mut gzip_bytes).unwrap(); - let mut output = String::new(); - MultiGzDecoder::new(&gzip_bytes[..]) - .read_to_string(&mut output) - .unwrap(); - output.lines().map(|s| s.to_owned()).collect() + let output = CappedDecoder::gzip(&gzip_bytes[..]).decompress().unwrap(); + String::from_utf8(output) + .unwrap() + .lines() + .map(|s| s.to_owned()) + .collect() } pub fn lines_from_zstd_file>(path: P) -> Vec { trace!(message = "Reading zstd file.", path = %path.as_ref().display()); let file = File::open(path).unwrap(); - let mut output = String::new(); - ZstdDecoder::new(file) + let output = CappedDecoder::zstd(file).unwrap().decompress().unwrap(); + String::from_utf8(output) .unwrap() - .read_to_string(&mut output) - .unwrap(); - output.lines().map(|s| s.to_owned()).collect() + .lines() + .map(|s| s.to_owned()) + .collect() } pub fn runtime() -> runtime::Runtime { diff --git a/src/sinks/aws_s3/integration_tests.rs b/src/sinks/aws_s3/integration_tests.rs index 9e1645778b..865d9cecdb 100644 --- a/src/sinks/aws_s3/integration_tests.rs +++ b/src/sinks/aws_s3/integration_tests.rs @@ -15,10 +15,10 @@ use aws_sdk_s3::{ }; use aws_smithy_runtime_api::client::result::SdkError; use bytes::Buf; -use flate2::read::MultiGzDecoder; use futures::{stream, Stream}; use similar_asserts::assert_eq; use tokio_stream::StreamExt; +use vector_common::decompression::CappedDecoder; use vector_lib::codecs::{encoding::FramingConfig, TextSerializerConfig}; use vector_lib::{ config::proxy::ProxyConfig, @@ -608,14 +608,17 @@ async fn get_lines(obj: GetObjectOutput) -> Vec { async fn get_gzipped_lines(obj: GetObjectOutput) -> Vec { let body = get_object_output_body(obj).await; - let buf_read = BufReader::new(MultiGzDecoder::new(body)); + let buf_read = BufReader::new(CappedDecoder::gzip(body).into_reader()); buf_read.lines().map(|l| l.unwrap()).collect() } async fn get_zstd_lines(obj: GetObjectOutput) -> Vec { let body = get_object_output_body(obj).await; - let decoder = zstd::Decoder::new(body).expect("zstd decoder initialization failed"); - let buf_read = BufReader::new(decoder); + let buf_read = BufReader::new( + CappedDecoder::zstd(body) + .expect("zstd decoder initialization failed") + .into_reader(), + ); buf_read.lines().map(|l| l.unwrap()).collect() } diff --git a/src/sinks/azure_blob/integration_tests.rs b/src/sinks/azure_blob/integration_tests.rs index 9e36ed1a6a..03818a01b5 100644 --- a/src/sinks/azure_blob/integration_tests.rs +++ b/src/sinks/azure_blob/integration_tests.rs @@ -6,9 +6,9 @@ use std::{ use azure_core::{error::HttpError, prelude::Range}; use azure_storage_blobs::prelude::*; use bytes::{Buf, BytesMut}; -use flate2::read::GzDecoder; use futures::{stream, Stream, StreamExt}; use http::StatusCode; +use vector_common::decompression::CappedDecoder; use vector_lib::codecs::{ encoding::FramingConfig, JsonSerializerConfig, NewlineDelimitedEncoderConfig, TextSerializerConfig, @@ -320,7 +320,7 @@ impl AzureBlobSinkConfig { if self.compression == Compression::None { BufReader::new(body).lines().map(|l| l.unwrap()).collect() } else { - BufReader::new(GzDecoder::new(body)) + BufReader::new(CappedDecoder::gzip(body).into_reader()) .lines() .map(|l| l.unwrap()) .collect() diff --git a/src/sinks/datadog/metrics/encoder.rs b/src/sinks/datadog/metrics/encoder.rs index 9f0608df80..64c140429c 100644 --- a/src/sinks/datadog/metrics/encoder.rs +++ b/src/sinks/datadog/metrics/encoder.rs @@ -981,20 +981,16 @@ fn write_payload_footer( #[cfg(test)] mod tests { - use std::{ - io::{self, copy}, - num::NonZeroU32, - sync::Arc, - }; + use std::{io, num::NonZeroU32, sync::Arc}; - use bytes::{BufMut, Bytes, BytesMut}; + use bytes::{BufMut, Bytes}; use chrono::{DateTime, TimeZone, Timelike, Utc}; - use flate2::read::ZlibDecoder; use proptest::{ arbitrary::any, collection::btree_map, num::f64::POSITIVE as ARB_POSITIVE_F64, prop_assert, proptest, strategy::Strategy, string::string_regex, }; use prost::Message; + use vector_common::decompression::CappedDecoder; use vector_lib::{ config::{log_schema, LogSchema}, event::{ @@ -1068,10 +1064,9 @@ mod tests { } fn decompress_payload(payload: Bytes) -> io::Result { - let mut decompressor = ZlibDecoder::new(&payload[..]); - let mut decompressed = BytesMut::new().writer(); - let result = copy(&mut decompressor, &mut decompressed); - result.map(|_| decompressed.into_inner().freeze()) + CappedDecoder::zlib(&payload[..]) + .decompress() + .map(Bytes::from) } fn ts() -> DateTime { diff --git a/src/sinks/datadog/metrics/integration_tests.rs b/src/sinks/datadog/metrics/integration_tests.rs index 5a3bda4a20..e71e565a41 100644 --- a/src/sinks/datadog/metrics/integration_tests.rs +++ b/src/sinks/datadog/metrics/integration_tests.rs @@ -2,13 +2,13 @@ use std::num::NonZeroU32; use bytes::Bytes; use chrono::{SubsecRound, Utc}; -use flate2::read::ZlibDecoder; use futures::{channel::mpsc::Receiver, stream, StreamExt}; use http::request::Parts; use hyper::StatusCode; use indoc::indoc; use prost::Message; use rand::{thread_rng, Rng}; +use vector_common::decompression::CappedDecoder; use vector_lib::{ config::{init_telemetry, Tags, Telemetry}, @@ -143,10 +143,7 @@ async fn start_test(events: Vec) -> (Vec, Receiver<(http::request: } fn decompress_payload(payload: Vec) -> std::io::Result> { - let mut decompressor = ZlibDecoder::new(&payload[..]); - let mut decompressed = Vec::new(); - let result = std::io::copy(&mut decompressor, &mut decompressed); - result.map(|_| decompressed) + CappedDecoder::zlib(&payload[..]).decompress() } #[tokio::test] diff --git a/src/sinks/datadog/traces/apm_stats/integration_tests.rs b/src/sinks/datadog/traces/apm_stats/integration_tests.rs index fbd2eb6d19..4d4d41e531 100644 --- a/src/sinks/datadog/traces/apm_stats/integration_tests.rs +++ b/src/sinks/datadog/traces/apm_stats/integration_tests.rs @@ -6,13 +6,13 @@ use axum::{ Router, }; use chrono::Utc; -use flate2::read::GzDecoder; use indoc::indoc; use rmp_serde; use serde::Serialize; -use std::{collections::HashMap, io::Read, net::SocketAddr, sync::Arc}; +use std::{collections::HashMap, net::SocketAddr, sync::Arc}; use tokio::sync::mpsc::{self, Receiver, Sender}; use tokio::time::{sleep, Duration}; +use vector_common::decompression::CappedDecoder; use crate::{ config::ConfigBuilder, @@ -122,9 +122,8 @@ async fn process_stats(Extension(state): Extension>, mut request: .await .expect("could not decode body into bytes"); - let mut gz = GzDecoder::new(compressed_body_bytes.as_ref()); - let mut decompressed_body_bytes = vec![]; - gz.read_to_end(&mut decompressed_body_bytes) + let decompressed_body_bytes = CappedDecoder::gzip(compressed_body_bytes.as_ref()) + .decompress() .expect("unable to decompress gzip stats payload"); let payload: StatsPayload = rmp_serde::from_slice(&decompressed_body_bytes).unwrap(); diff --git a/src/sinks/http/tests.rs b/src/sinks/http/tests.rs index 363877380c..2784bf9675 100644 --- a/src/sinks/http/tests.rs +++ b/src/sinks/http/tests.rs @@ -3,11 +3,11 @@ use std::sync::{atomic, Arc}; use bytes::{Buf, Bytes}; -use flate2::{read::MultiGzDecoder, read::ZlibDecoder}; use futures::stream; use headers::{Authorization, HeaderMapExt}; use hyper::{Body, Method, Response, StatusCode}; use serde::{de, Deserialize}; +use vector_common::decompression::CappedDecoder; use vector_lib::codecs::{ encoding::{Framer, FramingConfig}, JsonSerializerConfig, NewlineDelimitedEncoderConfig, TextSerializerConfig, @@ -533,9 +533,10 @@ where T: de::DeserializeOwned, { match compression { - "gzip" => serde_json::from_reader(MultiGzDecoder::new(buf.reader())).unwrap(), - "zstd" => serde_json::from_reader(zstd::Decoder::new(buf.reader()).unwrap()).unwrap(), - "zlib" => serde_json::from_reader(ZlibDecoder::new(buf.reader())).unwrap(), + "gzip" => serde_json::from_reader(CappedDecoder::gzip(buf.reader()).into_reader()).unwrap(), + "zstd" => serde_json::from_reader(CappedDecoder::zstd(buf.reader()).unwrap().into_reader()) + .unwrap(), + "zlib" => serde_json::from_reader(CappedDecoder::zlib(buf.reader()).into_reader()).unwrap(), _ => panic!("undefined compression: {}", compression), } } diff --git a/src/sinks/prometheus/exporter.rs b/src/sinks/prometheus/exporter.rs index e272a6d9dd..6dd3bb4947 100644 --- a/src/sinks/prometheus/exporter.rs +++ b/src/sinks/prometheus/exporter.rs @@ -596,12 +596,11 @@ impl StreamSink for PrometheusExporter { #[cfg(test)] mod tests { use chrono::{Duration, Utc}; - use flate2::read::GzDecoder; use futures::stream; use indoc::indoc; use similar_asserts::assert_eq; - use std::io::Read; use tokio::{sync::oneshot::error::TryRecvError, time}; + use vector_common::decompression::CappedDecoder; use vector_lib::{ event::{MetricTags, StatisticKind}, finalization::{BatchNotifier, BatchStatus}, @@ -801,9 +800,8 @@ mod tests { name = name1, ); - let mut gz = GzDecoder::new(&body_raw[..]); - let mut body_decoded = String::new(); - let _ = gz.read_to_string(&mut body_decoded); + let body_decoded = + String::from_utf8(CappedDecoder::gzip(&body_raw[..]).decompress().unwrap()).unwrap(); assert!(body_raw.len() < expected.len()); assert_eq!(body_decoded, expected); diff --git a/src/sinks/util/buffer/mod.rs b/src/sinks/util/buffer/mod.rs index 427ada410a..fddc84bd55 100644 --- a/src/sinks/util/buffer/mod.rs +++ b/src/sinks/util/buffer/mod.rs @@ -162,10 +162,7 @@ impl Batch for Buffer { #[cfg(test)] mod test { - use std::{ - io::Read, - sync::{Arc, Mutex}, - }; + use std::sync::{Arc, Mutex}; use bytes::{Buf, BytesMut}; use futures::{future, stream, SinkExt, StreamExt}; @@ -177,7 +174,7 @@ mod test { #[tokio::test] async fn gzip() { - use flate2::read::MultiGzDecoder; + use vector_common::decompression::CappedDecoder; let sent_requests = Arc::new(Mutex::new(Vec::new())); @@ -220,13 +217,9 @@ mod test { assert!(output.len() > 1); assert!(output.iter().map(|o| o.len()).sum::() < 80_000); - let decompressed = output.into_iter().flat_map(|batch| { - let mut decompressed = vec![]; - MultiGzDecoder::new(batch.reader()) - .read_to_end(&mut decompressed) - .unwrap(); - decompressed - }); + let decompressed = output + .into_iter() + .flat_map(|batch| CappedDecoder::gzip(batch.reader()).decompress().unwrap()); assert!(decompressed.eq(std::iter::repeat( b"It's going down, I'm yelling timber, You better move, you better dance".to_vec() diff --git a/src/sinks/util/test.rs b/src/sinks/util/test.rs index c029ce8d74..29ec736ce5 100644 --- a/src/sinks/util/test.rs +++ b/src/sinks/util/test.rs @@ -1,5 +1,4 @@ use bytes::{Buf, Bytes}; -use flate2::read::{MultiGzDecoder, ZlibDecoder}; use futures::{channel::mpsc, stream, FutureExt, SinkExt, TryFutureExt}; use futures_util::StreamExt; use http::request::Parts; @@ -14,6 +13,7 @@ use std::{ net::SocketAddr, }; use stream_cancel::{Trigger, Tripwire}; +use vector_common::decompression::CappedDecoder; use crate::{ config::{SinkConfig, SinkContext}, @@ -116,14 +116,20 @@ pub async fn get_received_gzip( rx: mpsc::Receiver<(Parts, Bytes)>, assert_parts: impl Fn(Parts), ) -> Vec { - get_received(rx, assert_parts, |body| MultiGzDecoder::new(body.reader())).await + get_received(rx, assert_parts, |body| { + CappedDecoder::gzip(body.reader()).into_reader() + }) + .await } pub async fn get_received_zlib( rx: mpsc::Receiver<(Parts, Bytes)>, assert_parts: impl Fn(Parts), ) -> Vec { - get_received(rx, assert_parts, |body| ZlibDecoder::new(body.reader())).await + get_received(rx, assert_parts, |body| { + CappedDecoder::zlib(body.reader()).into_reader() + }) + .await } async fn get_received( diff --git a/src/sources/util/http/mod.rs b/src/sources/util/http/mod.rs index 7d11f939d7..f0372f6242 100644 --- a/src/sources/util/http/mod.rs +++ b/src/sources/util/http/mod.rs @@ -24,6 +24,7 @@ mod query; pub use auth::{HttpSourceAuth, HttpSourceAuthConfig}; #[cfg(any( feature = "sources-aws_kinesis_firehose", + feature = "sources-datadog_agent", feature = "sources-opentelemetry", feature = "sources-splunk_hec", feature = "sources-utils-http-prelude", diff --git a/tests/e2e/datadog/metrics/mod.rs b/tests/e2e/datadog/metrics/mod.rs index 875d74f0fc..fe2763c71f 100644 --- a/tests/e2e/datadog/metrics/mod.rs +++ b/tests/e2e/datadog/metrics/mod.rs @@ -1,7 +1,6 @@ use base64::{prelude::BASE64_STANDARD, Engine}; use bytes::Bytes; -use flate2::read::ZlibDecoder; -use std::io::Read; +use vector_common::decompression::CappedDecoder; use vector::test_util::trace_init; @@ -11,10 +10,7 @@ mod sketches; use super::*; fn decompress_payload(payload: Vec) -> std::io::Result> { - let mut decompressor = ZlibDecoder::new(&payload[..]); - let mut decompressed = Vec::new(); - let result = decompressor.read_to_end(&mut decompressed); - result.map(|_| decompressed) + CappedDecoder::zlib(&payload[..]).decompress() } fn unpack_proto_payloads(in_payloads: &FakeIntakeResponseRaw) -> Vec From 61b436a010713eabd8b5079b4599167e22d4641a Mon Sep 17 00:00:00 2001 From: Harshvardhan Shrivastava Date: Fri, 7 Aug 2026 13:00:45 +0530 Subject: [PATCH 6/6] OBE-11559 - logstash source: close the connection on any malformed frame --- src/sources/logstash.rs | 168 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 158 insertions(+), 10 deletions(-) diff --git a/src/sources/logstash.rs b/src/sources/logstash.rs index ceb41c8e41..85cac85d50 100644 --- a/src/sources/logstash.rs +++ b/src/sources/logstash.rs @@ -318,12 +318,27 @@ enum LogstashDecoderReadState { #[derive(Debug)] struct LogstashDecoder { state: LogstashDecoderReadState, + // Set for the decoder used to parse a decompressed payload. No known + // Lumberjack/Beats client emits a compressed frame nested inside another, + // so a nested `C` frame here is rejected rather than recursed into. + // Without this, an attacker could nest compressed frames arbitrarily deep + // and drive unbounded recursion in `decode_compressed_frame`, exhausting + // the stack (CWE-674). + nested: bool, } impl LogstashDecoder { const fn new() -> Self { Self { state: LogstashDecoderReadState::ReadProtocol, + nested: false, + } + } + + const fn new_nested() -> Self { + Self { + state: LogstashDecoderReadState::ReadProtocol, + nested: true, } } } @@ -340,19 +355,18 @@ pub enum DecodeError { JsonFrameFailedDecode { source: serde_json::Error }, #[snafu(display("Failed to decompress compressed frame: {}", source))] DecompressionFailed { source: io::Error }, + #[snafu(display("Compressed frame contains a nested compressed frame"))] + NestedCompressedFrame, } impl StreamDecodingError for DecodeError { fn can_continue(&self) -> bool { - use DecodeError::*; - - match self { - IO { .. } => false, - UnknownProtocolVersion { .. } => false, - UnknownFrameType { .. } => false, - JsonFrameFailedDecode { .. } => true, - DecompressionFailed { .. } => true, - } + // No decode error is recoverable on this stream. Lumberjack is a + // length-prefixed binary protocol with no resync marker, so once a + // frame fails to decode the stream position is no longer trustworthy: + // continuing would misframe subsequent bytes and emit ACKs for bogus + // sequence numbers. + false } } @@ -538,6 +552,10 @@ impl Decoder for LogstashDecoder { } // https://github.com/logstash-plugins/logstash-input-beats/blob/master/PROTOCOL.md#compressed-frame-type LogstashDecoderReadState::ReadFrame(_protocol, LogstashFrameType::Compressed) => { + if self.nested { + return Err(DecodeError::NestedCompressedFrame); + } + let Some(frames) = decode_compressed_frame(src)? else { return Ok(None); }; @@ -689,7 +707,7 @@ fn decode_compressed_frame( let mut buf = res?; - let mut decoder = LogstashDecoder::new(); + let mut decoder = LogstashDecoder::new_nested(); let mut frames = VecDeque::new(); @@ -1000,6 +1018,136 @@ mod test { assert!(result.is_none(), "expected the decoder to await more bytes"); } + + fn push_req(req: &mut BytesMut, seq: u32, pairs: &[(&str, &str)]) { + req.put_slice(&encode_req(seq, pairs)); + } + + /// Wraps `inner` in a `'2' 'C'` compressed frame. + fn push_compressed(req: &mut BytesMut, inner: &[u8]) { + use std::io::Write as _; + + let mut encoder = + flate2::write::ZlibEncoder::new(Vec::new(), flate2::Compression::default()); + encoder.write_all(inner).unwrap(); + let compressed = encoder.finish().unwrap(); + + req.put_u8(b'2'); + req.put_u8(b'C'); + req.put_u32(compressed.len() as u32); + req.put_slice(&compressed); + } + + // A malformed frame must be a fatal (non-continuable) decode error: the + // Lumberjack stream can't be resynced, so the connection is closed rather + // than continuing with a desynced decoder (which would emit bogus ACKs). + // This matches upstream logstash-input-beats, which closes the channel on + // any decode exception. + + #[test] + fn malformed_json_frame_is_a_fatal_decode_error() { + let mut decoder = LogstashDecoder::new(); + let mut src = BytesMut::new(); + src.put_u8(b'2'); + src.put_u8(b'J'); + src.put_u32(1); // sequence number + let bad = b"{ not valid json "; + src.put_u32(bad.len() as u32); // payload size + src.put_slice(&bad[..]); + + let err = decoder.decode(&mut src).unwrap_err(); + assert!(matches!(err, DecodeError::JsonFrameFailedDecode { .. })); + assert!( + !err.can_continue(), + "a malformed JSON frame must be fatal so the connection closes", + ); + } + + #[test] + fn malformed_compressed_frame_is_a_fatal_decode_error() { + let mut decoder = LogstashDecoder::new(); + let mut src = BytesMut::new(); + src.put_u8(b'2'); + src.put_u8(b'C'); + let garbage = b"this is not a zlib stream"; + src.put_u32(garbage.len() as u32); // payload size + src.put_slice(&garbage[..]); + + let err = decoder.decode(&mut src).unwrap_err(); + assert!(matches!(err, DecodeError::DecompressionFailed { .. })); + assert!(!err.can_continue()); + } + + /// A compressed frame nested inside another must be refused rather than recursed into, so a + /// frame nested arbitrarily deep cannot exhaust the stack. + #[test] + fn nested_compressed_frame_is_a_fatal_decode_error() { + let mut inner = BytesMut::new(); + push_req(&mut inner, 1, &[("message", "should never be reached")]); + + let mut middle = BytesMut::new(); + push_compressed(&mut middle, &inner); + + let mut req = BytesMut::new(); + push_compressed(&mut req, &middle); + + let mut decoder = LogstashDecoder::new(); + let err = decoder.decode(&mut req).unwrap_err(); + assert!(matches!(err, DecodeError::NestedCompressedFrame)); + assert!(!err.can_continue()); + } + + /// The nesting guard must not disturb a single (non-nested) compressed frame carrying ordinary + /// data frames — the accept half of the pair above. + #[test] + fn singly_compressed_frame_is_still_accepted() { + let mut inner = BytesMut::new(); + push_req(&mut inner, 1, &[("message", "hello")]); + + let mut req = BytesMut::new(); + push_compressed(&mut req, &inner); + + let mut decoder = LogstashDecoder::new(); + let frame = decoder + .decode(&mut req) + .expect("a singly-compressed frame must decode") + .expect("expected one decoded frame"); + + assert_eq!( + frame.0.fields.get("message"), + Some(&serde_json::Value::from("hello")), + ); + } + + #[tokio::test] + async fn malformed_frame_closes_connection_without_ack() { + let (address, _recv) = start_logstash(EventStatus::Delivered).await; + + let mut socket = tokio::net::TcpStream::connect(address).await.unwrap(); + + // A '2' 'J' frame whose payload is not valid JSON. + let mut req = BytesMut::new(); + req.put_u8(b'2'); + req.put_u8(b'J'); + req.put_u32(1); // sequence number + let bad = b"{ not valid json "; + req.put_u32(bad.len() as u32); // payload size + req.put_slice(&bad[..]); + socket.write_all(&req).await.unwrap(); + + // The source must close the connection on the decode error and send no + // ACK; the client will reconnect and retransmit. + let mut output = BytesMut::new(); + let result = socket.read_buf(&mut output).await; + assert!( + matches!(result, Ok(0)) || result.is_err(), + "expected the connection to close; read returned {result:?} with {output:?}", + ); + assert!( + output.is_empty(), + "no ACK should be sent for a malformed frame, got {output:?}", + ); + } } #[cfg(all(test, feature = "logstash-integration-tests"))]