Reject control characters in the request target to prevent request splitting

[h-t-m]

Reject control characters in the request target (request splitting)

Problem

ClientRequest.send() builds the status line from self.url.raw_path_qs
without any validation:

path = self.url.raw_path_qs
...
status_line = f"{self.method} {path} HTTP/{v.major}.{v.minor}"

When the request target is a yarl.URL constructed with encoded=True
(a documented, supported public API) and its path contains CR/LF bytes,
those bytes are serialized verbatim into the request line. The server then
parses the trailing bytes as additional request lines — full
request splitting / smuggling controlled by whoever influences the URL
(CWE-444).

Dynamically reproduced on 3.8.6 and confirmed by source inspection that the
status-line construction remains unvalidated on master (3.14.x line):
client_reqrep.py still assigns path = self.url.raw_path_qs with no
newline check, while header names/values ARE checked (_safe_header).

Why fix this in aiohttp

1. Precedent — CPython stdlib: http.client.putrequest has rejected
   control characters in the request target since 2019
   (CVE-2019-9740, _contains_disallowed_url_pchar_re,
   Lib/http/client.py:159): "Prevents CVE-2019-9740. Includes control
   characters such as \r\n." The stdlib treats the client — not the URL
   parser — as the enforcement point for request-line integrity.
2. Precedent — aiohttp itself: the two 2023 request-line injection CVEs
   were fixed at this exact level: version is now type-constrained to
   HttpVersion (1e86b77, CVE-2023-49081) and the method fix followed
   the same principle (CVE-2023-49082). The request target is the only
   status-line component still concatenated without validation — this PR
   closes that remaining gap symmetrically.
3. yarl's encoded=True is documented pass-through behavior ("the URL is
   assumed to be already encoded"), so it is not a security boundary we can
   rely on.

Fix

Mirror the stdlib check: reject the request target when it contains control
characters ([\x00-\x1f\x7f]), raising ValueError at send time. Plain
str URLs are unaffected (yarl re-quotes and strips CR/LF); only
pre-encoded paths carrying raw control bytes are rejected — precisely the
splitting vector.

_contains_disallowed_target_pchar_re = re.compile("[\x00-\x1f\x7f]")
...
if _contains_disallowed_target_pchar_re.search(path):
    raise ValueError(
        "Request URL can't contain control characters. "
        "Potential request splitting attack."
    )

Testing

• test_request_target_with_crlf_raises: builds the encoded URL from the
  report PoC and asserts ValueError on send.
• Existing suite unaffected (plain/quoted URLs never contain raw control
  bytes after yarl re-quoting).

Related

• Private advisory GHSA-pwcp-vr8x-fp2x (closed with maintainer guidance to
  follow up via PR — this is that PR).
• yarl side: we chose not to file a separate report. encoded=True is
  documented pass-through behavior, and per the discussion the client-level
  request-line integrity check (this PR) is the appropriate enforcement
  point — the same conclusion the stdlib reached in 2019.

[greptile-apps]

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

_{Reviews (2): Last reviewed commit: "Fix test: use make_client_request fixtur..." | Re-trigger Greptile}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.02%. Comparing base (20acdf4) to head (66138ac).
⚠️ Report is 3 commits behind head on master.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #13593   +/-   ##
=======================================
  Coverage   99.02%   99.02%           
=======================================
  Files         135      135           
  Lines       50500    50527   +27     
  Branches     2652     2653    +1     
=======================================
+ Hits        50007    50034   +27     
  Misses        370      370           
  Partials      123      123           

Flag	Coverage Δ
Autobahn	22.03% <33.33%> (+<0.01%)	⬆️
CI-GHA	98.91% <100.00%> (+<0.01%)	⬆️
OS-Linux	98.69% <100.00%> (+<0.01%)	⬆️
OS-Windows	97.09% <100.00%> (+<0.01%)	⬆️
OS-macOS	97.96% <100.00%> (-0.01%)	⬇️
Py-3.10	98.11% <100.00%> (+<0.01%)	⬆️
Py-3.11	98.34% <100.00%> (+<0.01%)	⬆️
Py-3.12	98.43% <100.00%> (+<0.01%)	⬆️
Py-3.13	98.41% <100.00%> (-0.01%)	⬇️
Py-3.14	98.44% <100.00%> (-0.01%)	⬇️
Py-3.14t	97.61% <100.00%> (+<0.01%)	⬆️
Py-pypy-3.11	97.39% <100.00%> (-0.01%)	⬇️
VM-macos	97.96% <100.00%> (-0.01%)	⬇️
VM-ubuntu	98.69% <100.00%> (+<0.01%)	⬆️
VM-windows	97.09% <100.00%> (+<0.01%)	⬆️
cython-coverage	83.06% <50.00%> (-0.02%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

[codspeed-hq]

Merging this PR will not alter performance

✅ 96 untouched benchmarks
⏩ 83 skipped benchmarks¹

---

_{Comparing h-t-m:fix/request-target-crlf (66138ac) with master (6b97ebd)}

Footnotes

1. 83 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

[h-t-m]

Closing this — I misread "a public PR for this one" in the advisory as
referring to aiohttp, when it referred to yarl. The technical facts in
the report (unvalidated request target vs. checked headers; the
stdlib client-side precedent) stand either way.

Thanks for the CI runs and bot reviews.

h-t-m (@h-t-m)