Hijack path does not release buffered writer resources
low swesweep-fasthttp · fasthttpadaptor/adaptor.go · writer.Hijack · run #4
Description
When a request buffers writes and then calls Hijack(), consumePreflush() moves responseBody out, but w.bufPool remains set and releaseWriter() is not called in the modeHijacked branch. The pooled 32 KiB buffer is never returned. Repeated hijacked requests with buffered writes can leak memory and exhaust host resources. The buffer should be released after the preflush data is written or when the hijacked connection closes.
Triage
Not triaged.
Raw scan report for this file
## Security analysis
This file is a bridge between fasthttp and `net/http`. The trust boundary is:
remote client → `fasthttp.RequestCtx` → `ConvertRequest` → `http.Request` → wrapped `http.Handler` → `writer` → `fasthttp.ResponseCtx`.
The attacker controls the incoming request URI, headers, and body. The wrapped handler may then generate a response whose content is influenced by that untrusted input. The code is written in Go, so classic C-style stack overflows are not expected, but there are several realistic concurrency/lifetime, resource-exhaustion, and protocol-state issues.
### Most important issue: pooled response buffer lifetime
In the normal buffered path, the handler writes into `w.responseBody`. If the total response size stays within the 32 KiB pooled buffer, `w.responseBody` still aliases memory acquired from `bufferPool`. The code then does:
```go
ctx.Response.SetBody(w.responseBody)
releaseWriter(w)
```
`releaseWriter` returns that same pooled buffer to `bufferPool`. fasthttp typically stores the body slice and writes it after the handler returns. If another concurrent request obtains the same buffer from the pool, it can overwrite it while the first response is still being serialized.
This is a data race and can cause:
- cross-request response corruption,
- attacker-controlled tampering of another client’s response body,
- HTTP desynchronization,
- host-process memory integrity issues.
This is triggerable by an external attacker who simply issues concurrent requests that produce small responses through any wrapped handler.
### Other relevant issues
- `writer.Write` buffers arbitrarily before `Flush()`, allowing memory exhaustion if response size is derived from request input.
- `writer.Flush` can block forever if called after `modeDone` or `modeHijacked` has already been selected.
- The `modePanicked` branch re-panics in the fasthttp handler goroutine. If the host has not enabled fasthttp panic recovery, a single malformed request that causes the wrapped handler to panic can crash the entire process.
- `Hijack()` after `Flush()` can lead to two writers using the same connection, corrupting the response stream.
- The `Hijack()` path does not release buffered writer resources, causing memory leaks.
- Late writes to the `ResponseWriter` after the response has been released can race with recycled pooled buffers.
The fixed-size 32 KiB buffer and the `min(len(...), 512)` usage do not appear to be directly vulnerable to out-of-bounds access. The main risks are lifetime/concurrency and resource bounds.
```json
[
{
"severity": "high",
"title": "Pooled response buffer reused before fasthttp finishes writing response",
"function": "NewFastHTTPHandler",
"description": "In the modeDone path, ctx.Response.SetBody(w.responseBody) may store a reference to a slice backed by bufferPool. releaseWriter(w) immediately returns that buffer to the pool, but fasthttp writes the response after the handler returns. A concurrent request can acquire the same buffer and overwrite it while the first response is still being serialized. This creates a data race and can corrupt or tamper with response bodies across requests. An external attacker can trigger this by issuing concurrent requests that produce small responses through any wrapped net/http handler."
},
{
"severity": "medium",
"title": "Unbounded pre-Flush response buffering allows memory exhaustion",
"function": "writer.Write",
"description": "Before Flush() is called, Write appends all written data to w.responseBody with no size limit. The initial 32 KiB pooled buffer can be reallocated and grow without bound. If a wrapped handler echoes or derives response size from untrusted request input, a remote attacker can cause large allocations or OOM in the host process. A response buffer cap or automatic transition to streaming would mitigate this."
},
{
"severity": "medium",
"title": "Wrapped handler panic is re-panicked in the fasthttp handler goroutine",
"function": "NewFastHTTPHandler",
"description": "If h.ServeHTTP panics, the spawned goroutine recovers the panic and sends modePanicked. The main goroutine then executes panic(\"net/http handler panicked\"). If the fasthttp server does not have panic recovery configured, a single malicious request that triggers a panic in the wrapped handler can crash the entire process. Even if recovery is configured, this converts a handler-level error into a broader server-level panic. A safer behavior would be to send a 500 response when possible."
},
{
"severity": "medium",
"title": "Flush() can block forever after completion or hijack",
"function": "writer.Flush",
"description": "Flush() nonblockingly sends modeFlushed and then blocks on <-w.streamReady. If the state machine has already selected modeDone, for example because Flush is invoked from a goroutine after the handler returned, or modeHijacked, streamReady is never closed. The calling goroutine leaks and the writer remains allocated. A handler that performs asynchronous flushing based on request input can be used to exhaust goroutines and memory."
},
{
"severity": "low",
"title": "Hijack after Flush can interleave writes on the same connection",
"function": "writer.Hijack",
"description": "If Flush() has already selected modeFlushed, the main goroutine has started ctx.SetBodyStreamWriter. A subsequent Hijack() still obtains ctx.Conn(), calls ctx.Hijack, and writes consumePreflush() into a bufio writer. The streaming writer and the hijacked connection writer can then write concurrently to the same client connection, corrupting the response stream. Calling ctx.Hijack after streaming has begun may also be unsupported and cause a panic. Hijack should be rejected once streaming has started."
},
{
"severity": "low",
"title": "Hijack path does not release buffered writer resources",
"function": "writer.Hijack",
"description": "When a request buffers writes and then calls Hijack(), consumePreflush() moves responseBody out, but w.bufPool remains set and releaseWriter() is not called in the modeHijacked branch. The pooled 32 KiB buffer is never returned. Repeated hijacked requests with buffered writes can leak memory and exhaust host resources. The buffer should be released after the preflush data is written or when the hijacked connection closes."
},
{
"severity": "low",
"title": "Late ResponseWriter writes can race with released pooled buffers",
"function": "writer.Write",
"description": "The writer does not prevent writes after the handler has returned. In the modeDone path, releaseWriter returns the pooled buffer and closes pipes. A delayed goroutine that later calls Write() may see streamReady not closed, acquire or reuse pooled buffer memory, and append to w.responseBody after it was released. This can cause data races and cross-request corruption. Although this is a misuse of http.ResponseWriter, a handler that performs async writes based on request input can make the condition reachable from external input."
}
]
```