Skip to content

compression: enforce maximum uncompressed packet size#1498

Open
VasilisDragon wants to merge 1 commit into
PrismarineJS:masterfrom
VasilisDragon:vasilis-decompression-cap
Open

compression: enforce maximum uncompressed packet size#1498
VasilisDragon wants to merge 1 commit into
PrismarineJS:masterfrom
VasilisDragon:vasilis-decompression-cap

Conversation

@VasilisDragon

Copy link
Copy Markdown

Problem

Decompressor in src/transforms/compression.js inflates compressed packets with
zlib.unzipSync(..., { finishFlush: 2 }) and no bound on the output size. The declared
uncompressed length is read but only used for a console.error, never enforced, so a peer can
send a small, highly compressible packet that inflates to an arbitrarily large synchronous
allocation and OOMs the process. This is the behavior reported in #664 ("Players know about
this bug, and are actively exploiting it to ... effectively 'ban' them"
); the earlier #729
capped only the outbound Compressor and was closed, so the inbound path is still unbounded.
Resolves #664.

The transform is shared by both pipelines, so this affects clients/bots (malicious server) and
any node-minecraft-protocol server (malicious client) symmetrically.

Fix

Enforce the protocol's maximum uncompressed packet size of 2 ** 23 (8388608) bytes — the value
vanilla applies in NettyCompressionDecoder:

  • pass maxOutputLength to zlib.unzipSync, so a packet that inflates past the cap throws
    ERR_BUFFER_TOO_LARGE and is dropped by the existing error handler instead of allocating
    without bound; and
  • reject up front any packet whose declared uncompressed length already exceeds the cap.

No behavior change for spec-compliant traffic — every legitimate packet is ≤ 8 MiB uncompressed,
and anything larger is already rejected by a vanilla client.

Test

New test/compressionTest.js, exercising the transforms directly (no server jar):

  • compressed round-trip and uncompressed (value === 0) passthrough — the no-regression cases;
  • a decompression bomb (declares a small length, inflates past the cap) is dropped instead of
    emitted — the assertion fails on master, which inflates and forwards it;
  • a packet whose declared length already exceeds the cap is rejected before inflating;
  • boundary: an exactly-2 ** 23-byte packet still round-trips, since the cap rejects only output
    past the maximum — legitimate max-size packets are unaffected.

npm test green locally (standard plus mocha).

Scope

Intentionally narrow. The Splitter in framing.js also buffers an attacker-declared frame
length with no maximum — the same class of issue, but a separate change; I'll propose a
max-frame-size cap in a follow-up PR rather than widen this one.

The decompressor inflated packets with no bound on the output size, so a peer could send a small packet that expands to an unbounded synchronous allocation. Cap the inflate at the protocol maximum (2^23 bytes) via zlib's maxOutputLength and reject packets whose declared length already exceeds it.
@VasilisDragon VasilisDragon force-pushed the vasilis-decompression-cap branch from b493dc7 to d816de5 Compare June 26, 2026 00:17
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.

Client connection times out when packet over 2097152 bytes (2mb) is sent to client

2 participants