Skip to content

Fix host validation: IPv6 zone ID characters and NFKC percent bypass#1655

Merged
bdraco merged 10 commits into
aio-libs:masterfrom
rodrigobnogueira:fix-host-validation
May 26, 2026
Merged

Fix host validation: IPv6 zone ID characters and NFKC percent bypass#1655
bdraco merged 10 commits into
aio-libs:masterfrom
rodrigobnogueira:fix-host-validation

Conversation

@rodrigobnogueira

@rodrigobnogueira rodrigobnogueira commented Apr 12, 2026

Copy link
Copy Markdown
Member

What do these changes do?

Fixes two incomplete validation cases in host parsing.


Finding 1 — IPv6 zone ID character validation

The bug: URL.build() with validate_host=True (the default) did not validate the zone ID portion of IPv6 addresses. Any character — including CR, LF, null bytes — could be embedded in url.host via URL.build(host='::1%<bad>'). This creates an asymmetry:

# Correctly rejected:
URL.build(scheme='http', host='example.com\r\nX-Injected: evil')
# ValueError: Host '...' cannot contain '\r' ...

# Was NOT rejected (bug):
URL.build(scheme='http', host='::1%\r\nX-Injected: evil', path='/')
# url.host = '::1%\r\nX-Injected: evil'

Root cause: _encode_host() validates the IP address portion via ip_address() but the zone string was concatenated back verbatim without any character inspection.

Fix: Added _ZONE_ID_UNSAFE_RE to reject ASCII control characters (CTL, \x00–\x1f and \x7f) in the zone ID. Per RFC 4007 §11.2, zone IDs are OS-specific text strings with no defined format. RFC 9844 §6.3 recommends rejecting characters inappropriate for the environment; for yarl we reject ASCII control characters. All other characters — spaces, Unicode, parentheses, etc. — are accepted.


Finding 2 — NFKC fullwidth/small percent sign bypass

The bug: _check_netloc() normalizes the netloc via NFKC and checks for URL-reserved characters, but % was missing from the checked set. U+FF05 (FULLWIDTH PERCENT SIGN ) and U+FE6A (SMALL PERCENT SIGN ) both normalize to % under NFKC, so they passed validation and ultimately produced a literal % in url.host via the standard library IDNA fallback in _idna_encode():

URL('http://evil.com\uff052e.internal/').host
# 'evil.com%2e.internal'   ← literal % injected

Root cause: The idna library correctly rejects % as invalid in a hostname label, but the except UnicodeError fallback to standard library host.encode('idna') does its own NFKC normalization and silently accepts the character.

Fix: Add % to the character set checked after NFKC normalization in _check_netloc().


Changes

File Change
yarl/_url.py Add _ZONE_ID_UNSAFE_RE regex; validate zone ID in _encode_host() by rejecting CTL characters
yarl/_parse.py Add % to NFKC character check in _check_netloc()
tests/test_url_build.py Tests for zone ID CTL rejection (CRLF, null byte) and acceptance of non-CTL (spaces, Unicode, parens)
tests/test_url.py Tests for U+FF05 and U+FE6A being rejected

@psf-chronographer psf-chronographer Bot added the bot:chronographer:provided There is a change note present in this PR label Apr 12, 2026
@codspeed-hq

codspeed-hq Bot commented Apr 12, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 97 untouched benchmarks


Comparing rodrigobnogueira:fix-host-validation (06047a9) with master (fbf8cfd)

Open in CodSpeed

Finding 1: IPv6 zone IDs were not validated even when validate_host=True.
Any character — including CR, LF, and null bytes — could be embedded in
url.host via URL.build(host='::1%<bad>'). This creates an asymmetry: regular
hostnames are correctly rejected for control characters but zone IDs were
passed through verbatim.

Fix: add _ZONE_ID_RE regex (RFC 6874 unreserved + sub-delims) and validate
the zone portion of IPv6 addresses in _encode_host() when validate_host=True.

Finding 2: _check_netloc() normalizes the netloc via NFKC and checks for
URL-reserved characters but '%' was missing from the checked set. U+FF05
(FULLWIDTH PERCENT SIGN) and U+FE6A (SMALL PERCENT SIGN) both normalize to
'%' under NFKC and were accepted, ultimately producing a literal '%' in
url.host via the stdlib IDNA fallback in _idna_encode().

Fix: add '%' to the character set checked in _check_netloc().
@codecov

codecov Bot commented Apr 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.85%. Comparing base (fbf8cfd) to head (06047a9).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1655      +/-   ##
==========================================
+ Coverage   99.81%   99.85%   +0.04%     
==========================================
  Files          21       21              
  Lines        4249     4273      +24     
  Branches      250      251       +1     
==========================================
+ Hits         4241     4267      +26     
+ Misses          5        4       -1     
+ Partials        3        2       -1     
Flag Coverage Δ
CI-GHA 99.78% <100.00%> (+0.04%) ⬆️
OS-Linux 99.78% <100.00%> (+0.04%) ⬆️
OS-Windows 98.52% <100.00%> (+<0.01%) ⬆️
OS-macOS 98.71% <100.00%> (+0.05%) ⬆️
Py-3.10 99.69% <100.00%> (+<0.01%) ⬆️
Py-3.11 99.69% <100.00%> (+<0.01%) ⬆️
Py-3.12 99.69% <100.00%> (+<0.01%) ⬆️
Py-3.13 99.74% <100.00%> (+0.04%) ⬆️
Py-3.14 99.73% <100.00%> (+0.04%) ⬆️
Py-3.14t 99.73% <100.00%> (+0.04%) ⬆️
Py-pypy-3.11 99.36% <100.00%> (+<0.01%) ⬆️
VM-macos-latest 98.71% <100.00%> (+0.05%) ⬆️
VM-ubuntu-latest 99.78% <100.00%> (+0.04%) ⬆️
VM-windows-latest 98.52% <100.00%> (+<0.01%) ⬆️
pytest 99.78% <100.00%> (+0.04%) ⬆️

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

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread tests/test_url_build.py Outdated
Comment thread yarl/_url.py Outdated
- Remove unused 'desc' parameter from zone ID test parametrize tuple
- Update _ZONE_ID_RE comment: cite RFC 9844 (which obsoletes RFC 6874
  for UI usage) and add a direct link to RFC 6874 §2 for the ZoneID
  ABNF grammar (unreserved / pct-encoded)
Comment thread yarl/_url.py Outdated
rodrigobnogueira and others added 2 commits April 19, 2026 17:05
The _ZONE_ID_RE allowlist was based on RFC 6874's ABNF grammar, which
was overly restrictive. RFC 4007 §11.2 specifies that zone IDs are
OS-defined text strings with no format restriction (interface names like
'eth0', 'Ethernet (LAN)', and numeric indices are all valid).

RFC 9844 §6.3 recommends rejecting characters inappropriate for the
environment. For yarl this means ASCII control characters (CTL).

Changes:
- Replace _ZONE_ID_RE with _ZONE_ID_UNSAFE_RE that rejects CTL chars
- Accept empty-zone check (::1% is still invalid)
- Update tests: remove 'spaces' from invalid cases, add valid cases
- Update changelog to cite RFC 9844 §6.3
@bdraco

bdraco commented May 17, 2026

Copy link
Copy Markdown
Member

@aiolibsbot review

@aiolibsbot

aiolibsbot commented May 17, 2026

Copy link
Copy Markdown
Contributor

PR Review — Fix host validation: IPv6 zone ID characters and NFKC percent bypass

Both fixes are well-targeted and minimal. The IPv6 zone ID validation correctly rejects ASCII CTL (\x00-\x1f, \x7f) and empty zones, gated on sep and validate_host so non-IPv6 hosts and explicit validate_host=False callers are untouched. The error message is a static string that never reflects the offending zone, which the new test locks in via assert zone not in error. The % addition to _check_netloc() closes a real bypass (U+FF05 / U+FE6A NFKC-normalizing to % and bleeding through the stdlib IDNA fallback in _idna_encode) and only fires when NFKC has already mutated the netloc, so legitimate percent-encoded ASCII userinfo is unaffected. Test coverage for both fixes is thorough (CRLF, NUL, empty zone, spaces, parens, Unicode for the zone; U+FF05 and U+FE6A for the netloc bypass), the two CHANGES/1655.bugfix.*.rst fragments are correctly formatted with the right user attribution, and previous reviewer concerns (unused desc, RFC link freshness, control-char-only policy) have all been addressed. Merge-ready.



Checklist

  • Input validation at system boundaries
  • Error messages do not expose unsafe content to logs/terminal
  • New behaviour covered by tests
  • Existing reviewer feedback addressed
  • CHANGES fragments present and well-formed
  • Backward compatibility for validate_host=False callers

Summary

Both fixes are well-targeted and minimal. The IPv6 zone ID validation correctly rejects ASCII CTL (\x00-\x1f, \x7f) and empty zones, gated on sep and validate_host so non-IPv6 hosts and explicit validate_host=False callers are untouched. The error message is a static string that never reflects the offending zone, which the new test locks in via assert zone not in error. The % addition to _check_netloc() closes a real bypass (U+FF05 / U+FE6A NFKC-normalizing to % and bleeding through the stdlib IDNA fallback in _idna_encode) and only fires when NFKC has already mutated the netloc, so legitimate percent-encoded ASCII userinfo is unaffected. Test coverage for both fixes is thorough (CRLF, NUL, empty zone, spaces, parens, Unicode for the zone; U+FF05 and U+FE6A for the netloc bypass), the two CHANGES/1655.bugfix.*.rst fragments are correctly formatted with the right user attribution, and previous reviewer concerns (unused desc, RFC link freshness, control-char-only policy) have all been addressed. Merge-ready.


Automated review by Kōanf7aa293
979a271
ad025da
eb19850
f786fd3
24c6ebf
5b69d86
4e1b84a
bd210eb
06047a9

Comment thread yarl/_url.py Outdated
@bdraco

bdraco commented May 18, 2026

Copy link
Copy Markdown
Member

@aiolibsbot review

- Reword the error from "Invalid characters in IPv6 zone ID" to
  "Invalid characters in zone identifier"; the same check fires
  for inputs like 127.0.0.1%\x001 where the IPv6 wording is
  misleading. Validation behavior is unchanged.
- Add a round-trip assertion (URL(str(u)).host == ...) to
  test_url_build_ipv6_zone_id_valid so any future regression in
  serialization of zone IDs is caught.
- Add test_url_build_ipv6_zone_id_empty for the bare-trailing-%
  case and note the empty-zone rejection in CHANGES/1655.bugfix.1.rst.
@bdraco

bdraco commented May 26, 2026

Copy link
Copy Markdown
Member

Thanks @rodrigobnogueira

@bdraco bdraco merged commit 50ba229 into aio-libs:master May 26, 2026
56 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:chronographer:provided There is a change note present in this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants