Nil StreamWriter accepted without validation
medium swesweep-fasthttp · stream.go · NewStreamReader · run #4
Description
NewStreamReader accepts a nil StreamWriter. The goroutine invokes sw(bw) without checking for nil, causing a nil function-call panic. If host code selects a nil writer based on an attacker-controlled path, header, or parameter, this is a remotely triggerable denial-of-service condition.
Triage
- Verdict
- INVALID confidence 0.17
- Model
- qwen/qwen3.8-27b
- Rounds
- 6
- Cost
- $0.0144 · 319.9 s
[ARBITER] The threat model defines the attacker as controlling data payloads passed through public APIs, not the control flow or code objects (function pointers) of the host application. The `StreamWriter` parameter is a Go function type (`func(w *bufio.Writer)`), which is a code object defined by the developer of the host application, not an untrusted data payload (like a string or byte slice) that can be remotely manipulated to be nil. Passing a nil function is a programming error (violating a documented or implicit precondition that the callback must be valid), not a remotely triggerable denial-of-service vulnerability under the specified threat model.
All rounds
[
{
"finding_title": "Nil StreamWriter accepted without validation",
"verdict": "VALID",
"reasoning": "The vulnerability report claims that passing a nil `StreamWriter` to `NewStreamReader` results in a panic due to calling a nil function (`sw(bw)`). I verified the code in `stream.go`: the function accepts `sw StreamWriter`, creates a goroutine, and invokes `sw(bw)`. In Go, calling a nil function value indeed causes a panic. The function `NewStreamReader` is public (exported). The threat model defines the attacker as code calling the public API. If an application (e.g., a proxy or API server) allows user input to dictate which stream writer is used, or if a developer mistakenly passes a nil writer due to a logic error triggered by untrusted input, the host process will crash (panic). Since fasthttp is a library used in server applications, a crash in the server process constitutes a Denial of Service (DoS). The lack of a nil check in a public API function that spawns a goroutine and calls the argument is a valid security concern under the provided threat model, as it allows for remote triggering of a crash if the application logic maps untrusted input to the selection of this writer. While one might argue that passing nil is programmer error, the rules state that if an external attacker can trigger it (via the public API surface) to cause harm (crash/DoS), it is VALID. The absence of a defensive check makes this a real, reachable bug pattern for DoS.\n\nCRUX: The public function NewStreamReader calls the StreamWriter argument sw(bw) without checking if sw is nil, allowing a nil function call panic if an attacker or application logic passes a nil writer.\nGREP: func NewStreamReader",
"elapsed": 58.8,
"tokens": 2511,
"tokens_in": 2154,
"tokens_out": 357,
"cost_usd": 0.0019756799999999996,
"file": "stream.go",
"round": 1,
"grep_used": true,
"grep_results": "GREP `func NewStreamReader`:\n```\nstream.go:29:func NewStreamReader(sw StreamWriter) io.ReadCloser {\n```"
},
{
"finding_title": "Nil StreamWriter accepted without validation",
"verdict": "INVALID",
"reasoning": "The report claims that passing a nil `StreamWriter` to `NewStreamReader` causes a panic. While this is technically true in Go (calling a nil function panics), the critical question is whether an *external attacker* can trigger this. \n\n1. **Nature of the Input**: The `sw` parameter is a `StreamWriter`, which is a Go function type (`func(w *bufio.Writer)`). It is not a string, byte slice, or network payload. It is a code object defined in the host application's source code.\n2. **Attacker Capability**: An external attacker (e.g., a client sending an HTTP request) cannot inject or modify the code of the server application. They cannot pass a `nil` function pointer to the library; they can only send data *to* the server. The server application's logic decides which `StreamWriter` to instantiate and pass to `NewStreamReader`.\n3. **Threat Model**: The threat model defines the attacker as \"Code that calls the public API... and above all the data it passes through... parsers, decoders... receive adversarial payloads\". The `sw` argument is code logic, not an adversarial payload. A `nil` value for `sw` would result from a bug in the host application (e.g., a missing initialization or a logic error in a handler), not from an external attack vector. \n4. **Code Quality vs Security**: This is a missing parameter validation in a public API, but the \"input\" is the function itself. In a library context, panicking on invalid input (like a nil function where a non-nil callback is expected) is often considered acceptable or even desirable for catching programming errors early. It is not a memory safety vulnerability or a remote exploitability issue because the attacker does not control the *existence* or *nil-ness* of the function variable in the host's memory space.\n\nTherefore, this is a code quality issue (missing defensive check) and not a security vulnerability under the specified threat model where an external attacker controls untrusted data (payloads), not the application's control flow or code objects.\n\nCRUX: The `StreamWriter` argument is a function object defined by the host application, not an untrusted data payload controlled by an external attacker; thus, passing nil is a programmer error, not a remote trigger.\nGREP: func NewStreamReader",
"elapsed": 53.7,
"tokens": 3181,
"tokens_in": 2675,
"tokens_out": 506,
"cost_usd": 0.0026414999999999998,
"file": "stream.go",
"round": 2,
"grep_used": true,
"grep_results": "GREP `func NewStreamReader`:\n```\nstream.go:29:func NewStreamReader(sw StreamWriter) io.ReadCloser {\n```"
},
{
"finding_title": "Nil StreamWriter accepted without validation",
"verdict": "INVALID",
"reasoning": "The report claims that a nil `StreamWriter` passed to `NewStreamReader` causes a panic, which is technically true in Go. However, to be a valid security vulnerability under the provided threat model, an external attacker must be able to trigger this condition via untrusted input (e.g., data from a network request). `NewStreamReader` takes a `StreamWriter` (a function type) as an argument. In Go, function values are code objects managed by the host application's runtime and compiler; they are not serialized data. An external attacker sending HTTP requests to a server using fasthttp cannot influence whether the host application passes a `nil` function pointer or a valid function pointer to this library call. The decision to pass `nil` or a function is a control-flow decision made by the developer's code logic. While a developer *could* write logic that passes `nil` based on user input (e.g., `if (userChoice == 'A') { sw = nil } else { sw = myWriter }`), this would be a bug in the host application's logic, not a vulnerability in the fasthttp library itself. The library correctly assumes it is called with a valid `StreamWriter` as per its documentation (`StreamWriter` is defined as a callback). Panicking on invalid arguments (like a nil function where a valid one is expected) is a standard behavior in many libraries to catch programming errors. It does not constitute a remote denial-of-service vulnerability attributable to the library, as the attacker does not control the 'input' (the function pointer) in a way that bypasses the trust boundary. This falls under the 'misuse that violates a documented precondition' or 'programmer error' category, which are not vulnerabilities in the library context described by the threat model rules ('Not a vulnerability under this model: ... misuse that violates a documented precondition'). Therefore, it is a code quality issue or a host application bug, not a security vulnerability in swesweep-fasthttp.\n\nCRUX: The `sw` parameter is a function value (code object) controlled by the host application's logic, not an untrusted data payload that an external attacker can directly manipulate to be nil.\nGREP: type StreamWriter func(w *bufio.Writer)",
"elapsed": 48.6,
"tokens": 3662,
"tokens_in": 3193,
"tokens_out": 469,
"cost_usd": 0.00274806,
"file": "stream.go",
"round": 3,
"grep_used": true,
"grep_results": "GREP `type StreamWriter func(w *bufio.Writer) (simplified to: StreamWriter)`:\n```\nfasthttpadaptor/adaptor.go:118:\t\t\t// Streaming: send headers and start SetBodyStreamWriter.\nfasthttpadaptor/adaptor.go:143:\t\t\tctx.SetBodyStreamWriter(func(bw *bufio.Writer) {\nREADME.md:396: [ctx.SetBodyStreamWriter()](https://pkg.go.dev/github.com/valyala/fasthttp#RequestCtx.SetBodyStreamWriter)\nrequestctx_setbodystreamwriter_example_test.go:12:func ExampleRequestCtx_SetBodyStreamWriter() {\nrequestctx_setbodystreamwriter_example_test.go:21:\tctx.SetBodyStreamWriter(func(w *bufio.Writer) {\nserver_test.go:696:\t\tctx.SetBodyStreamWriter(func(w *bufio.Writer) {\nserver_test.go:704:\t\t\t// be flushed automatically after returning from StreamWriter.\nserver_test.go:2878:func TestRequestCtxSetBodyStreamWriter(t *testing.T) {\nserver_test.go:2888:\tctx.SetBodyStreamWriter(func(w *bufio.Writer) {\nserver_test.go:4632:\t\t\tctx.SetBodyStreamWriter(func(w *bufio.Writer) {\nserver_test.go:4727:\t\tctx.Response.SetBodyStreamWriter(func(w *bufio.Writer) {\nstream.go:11:// StreamWriter must write data to w.\nstream.go:13:// Usually StreamWriter writes data to w in a loop (aka 'data streaming').\nstream.go:15:// StreamWriter must return immediately if w returns error.\nstream.go:19:type StreamWriter func(w *bufio.Writer)\nstream.go:28:// See also Response.SetBodyStreamWriter.\nstream.go:29:func NewStreamReader(sw StreamWriter) io.ReadCloser {\nserver.go:1617:// See also SetBodyStreamWriter.\nserver.go:1622:// SetBodyStreamWriter registers the given stream writer for populating\nserver.go:1633:func (ctx *RequestCtx) SetBodyStreamWriter(sw StreamWriter) {\nserver.go:1634:\tctx.Response.SetBodyStreamWriter(sw)\nhttp.go:247:// See also SetBodyStreamWriter.\nhttp.go:264:// See also SetBodyStreamWriter.\nhttp.go:281:// SetBodyStreamWriter registers the given sw for populating request body.\nhttp.go:293:func (req *Request) SetBodyStreamWriter(sw StreamWriter) {\nhttp.go:298:// SetBodyStreamWriter registers the given sw for populating response body.\nhttp.go:308:func (resp *Response) SetBodyStreamWriter(sw StreamWriter) {\nhttp.go:317:// or SetBodyStreamWriter in this case.\nhttp_test.go:1496:\tr.SetBodyStreamWriter(func(w *bufio.Writer) {\nhttp_test.go:1521:\tr.SetBodyStreamWriter(func(w *bufio.Writer) {\n```"
},
{
"finding_title": "Nil StreamWriter accepted without validation",
"verdict": "INVALID",
"reasoning": "The reported vulnerability is that passing a nil `StreamWriter` to the public API `NewStreamReader` causes a panic due to a nil function call. While the code indeed lacks a nil check, the threat model for a reusable library defines the attacker as an entity that controls **data payloads** (parsers, decoders, network input) passed through public APIs, not the application's control flow or code objects. The `sw` parameter is a Go function value (a code pointer) defined in the host application's source code. An external attacker sending HTTP requests or binary data cannot manipulate the host application's memory layout or logic to force the passing of a `nil` function pointer into this library function. Passing `nil` is a programmer error or a misuse of the API that violates the precondition that a valid callback is provided. According to the threat model rules, 'misuse that violates a documented precondition' and issues where the attacker does not control the specific input (the function reference) are not considered security vulnerabilities. Therefore, this is a code quality issue (missing defensive validation) rather than a security vulnerability reachable by an external attacker.\n\nCRUX: The `sw` parameter is a function code object controlled by the host application's logic, not an untrusted data payload that an external attacker can manipulate to be nil.\nGREP: func NewStreamReader",
"elapsed": 39.1,
"tokens": 4572,
"tokens_in": 4285,
"tokens_out": 287,
"cost_usd": 0.0026607000000000002,
"file": "stream.go",
"round": 4,
"grep_used": true,
"grep_results": "GREP `func NewStreamReader`:\n```\nstream.go:29:func NewStreamReader(sw StreamWriter) io.ReadCloser {\n```"
},
{
"finding_title": "Nil StreamWriter accepted without validation",
"verdict": "INVALID",
"reasoning": "The report claims that a nil `StreamWriter` passed to `NewStreamReader` causes a panic, leading to Denial of Service. While it is true that calling a nil function in Go results in a panic, the critical question is whether an external attacker can trigger this condition via untrusted input. The `StreamWriter` parameter is a Go function type (`func(w *bufio.Writer)`). In the context of a reusable library like fasthttp, the `sw` argument is a code object defined by the host application's developer, not a data payload received from an external source. An external attacker (e.g., sending HTTP requests) cannot directly control the Go source code or the memory layout to force the host application to pass a `nil` function pointer to this library call. Any scenario where `nil` is passed would require a bug in the host application's logic (e.g., a missing initialization or a logic error in a handler), which constitutes a violation of the API's documented precondition (that a valid callback is provided). According to the threat model, 'misuse that violates a documented precondition' and issues where the attacker does not control the specific input (the function reference) are not considered security vulnerabilities in the library itself. This is a code quality issue (missing defensive check) rather than a remotely triggerable security vulnerability.\n\nCRUX: The `sw` parameter is a function code object controlled by the host application's logic, not an untrusted data payload that an external attacker can manipulate to be nil.\nGREP: type StreamWriter func(w *bufio.Writer)",
"elapsed": 49.1,
"tokens": 4399,
"tokens_in": 4068,
"tokens_out": 331,
"cost_usd": 0.00270156,
"file": "stream.go",
"round": 5,
"grep_used": true,
"grep_results": "GREP `type StreamWriter func(w *bufio.Writer) (simplified to: StreamWriter)`:\n```\nfasthttpadaptor/adaptor.go:118:\t\t\t// Streaming: send headers and start SetBodyStreamWriter.\nfasthttpadaptor/adaptor.go:143:\t\t\tctx.SetBodyStreamWriter(func(bw *bufio.Writer) {\nREADME.md:396: [ctx.SetBodyStreamWriter()](https://pkg.go.dev/github.com/valyala/fasthttp#RequestCtx.SetBodyStreamWriter)\nserver.go:1617:// See also SetBodyStreamWriter.\nserver.go:1622:// SetBodyStreamWriter registers the given stream writer for populating\nserver.go:1633:func (ctx *RequestCtx) SetBodyStreamWriter(sw StreamWriter) {\nserver.go:1634:\tctx.Response.SetBodyStreamWriter(sw)\nrequestctx_setbodystreamwriter_example_test.go:12:func ExampleRequestCtx_SetBodyStreamWriter() {\nrequestctx_setbodystreamwriter_example_test.go:21:\tctx.SetBodyStreamWriter(func(w *bufio.Writer) {\nserver_test.go:696:\t\tctx.SetBodyStreamWriter(func(w *bufio.Writer) {\nserver_test.go:704:\t\t\t// be flushed automatically after returning from StreamWriter.\nserver_test.go:2878:func TestRequestCtxSetBodyStreamWriter(t *testing.T) {\nserver_test.go:2888:\tctx.SetBodyStreamWriter(func(w *bufio.Writer) {\nserver_test.go:4632:\t\t\tctx.SetBodyStreamWriter(func(w *bufio.Writer) {\nserver_test.go:4727:\t\tctx.Response.SetBodyStreamWriter(func(w *bufio.Writer) {\nstream.go:11:// StreamWriter must write data to w.\nstream.go:13:// Usually StreamWriter writes data to w in a loop (aka 'data streaming').\nstream.go:15:// StreamWriter must return immediately if w returns error.\nstream.go:19:type StreamWriter func(w *bufio.Writer)\nstream.go:28:// See also Response.SetBodyStreamWriter.\nstream.go:29:func NewStreamReader(sw StreamWriter) io.ReadCloser {\nhttp.go:247:// See also SetBodyStreamWriter.\nhttp.go:264:// See also SetBodyStreamWriter.\nhttp.go:281:// SetBodyStreamWriter registers the given sw for populating request body.\nhttp.go:293:func (req *Request) SetBodyStreamWriter(sw StreamWriter) {\nhttp.go:298:// SetBodyStreamWriter registers the given sw for populating response body.\nhttp.go:308:func (resp *Response) SetBodyStreamWriter(sw StreamWriter) {\nhttp.go:317:// or SetBodyStreamWriter in this case.\nhttp_test.go:1496:\tr.SetBodyStreamWriter(func(w *bufio.Writer) {\nhttp_test.go:1521:\tr.SetBodyStreamWriter(func(w *bufio.Writer) {\n```"
},
{
"verdict": "INVALID",
"reasoning": "[ARBITER] The threat model defines the attacker as controlling data payloads passed through public APIs, not the control flow or code objects (function pointers) of the host application. The `StreamWriter` parameter is a Go function type (`func(w *bufio.Writer)`), which is a code object defined by the developer of the host application, not an untrusted data payload (like a string or byte slice) that can be remotely manipulated to be nil. Passing a nil function is a programming error (violating a documented or implicit precondition that the callback must be valid), not a remotely triggerable denial-of-service vulnerability under the specified threat model.",
"round": 6,
"file": "stream.go",
"finding_title": "Nil StreamWriter accepted without validation",
"elapsed": 70.6,
"tokens_in": 3020,
"tokens_out": 142,
"cost_usd": 0.0016943999999999998
}
]Raw scan report for this file
## Security analysis
The exported entry point is `NewStreamReader`. It accepts a caller-supplied `StreamWriter` callback, creates an in-memory pipe pair, wraps the write end in a `bufio.Writer`, starts a goroutine to invoke the callback, and returns the read end.
Relevant data flow:
```text
attacker-derived data
-> host handler / sw closure
-> bw.Write(...)
-> bufio buffer
-> bw.Flush()
-> pw PipeConn
-> pr PipeConn
-> HTTP response body written to client
```
Because this is Go code, there are no C-style stack/heap overflows from the `bufio` or pipe buffers. The main risks are:
1. **Goroutine panic isolation / nil callback**
- `sw` is a function value and may be `nil`.
- The goroutine calls `sw(bw)` without checking it.
- There is no `recover()` in the goroutine.
- If `sw` panics while processing untrusted input, the panic occurs outside the normal request-handler recovery path and can crash the host process.
- If the process is somehow kept alive by an external recovery mechanism, the panic path also skips `pw.Close()` and `streamWriterBufPool.Put(bw)`, leaking pipe and buffer resources.
2. **Reader lifecycle / backpressure**
- The returned reader `pr` must be read and `Close`d.
- If a client stops reading, or the caller abandons `pr`, the writer goroutine can block indefinitely once the pipe/bufio buffers fill.
- There is no timeout, cancellation, or way to abort the stream writer.
- An attacker can potentially open many connections and stop reading response bodies to accumulate blocked goroutines and pipe objects.
3. **Non-findings / lower-priority issues**
- `v.(*bufio.Writer)` could panic if the pool were poisoned, but the pool is package-private and only this code puts `*bufio.Writer` values into it. This is not realistically attacker-controlled.
- Ignored errors from `bw.Flush()` and `pw.Close()` are not directly exploitable here; they may mask errors, but they do not create memory corruption or obvious remote DoS beyond the existing lifecycle issues.
- Unbounded streamed input is a property of the streaming API. The file itself does not enforce length limits, but that is usually a host/handler policy issue.
```json
[
{
"severity": "high",
"title": "Stream writer goroutine lacks panic recovery and cleanup",
"function": "NewStreamReader",
"description": "The goroutine created by NewStreamReader calls the caller-supplied StreamWriter without recover and without guaranteed cleanup. If sw panics while processing attacker-controlled request data, the panic occurs outside the normal request-handler recovery path and can terminate the host process, causing denial of service. On the panic path pw is not closed and bw is not returned to streamWriterBufPool, so any recovery harness would also leak pipe and buffered-writer resources."
},
{
"severity": "medium",
"title": "Nil StreamWriter accepted without validation",
"function": "NewStreamReader",
"description": "NewStreamReader accepts a nil StreamWriter. The goroutine invokes sw(bw) without checking for nil, causing a nil function-call panic. If host code selects a nil writer based on an attacker-controlled path, header, or parameter, this is a remotely triggerable denial-of-service condition."
},
{
"severity": "low",
"title": "Abandoned reader can leave stream writer blocked indefinitely",
"function": "NewStreamReader",
"description": "The returned reader must be consumed and Close()d, but the API provides no timeout or cancellation. If a client stops reading or the caller abandons the reader, the writer goroutine can block indefinitely in bufio/pipe backpressure. An attacker can open many connections and not read response bodies to accumulate blocked goroutines and pipe objects, causing resource exhaustion. Severity is low because the documented contract requires Close, but the primitive is unsafe in hostile HTTP environments."
}
]
```