Skip to content

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

Open
JuanMantica45 wants to merge 16 commits into
Sentinel-One:masterfrom
JuanMantica45:security-oom-bounds
Open

Conversation

@JuanMantica45

Copy link
Copy Markdown

What

Ten security fixes across six sources to eliminate OOM/unbounded allocation attack surfaces:

  • GCS (OBE-10709): cap decompressed object size via .take(max_decompressed_bytes) (256 MiB default); set newline-framer max_length to 1 MiB (was None/unbounded)
  • Logstash (OBE-10712): cap zlib decompression at 256 MiB; reject nested C frames (recursion bomb)
  • WEF (OBE-10718, OBE-11236): enforce max_content_length on HTTP body via http_body_util::Limited; cap SLDC decompressor output at max_content_length × 100
  • Newline framer (OBE-11232): NewlineDelimitedDecoder::new() now defaults to 100 KiB max-line length (was usize::MAX); applied to socket/TCP and statsd sources
  • GELF (OBE-11235 parts 1 & 2): default pending_messages_limit to 1 000 and max_length to 5 MiB; replace O(N) per-message `tokio::spawn(sleep)` timeout tasks with a single `DelayQueue`-based reaper (O(1) tasks regardless of in-flight message count)
  • STCP (OBE-11238): max_frame_bytes = 1 MiB buffer cap in decode(); BufferOverflow.can_continue() = false to terminate the stream; non-InSufficientData errors now propagate instead of being swallowed as Ok(None)
  • STCP (OBE-11234): read_leb128_i64 returns Err(InSufficientData) on buffer exhaustion instead of Ok(0) (silent truncation that bypassed loop-count guards); field/header count caps in parse_event and read_legacy_event
  • STCP (OBE-11556): max_lines_per_event = 10 000 cap in parse_lines — eliminates O(N×M) clone amplification from large field maps × many newlines in RAW
  • TCP (OBE-11555): RequestLimiterPermit is now released before stream.write_all(&ack_bytes) — a zero-window peer can no longer park the permit indefinitely; 30-second write timeout added as defense-in-depth

Private submodule (lib/observo/private) is updated to the security-oom-bounds tip for WEF, STCP, and GCS changes.

Why

Multiple sources accepted arbitrary-size inputs from remote senders without any upper bound on allocation:

  • A zip bomb to the GCS/Logstash/WEF decompressors would exhaust heap
  • Sending a GELF stream that never closes message IDs would spawn an unbounded number of timeout tasks
  • A STCP stream with many fields and many newlines in RAW would trigger O(N×M) clone amplification
  • A slow peer holding a TCP connection open during ack write starved the RequestLimiter semaphore

How to Test

  1. cargo test -p codecs --lib decoding::framing::chunked_gelf — 36 tests including new reaper eviction and limit tests
  2. cargo test -p codecs --lib decoding::framing::newline_delimited — 7 tests including the 100 KiB default test
  3. cargo test -p vector --lib sources::logstash (CI/Linux only — requires librdkafka) — includes decompression bomb and nested-C rejection tests
  4. Private submodule tests require Linux (GSSAPI link issue on Mac); CI covers STCP and WEF tests
  5. See docs/adr/security-oom-allocation-bounds.md for design rationale on non-obvious choices (LEB128 EOF semantics, SLDC expansion ratio, Arc-sharing deferral)

Jira: OBE-10709, OBE-10712, OBE-10718, OBE-11232, OBE-11234, OBE-11235, OBE-11236, OBE-11238, OBE-11555, OBE-11556

JuanMantica45 and others added 16 commits August 7, 2026 16:25
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant