BudgetScan

Error path can retain pooled output buffer and allow unbounded growth

low swesweep-fasthttp · stackless/writer.go · (*writer).do · run #4

Description

When w.err is non-nil, do() returns before calling w.xw.Reset(), leaving the pooled ByteBuffer allocated. If the underlying writer from newWriter can both append output to xWriter and return errors, repeated operations can retain and grow the same buffer without returning it to the pool. This is memory-retention or resource-exhaustion risk, especially if the underlying writer is faulty or adversarial.

Triage

Not triaged.

Raw scan report for this file

`stackless/writer.go` exposes a small adapter whose security depends heavily on the correctness of the stackless runtime, the caller-supplied `dstW`, and the caller-supplied `newWriter`. There are no fixed-size buffers or direct integer-arithmetic bugs in this file, but there are robustness and state-integrity problems that can panic the host process or silently corrupt compressed output.

Key observations:

- `NewWriter` accepts `io.Writer` and `NewWriterFunc` without validation.
- `Write` / `WriteString` store caller input in `w.p` and run the underlying writer through `stacklessWriterFunc`.
- `do` executes the operation, then flushes the pooled `xWriter` buffer to `w.dstW`.
- `Reset` changes the destination writer even if the low-stack operation cannot be executed.
- `xWriter` uses `bytebufferpool`, so buffer lifecycle matters.

Most serious issue is `Reset()`:

```go
func (w *writer) Reset(dstW io.Writer) {
	w.xw.Reset()
	w.do(opReset) //nolint:errcheck
	w.dstW = dstW
}
```

`w.do(opReset)` can return `errHighLoad` when the stackless runtime is saturated. In that case `writerFunc` is not executed, so `w.zw.Reset(&w.xw)` is not called. However, `Reset` still updates `w.dstW`. The caller receives no error because `Reset` has a `void` signature. This can leave the underlying compression writer in a stale state while subsequent writes are sent to a new destination, causing stream corruption, cross-request state confusion, or availability impact under load.

Other issues:

- `do()` dereferences `w.dstW` when flushing `w.xw.bb.B`. If `NewWriter` was called with a nil `dstW`, the first write that produces buffered output panics.
- `NewWriter` does not check `newWriter` for nil, nor does it check the returned `Writer` for nil. A nil `newWriter` panics immediately; a nil returned `zw` panics later in `writerFunc`.
- If `w.err` is non-nil, `do()` returns before calling `w.xw.Reset()`. This can retain the pooled output buffer. If the underlying writer continues to append output while returning errors, the retained buffer can grow, causing memory retention or exhaustion.

Concurrent use of a single `writer` is also unsafe because `w.p`, `w.n`, `w.err`, `w.op`, and `w.xw.bb` are mutated without synchronization. This file does not document the writer as thread-safe, so this is treated more as an API contract issue than an exploitable zero-day.

```json
[
  {
    "severity": "high",
    "title": "Reset silently changes destination writer when stackless reset cannot execute",
    "function": "(*writer).Reset",
    "description": "Reset calls w.xw.Reset(), then w.do(opReset) and ignores the returned error. If do() returns errHighLoad, writerFunc is not executed and w.zw.Reset(&w.xw) is skipped, but Reset still assigns w.dstW = dstW. The underlying compression writer remains in its old state while future writes are flushed to the new destination. An attacker can increase concurrent load to make stackless execution unavailable, causing stream corruption, state confusion, or denial of service without the caller receiving an error."
  },
  {
    "severity": "medium",
    "title": "Nil destination writer causes panic when flushing buffered output",
    "function": "(*writer).do",
    "description": "do() calls w.dstW.Write(w.xw.bb.B) when the internal buffer is non-empty, but NewWriter does not validate dstW. If dstW is a nil io.Writer, the first operation that produces buffered output dereferences a nil interface and panics the host process."
  },
  {
    "severity": "low",
    "title": "Missing validation of newWriter and returned underlying writer can cause nil interface panics",
    "function": "NewWriter",
    "description": "NewWriter calls newWriter(&w.xw) without checking whether newWriter is nil, and it does not check whether the returned Writer is nil. A nil newWriter panics immediately, while a nil returned zw causes a nil interface method call later in writerFunc during Write, Flush, Close, or Reset."
  },
  {
    "severity": "low",
    "title": "Error path can retain pooled output buffer and allow unbounded growth",
    "function": "(*writer).do",
    "description": "When w.err is non-nil, do() returns before calling w.xw.Reset(), leaving the pooled ByteBuffer allocated. If the underlying writer from newWriter can both append output to xWriter and return errors, repeated operations can retain and grow the same buffer without returning it to the pool. This is memory-retention or resource-exhaustion risk, especially if the underlying writer is faulty or adversarial."
  }
]
```