Nil io.Writer causes panic in WriteBrotliLevel default branch
low swesweep-fasthttp · brotli.go · WriteBrotliLevel · run #2
Description
A nil io.Writer falls into the default type-switch branch, creating a stackless writer wrapping nil. The subsequent Write call panics with a nil pointer dereference. In a fasthttp server context this can crash the handler goroutine. The public API should guard against nil w.
Triage
Not triaged.
Raw scan report for this file
## Security analysis of `brotli.go`
This file exposes public brotli compression/decompression helpers for `fasthttp`. The trust boundary is the public API:
- `AppendBrotliBytesLevel(dst, src, level)`
- `WriteBrotliLevel(w, p, level)`
- `WriteBrotli(w, p)`
- `AppendBrotliBytes(dst, src)`
- `WriteUnbrotli(w, p)`
- `AppendUnbrotliBytes(dst, src)`
The main risks are:
1. **Unbounded decompression / decompression bomb**
- `WriteUnbrotli` calls the internal `writeUnbrotli(w, p, 0)`.
- `writeUnbrotli` passes `maxBodySize = 0` to `copyZeroAllocWithLimit(w, zr, maxBodySize)`.
- In `fasthttp`, `0` normally means “no limit”.
- Therefore, a small malicious brotli payload can expand to a very large amount of output.
- `AppendUnbrotliBytes` makes this worse because it wraps `dst` in `byteSliceWriter`, so the decompressed output is appended to `dst` with no external cap.
- The later check:
```go
nn := int(n)
if int64(nn) != n {
return 0, fmt.Errorf("too much data unbrotlied: %d", n)
}
```
only helps on 32-bit platforms and only after the copy has already happened. On 64-bit platforms it does not impose a limit.
- Impact: remote denial of service through memory exhaustion when an untrusted brotli payload is decompressed.
2. **Unvalidated compression level passed to the brotli writer**
- Public functions accept `level int` directly.
- `normalizeBrotliCompressLevel` clamps the value only for use as a pool index:
```go
if level < 0 || level > 11 {
level = CompressBrotliDefaultCompression
}
```
- However, `acquireRealBrotliWriter` passes the **original** `level` to `brotli.NewWriterLevel(w, level)`.
- If the dependency assumes brotli levels are `[0..11]`, out-of-range values such as `-1` or `12` can cause a panic, for example by indexing a level table outside bounds.
- Even if the dependency clamps the level internally, the pool key is normalized while the actual writer state is not. Writers created with out-of-range levels can be stored under the default pool bucket and later reused for a legitimate level such as `4`.
- This can lead to:
- process crash / panic,
- incorrect compression behavior,
- cross-contamination of pooled writers.
3. **Ignored stackless deferred-execution return value can cause a data race**
- For non-blocking writer types:
```go
case *byteSliceWriter,
*bytes.Buffer,
*bytebufferpool.ByteBuffer:
```
`WriteBrotliLevel` uses `stacklessWriteBrotli(ctx)`.
- `stacklessWriteBrotli` does this:
```go
stacklessWriteBrotliFunc(ctx)
```
and ignores the returned `bool`.
- In `fasthttp/stackless`, that boolean generally indicates whether the function ran synchronously in the current goroutine or whether execution was deferred to another goroutine.
- If the call is deferred, `WriteBrotliLevel` returns before the compressed data has actually been written.
- For `AppendBrotliBytesLevel`, that means `dst` can be returned to the caller before the brotli writer has appended the compressed bytes.
- This creates a data race on:
- `dst` / `byteSliceWriter.b`,
- possibly `src` / `p`,
- internal writer state.
- It can also cause:
- incomplete compressed output,
- corrupted compressed output,
- lost write errors,
- incorrect returned byte count.
- This is especially important because `AppendBrotliBytesLevel` is a public API that returns the final buffer.
4. **Incorrect returned byte count for non-blocking writers**
- For the non-blocking writer cases, `WriteBrotliLevel` returns:
```go
return len(p), nil
```
- The documentation says the function returns “the number of compressed bytes written to `w`”.
- `len(p)` is the **uncompressed input length**, not the compressed output length.
- If a caller uses this return value to set `Content-Length`, manage response buffers, or perform protocol framing, it may produce incorrect lengths or protocol desynchronization.
- This is lower severity than the previous issues, but it is still a public API correctness problem with security relevance.
## Areas reviewed and deprioritized
- **Pool type assertions** such as `v.(*brotli.Reader)`, `v.(*brotli.Writer)`, and `v.(stackless.Writer)` are internal. The pools are only populated by internal release functions, so they are not directly attacker-controllable.
- **Nil `p` or nil `dst`** are mostly safe:
- `p == nil` compresses/decompresses empty input.
- `dst == nil` works with `byteSliceWriter` because `append` can start from `nil`.
- **Nil `w`** can panic, but that is primarily a caller-error issue rather than a typical untrusted-data issue.
- **Ignored compression errors** in `nonblockingWriteBrotli` are real, but for the specific non-blocking writer types used here the practical impact is mostly correctness/data-race related rather than a direct memory-safety bug.
- There are **no fixed-size stack buffers** in this file, so there is no classic C-style buffer overflow.
```json
[
{
"severity": "high",
"title": "Unbounded brotli decompression can exhaust memory",
"function": "WriteUnbrotli",
"description": "WriteUnbrotli calls writeUnbrotli with maxBodySize=0, and writeUnbrotli passes that value to copyZeroAllocWithLimit. In fasthttp, 0 normally disables the limit, so an attacker-controlled brotli payload can decompress to arbitrarily large output. AppendUnbrotliBytes makes this exploitable as a decompression bomb because it appends the decompressed data to dst without any cap. The int overflow check in writeUnbrotli runs after the copy has already written data and is only effective on 32-bit platforms, so it does not prevent denial of service via memory exhaustion."
},
{
"severity": "medium",
"title": "Out-of-range compression level is passed to brotli.NewWriterLevel and pooled under a normalized key",
"function": "acquireRealBrotliWriter",
"description": "WriteBrotliLevel and AppendBrotliBytesLevel accept an arbitrary int level. normalizeBrotliCompressLevel only bounds the value for pool indexing, but acquireRealBrotliWriter passes the original level to brotli.NewWriterLevel. If the brotli dependency expects levels in [0..11], values such as -1 or 12 can cause a panic in the host process. Even if the dependency clamps the level, writers created with out-of-range levels are stored under the normalized default pool bucket, and Reset does not change their configured level. Later legitimate requests for the default level can reuse a writer configured for a different level, causing incorrect compression behavior or further undefined behavior."
},
{
"severity": "high",
"title": "Ignored stackless deferred-execution return value can cause a data race in brotli compression",
"function": "stacklessWriteBrotli",
"description": "stacklessWriteBrotli ignores the bool returned by stacklessWriteBrotliFunc. In the fasthttp stackless runtime, this return value indicates whether nonblockingWriteBrotli executed synchronously in the current goroutine or was deferred to another goroutine. If execution is deferred, WriteBrotliLevel returns len(p), nil and AppendBrotliBytesLevel returns dst before the compressed bytes have been written. This creates a data race on dst/src and internal writer state, can return incomplete or corrupted compressed output, and hides write errors. The race is reachable when compression is invoked with a stack near the limit or from a deep call stack."
},
{
"severity": "low",
"title": "WriteBrotliLevel returns uncompressed input length instead of compressed byte count for non-blocking writers",
"function": "WriteBrotliLevel",
"description": "For *byteSliceWriter, *bytes.Buffer, and *bytebufferpool.ByteBuffer, WriteBrotliLevel returns len(p), nil. That value is the uncompressed input length, not the number of compressed bytes written to the writer, contrary to the documented contract. Callers that use the returned value to set Content-Length, manage response buffers, or perform protocol framing may produce incorrect lengths or misuse memory, which can lead to protocol desynchronization or resource handling bugs."
}
]
```