Nil pointer dereference panics in all public Do* entry points
medium swesweep-fasthttp · client.go · Do, DoTimeout, DoDeadline, DoRedirects (package-level, Client, HostClient) · run #4
Description
All public Do* methods dereference req without a nil check: req.URI() in Do, req.timeout assignment in DoTimeout/DoDeadline, and req.URI().DisablePathNormalizing in DoRedirects. Passing a nil *Request causes an unrecoverable runtime panic that crashes the host process. In library consumers that process untrusted data (proxies, API gateways, crawlers), a nil request produced by a failed parse or lookup leads to denial of service. doNonNilReqResp confirms awareness via an explicit debug panic but provides no error return at the public API boundary.
Triage
- Verdict
- INVALID confidence 0.0
- Model
- qwen/qwen3.8-27b
- Rounds
- 5
- Cost
- $0.0630 · 359.4 s
The report claims that passing a nil *Request to public Do* methods causes a panic. While this is technically true in Go (dereferencing a nil pointer panics), the threat model for a library defines the attacker as 'code that calls the public API, and above all the data it passes through: parsers, decoders, deserializers and protocol handlers receive adversarial payloads'. The *Request object is constructed by the library consumer (the application developer). An external attacker sending malicious HTTP requests or responses over the network cannot inject a nil pointer into the calling application's memory space. They can only influence the content of valid pointers (URLs, headers, bodies). A nil pointer indicates a bug in the caller's logic (e.g., forgetting to initialize the struct), which is a programming error, not a security vulnerability reachable via untrusted input. The library explicitly treats this as an internal invariant violation by panicking in doNonNilReqResp with 'BUG: req cannot be nil'. Since the trigger (nil pointer) is not reachable via untrusted external input (network bytes) but only via malformed code logic in the trusted caller, this is a code quality/robustness issue rather than a security vulnerability. CRUX: The nil pointer originates from the library consumer's code logic and is not controllable by an external attacker processing untrusted network data. GREP: doNonNilReqResp
All rounds
[
{
"finding_title": "Nil pointer dereference panics in all public Do* entry points",
"verdict": "INVALID",
"reasoning": "The report claims that passing a nil `*Request` to public `Do*` functions causes a nil pointer dereference panic. While it is true that `Client.Do` (and `HostClient.Do`) will panic or error if `req` is nil, the threat model for a reusable library (fasthttp) defines the attacker as code that calls the public API and the data it processes. The input to `Do` is a `*Request` object constructed by the calling application (the library consumer), not untrusted data from the network. A nil pointer in this context results from a bug in the caller (the developer forgetting to initialize the request), not from an external attacker sending malicious data. The `doNonNilReqResp` function explicitly panics with 'BUG: req cannot be nil', treating this as a programming error in the caller. Since an external attacker cannot inject a nil pointer through the library's public API surface (they can only influence the *contents* of the request or the response data), this is a programming error / code quality issue, not a security vulnerability.\n\nCRUX: The nil pointer is passed by the application developer (caller), not derived from untrusted external input; the library is not vulnerable to a remote attacker sending a nil request.\nGREP: doNonNilReqResp",
"elapsed": 15.0,
"tokens": 27281,
"tokens_in": 26998,
"tokens_out": 283,
"cost_usd": 0.01218816,
"file": "client.go",
"round": 1,
"grep_used": true,
"grep_results": "GREP `doNonNilReqResp`:\n```\nclient_test.go:3257:\t_, err := client.doNonNilReqResp(req, &Response{})\nclient.go:1543:\treturn c.doNonNilReqResp(req, resp)\nclient.go:1546:func (c *HostClient) doNonNilReqResp(req *Request, resp *Response) (bool, error) {\n```"
},
{
"finding_title": "Nil pointer dereference panics in all public Do* entry points",
"verdict": "INVALID",
"reasoning": "The report claims that passing a nil `*Request` to public `Do*` functions causes a panic. While it is true that dereferencing a nil `req` would cause a panic (e.g., in `DoTimeout` via `req.timeout = timeout` or in `Client.Do` via `req.URI()`), the threat model for a library defines the attacker as the code calling the public API and the *data* passed through it. The `req` parameter is a pointer to a structure constructed by the library consumer (the application developer). An external attacker (e.g., sending malicious HTTP responses) cannot force the application developer to pass a `nil` pointer to `Do*`. The scenario described in the report ('nil request produced by a failed parse or lookup') refers to a logic error in the *consumer* application, not an input validation flaw in the fasthttp library itself. The library explicitly treats this as a programming error in the caller by panicking with 'BUG: req cannot be nil' in `doNonNilReqResp` and returning errors for invalid URIs in other paths. Since the nil pointer is not derived from untrusted external input but is a result of the caller's own code logic, this is a code quality/robustness issue (missing graceful error handling for programmer error) rather than a security vulnerability exploitable by an external attacker.\n\nCRUX: The nil pointer originates from the library consumer (caller) and is not controllable by an external attacker processing untrusted data.\nGREP: doNonNilReqResp",
"elapsed": 128.5,
"tokens": 27843,
"tokens_in": 27509,
"tokens_out": 334,
"cost_usd": 0.012555779999999999,
"file": "client.go",
"round": 2,
"grep_used": true,
"grep_results": "GREP `doNonNilReqResp`:\n```\nclient_test.go:3257:\t_, err := client.doNonNilReqResp(req, &Response{})\nclient.go:1543:\treturn c.doNonNilReqResp(req, resp)\nclient.go:1546:func (c *HostClient) doNonNilReqResp(req *Request, resp *Response) (bool, error) {\n```"
},
{
"finding_title": "Nil pointer dereference panics in all public Do* entry points",
"verdict": "INVALID",
"reasoning": "The reported vulnerability claims that passing a nil *Request to public Do* methods causes a panic, leading to Denial of Service if triggered by untrusted data. However, the *Request object is constructed and managed by the application developer (the library consumer). An external attacker (e.g., a malicious HTTP server or client) interacts with the library by sending bytes over the network (URLs, headers, bodies), not by injecting Go pointers into the application's memory space. A nil *Request pointer indicates a bug in the caller's logic (e.g., failing to initialize a request before passing it to fasthttp). Under the threat model for a library, where the attacker is the code calling the public API and the data it processes, the attacker cannot control whether the caller passes a nil pointer; they can only influence the content of valid pointers. Therefore, this is a programming error on the caller side, not a security vulnerability exploitable by an external attacker through untrusted input. The library correctly treats this as an internal invariant violation by panicking in doNonNilReqResp, which is appropriate for detecting developer bugs rather than handling malicious network payloads.\n\nCRUX: The nil pointer originates from the library consumer's code logic and is not controllable by an external attacker processing untrusted network data.\nGREP: doNonNilReqResp",
"elapsed": 68.5,
"tokens": 28212,
"tokens_in": 27927,
"tokens_out": 285,
"cost_usd": 0.01258434,
"file": "client.go",
"round": 3,
"grep_used": true,
"grep_results": "GREP `doNonNilReqResp`:\n```\nclient_test.go:3257:\t_, err := client.doNonNilReqResp(req, &Response{})\nclient.go:1543:\treturn c.doNonNilReqResp(req, resp)\nclient.go:1546:func (c *HostClient) doNonNilReqResp(req *Request, resp *Response) (bool, error) {\n```"
},
{
"finding_title": "Nil pointer dereference panics in all public Do* entry points",
"verdict": "INVALID",
"reasoning": "The report claims that passing a nil *Request to public Do* methods causes a panic. However, under the provided threat model for a library (fasthttp), the attacker is defined as the code calling the public API and the data it passes. The *Request parameter is a Go pointer constructed by the application developer (the library consumer). An external attacker (e.g., sending malicious HTTP packets) cannot inject a nil pointer into the calling application's memory space; they can only influence the content of valid pointers (strings, bytes) or trigger logic errors in the consumer. A nil pointer indicates a programming error in the consumer's code (e.g., failing to initialize a struct). The library explicitly treats this as a programmer error by panicking in doNonNilReqResp with the message 'BUG: req cannot be nil'. Since the trigger (nil pointer) is not reachable via untrusted external input (network bytes) but only via malformed code logic in the trusted caller, this is a code quality/robustness issue rather than a security vulnerability exploitable by an external attacker. Therefore, it is marked INVALID.\n\nCRUX: The nil pointer originates from the library consumer's code logic and is not controllable by an external attacker processing untrusted network data.\nGREP: doNonNilReqResp",
"elapsed": 78.7,
"tokens": 28574,
"tokens_in": 28296,
"tokens_out": 278,
"cost_usd": 0.01271832,
"file": "client.go",
"round": 4,
"grep_used": true,
"grep_results": "GREP `doNonNilReqResp`:\n```\nclient_test.go:3257:\t_, err := client.doNonNilReqResp(req, &Response{})\nclient.go:1543:\treturn c.doNonNilReqResp(req, resp)\nclient.go:1546:func (c *HostClient) doNonNilReqResp(req *Request, resp *Response) (bool, error) {\n```"
},
{
"finding_title": "Nil pointer dereference panics in all public Do* entry points",
"verdict": "INVALID",
"reasoning": "The report claims that passing a nil *Request to public Do* methods causes a panic. While this is technically true in Go (dereferencing a nil pointer panics), the threat model for a library defines the attacker as 'code that calls the public API, and above all the data it passes through: parsers, decoders, deserializers and protocol handlers receive adversarial payloads'. The *Request object is constructed by the library consumer (the application developer). An external attacker sending malicious HTTP requests or responses over the network cannot inject a nil pointer into the calling application's memory space. They can only influence the content of valid pointers (URLs, headers, bodies). A nil pointer indicates a bug in the caller's logic (e.g., forgetting to initialize the struct), which is a programming error, not a security vulnerability reachable via untrusted input. The library explicitly treats this as an internal invariant violation by panicking in doNonNilReqResp with 'BUG: req cannot be nil'. Since the trigger (nil pointer) is not reachable via untrusted external input (network bytes) but only via malformed code logic in the trusted caller, this is a code quality/robustness issue rather than a security vulnerability.\n\nCRUX: The nil pointer originates from the library consumer's code logic and is not controllable by an external attacker processing untrusted network data.\nGREP: doNonNilReqResp",
"elapsed": 68.7,
"tokens": 28958,
"tokens_in": 28658,
"tokens_out": 300,
"cost_usd": 0.01293636,
"file": "client.go",
"round": 5,
"grep_used": true,
"grep_results": "GREP `doNonNilReqResp`:\n```\nclient_test.go:3257:\t_, err := client.doNonNilReqResp(req, &Response{})\nclient.go:1543:\treturn c.doNonNilReqResp(req, resp)\nclient.go:1546:func (c *HostClient) doNonNilReqResp(req *Request, resp *Response) (bool, error) {\n```"
}
]Raw scan report for this file
# Security Analysis: `client.go` (fasthttp)
## Data Flow Tracing
I traced untrusted input through the public API surface:
- **`url`** → `hostnameFromURLString` → `host` → `c.hostClient` → `HostClient.Addr`
- **`req`** → `req.SetRequestURI` / `parseURI` → `req.URI().Host()` → connection target
- **`location`** (server response header) → `getRedirectURL` → `dst.UpdateBytes(location)` → next `url`
- **`postArgs`** → `req.BodyWriter`
- **Server response bytes** → `resp.Read` / `ReadLimitBody` → `dst` buffer
---
## Findings
### 1. Data race on `dst` buffer after timeout in `clientGetURLDeadline`
**Function:** `clientGetURLDeadline` (reachable via `GetDeadline`, `GetTimeout`, `Client.GetDeadline`, `Client.GetTimeout`, `HostClient.GetDeadline`, `HostClient.GetTimeout`)
**Flow:**
```
caller passes dst []byte
→ clientGetURLDeadline(dst, url, deadline, c)
→ spawns goroutine: doRequestFollowRedirectsBuffer(req, dst, url, c)
→ bodyBuf.B = dst // response body written directly into dst
→ doRequestFollowRedirects → c.Do → transport.RoundTrip
→ resp.ReadLimitBody(br, maxBody) // reads server body INTO dst
→ caller waits on select { ch | timer }
→ if timer fires: returns body = dst to caller
→ goroutine MAY STILL BE WRITING TO dst
```
The goroutine writes the response body directly into the caller-supplied `dst` slice (`bodyBuf.B = dst`). When the deadline timer fires, the main goroutine sets `timedout = true` and returns `body = dst` to the caller. The spawned goroutine is **not cancelled** — it continues executing `doRequestFollowRedirectsBuffer`, which is still reading the server's response body into `dst`. The code comment explicitly acknowledges this:
> "Note that the request continues execution on ErrTimeout until client-specific ReadTimeout exceeds."
The `sync.Mutex` only guards the `timedout`/`responded` flags and the channel send — it does **not** protect the `dst` buffer itself. This creates a concurrent read (by the caller) and write (by the goroutine) on the same `[]byte` without synchronization.
**Exploitability:** An attacker controlling the remote server can trigger this by:
1. Returning a response with a large `Content-Length` (or chunked body)
2. Sending body data slowly so the client's deadline expires mid-transfer
3. Continuing to send body data after the timeout
The caller receives a partially-filled `dst` that is still being mutated. If the caller parses `dst` (e.g., as JSON, form data, or a protocol message), the attacker can inject corrupted or malicious content. In the worst case, if `dst` is a fixed-size buffer and the goroutine writes beyond the expected boundary, this could corrupt adjacent memory.
**Severity: High** — memory integrity / data corruption, attacker-controllable via slow response.
---
### 2. Nil pointer dereference in public `Do*` methods
**Functions:** `Do`, `DoTimeout`, `DoDeadline`, `DoRedirects` (package-level and on `Client`/`HostClient`)
**Flow:**
```
Do(req, resp)
→ defaultClient.Do(req, resp)
→ uri := req.URI() // PANIC: nil pointer dereference
```
```
DoTimeout(req, resp, timeout)
→ req.timeout = timeout // PANIC: nil pointer dereference
```
```
DoDeadline(req, resp, deadline)
→ req.timeout = time.Until(deadline) // PANIC: nil pointer dereference
```
```
DoRedirects(req, resp, n)
→ req.URI().DisablePathNormalizing = true // PANIC: nil pointer dereference
```
Every public `Do*` entry point dereferences `req` without a nil check. If `req` is nil, the host process crashes with a runtime panic. While the documentation states "Request must contain at least non-zero RequestURI," the functions are exported library APIs. In a host application that processes untrusted data (e.g., a proxy, API gateway, or web crawler), a code path that produces a nil `*Request` (from a failed parse, a nil map lookup, etc.) will crash the entire process.
The `doNonNilReqResp` method contains an explicit debug panic for nil `req`/`resp`, confirming the developers are aware of this risk but chose to panic rather than return an error.
**Severity: Medium** — process crash (DoS). Not directly attacker-controllable unless the host application has a bug that produces nil requests from untrusted data.
---
### 3. `AddMissingPort` mishandles unbracketed IPv6 literals
**Function:** `AddMissingPort`
```go
func AddMissingPort(addr string, isTLS bool) string {
...
isIP6 := addr[0] == '['
if isIP6 {
// handles [::1] correctly
} else { // IPv4
columnPos := strings.LastIndexByte(addr, ':')
if columnPos > 0 {
return addr // BUG: treats unbracketed IPv6 as having a port
}
}
...
}
```
For an unbracketed IPv6 address like `::1` (no port, no brackets), `addr[0]` is `:` (not `[`), so it enters the "IPv4" branch. `strings.LastIndexByte("::1", ':')` returns `2`, which is `> 0`, so the function returns the address as-is **without appending a port**. The dialer then receives `::1` with no port, which will fail to dial or dial an unintended address.
While unbracketed IPv6 literals are non-standard, they can appear in configuration files or user-supplied addresses. This is a correctness bug rather than a direct security vulnerability, but it could cause connection failures or, in edge cases, connection to an unintended port.
**Severity: Low** — incorrect port handling for non-standard input.
---
## Functions Reviewed and Found Safe
- **`stripSensitiveHeadersOnRedirect` / `shouldStripSensitiveHeadersOnRedirect` / `isDomainOrSubdomainBytes`**: The subdomain check correctly uses `EqualFold` and requires a `.` separator. No bypass via encoding tricks. Sensitive headers (Authorization, Cookie, etc.) are properly stripped on cross-domain redirects.
- **`splitHostPortBytes` / `hostnameFromHostPortBytes`**: Correctly handles bracketed IPv6. The unbracketed IPv6 edge case in `splitHostPortBytes` (multiple colons → no port split) is consistent with `AddMissingPort` behavior.
- **`hostClient`**: Double-checked locking with `mLock` is correct. `mOnce.Do` guarantees map initialization before use.
- **`AcquireConn` / `ReleaseConn` / `decConnsCount`**: Connection pool operations are protected by `connsLock`. The `wantConnQueue` is properly managed.
- **`connsCleaner` / `mCleaner`**: Goroutine lifecycle is managed correctly with proper lock acquisition.
- **`PipelineClient.Do` overflow handling**: Correctly evicts oldest work item and returns `ErrPipelineOverflow`.
- **`transport.RoundTrip`**: Properly handles timeouts, retries, and connection lifecycle. `MaxResponseBodySize` bounds body reads.
- **`tlsServerName` / `newClientTLSConfig`**: The `InsecureSkipVerify = true` path is only triggered by malformed addresses set by the application developer, not by attacker input.
- **`dialHostHard`**: Attempts all configured addresses before giving up. Proper deadline enforcement.
```json
[
{
"severity": "high",
"title": "Data race on caller-supplied dst buffer after request timeout",
"function": "clientGetURLDeadline",
"description": "When the deadline timer fires, the function returns dst to the caller while the spawned goroutine may still be writing the server response body into dst via doRequestFollowRedirectsBuffer (bodyBuf.B = dst). The sync.Mutex only guards the timedout/responded flags and channel send, not the dst buffer. An attacker controlling the remote server can trigger this by sending a slow, large response body that causes the client to time out mid-transfer. The caller then reads a buffer that is concurrently being mutated, leading to data corruption, injection of attacker-controlled content into the caller's processing pipeline, or potential memory corruption."
},
{
"severity": "medium",
"title": "Nil pointer dereference panics in all public Do* entry points",
"function": "Do, DoTimeout, DoDeadline, DoRedirects (package-level, Client, HostClient)",
"description": "All public Do* methods dereference req without a nil check: req.URI() in Do, req.timeout assignment in DoTimeout/DoDeadline, and req.URI().DisablePathNormalizing in DoRedirects. Passing a nil *Request causes an unrecoverable runtime panic that crashes the host process. In library consumers that process untrusted data (proxies, API gateways, crawlers), a nil request produced by a failed parse or lookup leads to denial of service. doNonNilReqResp confirms awareness via an explicit debug panic but provides no error return at the public API boundary."
},
{
"severity": "low",
"title": "AddMissingPort fails to append default port for unbracketed IPv6 literals",
"function": "AddMissingPort",
"description": "For an unbracketed IPv6 address like '::1' (no square brackets, no port), the function enters the IPv4 branch where LastIndexByte finds a colon at position > 0 and returns the address unchanged without appending :80 or :443. The resulting address lacks a port, causing dial failures or unintended port selection. While non-standard, such addresses can appear in user-supplied configuration or upstream host lists."
}
]
```