Uh oh!
There was an error while loading. Please reload this page.
fix(security): [OBE-10709,OBE-10712,OBE-10718,OBE-11232,OBE-11234,OBE-11235,OBE-11236,OBE-11238,OBE-11555,OBE-11556] OOM/unbounded allocation bounds - #138
Closed
JuanMantica45 wants to merge 20 commits into
Conversation
Covers OBE-11232, OBE-11234, OBE-11235, OBE-11236, OBE-11238, OBE-11555, OBE-11556, OBE-10709, OBE-10712, OBE-10718. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Covers 10 tickets (OBE-10709, -10712, -10718, -11232, -11234, -11235, -11236, -11238, -11555, -11556) across 4 fix families: decompression output caps, newline framer max_length, GELF chunk-reassembly bounds, and STCP buffer/header/clone/permit fixes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… compressed frames - Add `max_decompressed_bytes` config field (default 256 MiB) - Wrap ZlibDecoder with `.take(max_decompressed_bytes)` and error if limit reached - Track `inside_compressed` flag; reject nested C-frames immediately - New error variant `NestedCompressionRejected` with `can_continue() = false` Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Drop the permit after receiver.await completes, before stream.write_all, so a zero-window peer cannot hold the semaphore slot during a potentially blocking write and starve other connections. Also wrap write_all in a 30-second timeout to bound worst-case connection hold time when the peer stops draining its TCP receive window. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ax_length Previously new() delegated to CharacterDelimitedDecoder::new() which uses usize::MAX as the limit, leaving the internal BytesMut unbounded. Any stream that never emits a newline would grow the buffer until OOM. Change new() to call new_with_max_length(DEFAULT_MAX_LENGTH) (100 KiB). Callers that need a higher limit must opt in explicitly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…s_limit and max_length Previously both were None (unbounded): a sender could open many message IDs without completing them to exhaust the in-memory HashMap, or send a very large multi-chunk message to exhaust per-message allocation. Defaults now: pending_messages_limit = Some(1000) max_length = Some(5 MiB) Operators who need higher limits can override via config. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…elayQueue reaper Each incomplete GELF chunk-reassembly message used to spawn a dedicated tokio task to expire it after the timeout. With many concurrent senders opening message IDs without completing them, this could grow the task pool unboundedly (O(N) tasks for N in-flight message IDs). Replace with a single background reaper task per ChunkedGelfDecoder that owns a tokio_util::time::DelayQueue<u64>. The decode path sends the message_id to the reaper via an UnboundedSender; the reaper inserts it into the DelayQueue with the configured timeout. When a timeout fires the reaper removes the entry from the shared state HashMap and logs the existing warning. Task count is now O(1) regardless of concurrent senders. JoinHandle is removed from MessageState (no per-message abort needed; completed messages are removed from state before the timer fires, so the reaper's remove is a no-op). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…coder The timeout Duration is now fully captured in the reaper closure; keep it only as a local in new(). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Points to 18fac46 — LEB128 InSufficientData fix and max_lines_per_event cap. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ounds Add ADR with 7 non-obvious design decisions (GELF defaults, reaper channel design, LEB128 EOF semantics, Arc-sharing deferral, bomb detection boundary, SLDC expansion ratio, TCP permit drop idiom). Delete spec and plan — decisions are now in the ADR; task breakdown is in git history. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ew on OOM bounds Applies the PR Sentinel-One#138 review feedback. The security intent of each bound is unchanged; what changes is where the bound is applied and how loudly it reports when it bites. Newline framing (OBE-11232) Bounding `NewlineDelimitedDecoder::new()` silently overrode every caller that had deliberately chosen no limit — including `aws_s3::default_framing`, which spells out `max_length: None` under a backwards-compatibility comment. At 100 KiB that dropped ordinary large JSON records (CloudTrail, EDR telemetry) with only a rate-limited warning. Restore `new()` to unbounded and move the default to `NewlineDelimitedDecoderConfig::build()` at 1 MiB, matching what the GCS source had already chosen independently. Callers that construct the decoder directly keep control of their own limit; the statsd call sites now opt in explicitly, since they were the intended targets of the original ticket. GELF reaper (OBE-11235) The DelayQueue timer was never cancelled when a message completed, so a sender that reused a message id inside the timeout window had its new message evicted by the previous message's timer. Track each pending id's DelayQueue key so completion (and the max-length drop path) can cancel it. Three new tests fail against the previous behaviour. Logstash (OBE-10712) Lower `max_decompressed_bytes` from 256 MiB to 32 MiB: the bound is per frame, so at 256 MiB a few concurrent connections could still exhaust heap. Fix an off-by-one — reading up to `max` made "exactly at the limit" (legal) indistinguishable from "truncated at the limit", so a payload of exactly `max_decompressed_bytes` was rejected. Read `max + 1` and compare with `>`. The two decompression-bomb tests were behind the `logstash-integration-tests` feature and never ran in normal CI. Move them into the unit test module and add boundary, buffer-drain, and stream-continuation coverage. TCP ack write (OBE-11555) Replace the empty `test_permit_released_before_ack_write` stub — which asserted nothing and always passed — with real coverage. Extract the ack write into `write_ack`, testable over `tokio::io::duplex`, and cover the success, timeout, slow-but-progressing, and hangup paths plus the permit ordering the fix depends on. Docs Add a breaking-change changelog entry, and refresh the generated Cue docs for the changed `max_length` default and the new logstash option. NOTE: the Cue files under website/cue are machine-generated. They were updated by hand because `make generate-component-docs` needs a full vector build, which does not link on macOS (librdkafka/GSSAPI). CI must re-run the generator to confirm no drift. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Re-derived every cap against documented producer maxima so it only trips on abuse, never on real traffic. Also shrinks the chunked_gelf diff to the two default values; the DelayQueue reaper rewrite moves to its own PR. - Newline framing: 1 MiB -> 10 MiB. - logstash max_decompressed_bytes: 32 MiB -> 256 MiB. One Beats `C` frame carries a whole window (bulk_max_size 2048, go-lumber maxWindowSize 10000), so inflated batches legitimately reach tens of MiB. - GELF: max_length 5 -> 8 MiB and pending_messages_limit 1000 -> 10000. The protocol ceiling is 128 chunks x 65507 bytes, so 8 MiB is above anything the wire format can produce; Graylog's own decompress_size_limit is also 8 MiB and Graylog caps pending messages not at all. - chunked_gelf.rs is back to master apart from the two defaults, which also drops the tokio-util "time" feature. Changelog is no longer a breaking entry. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Research on realistic per-record maxima says 10 MiB is the right number but was
applied in the wrong place. `build()` turned an explicit `max_length: None`
into 10 MiB, silently overriding `aws_s3::default_framing`, which sets `None`
deliberately.
That matters because some S3 objects are a single newline-free JSON document —
CloudTrail delivers `{"Records":[...]}`, AWS Config `{"configurationItems":
[...]}` — so after gunzip the whole object is one "line" and a per-line cap
drops it wholesale.
Moving the default onto the field as a serde default keeps both behaviors: a
user who omits the key gets 10 MiB, while a component that constructs `None` in
Rust stays unbounded.
10 MiB clears every documented per-record maximum by >=10x (CloudTrail 256 KB /
1 MB Lake, CloudWatch Logs 1 MB, EventBridge 1 MB, Pub/Sub 10 MB, CRI 16 KiB,
ETW 64 KB) and is already 100x upstream Vector's `file` source `max_line_bytes`
of 100 KiB.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>…ection A blast-radius audit found the newline framer bound does not prevent the OOM it was added for. CharacterDelimitedDecoder::decode returns Ok(None) with no size check when no delimiter is present (character_delimited.rs:117), so a peer that never sends '\n' still grows the BytesMut without limit. max_length only fires once a delimiter arrives or at EOF. So the default was pure cost: silently dropping legitimate long lines (S3 CloudTrail objects are one newline-free JSON document) with no metric and no back-pressure, while leaving the actual attack open. Reverted across codecs, statsd, gcs and the generated docs. Closing that vector properly means bounding buffer growth inside CharacterDelimitedDecoder when no delimiter has been found yet, which is a separate change to shared codec behavior and belongs in its own PR. Also bumps the private submodule for the stcp frame-cap headroom and the wef body-limit and SLDC fixes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
JuanMantica45
commented
Aug 10, 2026
ContributorAuthor
Superseded — split into four independent PRs off
Two things changed during the split, both from follow-up research: The newline framer bound was dropped entirely. Every remaining bound was re-derived against documented producer maxima so it trips on abuse and not on real traffic. Details in each PR. This PR can be closed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds default upper bounds to allocation paths that a remote sender could previously drive without limit.
max_lengthlogstashmax_decompressed_byteslogstashCframesgcp_gcsmax_decompressed_bytesgcs_object_truncated_totalstcpmax_frame_bytesmax_event_size(16 MiB)stcpmax_lines_per_eventwefmax_content_lengthpending_messages_limit/max_lengthtcpAlso fixes, in the same paths:
warn!was used without being imported.read_leb128_i64) returnedOk(0)on buffer exhaustion, a silent truncation that bypassed loop-count guards; now returnsInSufficientData.RequestLimiterpermit only after the ack write, so a zero-window peer parked a permit indefinitely and starved other connections.Why these values
Every default is set above documented producer maxima, so it trips on abuse and not on real traffic:
decompress_size_limitis also 8 MiB. Graylog caps pending messages not at all; 10,000 is well above what a sender holds in flight inside the 5s reassembly window.Cframe carries an entire window, not one event.bulk_max_sizedefaults to 2048 and go-lumber'smaxWindowSizeallows 10,000, so legitimate inflated batches reach tens of MiB.FramedRead, never buffered whole, so the per-line cap is what actually bounds memory; this value only stops a runaway decompressor. BigQuery exports up to 1 GB uncompressed per file and Cloud Logging up to 3.5 GiB, so anything lower silently truncated real objects.max_frame_bytes— bounds the whole receive buffer, so it is derived frommax_event_sizerather than fixed; a lower value would disconnect a forwarder sending a legitimate max-size event.Testing
codecs358 ·stcp63 ·wef116 ·gcs8 ·vectorsources 90 — all passing. The two logstash decompression-bomb tests were behind thelogstash-integration-testsfeature and never ran in CI; they are now unit tests.website/cue/**is machine-generated and was updated by hand —make generate-component-docsneeds a full build, which does not link on macOS (librdkafka/GSSAPI). CI must re-run the generator to confirm no drift.Jira: OBE-10709, OBE-10712, OBE-10718, OBE-11232, OBE-11234, OBE-11235, OBE-11236, OBE-11238, OBE-11555, OBE-11556