Ignored error on fallback compression writer creation can yield nil writer
low swesweep-fasthttp · compress.go · acquireRealGzipWriter / acquireRealDeflateWriter · run #4
Description
When gzip.NewWriterLevel / zlib.NewWriterLevel fails on the first attempt (invalid level), the code clamps the level and retries, but discards the second error: 'zw, _ = gzip.NewWriterLevel(w, level)'. If the retry also fails (e.g. a future library change adds a new failure mode), zw is nil. The corresponding release function (releaseRealGzipWriter / releaseRealDeflateWriter) then calls zw.Close() on a nil pointer, causing a panic and process crash. This path is currently unreachable because the clamped level is always valid, but the unchecked error is a latent nil-pointer dereference.
Triage
Not triaged.
Raw scan report for this file
## Security Analysis: `compress.go` (fasthttp)
This file is fasthttp's compression helper layer, providing gzip/deflate compression and decompression utilities. The public API (`WriteGzip*`, `WriteGunzip`, `WriteInflate*`, `Append*`) accepts attacker-controlled data (request/response bodies, file contents) and passes it through compression/decompression codecs.
### Function-by-Function Trace
**Decompression path (highest risk):** `WriteGunzip(w, p)` → `writeGunzip(w, p, 0)` → `byteSliceReader{b: p}` → `gzip.NewReader(r)` → `copyZeroAllocWithLimit(w, zr, 0)`. The `maxBodySize=0` means **no output size cap**. An attacker-supplied `p` (a few hundred bytes of highly compressed data) can expand to gigabytes of output, allocating via `byteSliceWriter.b = append(w.b, p...)` inside `copyZeroAllocWithLimit`. The `AppendGunzipBytes`/`AppendInflateBytes` variants make this worse: the decompressed output is appended to `dst` with no limit, and the resulting slice is returned to the caller.
**Compression path:** `WriteGzipLevel(w, p, level)` → for non-blocking writer types (`*byteSliceWriter`, `*bytes.Buffer`, `*bytebufferpool.ByteBuffer`), the level is clamped by `normalizeCompressLevel` into `[0..11]` for pool indexing, and the actual compression is done in `nonblockingWriteGzip` via a pooled `gzip.Writer`. The level passed to `gzip.NewWriterLevel` is the raw `level`, but `acquireRealGzipWriter` clamps invalid levels before the second attempt. No buffer overflow or out-of-bounds index is possible.
**Pool invariants:** `sync.Pool` entries are always the correct type because they are only populated by the corresponding `acquire*`/`release*` pairs. The interface assertions (`v.(*gzip.Reader)`, `ctxv.(*compressCtx)`, `zr.(zlib.Resetter)`) are safe under normal operation.
**`byteSliceReader` / `byteSliceWriter`:** Both are trivially safe. `Read` copies `min(len(p), len(r.b))` bytes; `Write` uses `append` which handles growth. No fixed-size buffers.
**`isFileCompressible`:** Uses `io.LimitedReader{N: 4096}` to bound reads. The `n := 4096 - lr.N` computation is safe (result ∈ [0, 4096]). The ignored `Seek` error is benign — worst case the compressibility check is slightly inaccurate. A nil `f` would panic, but this is an internal function and the caller validates file opens.
---
### Findings
```json
[
{
"severity": "medium",
"title": "Decompression bomb: no output size limit in public decompression API",
"function": "WriteGunzip / WriteInflate / AppendGunzipBytes / AppendInflateBytes",
"description": "All four public decompression functions call writeGunzip/writeInflate with maxBodySize=0 (unlimited). An attacker can supply a small gzip or deflate stream (e.g. a few KB) that decompresses to tens or hundreds of GB, causing unbounded memory allocation via byteSliceWriter.append inside copyZeroAllocWithLimit and crashing the host process with OOM. The internal writeGunzip/writeInflate accept a maxBodySize parameter for limiting, but the public API never exposes it. In a web-server context, any code path that calls AppendGunzipBytes or WriteGunzip on an attacker-controlled request body is vulnerable to a classic decompression-bomb resource-exhaustion attack."
},
{
"severity": "low",
"title": "WriteGzipLevel / WriteDeflateLevel return input length instead of compressed byte count for non-blocking writers",
"function": "WriteGzipLevel / WriteDeflateLevel",
"description": "When w is a *byteSliceWriter, *bytes.Buffer, or *bytebufferpool.ByteBuffer, the function synchronously compresses p into w via stacklessWriteGzip/stacklessWriteDeflate, but returns len(p), nil — the size of the uncompressed input, not the number of compressed bytes actually written. The API contract states it returns 'the number of compressed bytes written to w'. A caller that uses this return value to slice, buffer, or frame the compressed output (e.g. buf.Bytes()[:n]) will read too many or too few bytes, producing a malformed compressed payload on the wire. This is a correctness bug that could cause protocol desynchronisation or data corruption in dependent code, though it is not a direct memory-safety issue in Go."
},
{
"severity": "low",
"title": "Ignored error on fallback compression writer creation can yield nil writer",
"function": "acquireRealGzipWriter / acquireRealDeflateWriter",
"description": "When gzip.NewWriterLevel / zlib.NewWriterLevel fails on the first attempt (invalid level), the code clamps the level and retries, but discards the second error: 'zw, _ = gzip.NewWriterLevel(w, level)'. If the retry also fails (e.g. a future library change adds a new failure mode), zw is nil. The corresponding release function (releaseRealGzipWriter / releaseRealDeflateWriter) then calls zw.Close() on a nil pointer, causing a panic and process crash. This path is currently unreachable because the clamped level is always valid, but the unchecked error is a latent nil-pointer dereference."
}
]
```