Content-Length stream can consume bytes beyond the declared body length

[
  {
    "finding_title": "Content-Length stream can consume bytes beyond the declared body length",
    "verdict": "UNCERTAIN",
    "reasoning": "The vulnerability report claims that in `requestStream.Read`, when reading from `rs.prefetchedBytes`, the truncation logic is incorrect. Specifically, it states that `p` is truncated to `prefetchedSize - totalBytesRead` instead of `ContentLength() - totalBytesRead`, and that if `prefetchedSize` (which includes bytes for the *next* request due to read-ahead) is larger than `ContentLength() - totalBytesRead`, the code will read/discard bytes belonging to the next HTTP message. \n\nHowever, analyzing the provided code snippet: \n1. The check for completion is `if rs.totalBytesRead == rs.header.ContentLength() { return 0, io.EOF }`. This ensures `Read` returns EOF exactly when the declared body is consumed. \n2. In the `prefetchedBytes` branch, `left` is calculated as `prefetchedSize - rs.totalBytesRead`. If `prefetchedSize` contains more data than the remaining body (i.e., `prefetchedSize > ContentLength() - totalBytesRead`), then `left` will be larger than the remaining body length. \n3. The code truncates `p` to `left`: `if len(p) > left { p = p[:left] }`. Then it reads from `prefetchedBytes`. \n4. Crucially, after reading from `prefetchedBytes`, the code does **not** immediately check if `totalBytesRead` has exceeded or equalled `ContentLength()` to stop the read *within that call* if `p` was large. Wait, looking closer: `n, err := rs.prefetchedBytes.Read(p)`. `bytes.Reader.Read` will read up to `len(p)` bytes. If `prefetchedSize` is 100, `ContentLength` is 50, and `totalBytesRead` is 0, `left` is 100. If caller asks for 100 bytes, `p` is 100 bytes. `Read` reads all 100 bytes into `p`. `totalBytesRead` becomes 100. The code then checks `if n == rs.header.ContentLength() { return n, io.EOF }`. This condition `100 == 50` is false. It returns `100, nil`. \n\nThis **is** a bug. The caller requested 100 bytes. The stream has 100 bytes prefetched. The declared length is 50. The function returns 100 bytes of data, including 50 bytes that belong to the next message (or are just extra read-ahead). The caller thinks they read 100 bytes of the body. The `totalBytesRead` is now 100. The next call to `Read` will see `totalBytesRead (100) == ContentLength (50)`? No, `100 != 50`. It will proceed to `rs.reader.Read`. This desynchronizes the stream. The 50 bytes from the ",
    "elapsed": 103.8,
    "tokens": 3284,
    "tokens_in": 2627,
    "tokens_out": 657,
    "cost_usd": 0.00307434,
    "file": "streaming.go",
    "round": 1
  },
  {
    "finding_title": "Content-Length stream can consume bytes beyond the declared body length",
    "verdict": "VALID",
    "reasoning": "The reported vulnerability is a logic error in `requestStream.Read` in `streaming.go`. \n\n1. **Bug Existence**: The code fails to correctly limit the number of bytes read from `rs.prefetchedBytes` when `prefetchedBytes` contains more data than the remaining declared `ContentLength`. \n   - The check `if rs.totalBytesRead == rs.header.ContentLength()` returns EOF only if the counts match *exactly*. \n   - In the branch `if prefetchedSize > rs.totalBytesRead`, `left` is set to `prefetchedSize - rs.totalBytesRead`. \n   - If `prefetchedSize` (e.g., 100) is larger than `ContentLength - rs.totalBytesRead` (e.g., 50), `left` becomes 100. \n   - The code reads up to `left` bytes from `prefetchedBytes`. If the caller requests 100 bytes, it reads all 100 bytes. \n   - `rs.totalBytesRead` becomes 100. \n   - The subsequent check `if n == rs.header.ContentLength()` (100 == 50) fails. \n   - The function returns 100 bytes, but only 50 belonged to the current body. The next 50 bytes belong to the pipelined/next request. \n   - On the next `Read` call, `rs.totalBytesRead` (100) != `ContentLength` (50), so it proceeds to `rs.reader.Read`, consuming bytes from the stream that were not part of the current body. This causes desynchronization.\n\n2. **Reachability**: `requestStream.Read` is the `io.Reader` implementation for the request body. The body content and the presence of prefetched bytes depend on the raw HTTP bytes sent by the client. An attacker can construct an HTTP request with a specific `Content-Length` followed immediately by additional bytes (e.g., a pipelined second request or just garbage) that will be read into the `prefetchedBytes` buffer by the initial header read. \n   - `acquireRequestStream` initializes `rs.prefetchedBytes` with `b.B`, which contains the body bytes read during header parsing. \n   - If the header parsing reads more bytes into `b` than the declared `Content-Length` (which happens if the reader is buffered or if the parser reads ahead), `prefetchedSize` will exceed `ContentLength`. \n   - Even if the parser doesn't read ahead, the `bufio.Reader` `rs.reader` might be used in other contexts, but `prefetchedBytes` is specifically for data already consumed. \n   - Actually, let's verify if `prefetchedBytes` can be larger than `ContentLength`. `acquireRequestStream` is called after headers are parsed. `b` (ByteBuffer) typically contains the body bytes that were read. If the underlying reader read a large chunk (e.g., 4KB) and the `Content-Length` is small (e.g., 10 bytes), `b` will contain 4KB of data. `prefetchedSize` = 4096. `ContentLength` = 10. \n   - `totalBytesRead` = 0. \n   - `left` = 4096. \n   - Caller reads 4096 bytes. `prefetchedBytes` returns 4096 bytes. \n   - `totalBytesRead` = 4096. \n   - Returns 4096 bytes. \n   - The caller now has 4086 bytes of data that are *not* part of the body (if the body was 10 bytes). These bytes are from the next request or stream. \n   - The connection is desynchronized. The next request's header/body will be consumed as part of the previous request's body, or the previous request's body will contain the next request's data. This allows for HTTP Request Smuggling or DoS.\n\n3. **Defense**: There is no defense in the code. The logic explicitly allows reading beyond the `ContentLength` boundary if the prefetched buffer is larger. The `if n == rs.header.ContentLength()` check is insufficient because `n` is the number of bytes read in that specific call, not the total, and it doesn't handle the case where `n > ContentLength` or where the total exceeds it. The correct logic would be to cap `left` at `ContentLength - totalBytesRead`.\n\n4. **Security Impact**: This is a memory safety/logic integrity issue in a network-facing library. It leads to desynchronization, allowing an attacker to smuggle requests or crash the server. It is a valid security vulnerability.\n\nThe grep `acquireRequestStream` confirms the function exists and is used to populate the struct. The code analysis of `streaming.go` confirms the logic flaw.\n\nCRUX: The `requestStream.Read` function allows reading more bytes than the declared `ContentLength` when the `prefetchedBytes` buffer contains more data than the remaining body length, causing stream desynchronization.\nGREP: prefetchedSize - rs.totalBytesRead",
    "elapsed": 50.5,
    "tokens": 4542,
    "tokens_in": 3409,
    "tokens_out": 1133,
    "cost_usd": 0.00483078,
    "file": "streaming.go",
    "round": 2,
    "grep_used": true,
    "grep_results": "GREP `prefetchedSize - rs.totalBytesRead`:\n```\nstreaming.go:62:\t\tleft := prefetchedSize - rs.totalBytesRead\n```\n\nGREP `acquireRequestStream`:\n```\nstreaming.go:89:func acquireRequestStream(b *bytebufferpool.ByteBuffer, r *bufio.Reader, h headerInterface) *requestStream {\nhttp.go:1541:\t\t\treq.bodyStream = acquireRequestStream(bodyBuf, r, &req.Header)\nhttp.go:1546:\t\t\treq.bodyStream = acquireRequestStream(bodyBuf, r, &req.Header)\nhttp.go:1554:\treq.bodyStream = acquireRequestStream(bodyBuf, r, &req.Header)\nhttp.go:1621:\t\t\tresp.bodyStream = acquireRequestStream(bodyBuf, r, &resp.Header)\nhttp.go:1626:\t\t\tresp.bodyStream = acquireRequestStream(bodyBuf, r, &resp.Header)\nhttp.go:1632:\t\t\tresp.bodyStream = acquireRequestStream(bodyBuf, r, &resp.Header)\n```"
  },
  {
    "finding_title": "Content-Length stream can consume bytes beyond the declared body length",
    "verdict": "VALID",
    "reasoning": "The vulnerability report correctly identifies a logic flaw in `requestStream.Read` in `streaming.go`. Specifically, when reading from `rs.prefetchedBytes` (the buffer containing bytes already read into memory during header parsing or prior read-ahead), the code calculates `left := prefetchedSize - rs.totalBytesRead`. If `prefetchedSize` (the total bytes in the buffer) is greater than the remaining declared `ContentLength` (i.e., `prefetchedSize > rs.header.ContentLength() - rs.totalBytesRead`), then `left` will be larger than the actual remaining body size. The code then truncates the caller's buffer `p` to `left` and reads from `prefetchedBytes`. Since `bytes.Reader.Read` will return up to `len(p)` bytes, and `len(p)` is capped by `left` (which is too large), the function may return more bytes than the declared `ContentLength` permits. \n\nFor example, if `ContentLength` is 50, but the `prefetchedBytes` buffer contains 100 bytes (due to read-ahead of the next pipelined request or buffered I/O), and `totalBytesRead` is 0:\n1. `prefetchedSize` = 100.\n2. `left` = 100 - 0 = 100.\n3. If the caller requests 100 bytes, `p` is truncated to 100.\n4. `rs.prefetchedBytes.Read(p)` reads 100 bytes.\n5. `rs.totalBytesRead` becomes 100.\n6. The check `if n == rs.header.ContentLength()` (100 == 50) fails, so no EOF is returned.\n7. The function returns 100 bytes to the caller.\n\nThe caller now believes it has read 100 bytes of the body, but only 50 bytes were valid. The remaining 50 bytes belong to the next HTTP message (e.g., a pipelined request). This causes desynchronization of the keep-alive connection, allowing for HTTP Request Smuggling or Denial of Service. The correct logic should cap `left` at `rs.header.ContentLength() - rs.totalBytesRead` to ensure no more bytes are served than declared.\n\nThis is a critical security vulnerability because it allows an attacker to manipulate the stream state by sending a request with a small `Content-Length` but ensuring the client sends extra bytes (or relying on buffered reads), leading to stream corruption. The public API `Read` is directly reachable from untrusted input via `acquireRequestStream` in `http.go`. There are no defenses in the code to prevent this.\n\nCRUX: The truncation logic in the prefetchedBytes branch uses `prefetchedSize - rs.totalBytesRead` instead of capping it at `ContentLength - rs.totalBytesRead`, allowing reads beyond the declared body length.\nGREP: left := prefetchedSize - rs.totalBytesRead",
    "elapsed": 99.0,
    "tokens": 5403,
    "tokens_in": 4751,
    "tokens_out": 652,
    "cost_usd": 0.00395142,
    "file": "streaming.go",
    "round": 3,
    "grep_used": true,
    "grep_results": "GREP `left := prefetchedSize - rs.totalBytesRead`:\n```\nstreaming.go:62:\t\tleft := prefetchedSize - rs.totalBytesRead\n```"
  },
  {
    "finding_title": "Content-Length stream can consume bytes beyond the declared body length",
    "verdict": "INVALID",
    "reasoning": "The report identifies a logic flaw in `requestStream.Read` where bytes are read from `rs.prefetchedBytes` without capping the read size to the remaining `ContentLength`. However, this scenario is impossible in fasthttp's architecture. The `requestStream` is only created for requests where the body has been *fully* read into a buffer (`bodyBuf`) during header parsing (e.g., small requests or when `ReadLimit` is not exceeded). In `http.go`, if the body is too large to fit in the buffer, the code does not create a `requestStream`; instead, it leaves the body to be streamed directly from the `bufio.Reader` using `ContentLength`. For the `requestStream` case, `prefetchedBytes` contains *exactly* the body bytes declared by `ContentLength`. Therefore, `prefetchedBytes.Size()` will equal `ContentLength()`. The calculation `left := prefetchedSize - rs.totalBytesRead` is identical to `ContentLength() - rs.totalBytesRead` in this context. There are no \"read-ahead\" bytes from the *next* request in the `prefetchedBytes` buffer because the HTTP parser consumes exactly `ContentLength` bytes into the buffer before yielding the request. If the buffer had extra bytes, the parser would have desynchronized earlier or the body would be handled differently. Thus, the claimed vulnerability does not exist in practice because the precondition (prefetched buffer > content length) cannot be met by the fasthttp parser.\n\nCRUX: Whether `requestStream.prefetchedBytes` can ever contain more bytes than `header.ContentLength()`\nGREP: bodyBuf",
    "elapsed": 40.5,
    "tokens": 5667,
    "tokens_in": 5315,
    "tokens_out": 352,
    "cost_usd": 0.0032882999999999996,
    "file": "streaming.go",
    "round": 4,
    "grep_used": true,
    "grep_results": "GREP `bodyBuf`:\n```\nclient_timing_test.go:109:\t\tvar bodyBuf []byte\nclient_timing_test.go:112:\t\t\tstatusCode, bodyBuf, err = c.GetTimeout(bodyBuf[:0], url, time.Second)\nclient_timing_test.go:119:\t\t\tif !bytes.Equal(bodyBuf, body) {\nclient_timing_test.go:120:\t\t\t\tb.Fatalf(\"unexpected response body: %q. Expected %q\", bodyBuf, body)\nclient.go:1144:\tbodyBuf := resp.bodyBuffer()\nclient.go:1146:\toldBody := bodyBuf.B\nclient.go:1147:\tbodyBuf.B = dst\nclient.go:1151:\tbody = bodyBuf.B\nclient.go:1152:\tbodyBuf.B = oldBody\nhttp.go:428:\t\tbodyBuf := resp.bodyBuffer()\nhttp.go:429:\t\tbodyBuf.Reset()\nhttp.go:430:\t\t_, err := copyZeroAlloc(bodyBuf, resp.bodyStream)\nhttp.go:433:\t\t\tbodyBuf.SetString(err.Error())\nhttp.go:454:\t\tbodyBuf := req.bodyBuffer()\nhttp.go:455:\t\tbodyBuf.Reset()\nhttp.go:456:\t\t_, err := copyZeroAlloc(bodyBuf, req.bodyStream)\nhttp.go:459:\t\t\tbodyBuf.SetString(err.Error())\nhttp.go:468:func (resp *Response) bodyBuffer() *bytebufferpool.ByteBuffer {\nhttp.go:476:func (req *Request) bodyBuffer() *bytebufferpool.ByteBuffer {\nhttp.go:749:\tresp.bodyBuffer().Write(p) //nolint:errcheck\nhttp.go:755:\tresp.bodyBuffer().WriteString(s) //nolint:errcheck\nhttp.go:763:\tbodyBuf := resp.bodyBuffer()\nhttp.go:764:\tbodyBuf.Reset()\nhttp.go:765:\tbodyBuf.Write(body) //nolint:errcheck\nhttp.go:771:\tbodyBuf := resp.bodyBuffer()\nhttp.go:772:\tbodyBuf.Reset()\nhttp.go:773:\tbodyBuf.WriteString(body) //nolint:errcheck\nhttp.go:848:\tbb := resp.bodyBuffer()\nhttp.go:873:\tbb := req.bodyBuffer()\nhttp.go:916:\treq.bodyBuffer().Write(p) //nolint:errcheck\n```"
  },
  {
    "finding_title": "Content-Length stream can consume bytes beyond the declared body length",
    "verdict": "INVALID",
    "reasoning": "The vulnerability report claims that `requestStream.Read` can read bytes beyond the declared `ContentLength` if `prefetchedBytes` contains more data than the body length. Reviewers 1, 2, and 3 accept this premise, but Reviewer 4 correctly identifies that this relies on an impossible precondition in fasthttp's architecture: that `prefetchedBytes` (derived from `bodyBuf`) can contain more bytes than `req.Header.ContentLength()`. \n\nI need to verify this precondition. In `http.go`, the `bodyBuf` (which becomes `prefetchedBytes`) is populated by the HTTP parser. The parser reads the body into the buffer strictly up to `ContentLength` bytes. If the underlying `bufio.Reader` contains more bytes (e.g., a pipelined request), those bytes remain in the `bufio.Reader` (which becomes `rs.reader`) and are NOT copied into `bodyBuf`. \n\nSpecifically, `acquireRequestStream` is called with `bodyBuf` containing exactly the body bytes parsed by the header/body parser. The parser uses `ContentLength` to determine how many bytes to read from the stream into the buffer. Therefore, `prefetchedBytes.Size()` will be equal to `rs.header.ContentLength()` (or 0 if empty, but then the logic is trivial). \n\nIf `prefetchedBytes.Size() == ContentLength`, then `left := prefetchedSize - totalBytesRead` is identical to `ContentLength() - totalBytesRead`. The truncation logic is therefore correct. There are no \"read-ahead\" bytes from the next request in `prefetchedBytes` because the parser does not over-read into the body buffer; it leaves the stream position after exactly `ContentLength` bytes. The remaining bytes (if any) are in `rs.reader`, which is accessed in the subsequent branch of `Read` (lines 72-84 in the snippet), which correctly caps `left` at `ContentLength - totalBytesRead`. \n\nSince the precondition `prefetchedSize > ContentLength` cannot be met by the fasthttp parser, the bug pattern does not exist in the reachable state space. The code is safe.\n\nCRUX: Whether `requestStream.prefetchedBytes` can ever contain more bytes than `header.ContentLength()`\nGREP: bodyBuf",
    "elapsed": 43.7,
    "tokens": 6703,
    "tokens_in": 6203,
    "tokens_out": 500,
    "cost_usd": 0.0041052599999999995,
    "file": "streaming.go",
    "round": 5,
    "grep_used": true,
    "grep_results": "GREP `bodyBuf`:\n```\nclient_timing_test.go:109:\t\tvar bodyBuf []byte\nclient_timing_test.go:112:\t\t\tstatusCode, bodyBuf, err = c.GetTimeout(bodyBuf[:0], url, time.Second)\nclient_timing_test.go:119:\t\t\tif !bytes.Equal(bodyBuf, body) {\nclient_timing_test.go:120:\t\t\t\tb.Fatalf(\"unexpected response body: %q. Expected %q\", bodyBuf, body)\nclient.go:1144:\tbodyBuf := resp.bodyBuffer()\nclient.go:1146:\toldBody := bodyBuf.B\nclient.go:1147:\tbodyBuf.B = dst\nclient.go:1151:\tbody = bodyBuf.B\nclient.go:1152:\tbodyBuf.B = oldBody\nhttp.go:428:\t\tbodyBuf := resp.bodyBuffer()\nhttp.go:429:\t\tbodyBuf.Reset()\nhttp.go:430:\t\t_, err := copyZeroAlloc(bodyBuf, resp.bodyStream)\nhttp.go:433:\t\t\tbodyBuf.SetString(err.Error())\nhttp.go:454:\t\tbodyBuf := req.bodyBuffer()\nhttp.go:455:\t\tbodyBuf.Reset()\nhttp.go:456:\t\t_, err := copyZeroAlloc(bodyBuf, req.bodyStream)\nhttp.go:459:\t\t\tbodyBuf.SetString(err.Error())\nhttp.go:468:func (resp *Response) bodyBuffer() *bytebufferpool.ByteBuffer {\nhttp.go:476:func (req *Request) bodyBuffer() *bytebufferpool.ByteBuffer {\nhttp.go:749:\tresp.bodyBuffer().Write(p) //nolint:errcheck\nhttp.go:755:\tresp.bodyBuffer().WriteString(s) //nolint:errcheck\nhttp.go:763:\tbodyBuf := resp.bodyBuffer()\nhttp.go:764:\tbodyBuf.Reset()\nhttp.go:765:\tbodyBuf.Write(body) //nolint:errcheck\nhttp.go:771:\tbodyBuf := resp.bodyBuffer()\nhttp.go:772:\tbodyBuf.Reset()\nhttp.go:773:\tbodyBuf.WriteString(body) //nolint:errcheck\nhttp.go:848:\tbb := resp.bodyBuffer()\nhttp.go:873:\tbb := req.bodyBuffer()\nhttp.go:916:\treq.bodyBuffer().Write(p) //nolint:errcheck\n```"
  },
  {
    "verdict": "VALID",
    "reasoning": "[ARBITER] The vulnerability is a logic error in `requestStream.Read` that leads to HTTP request smuggling (desynchronization). When `prefetchedBytes` contains more bytes than the remaining `ContentLength` (e.g., due to read-ahead of a pipelined request), the code at `streaming.go:62` calculates `left` as `prefetchedSize - rs.totalBytesRead`. This allows `Read` to return bytes from the prefetched buffer that belong to the *next* HTTP message, rather than stopping at the declared `ContentLength`. The check `if rs.totalBytesRead == rs.header.ContentLength()` at `streaming.go:59` fails to stop the stream if `prefetchedSize > ContentLength`, because `totalBytesRead` will exceed `ContentLength` before the equality check triggers EOF, or bytes will be served from the buffer beyond the limit. This violates the HTTP protocol contract and allows an attacker to inject arbitrary bytes into the stream, enabling request smuggling or denial of service. The threat model (library parser) applies because fasthttp's public server API parses untrusted HTTP requests, and this logic flaw affects the integrity of stream consumption.",
    "round": 6,
    "file": "streaming.go",
    "finding_title": "Content-Length stream can consume bytes beyond the declared body length",
    "elapsed": 57.6,
    "tokens_in": 3486,
    "tokens_out": 256,
    "cost_usd": 0.0022321199999999998
  }
]