Stream writer goroutine lacks panic recovery and cleanup
high swesweep-fasthttp · stream.go · NewStreamReader · run #4
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.
Triage
- Verdict
- INVALID confidence 0.0
- Model
- qwen/qwen3.8-27b
- Rounds
- 5
- Cost
- $0.0130 · 675.1 s
The reported vulnerability claims that a panic in the goroutine created by `NewStreamReader` will terminate the host process because there is no `recover` call. While it is true that an unhandled panic in a goroutine crashes the Go process, the threat model for a library defines the attacker as an external entity providing untrusted data (e.g., HTTP payloads) to the public API, not the entity writing the host application code. The `StreamWriter` (`sw`) is a callback function supplied by the host application (the library consumer). For an external attacker to trigger this panic, they would need to cause the host application's own code to panic via crafted input. This implies a bug in the host application's logic (e.g., nil pointer dereference, division by zero) rather than a vulnerability in `fasthttp`'s internal code. `fasthttp` explicitly documents in `server.go` that it does not perform panic recovery, shifting the responsibility for panic safety to the host application. Since the panic originates from trusted internal code (the host's callback) and the library explicitly disclaims responsibility for panic recovery in user-supplied callbacks, this is not a security vulnerability in the library under the defined threat model. It is a code quality/robustness issue or expected behavior for user-supplied callbacks. CRUX: The panic originates from host-supplied code (StreamWriter), and the library explicitly documents that it does not provide panic recovery, shifting the responsibility to the host application. GREP: no `panic` recovery is done by `fasthttp
All rounds
[
{
"finding_title": "Stream writer goroutine lacks panic recovery and cleanup",
"verdict": "INVALID",
"reasoning": "The report claims that a panic in the `StreamWriter` callback within the goroutine spawned by `NewStreamReader` can terminate the host process because there is no `recover` call. While it is true that an unhandled panic in a goroutine will crash the entire process in Go, this scenario requires the `StreamWriter` (`sw`) function itself to panic. The `StreamWriter` is supplied by the host application (the user of the `fasthttp` library). If the host application writes malformed data or encounters a bug in their own streaming logic (e.g., dividing by zero, nil pointer dereference in their closure), that is a bug in the host application's code, not a vulnerability in `fasthttp`. The library correctly passes the function call to the host's closure. The threat model for a library defines the attacker as someone controlling data passed through the public API. Here, the 'attack' would be the host application's own code panicking. A library cannot prevent the host from crashing if the host's own code panics; this is standard Go semantics. Furthermore, `fasthttp`'s core HTTP server typically wraps request handling in a `recover` (often in `serve.go` or similar), but `NewStreamReader` is a utility that can be used anywhere. Even if used in a context without a global recover, the responsibility for handling panics in user-supplied callbacks lies with the user (the host app), not the library providing the wrapper. The library has not violated a security invariant; it has simply exposed a mechanism where host bugs (panics in host code) propagate. This is not a security vulnerability in the library but a code quality/robustness issue or expected behavior for user-supplied callbacks. Therefore, it is INVALID under the threat model.\n\nCRUX: The panic originates from the host application's supplied StreamWriter callback, which is user code, not an internal flaw in the library triggered by malicious network input.\nGREP: recover",
"elapsed": 62.0,
"tokens": 2603,
"tokens_in": 2187,
"tokens_out": 416,
"cost_usd": 0.00216654,
"file": "stream.go",
"round": 1,
"grep_used": true,
"grep_results": "GREP `recover`:\n```\nfasthttpadaptor/adaptor.go:66:\t\t\t\tif rec := recover(); rec != nil {\nfasthttpadaptor/adaptor_test.go:699:\t\trecover() //nolint:errcheck\nprefork/prefork_test.go:282:\t\t\trecord(\"recover\", oldPid, newPid)\nprefork/prefork_test.go:297:\tvar recoverCount int\nprefork/prefork_test.go:310:\t\tcase \"recover\":\nprefork/prefork_test.go:311:\t\t\trecoverCount++\nprefork/prefork_test.go:313:\t\t\t\tt.Errorf(\"recover event has invalid PIDs: %v\", e.pids)\nprefork/prefork_test.go:316:\t\t\t\tt.Error(\"recover old and new PID should differ\")\nprefork/prefork_test.go:327:\t// Initial spawns + at least one recovery spawn\nprefork/prefork_test.go:332:\tif recoverCount == 0 {\nprefork/prefork_test.go:355:\trecoveredSpawnByPID := make(map[int]bool)\nprefork/prefork_test.go:356:\trecoveredPIDs := make(map[int]bool)\nprefork/prefork_test.go:359:\t\t\trecoveredSpawnByPID[e.pids[0]] = true\nprefork/prefork_test.go:361:\t\tif e.name == \"recover\" {\nprefork/prefork_test.go:362:\t\t\tif !recoveredSpawnByPID[e.pids[1]] {\nprefork/prefork_test.go:365:\t\t\trecoveredPIDs[e.pids[1]] = true\nprefork/prefork_test.go:368:\tfor pid := range recoveredPIDs {\nprefork/prefork_test.go:369:\t\tif !recoveredSpawnByPID[pid] {\nprefork/prefork_test.go:383:\tvar recoverCount int\nprefork/prefork_test.go:406:\t\t\trecoverCount++\nprefork/prefork_test.go:414:\tif recoverCount != 0 {\nprefork/prefork_test.go:415:\t\tt.Fatalf(\"OnChildRecover called %d times, want 0\", recoverCount)\nprefork/prefork.go:373:\t\t\t\tp.logger().Printf(\"OnChildSpawn callback failed for recovered PID %d: %v\\n\", newPid, err)\nserver_test.go:4221:\t\t\t// This will panic, but I was not able to intercept with recover()\nserver.go:166:\t// Take into account that no `panic` recovery is done by `fasthttp` (thus any `panic` will take down the entire server).\nserver.go:167:\t// Instead the user should use `recover` to handle these situations.\nhttp.go:2194:\t\tif r := recover(); r != nil {\n```"
},
{
"finding_title": "Stream writer goroutine lacks panic recovery and cleanup",
"verdict": "INVALID",
"reasoning": "The report claims that a panic in the `StreamWriter` callback within the `NewStreamReader` goroutine can terminate the host process, constituting a Denial of Service vulnerability. The threat model defines the attacker as an external party providing adversarial data (e.g., HTTP request bodies, headers) to the library. For this vulnerability to be valid, an external attacker must be able to cause the `StreamWriter` function to panic using only untrusted input data. \n\nHowever, the `StreamWriter` (`sw`) is a function pointer supplied by the host application (the library consumer) when calling `NewStreamReader`. It represents the host's own logic for processing data. If the host application writes code that panics (e.g., due to a bug in their own logic, or handling of edge cases in the data), that is a bug in the host application, not a security vulnerability in the `fasthttp` library. The library's role is to facilitate the streaming mechanism. \n\nCrucially, the `fasthttp` documentation and code comments (verified via grep on `server.go`) explicitly state that `fasthttp` does *not* perform panic recovery (`no `panic` recovery is done by `fasthttp` (thus any `panic` will take down the entire server)`) and that users are responsible for implementing `recover` in their handlers if they wish to protect against such panics. The library does not claim to be panic-safe. Therefore, the lack of `recover` in the `NewStreamReader` goroutine is consistent with the library's documented behavior and threat model. An attacker cannot force the host's `StreamWriter` to panic merely by sending HTTP data, unless the host's code is flawed. Since the threat model excludes issues arising from host code bugs (only external data triggering library flaws), and the library explicitly disclaims panic protection, this is not a security vulnerability in the library itself. It is a code quality/robustness issue or expected behavior for user-supplied callbacks.\n\nCRUX: The panic originates from host-supplied code (StreamWriter), and the library explicitly documents that it does not provide panic recovery, shifting the responsibility to the host application.\nGREP: no `panic` recovery is done by `fasthttp",
"elapsed": 133.7,
"tokens": 3876,
"tokens_in": 3400,
"tokens_out": 476,
"cost_usd": 0.002856,
"file": "stream.go",
"round": 2,
"grep_used": true,
"grep_results": "GREP `no `panic` recovery is done by `fasthttp`:\n```\nserver.go:166:\t// Take into account that no `panic` recovery is done by `fasthttp` (thus any `panic` will take down the entire server).\n```"
},
{
"finding_title": "Stream writer goroutine lacks panic recovery and cleanup",
"verdict": "INVALID",
"reasoning": "The reported vulnerability claims that a panic in the `StreamWriter` callback within the `NewStreamReader` goroutine can terminate the host process, constituting a Denial of Service. \n\n1. **Bug Pattern**: The code `go func() { sw(bw) ... }()` does not contain a `recover` call. If `sw` panics, the process crashes. This part is true.\n2. **Attacker Reachability**: The `StreamWriter` (`sw`) is a function pointer supplied by the host application (the library consumer). The threat model defines the attacker as providing adversarial data (payloads) to the library, not writing the library's consumer code. For an external attacker to trigger a panic, the host application's `StreamWriter` implementation must contain a logic bug that results in a panic when processing specific input. However, if the host application's code panics due to malformed input, that is a bug in the host application, not a security vulnerability in the `fasthttp` library itself. The library correctly executes the callback provided by the user. \n3. **Threat Model & Defenses**: The `fasthttp` library explicitly documents in `server.go` that it does not perform panic recovery (`no `panic` recovery is done by `fasthttp``). The responsibility for handling panics in user-supplied handlers or callbacks lies with the host application. The library cannot prevent the host from crashing if the host's own code (the `sw` function) panics. This is standard Go semantics for user-supplied callbacks. \n4. **Conclusion**: This is not a security vulnerability in the library where an attacker exploits a flaw in the library's code using untrusted data. It is a consequence of the host application's code panicking, which is outside the scope of library vulnerabilities under this threat model. The lack of `recover` is consistent with the library's design and documentation.\n\nCRUX: The panic originates from host-supplied code (StreamWriter), and the library explicitly documents that it does not provide panic recovery, shifting the responsibility to the host application.\nGREP: no `panic` recovery is done by `fasthttp",
"elapsed": 362.4,
"tokens": 3810,
"tokens_in": 3350,
"tokens_out": 460,
"cost_usd": 0.002787,
"file": "stream.go",
"round": 3,
"grep_used": true,
"grep_results": "GREP `no `panic` recovery is done by `fasthttp`:\n```\nserver.go:166:\t// Take into account that no `panic` recovery is done by `fasthttp` (thus any `panic` will take down the entire server).\n```"
},
{
"finding_title": "Stream writer goroutine lacks panic recovery and cleanup",
"verdict": "INVALID",
"reasoning": "The reported vulnerability claims that a panic in the `StreamWriter` goroutine crashes the process and leaks resources. Under the provided threat model for a library, the attacker is an external entity providing untrusted data (e.g., HTTP payloads) to the public API, not the entity writing the host application code that consumes the library. The `StreamWriter` is a callback function supplied by the host application. If the host application's code panics (due to a bug in its logic or handling of data), that is a defect in the host application, not a security vulnerability in the `fasthttp` library. The library's responsibility is to handle the data passed through its own internal mechanisms safely; it does not need to protect the host from panics caused by the host's own logic. Furthermore, `fasthttp` explicitly documents in `server.go` that it does not perform panic recovery (\"no `panic` recovery is done by `fasthttp`\"), establishing a clear invariant that the host must handle panic safety. Since the panic originates from trusted internal code (the host's callback) and the library explicitly disclaims responsibility for panic recovery, this is not a security vulnerability in the library under the defined threat model.\n\nCRUX: The panic originates from the host-supplied `StreamWriter` callback, which is user code, and the library explicitly documents that it does not perform panic recovery.\nGREP: no `panic` recovery is done by `fasthttp",
"elapsed": 64.7,
"tokens": 4108,
"tokens_in": 3798,
"tokens_out": 310,
"cost_usd": 0.00252516,
"file": "stream.go",
"round": 4,
"grep_used": true,
"grep_results": "GREP `no `panic` recovery is done by `fasthttp`:\n```\nserver.go:166:\t// Take into account that no `panic` recovery is done by `fasthttp` (thus any `panic` will take down the entire server).\n```"
},
{
"finding_title": "Stream writer goroutine lacks panic recovery and cleanup",
"verdict": "INVALID",
"reasoning": "The reported vulnerability claims that a panic in the goroutine created by `NewStreamReader` will terminate the host process because there is no `recover` call. While it is true that an unhandled panic in a goroutine crashes the Go process, the threat model for a library defines the attacker as an external entity providing untrusted data (e.g., HTTP payloads) to the public API, not the entity writing the host application code. The `StreamWriter` (`sw`) is a callback function supplied by the host application (the library consumer). For an external attacker to trigger this panic, they would need to cause the host application's own code to panic via crafted input. This implies a bug in the host application's logic (e.g., nil pointer dereference, division by zero) rather than a vulnerability in `fasthttp`'s internal code. `fasthttp` explicitly documents in `server.go` that it does not perform panic recovery, shifting the responsibility for panic safety to the host application. Since the panic originates from trusted internal code (the host's callback) and the library explicitly disclaims responsibility for panic recovery in user-supplied callbacks, this is not a security vulnerability in the library under the defined threat model. It is a code quality/robustness issue or expected behavior for user-supplied callbacks.\n\nCRUX: The panic originates from host-supplied code (StreamWriter), and the library explicitly documents that it does not provide panic recovery, shifting the responsibility to the host application.\nGREP: no `panic` recovery is done by `fasthttp",
"elapsed": 52.3,
"tokens": 4429,
"tokens_in": 4098,
"tokens_out": 331,
"cost_usd": 0.0027141599999999997,
"file": "stream.go",
"round": 5,
"grep_used": true,
"grep_results": "GREP `no `panic` recovery is done by `fasthttp`:\n```\nserver.go:166:\t// Take into account that no `panic` recovery is done by `fasthttp` (thus any `panic` will take down the entire server).\n```"
}
]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."
}
]
```