Skip to content

Harden compression and decompression against memory amplification and malformed blocks - #419

Open
mattrm456 wants to merge 3 commits into
bloomberg:mainfrom
mattrm456:ntcd-compression-rle-decoder-underflow
Open

mattrm456 wants to merge 3 commits into
bloomberg:mainfrom
mattrm456:ntcd-compression-rle-decoder-underflow

Conversation

@mattrm456

@mattrm456 mattrm456 commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Summary

This PR fixes a bug in the built-in RLE decoder (ntcd::CompressionDecoder), where a peer-controlled frame length can underflow the decoder's internal state. It also adds configurable per-operation limits on inflated and deflated output, honored by the RLE decoder and by the zlib, gzip, LZ4 and Zstd plugins. Applying those limits across the plugins uncovered several existing defects in their error handling, which are fixed here, including a remotely triggerable infinite loop in the Zstd inflater.

Problem

RLE frame length underflow (reported). ntcd::CompressionDecoder::process() loaded the 32-bit frame length from the frame header into d_frameContentBytesNeeded. It then subtracted block-header and RAW payload sizes from it without checking bounds:

  • A frame length of 1–3, or any length that doesn't fall exactly on a block boundary, wrapped the counter to about 2^64.
  • A RAW block longer than the rest of the frame did the same.

Once the counter wrapped, the decoder never reached the frame footer and never verified the checksum. It kept expanding RLE blocks indefinitely: up to 65,535 output bytes per 4 input bytes, all appended to the caller's blob. Even well-formed frames had no output cap: a single frame could legitimately inflate to about 70 TB before its checksum was checked.

No decompression limits. None of the compression backends limited how much output one operation could produce. A small compressed read could expand into an arbitrarily large append (a decompression bomb).

Changes

ntca::CompressionConfig

  • Adds two optional fields, maxInflateSize and maxDeflateSize: the maximum number of bytes a single inflate or deflate operation may produce. An operation that would exceed the limit fails with ntsa::Error::e_LIMIT.
  • When a field isn't set, the limit defaults to INT_MAX.
  • Because the limits live in the compression configuration, sockets created with compressionConfig get them without any change to socket code.

ntcd::CompressionDecoder / ntcd::CompressionEncoder (RLE)

  • Underflow fixes:
    • A block header is rejected if fewer than 4 bytes are left in the frame.
    • A RAW block is rejected if its payload is longer than the rest of the frame.
    • The frame-length counter can no longer underflow.
  • Limits:
    • The decoder checks maxInflateSize before each RLE expansion and each RAW payload is added to the result.
    • The encoder checks maxDeflateSize before adding the frame header, each block and the footer.
    • Both also refuse anything that would grow the result blob past INT_MAX bytes.
  • Reset after failure: the decoder now discards buffered input and the half-decoded frame and waits for a new frame header. Previously it stayed failed for good. Staying failed meant one malformed datagram permanently disabled decompression on a datagram socket.
  • Checksum-update errors now go through the decoder's normal failure path.
  • The last RAW block's encode error, which was previously ignored, is now checked.

ntctlc plugin (zlib, gzip, LZ4, Zstd)

  • Limits:
    • For zlib, gzip, Zstd, and LZ4 inflate, the output space given to the library on each call is capped at what's left of the limit plus one byte. The library never does more than one byte of extra work, and producing that byte means the limit was exceeded.
    • LZ4 deflate requires worst-case output space, so its output is checked after each call and before it is added to the result.
    • In every case, no more than the limit is ever appended to the result.
  • Failure handling: every error path now goes through new inflateFail/deflateFail helpers. They discard pending output and stream state, and the inflate version also resets the decompression library. This fixes three existing defects:
    • Bytes left over from a failed operation could be appended to the next operation's result.
    • The zlib/gzip inflaters left next_in set after an error, so the next call tripped BSLS_ASSERT(d_inflaterStream.next_in == 0). In builds with assertions enabled, one corrupt frame could crash the process.
    • Inflaters now reset after an error, so later well-formed frames still decode.
  • Zstd infinite loop: the inflateNext and deflateEnd loops treated every non-zero size_t return as "continue", including Zstd error codes. Malformed input made the inflater spin forever; reproduced with 31 bytes of garbage. Errors are now checked first.
  • Pending output delivered sooner: zlib, gzip and Zstd now commit buffered output at the end of every inflate operation, as LZ4 already did. Previously, data inflated during an operation could sit in a private buffer until the buffer filled or the frame ended.
  • Large inputs: zlib and gzip now feed inputs larger than 4 GiB to the library in chunks, instead of truncating the length to uInt.

ntci::Compression

  • inflate(..., const ntsa::Data&, ...) never checked the error from its inflateRep call; it went on to inflateEnd and overwrote the error with that call's result. Every inflate error on that overload was reported as success. It now returns the error, as the matching deflate overload already did. The bdlbb::Blob overload, which the sockets use, was not affected.

Behaviour changes

  • Inflate and deflate operations can now fail with e_LIMIT, but only when a limit is configured or the result blob would exceed INT_MAX bytes.
  • After a failure, decoders and inflaters reset instead of staying failed. On a stream socket, the bytes that follow are mid-frame, so they fail again and the connection is torn down as before. On a datagram socket, later well-formed datagrams now decode.
  • zlib, gzip and Zstd deliver inflated data at the end of each operation instead of holding it until a buffer fills or a frame ends.
  • ntsa::Data inflate callers now receive errors that were previously dropped.

Testing

  • ntcd_compression.t:
    • verifyMalformed:
      • Frames whose header length is too short to hold a block.
      • Frames with a leftover of 1–3 bytes after a valid block.
      • RAW blocks longer than the rest of the frame.
      • A second RAW block that runs past the frame.
      • In each case, the test checks the error, a bound on the output, and that a later well-formed frame decodes.
    • verifyLimits:
      • Every limit from 0 up to the inflated size + 1 (and the deflated size + 1).
      • Input fed one byte per call within a single operation.
      • The limit applies per operation, not across operations.
      • The INT_MAX defaults.
  • ntctlc_plugin.t: runs for every compiled-in algorithm.
    • verifyLimits:
      • Limits around the boundary, for both inflate and deflate.
      • A fragmented blob of 1-byte buffers, and an ntsa::Data buffer array (a regression test for the ntci fix).
      • The limit applies per operation.
      • No leftover bytes after a failed inflate or deflate.
      • A 16 MiB run of zeros, compressed, then inflated with a 1 MiB limit.
    • verifyMalformed: garbage input returns e_INVALID, and a later well-formed frame decodes, repeated twice.
  • Run against the old code:
    • The new RLE test fails without the fix, with 1,048,560 bytes inflated from a frame that declared 1 byte of content.
    • The new plugin tests fail against the original plugin; the Zstd malformed-input case hangs until killed by a timeout.
    • The ntsa::Data regression test fails without the ntci fix.
  • Existing tests: all existing ntcd_compression.t and ntctlc_plugin.t cases pass. The plugin's verifyAll sweep was also run temporarily across all four algorithms (it is normally pinned to LZ4 by NTCTLC_PLUGIN_TEST_COMPRESSION_TYPE).

Known limitations and follow-ups

  • Zstd window memory is not capped. A peer can still make a Zstd inflater allocate up to zstd's default window limit (2^27, about 128 MiB) per frame. Capping it would reject frames from existing peers using e_BEST_SIZE, so it is left to a follow-up, possibly as an opt-in configuration setting.
  • Interface-level defaults don't merge field by field. Interface-level compressionConfig defaults are applied only when a socket's own compressionConfig is unset. A socket that sets compressionConfig with only type will not inherit an interface-level maxInflateSize. This is existing behaviour.
  • The limit is per operation, not per message. On a stream socket, one operation is one read, so the right value depends on the read size.

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