Stale TransferEncoding and RequestURI state when http.Request is reused

[
  {
    "finding_title": "Stale TransferEncoding and RequestURI state when http.Request is reused",
    "verdict": "VALID",
    "reasoning": "The vulnerability report claims that `r.TransferEncoding` and `r.RequestURI` are not reset when reusing an `http.Request`, leading to stale state. \n\n1. **TransferEncoding**: The code executes `r.TransferEncoding = append(r.TransferEncoding, sv)` inside a loop iterating over `ctx.Request.Header.All()`. If the current request `ctx` does not contain a `Transfer-Encoding` header, the loop never hits the `case \"Transfer-Encoding\"`, and `r.TransferEncoding` is never touched. It retains the values from the previous request. Since `ctx` is attacker-controlled (incoming HTTP request headers), an attacker can send a request with `Transfer-Encoding: chunked` followed by a request without it. If the caller reuses `r`, the second request will still have `r.TransferEncoding = [\"chunked\"]`, potentially causing body parsing confusion or request smuggling in downstream handlers that trust `r.TransferEncoding`. The function does not clear `r.TransferEncoding` before populating it (unlike `r.Header`, which is explicitly cleared). \n\n2. **RequestURI**: The code only sets `r.RequestURI = strRequestURI` if `forServer` is true. If `forServer` is false, `r.RequestURI` is not modified. If `forServer` is true, it is overwritten. The report states it is only overwritten when `forServer` is true, which is a conditional write. However, the `forServer` flag is a parameter, and the function is designed to set it. If `forServer` is true, it is reset. If false, it might be stale. But `TransferEncoding` is definitely never reset regardless of `forServer`.\n\nThe core issue is that `ConvertRequest` is a mutation function on a caller-provided struct. It is not idempotent/reset-safe for `TransferEncoding` and potentially `RequestURI`. In a library context where reuse is common (e.g., in a pool or loop), this is a real bug. The threat model states that the public API must validate inputs and maintain correct behavior. A stale `TransferEncoding` from a previous request on a reused `http.Request` object allows an attacker to influence the interpretation of the current request's body or metadata, which is a security-relevant state inconsistency (potential request smuggling or parsing error). The lack of reset for `TransferEncoding` is a clear defect. The `forServer` condition for `RequestURI` is less critical if the caller manages it, but `TransferEncoding` is purely managed by this function from `ctx` and never cleared, making the state leak real.\n\nTherefore, the bug pattern exists, is reachable via untrusted input (attacker controls headers in `ctx`), and causes incorrect state in a reused object, which is a security concern in the context of request parsing.\n\nVerdict: VALID\n\nCRUX: r.TransferEncoding is appended to but never cleared/reset before processing new headers, allowing stale values from previous requests to persist in reused http.Request objects.\nGREP: r.TransferEncoding",
    "elapsed": 10.1,
    "tokens": 3025,
    "tokens_in": 2371,
    "tokens_out": 654,
    "cost_usd": 0.00295782,
    "file": "fasthttpadaptor/request.go",
    "round": 1,
    "grep_used": true,
    "grep_results": "GREP `r.TransferEncoding`:\n```\nfasthttpadaptor/request.go:60:\t\t\tr.TransferEncoding = append(r.TransferEncoding, sv)\nfasthttpadaptor/adaptor_test.go:66:\t\tif len(r.TransferEncoding) != 0 {\nfasthttpadaptor/adaptor_test.go:67:\t\t\tt.Fatalf(\"unexpected transferEncoding %q. Expecting []\", r.TransferEncoding)\n```"
  },
  {
    "finding_title": "Stale TransferEncoding and RequestURI state when http.Request is reused",
    "verdict": "VALID",
    "reasoning": "The report identifies that `r.TransferEncoding` and `r.RequestURI` are not properly reset when reusing an `http.Request` object with `ConvertRequest`. \n\n1. **TransferEncoding Bug**: The code executes `r.TransferEncoding = append(r.TransferEncoding, sv)` only when a `Transfer-Encoding` header is present in the incoming request (`ctx`). It does not clear `r.TransferEncoding` at the start of the function. If a caller reuses the same `*http.Request` instance for multiple requests (a common pattern in pooled handlers or adapters), and the first request has `Transfer-Encoding: chunked` while the second does not, the second request will erroneously retain the `TransferEncoding` field from the first. Since `ctx` is derived from attacker-controlled network input, an attacker can craft a sequence of requests to inject stale `TransferEncoding` values into subsequent requests. This can lead to request smuggling or body parsing errors in downstream handlers that rely on `net/http` semantics.\n\n2. **RequestURI Bug**: `r.RequestURI` is only updated if `forServer` is true. If `forServer` is false, it remains unchanged. However, `TransferEncoding` is a more direct path to confusion because it affects how the body is interpreted regardless of the `forServer` flag's specific usage intent, and the failure to reset it is a clear state leakage bug.\n\nThe threat model for a library states that the public API must ensure correct behavior on inputs. Failing to reset state in a mutable struct passed by the caller is a defect. The attacker controls the headers in `ctx`, allowing them to manipulate the state of the reused `r`. The lack of a defense (initial clearing of `r.TransferEncoding`) makes this a valid vulnerability.\n\nCRUX: r.TransferEncoding is appended to but never cleared/reset before processing new headers, allowing stale values from previous requests to persist in reused http.Request objects.\nGREP: r.TransferEncoding",
    "elapsed": 6.6,
    "tokens": 3662,
    "tokens_in": 3239,
    "tokens_out": 423,
    "cost_usd": 0.0026293799999999997,
    "file": "fasthttpadaptor/request.go",
    "round": 2,
    "grep_used": true,
    "grep_results": "GREP `r.TransferEncoding`:\n```\nfasthttpadaptor/request.go:60:\t\t\tr.TransferEncoding = append(r.TransferEncoding, sv)\nfasthttpadaptor/adaptor_test.go:66:\t\tif len(r.TransferEncoding) != 0 {\nfasthttpadaptor/adaptor_test.go:67:\t\t\tt.Fatalf(\"unexpected transferEncoding %q. Expecting []\", r.TransferEncoding)\n```"
  },
  {
    "finding_title": "Stale TransferEncoding and RequestURI state when http.Request is reused",
    "verdict": "UNCERTAIN",
    "reasoning": "The report claims that `r.TransferEncoding` retains stale state when `http.Request` is reused. The code confirms that `r.TransferEncoding` is only appended to if the `Transfer-Encoding` header is present in the incoming request, and it is never cleared/reset at the start of `ConvertRequest` (unlike `r.Header`). However, the security relevance depends on the trust model of the library API. The `http.Request` struct (`r`) is provided by the caller. The caller is a trusted component (the host application or adapter wrapper) that manages the lifecycle of this object. The threat model states: \"Missing parameter validation in non-public ... functions ... Not a vulnerability under this model ... misuse that violates a documented precondition of the public API.\" While `ConvertRequest` is public, the *state* of the `http.Request` object is a precondition managed by the caller. If the caller reuses the object, they are responsible for resetting fields that `ConvertRequest` does not fully own or overwrite (or they must expect the function to handle it). However, looking closer at the threat model rule: \"Trust invariants: the public API must validate all inputs... A crash, memory corruption or unbounded resource use reachable through the public API with a crafted payload is VALID.\" Here, the issue is a logic error (state leakage) in a library utility. Does this lead to \"meaningful harm\" (crash, code exec, etc.)? A stale `TransferEncoding` might cause a downstream `net/http` handler to misinterpret the body. But is this a vulnerability in `fasthttpadaptor`? The function's purpose is to populate `r` from `ctx`. It explicitly clears `r.Header`. It fails to clear `r.TransferEncoding`. This is a bug. Is it a security vulnerability? If an attacker can trigger this by sending specific headers, and it causes a downstream crash or data corruption, it is valid. But wait: `r` is a pointer passed by the caller. If the caller allocates a new `http.Request` for every request (standard practice unless pooling), this bug does not trigger. The report assumes reuse. The threat model says: \"Misuse that violates a documented precondition... is NOT a vulnerability.\" Does the documentation say \"Do not reuse r\"? The doc says: \"The http.Request must not be used after the fasthttp handler has returned! Memory in use by the http.Request will be reused after your handler has returned!\" This implies `r` is tied to the `ctx` lifetime. Reusing `r` across different `ctx` instances in a way that relies on fields not explicitly reset might be considered misuse if the library doesn't promise idempotency on partial resets. However, `r.Header` IS cleared. This inconsistency is a code quality issue / bug, but does it meet the bar for a security vulnerability? The attacker controls `ctx`. If the *caller* reuses `r`, the caller is a trusted component. The ",
    "elapsed": 10.4,
    "tokens": 4367,
    "tokens_in": 3740,
    "tokens_out": 627,
    "cost_usd": 0.0034518,
    "file": "fasthttpadaptor/request.go",
    "round": 3
  },
  {
    "finding_title": "Stale TransferEncoding and RequestURI state when http.Request is reused",
    "verdict": "UNCERTAIN",
    "reasoning": "The vulnerability report claims that `r.TransferEncoding` and `r.RequestURI` retain stale values when the `http.Request` object is reused across multiple calls to `ConvertRequest`. I verified that `r.TransferEncoding` is appended to but never cleared at the start of the function (unlike `r.Header`, which is explicitly cleared). This is a real defect in the function's logic. However, to classify this as a security vulnerability under the provided threat model (library/SDK), it must be reachable by an external attacker through the public API to cause meaningful harm (crash, data corruption, auth bypass, etc.). \n\nThe key constraint is the caller. `ConvertRequest` takes `r *http.Request` as a parameter. The caller (the host application or adapter wrapper) is responsible for allocating and managing the lifecycle of this `http.Request` object. In standard `net/http` and `fasthttp` usage, a new `http.Request` (or one managed by the server pool) is typically used for each request. Reusing a single `http.Request` object across multiple distinct incoming requests from different clients or even different requests in a pool is a pattern managed by the *caller*, not the library function itself. The library function `ConvertRequest` does not pool `http.Request` objects internally; it only populates the one passed to it. \n\nIf a caller reuses an `http.Request` object across requests, they are managing shared mutable state. If they fail to reset fields that `ConvertRequest` does not explicitly clear (like `TransferEncoding` when the header is absent in the new request), that is a bug in the *caller's* state management, not a vulnerability in the library's parsing of untrusted input in the sense of memory safety or direct exploitability of the library's internals. The threat model states: \"Misuse that violates a documented precondition of the public API\" is not a vulnerability. While the documentation doesn't explicitly say \"do not reuse\", the function signature implies `r` is a container to be filled. The \"attacker",
    "elapsed": 7.3,
    "tokens": 4792,
    "tokens_in": 4355,
    "tokens_out": 437,
    "cost_usd": 0.0031401,
    "file": "fasthttpadaptor/request.go",
    "round": 4
  },
  {
    "finding_title": "Stale TransferEncoding and RequestURI state when http.Request is reused",
    "verdict": "INVALID",
    "reasoning": "The report claims that `r.TransferEncoding` is not reset, leading to stale state if the caller reuses the `http.Request` object. The code confirms that `r.TransferEncoding` is appended to only if the header exists in the incoming request, and unlike `r.Header`, it is not cleared at the start of the function. However, the threat model explicitly states that misuse violating a documented precondition is not a vulnerability. The function documentation states: 'The http.Request must not be used after the fasthttp handler has returned! Memory in use by the http.Request will be reused after your handler has returned!'. While this primarily refers to memory safety (pointers to fasthttp internal buffers), it implies a strict lifetime: the `http.Request` object is a transient view into the `fasthttp` context. Reusing this object across multiple requests (e.g., in a pool) is a deviation from the intended lifecycle where a new `fasthttp.RequestCtx` (and typically a new `http.Request` conversion target) is used per request. Even if one argues the documentation is ambiguous about reusability, the 'bug' relies on the *caller* failing to reset state between requests. Since the library function itself does not maintain the state across calls, and the attacker cannot force the *caller* to reuse the same `http.Request` object incorrectly (the attacker only controls the content of the `fasthttp` request, not the allocation strategy of the host application), this is not a security vulnerability in the library. It is a caller-side programming error. The attacker does not have control over the reuse pattern; they only control the data. If the caller correctly allocates a fresh `http.Request` or resets it, the issue does not occur. If the caller reuses it improperly, the caller is responsible for the state management. Therefore, this is INVALID as a security vulnerability in the library under the given threat model.\n\nCRUX: The bug requires the caller to reuse the same http.Request object across multiple requests, which is a caller-side state management issue, not a flaw in the library's parsing of untrusted input.\nGREP: TransferEncoding",
    "elapsed": 7.6,
    "tokens": 5238,
    "tokens_in": 4785,
    "tokens_out": 453,
    "cost_usd": 0.0033686999999999996,
    "file": "fasthttpadaptor/request.go",
    "round": 5,
    "grep_used": true,
    "grep_results": "GREP `TransferEncoding`:\n```\nfasthttpadaptor/request.go:60:\t\t\tr.TransferEncoding = append(r.TransferEncoding, sv)\nfasthttpadaptor/adaptor_test.go:66:\t\tif len(r.TransferEncoding) != 0 {\nfasthttpadaptor/adaptor_test.go:67:\t\t\tt.Fatalf(\"unexpected transferEncoding %q. Expecting []\", r.TransferEncoding)\nserver_test.go:2081:func TestServerRejectsInvalidContentLengthWithTransferEncoding(t *testing.T) {\nheader_test.go:2830:\texpectRequestHeaderGet(t, h, HeaderTransferEncoding, \"\")\nheader_test.go:2867:\texpectRequestHeaderGet(t, &h1, HeaderTransferEncoding, \"\")\nheader_test.go:2885:\th.Set(HeaderTransferEncoding, \"chunked\")\nheader_test.go:2894:\texpectResponseHeaderGet(t, h, HeaderTransferEncoding, \"\")\nheader_test.go:3169:\tif hasArg(h.h, HeaderTransferEncoding) {\nheader.go:212:\t\tte := peekArgBytes(h.h, strTransferEncoding)\nheader.go:241:\t\th.h = delAllArgs(h.h, HeaderTransferEncoding)\nheader.go:245:\t\th.h = setArgBytes(h.h, strTransferEncoding, strChunked, argsHasValue)\nheader.go:272:\t\th.h = delAllArgs(h.h, HeaderTransferEncoding)\nheader.go:275:\t\th.h = setArgBytes(h.h, strTransferEncoding, strChunked, argsHasValue)\nheader.go:479:\tErrUnsupportedTransferEncoding   = errors.New(\"unsupported Transfer-Encoding\")\nheader.go:1447:\t\tif caseInsensitiveCompare(strTransferEncoding, key) {\nheader.go:1501:\t\tif caseInsensitiveCompare(strTransferEncoding, key) {\nheader.go:2787:\t\t\tcaseInsensitiveCompare(key, strTransferEncoding)\nheader.go:3075:\t\t\tif caseInsensitiveCompare(s.key, strTransferEncoding) {\nheader.go:3082:\t\t\t\t\t\treturn 0, ErrUnsupportedTransferEncoding\nheader.go:3090:\t\t\t\t\t\treturn 0, ErrUnsupportedTransferEncoding\nheader.go:3095:\t\t\t\th.h = setArgBytes(h.h, strTransferEncoding, strChunked, argsHasValue)\nheader.go:3178:\t\tisTransferEncoding := false\nheader.go:3198:\t\t\tif caseInsensitiveCompare(s.key, strTransferEncoding) {\nheader.go:3199:\t\t\t\tisTransferEncoding = true\nheader.go:3246:\t\t\tif isTransferEncoding {\nheader.go:3253:\t\t\t\t\t\treturn 0, ErrUnsupportedTransferEncoding\nheader.go:3260:\t\t\t\t\th.h = setArgBytes(h.h, strTransferEncoding, strChunked, argsHasValue)\nstrings.go:42:\tstrTransferEncoding   = []byte(HeaderTransferEncoding)\nhttp_test.go:35:func TestResponseEmptyTransferEncodingError(t *testing.T) {\n```"
  },
  {
    "verdict": "INVALID",
    "reasoning": "[ARBITER] The finding relies on the caller violating a documented precondition of the public API. The `ConvertRequest` function explicitly states in its documentation: 'The http.Request must not be used after the fasthttp handler has returned! Memory in use by the http.Request will be reused after your handler has returned!'. Under the provided threat model for a library/SDK, misuse that violates a documented precondition is not considered a security vulnerability. The library is not responsible for state management if the caller incorrectly reuses the object against explicit instructions.",
    "round": 6,
    "file": "fasthttpadaptor/request.go",
    "finding_title": "Stale TransferEncoding and RequestURI state when http.Request is reused",
    "elapsed": 2.4,
    "tokens_in": 2655,
    "tokens_out": 118,
    "cost_usd": 0.0014690999999999999
  }
]