BudgetScan

IPv6 and non-TCP remote addresses collapse to the same counter key 0

medium swesweep-fasthttp · peripconn.go · getConnIP4 · run #4

Description

getConnIP4 returns nil for addresses whose IP is not IPv4, and ip2uint32 maps nil or len != 4 to 0. Consequently all pure IPv6 connections and non-*net.TCPAddr connections share key 0. If MaxConnsPerIP is configured, an attacker using IPv6, or non-TCP connections if supported, can exhaust the shared key and deny service to other IPv6/non-TCP clients.

Triage

Verdict
VALID confidence 0.83
Model
qwen/qwen3.8-27b
Rounds
6
Cost
$0.0191 · 313.8 s
[ARBITER] The vulnerability is valid under the threat model because it leads to a Denial of Service (DoS) reachable via the public server API. The code `getConnIP4` explicitly returns `net.IPv4zero` for any remote address that is not a `*net.TCPAddr` or whose IP does not convert to IPv4 (e.g., pure IPv6 addresses). Consequently, `ip2uint32` maps these to the key `0`. When `Server.MaxConnsPerIP` is configured, all IPv6 connections and non-TCP connections share this single counter key. An attacker can exploit this by opening a small number of IPv6 connections (or non-TCP connections, if supported) to exhaust the shared limit, thereby preventing all other legitimate IPv6/non-TCP clients from connecting, even if they are using distinct IP addresses. This violates the resource bounds invariant (correct behavior on malformed/differentiated input) and causes unavailability of the service.
All rounds
[
  {
    "finding_title": "IPv6 and non-TCP remote addresses collapse to the same counter key 0",
    "verdict": "VALID",
    "reasoning": "The report claims that IPv6 and non-TCP connections collapse to key 0, allowing an attacker to deny service to other IPv6/non-TCP clients by exhausting the `MaxConnsPerIP` limit. \n\n1. **Bug Pattern Verification**: The code in `peripconn.go` confirms the behavior. `getConnIP4` returns `net.IPv4zero` (which is `[]byte{0, 0, 0, 0}`) for any address that is not a `*net.TCPAddr` or whose IP is not IPv4 (i.e., `To4()` returns nil). `ip2uint32` converts this to `0`. Thus, all IPv6 connections and non-TCP connections are indeed mapped to the same counter key `0`.\n\n2. **Threat Model & Reachability**: The threat model defines the attacker as code calling the public API or providing data to it. In the context of the `fasthttp` client, the `perIPConnCounter` is used to track connections made by the client to remote servers. The `RemoteAddr` is determined by the server the client connects to. An external attacker can control the IP address they present (e.g., using IPv6 addresses or non-TCP protocols if supported/forced). If the `fasthttp.Client` is configured with `MaxConnsPerIP > 0`, and the attacker connects using multiple IPv6 addresses (which all map to key 0), they can saturate the limit for key 0. This would prevent the client from making *any* further connections to IPv6 addresses (since they all share key 0) or non-TCP addresses, effectively causing a Denial of Service for those types of connections in the host application.\n\n3. **Defense Verification**: There is no defense against this collapse. The code explicitly maps non-IPv4 to 0. The intent of `MaxConnsPerIP` is to limit connections per *IP*. Collapsing all IPv6 into a single bucket defeats this purpose and creates a vector for resource exhaustion (connection pool exhaustion for specific address families).\n\n4. **Security Relevance**: While this is a client-side component, a vulnerability in the client library that allows an external server to exhaust client resources (connections) via crafted IP responses or address types is a valid security issue (Denial of Service). The `MaxConnsPerIP` limit is intended to protect against a specific IP flooding the client. By collapsing IPv6 to one bucket, a single malicious IPv6 host (or a small range if they rotate) can block all IPv6 traffic handled by that client instance.\n\nTherefore, the bug is real, reachable by an external attacker (by using IPv6 addresses), and leads to a meaningful harm (DoS of IPv6 connectivity in the host application).\n\nCRUX: IPv6 and non-TCP addresses are all mapped to the counter key 0, allowing a single malicious entity (or few) to exhaust the MaxConnsPerIP limit for all such connections simultaneously, causing a Denial of Service.\nGREP: getConnIP4",
    "elapsed": 71.1,
    "tokens": 3451,
    "tokens_in": 2784,
    "tokens_out": 667,
    "cost_usd": 0.00317028,
    "file": "peripconn.go",
    "round": 1,
    "grep_used": true,
    "grep_results": "GREP `getConnIP4`:\n```\nperipconn.go:120:\treturn ip2uint32(getConnIP4(c))\nperipconn.go:123:func getConnIP4(c net.Conn) net.IP {\nserver.go:2094:\t\t\t\t\t\tgetConnIP4(c), s.MaxConnsPerIP)\n```"
  },
  {
    "finding_title": "IPv6 and non-TCP remote addresses collapse to the same counter key 0",
    "verdict": "VALID",
    "reasoning": "The report claims that collapsing IPv6/non-TCP addresses to key 0 allows an attacker to exhaust the `MaxConnsPerIP` limit. I must verify who controls the `RemoteAddr` used to compute this key. The threat model specifies the attacker is 'code that calls the public API' (i.e., the library consumer) and the data they pass. However, `fasthttp` is a dual client/server library. The `perIPConnCounter` is used in `acquirePerIPConn`. Looking at the call sites (verified via grep in the prompt: `server.go:2094`), the counter is initialized on the *server* side (`s.MaxConnsPerIP`). In a server context, `c.RemoteAddr()` is the IP of the client connecting *to* the server. If the attacker is an external network client (which fits the 'protocol handler receives adversarial payloads' part of the threat model, although the profile says 'code that calls the public API', the entry point is the public network listener which is part of the public API surface), they can control their source IP. Wait, the threat model profile is 'library'. 'Attacker: Code that calls the public API... and above all the data it passes through: parsers, decoders... receive adversarial payloads that the host application obtained from untrusted sources.' This implies the host app receives data from the network and passes it to the library. For a server, the library *is* the entry point receiving data from the network. The `RemoteAddr` is determined by the TCP stack, which is untrusted input from the network. An attacker can connect via IPv6. All IPv6 connections map to key 0. If the attacker opens `MaxConnsPerIP` connections, they exhaust the counter for key 0. This prevents *any* other IPv6 connection (key 0) from being accepted/registered, leading to a DoS for all IPv6 clients. Is this 'meaningful harm'? Yes, it's a DoS on a specific subset of users (IPv6). Is it reachable? Yes, via the public server listener. Is it a security vulnerability? Yes, unbounded resource usage (connection slots) due to input handling flaw (IP hashing). The bug is real. The previous reviewer was correct.\n\nCRUX: The perIPConnCounter maps all IPv6 addresses to the same key (0), allowing a single attacker to exhaust the MaxConnsPerIP limit for all IPv6 traffic via the public server API.\nGREP: getConnIP4",
    "elapsed": 71.6,
    "tokens": 4204,
    "tokens_in": 3657,
    "tokens_out": 547,
    "cost_usd": 0.00317694,
    "file": "peripconn.go",
    "round": 2,
    "grep_used": true,
    "grep_results": "GREP `getConnIP4`:\n```\nperipconn.go:120:\treturn ip2uint32(getConnIP4(c))\nperipconn.go:123:func getConnIP4(c net.Conn) net.IP {\nserver.go:2094:\t\t\t\t\t\tgetConnIP4(c), s.MaxConnsPerIP)\n```"
  },
  {
    "finding_title": "IPv6 and non-TCP remote addresses collapse to the same counter key 0",
    "verdict": "UNCERTAIN",
    "reasoning": "The bug pattern is real: `getConnIP4` returns `net.IPv4zero` (which converts to uint32 0) for any non-TCP or non-IPv4 address. Consequently, all IPv6 connections and any non-TCP connections are assigned the same counter key `0`. \n\nHowever, the critical question is whether this leads to a security vulnerability (DoS) or is a harmless implementation detail. To answer this, I must verify how the counter key is used in relation to `MaxConnsPerIP`. I need to check the call sites of `acquirePerIPConn` or wherever `getUint32IP` is used to ensure that the `ip` key is actually involved in the enforcement of `MaxConnsPerIP`. If the library only uses the counter for statistics or if the `MaxConnsPerIP` check is performed elsewhere using the full address, this finding is invalid. \n\nLooking at `server.go:2094`, it calls `getConnIP4`. Let's look at how `MaxConnsPerIP` is enforced. Typically, in fasthttp, `MaxConnsPerIP` is checked in the `serve` loop or connection handler. If the check is `if s.MaxConnsPerIP > 0 && connCounter.get(ip) >= s.MaxConnsPerIP`, then key collision matters. \n\nHowever, there is a more fundamental issue: `fasthttp`'s `Client` and `Server` handle connections. If an attacker connects via IPv6, they get key 0. If they open `MaxConnsPerIP` connections, they saturate key 0. Does this prevent *other* IPv6 users from connecting? Yes, because they also map to key 0. This is a classic ",
    "elapsed": 33.9,
    "tokens": 4667,
    "tokens_in": 4275,
    "tokens_out": 392,
    "cost_usd": 0.0029715,
    "file": "peripconn.go",
    "round": 3
  },
  {
    "finding_title": "IPv6 and non-TCP remote addresses collapse to the same counter key 0",
    "verdict": "VALID",
    "reasoning": "The reported bug is that `getConnIP4` and `ip2uint32` collapse all non-IPv4 (specifically IPv6) and non-TCP addresses to the key `0` in the `perIPConnCounter`. I need to verify if this leads to a security vulnerability (DoS) when `MaxConnsPerIP` is set.\n\n1. **Bug Verification**: The code in `peripconn.go` shows `getConnIP4` returns `net.IPv4zero` if the address is not `*net.TCPAddr` or if `To4()` returns nil (IPv6). `ip2uint32` converts `net.IPv4zero` (len 4) to `0`. Thus, all IPv6 connections map to key `0`.\n\n2. **Usage Context**: The grep results indicate usage in `server.go:2094`. In a server context, `MaxConnsPerIP` is a rate limiting feature to prevent a single client IP from exhausting resources. If all IPv6 clients are mapped to key `0`, a single malicious IPv6 client (or a few) can exhaust the `MaxConnsPerIP` limit for key `0`, preventing *all* other IPv6 clients from connecting. This is a Denial of Service (DoS) vulnerability for IPv6 traffic.\n\n3. **Threat Model Fit**: The threat model defines the attacker as code calling the public API or data passing through parsers/protocol handlers. In a server library, the network stack is the entry point. An external attacker controlling their source IP (IPv6) can trigger this collapse. The impact is resource exhaustion (connection limits) leading to DoS for a subset of users (IPv6). This fits the 'meaningful harm' criterion.\n\n4. **Defense Check**: There is no mechanism in `perIPConnCounter` to distinguish between different IPv6 addresses. It relies on the `uint32` key. Since IPv6 addresses are 128 bits, they cannot be mapped to `uint32` without collisions or hashing. The current implementation simply truncates/defaults to 0 for non-IPv4, which is a significant flaw for a library that claims to support IPv6 and provides per-IP rate limiting.\n\n5. **Conclusion**: The bug is real, attacker-reachable via the server's public network interface, and causes DoS for IPv6 clients when `MaxConnsPerIP` is configured. It is a security vulnerability.\n\nCRUX: All IPv6 connections are mapped to the same counter key (0) in perIPConnCounter, allowing a single attacker to exhaust the MaxConnsPerIP limit and Denial of Service all other IPv6 clients.\nGREP: MaxConnsPerIP",
    "elapsed": 64.3,
    "tokens": 5248,
    "tokens_in": 4647,
    "tokens_out": 601,
    "cost_usd": 0.00375474,
    "file": "peripconn.go",
    "round": 4,
    "grep_used": true,
    "grep_results": "GREP `MaxConnsPerIP`:\n```\nserver_timing_test.go:151:func BenchmarkServerMaxConnsPerIP(b *testing.B) {\nserver_timing_test.go:161:\t\tMaxConnsPerIP: clientsCount * 2,\nserver_test.go:849:func TestServerMaxConnsPerIPLimit(t *testing.T) {\nserver_test.go:856:\t\tMaxConnsPerIP: 1,\nserver.go:304:\tMaxConnsPerIP int\nserver.go:2089:\t\tif s.MaxConnsPerIP > 0 {\nserver.go:2093:\t\t\t\t\ts.logger().Printf(\"The number of connections from %s exceeds MaxConnsPerIP=%d\",\nserver.go:2094:\t\t\t\t\t\tgetConnIP4(c), s.MaxConnsPerIP)\nserver.go:2111:\tif n > s.MaxConnsPerIP {\nserver.go:2113:\t\ts.writeFastError(c, StatusTooManyRequests, \"The number of connections from your ip exceeds MaxConnsPerIP\")\nserver.go:2131:\t// per ip exceeds Server.MaxConnsPerIP.\nserver.go:2149:\tif s.MaxConnsPerIP > 0 {\n```"
  },
  {
    "finding_title": "IPv6 and non-TCP remote addresses collapse to the same counter key 0",
    "verdict": "VALID",
    "reasoning": "The report claims that all IPv6 and non-TCP connections collapse to the same counter key (0) in `perIPConnCounter`, allowing an attacker to exhaust the `MaxConnsPerIP` limit for key 0 and Denial of Service other IPv6 clients. \n\n1. **Bug Verification**: The code in `peripconn.go` confirms that `getConnIP4` returns `net.IPv4zero` for any address that is not a `*net.TCPAddr` or whose IP is not IPv4 (i.e., `To4()` returns nil). `ip2uint32` converts `net.IPv4zero` (len 4) to `0`. Thus, all IPv6 connections and non-TCP connections do indeed map to key `0`. \n\n2. **Reachability & Threat Model**: The threat model defines the attacker as code calling the public API or providing data to it. In the context of a server library (`fasthttp`), the public API is the network listener. An external attacker can control their source IP address. If they connect via IPv6, the server library's `getConnIP4` will map this to key `0`. If `MaxConnsPerIP` is configured, the attacker can open `MaxConnsPerIP` connections. These connections will increment the counter for key `0` to the limit. \n\n3. **Impact**: Once the counter for key `0` reaches `MaxConnsPerIP`, any subsequent connection that maps to key `0` (which includes *all* other IPv6 connections, regardless of their source IP) will be rejected with `StatusTooManyRequests`. This allows a single malicious IPv6 client to prevent *any* other IPv6 client from connecting to the server. This is a significant Denial of Service vulnerability for IPv6 traffic. \n\n4. **Defense**: There is no defense against this collapse. The implementation explicitly treats all non-IPv4 addresses as a single entity (key 0). The `MaxConnsPerIP` feature is intended to limit connections per unique IP, but the mapping logic fails to distinguish between unique IPv6 addresses, treating them all as the same address. \n\n5. **Conclusion**: The bug is real, reachable via the public network interface, and causes meaningful harm (DoS for all IPv6 clients) when `MaxConnsPerIP` is enabled. Therefore, it is a valid security vulnerability.\n\nCRUX: IPv6 and non-TCP addresses are mapped to the same counter key (0) in perIPConnCounter, allowing a single attacker to exhaust the MaxConnsPerIP limit for all such connections simultaneously, causing a Denial of Service for other IPv6/non-TCP clients.\nGREP: getConnIP4",
    "elapsed": 42.2,
    "tokens": 6131,
    "tokens_in": 5533,
    "tokens_out": 598,
    "cost_usd": 0.00411786,
    "file": "peripconn.go",
    "round": 5,
    "grep_used": true,
    "grep_results": "GREP `getConnIP4`:\n```\nperipconn.go:120:\treturn ip2uint32(getConnIP4(c))\nperipconn.go:123:func getConnIP4(c net.Conn) net.IP {\nserver.go:2094:\t\t\t\t\t\tgetConnIP4(c), s.MaxConnsPerIP)\n```"
  },
  {
    "verdict": "VALID",
    "reasoning": "[ARBITER] The vulnerability is valid under the threat model because it leads to a Denial of Service (DoS) reachable via the public server API. The code `getConnIP4` explicitly returns `net.IPv4zero` for any remote address that is not a `*net.TCPAddr` or whose IP does not convert to IPv4 (e.g., pure IPv6 addresses). Consequently, `ip2uint32` maps these to the key `0`. When `Server.MaxConnsPerIP` is configured, all IPv6 connections and non-TCP connections share this single counter key. An attacker can exploit this by opening a small number of IPv6 connections (or non-TCP connections, if supported) to exhaust the shared limit, thereby preventing all other legitimate IPv6/non-TCP clients from connecting, even if they are using distinct IP addresses. This violates the resource bounds invariant (correct behavior on malformed/differentiated input) and causes unavailability of the service.",
    "round": 6,
    "file": "peripconn.go",
    "finding_title": "IPv6 and non-TCP remote addresses collapse to the same counter key 0",
    "elapsed": 30.7,
    "tokens_in": 2975,
    "tokens_out": 217,
    "cost_usd": 0.0019004999999999998
  }
]
Raw scan report for this file

Tracing the untrusted input: `RemoteAddr` → `getUint32IP` → `ip` → `Register` → `acquirePerIPConn` → `Close` → `Unregister`.

Key exploitable issues:

1. **Unbounded map growth**: `Unregister` sets counts to zero but never deletes the map entry. Every IPv4 address ever observed remains in `cc.m`. A remote attacker with many source IPs, or a proxy-protocol path exposing many client IPs, can cause unbounded memory growth by opening/closing connections.

2. **IP normalization collapse**: Pure IPv6 and non-`*net.TCPAddr` addresses map to `0`. If `MaxConnsPerIP` is enabled, all such connections share the same counter key, allowing one attacker to deny service to other IPv6/non-TCP clients.

3. **Pooled wrapper lifecycle race**: `Close` puts the wrapper back into `sync.Pool` after niling `Conn`. `acquirePerIPConn` can reuse the same wrapper without an ownership token or synchronizing with a stale `Close`. A delayed `Close` can close the newly assigned underlying connection and unregister the new IP, causing connection resets, accounting errors, and a data race on `Conn`/`ip`.

Other helpers are mostly safe: `ip2uint32` bounds-checks `len(ip)`, `uint322ip` uses a fixed 4-byte buffer safely, and `Register`/`Unregister` are mutex-protected.

```json
[
  {
    "severity": "high",
    "title": "Unbounded per-IP counter map growth from retained zero-count entries",
    "function": "perIPConnCounter.Unregister",
    "description": "Unregister decreases the counter and clamps to zero but never deletes the map entry. Every remote IPv4 address that is ever seen remains in cc.m forever. A remote attacker with many source IPs, or a proxy-protocol path that exposes many client IPs, can open/close connections to cause unbounded memory growth and denial of service, even if connections are rejected after Register."
  },
  {
    "severity": "medium",
    "title": "IPv6 and non-TCP remote addresses collapse to the same counter key 0",
    "function": "getConnIP4",
    "description": "getConnIP4 returns nil for addresses whose IP is not IPv4, and ip2uint32 maps nil or len != 4 to 0. Consequently all pure IPv6 connections and non-*net.TCPAddr connections share key 0. If MaxConnsPerIP is configured, an attacker using IPv6, or non-TCP connections if supported, can exhaust the shared key and deny service to other IPv6/non-TCP clients."
  },
  {
    "severity": "medium",
    "title": "Pooled wrapper can be closed after reuse, closing a different connection",
    "function": "perIPConn.Close / perIPTLSConn.Close",
    "description": "Close nils c.Conn, unregisters the IP, and puts the wrapper back into sync.Pool. acquirePerIPConn may later retrieve the same wrapper and assign a new Conn/ip without an ownership token or synchronization with a stale Close. A delayed second Close from an old reference will close the newly assigned Conn and Unregister the new IP, causing connection resets, accounting errors, and a data race on c.Conn/c.ip. A remote endpoint can influence this by causing an early close/abort and rapid connection reuse."
  }
]
```