From 9fff50c82e4fd6baa8252cb573d4a5f3df1a9dcb Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 7 Jun 2026 00:40:24 -0500 Subject: [PATCH] [PR #12824/60b85e98 backport][3.14] Preserve host-only cookie scope across CookieJar save/load (#12833) CVE: CVE-2026-54279 Upstream-Status: Backport [https://github.com/aio-libs/aiohttp/commit/a329a7aacad5284f087af36103aff778746da0f2] (cherry picked from commit a329a7aacad5284f087af36103aff778746da0f2) Signed-off-by: Darsh Kelaiya --- CHANGES/12824.bugfix.rst | 1 + aiohttp/cookiejar.py | 52 ++++++++++----- tests/test_cookiejar.py | 135 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 171 insertions(+), 17 deletions(-) create mode 100644 CHANGES/12824.bugfix.rst diff --git a/CHANGES/12824.bugfix.rst b/CHANGES/12824.bugfix.rst new file mode 100644 index 000000000..f8dbd169c --- /dev/null +++ b/CHANGES/12824.bugfix.rst @@ -0,0 +1 @@ +Fixed :class:`~aiohttp.CookieJar` dropping the host-only flag of cookies when persisted with :meth:`~aiohttp.CookieJar.save` and reloaded with :meth:`~aiohttp.CookieJar.load`, so a cookie set without a ``Domain`` attribute is again scoped to the exact host that set it after a reload; the absolute expiration deadline is now persisted as well, so a reloaded cookie keeps its original lifetime instead of being rescheduled from the load time. :meth:`~aiohttp.CookieJar.load` now replaces the jar contents rather than merging onto prior state, and loaded cookies pass through the same acceptance rules as :meth:`~aiohttp.CookieJar.update_cookies`, so a cookie for an IP-address host is dropped when loaded into a jar created without ``unsafe=True`` -- by :user:`bdraco`. diff --git a/aiohttp/cookiejar.py b/aiohttp/cookiejar.py index 757cc10e8..b07d1fd50 100644 --- a/aiohttp/cookiejar.py +++ b/aiohttp/cookiejar.py @@ -48,6 +48,9 @@ _FORMAT_DOMAIN_REVERSED = "{1}.{0}".format _MIN_SCHEDULED_COOKIE_EXPIRATION = 100 _SIMPLE_COOKIE = SimpleCookie() +# Not persisted; the absolute deadline is saved instead. +_RELATIVE_EXPIRY_ATTRS = frozenset(("max-age", "expires")) + class _RestrictedCookieUnpickler(pickle.Unpickler): """A restricted unpickler that only allows cookie-related types. @@ -165,21 +168,28 @@ class CookieJar(AbstractCookieJar): :class:`str` or :class:`pathlib.Path` instance. """ file_path = pathlib.Path(file_path) - data: dict[str, dict[str, dict[str, str | bool]]] = {} + data: dict[str, dict[str, dict[str, str | bool | float]]] = {} for (domain, path), cookie in self._cookies.items(): key = f"{domain}|{path}" data[key] = {} for name, morsel in cookie.items(): - morsel_data: dict[str, str | bool] = { + morsel_data: dict[str, str | bool | float] = { "key": morsel.key, "value": morsel.value, "coded_value": morsel.coded_value, } - # Save all morsel attributes that have values + # Skip relative expiry; the absolute deadline is saved below. for attr in morsel._reserved: # type: ignore[attr-defined] + if attr in _RELATIVE_EXPIRY_ATTRS: + continue attr_val = morsel[attr] if attr_val: morsel_data[attr] = attr_val + # Persist or it reloads as a domain cookie and leaks to subdomains. + if (domain, name) in self._host_only_cookies: + morsel_data["host_only"] = True + if (exp := self._expirations.get((domain, path, name))) is not None: + morsel_data["expires_timestamp"] = exp data[key][name] = morsel_data with file_path.open(mode="w", encoding="utf-8") as f: json.dump(data, f, indent=2) @@ -191,6 +201,9 @@ class CookieJar(AbstractCookieJar): pickle format (using a restricted unpickler) for backward compatibility with existing cookie files. + Replaces the current jar contents; loaded cookies pass through the + same acceptance rules as :meth:`update_cookies`. + :param file_path: Path to file from where cookies will be imported, :class:`str` or :class:`pathlib.Path` instance. """ @@ -199,32 +212,28 @@ class CookieJar(AbstractCookieJar): try: with file_path.open(mode="r", encoding="utf-8") as f: data = json.load(f) - self._cookies = self._load_json_data(data) + self._load_json_data(data) except (json.JSONDecodeError, UnicodeDecodeError, ValueError): # Fall back to legacy pickle format with restricted unpickler with file_path.open(mode="rb") as f: self._cookies = _RestrictedCookieUnpickler(f).load() def _load_json_data( - self, data: dict[str, dict[str, dict[str, str | bool]]] - ) -> defaultdict[tuple[str, str], SimpleCookie]: - """Load cookies from parsed JSON data.""" - cookies: defaultdict[tuple[str, str], SimpleCookie] = defaultdict(SimpleCookie) + self, data: dict[str, dict[str, dict[str, str | bool | float]]] + ) -> None: + """Replace contents, routing cookies through update_cookies().""" + self.clear() for compound_key, cookie_data in data.items(): domain, path = compound_key.split("|", 1) - key = (domain, path) for name, morsel_data in cookie_data.items(): morsel: Morsel[str] = Morsel() - morsel_key = morsel_data["key"] - morsel_value = morsel_data["value"] - morsel_coded_value = morsel_data["coded_value"] # Use __setstate__ to bypass validation, same pattern # used in _build_morsel and _cookie_helpers. morsel.__setstate__( # type: ignore[attr-defined] { - "key": morsel_key, - "value": morsel_value, - "coded_value": morsel_coded_value, + "key": morsel_data["key"], + "value": morsel_data["value"], + "coded_value": morsel_data["coded_value"], } ) # Restore morsel attributes @@ -235,8 +244,17 @@ class CookieJar(AbstractCookieJar): "coded_value", ): morsel[attr] = morsel_data[attr] - cookies[key][name] = morsel - return cookies + # Drop the domain so update_cookies() re-marks it host-only. + if morsel_data.get("host_only"): + morsel["domain"] = "" + response_url = ( + URL.build(scheme="https", host=domain) if domain else URL() + ) + self.update_cookies({name: morsel}, response_url) + # Restore the absolute deadline; update_cookies() schedules none. + if (exp := morsel_data.get("expires_timestamp")) is not None: + self._expire_cookie(float(exp), domain, path, name) + self._do_expiration() def clear(self, predicate: Optional[ClearCookiePredicate] = None) -> None: if predicate is None: diff --git a/tests/test_cookiejar.py b/tests/test_cookiejar.py index 694514067..ba7ee3220 100644 --- a/tests/test_cookiejar.py +++ b/tests/test_cookiejar.py @@ -2,6 +2,7 @@ import asyncio import datetime import heapq import itertools +import json import logging import pathlib import pickle @@ -1760,6 +1761,140 @@ async def test_save_load_json_partitioned_cookies(tmp_path: Path) -> None: assert s["path"] == lo["path"] +async def test_save_load_json_preserves_host_only_scope(tmp_path: Path) -> None: + """Verify save/load keeps host-only cookies off subdomains.""" + file_path = tmp_path / "host_only.json" + issuer = URL("https://auth.example.com/login") + subdomain = URL("https://sub.auth.example.com/") + + jar_save = CookieJar() + jar_save.update_cookies({"sid": "hostonly"}, response_url=issuer) + assert "sid" not in jar_save.filter_cookies(subdomain) + jar_save.save(file_path=file_path) + + jar_load = CookieJar() + jar_load.load(file_path=file_path) + + assert jar_load.host_only_cookies == frozenset({("auth.example.com", "sid")}) + assert "sid" not in jar_load.filter_cookies(subdomain) + assert "sid" in jar_load.filter_cookies(issuer) + + +async def test_save_load_json_domain_cookie_still_matches_subdomain( + tmp_path: Path, +) -> None: + """Verify save/load keeps an explicit Domain cookie valid for subdomains.""" + file_path = tmp_path / "domain.json" + subdomain = URL("https://sub.example.com/") + + jar_save = CookieJar() + jar_save.update_cookies_from_headers( + ["sid=domaincookie; Domain=example.com"], URL("https://example.com/") + ) + jar_save.save(file_path=file_path) + + jar_load = CookieJar() + jar_load.load(file_path=file_path) + + assert jar_load.host_only_cookies == frozenset() + assert "sid" in jar_load.filter_cookies(subdomain) + + +async def test_save_load_json_preserves_max_age_deadline(tmp_path: Path) -> None: + """Verify save/load restores the absolute deadline without resetting it.""" + file_path = tmp_path / "max_age.json" + url = URL("https://example.com/") + + jar_save = CookieJar() + jar_save.update_cookies_from_headers( + ["sid=x; Max-Age=3600; Domain=example.com"], url + ) + expirations = dict(jar_save._expirations) + jar_save.save(file_path=file_path) + + jar_load = CookieJar() + jar_load.load(file_path=file_path) + + # The deadline is restored as the original absolute time, not now + Max-Age. + assert dict(jar_load._expirations) == expirations + assert "sid" in jar_load.filter_cookies(url) + + +async def test_save_load_json_drops_expired_cookie(tmp_path: Path) -> None: + """Verify a cookie whose persisted deadline is in the past is dropped on load.""" + file_path = tmp_path / "expired.json" + url = URL("https://example.com/") + + # Save a future-expiring cookie, then rewrite its persisted deadline to the + # past so the cookie survives save() and the drop happens on the load path. + jar_save = CookieJar() + jar_save.update_cookies_from_headers( + ["sid=x; Expires=Tue, 1 Jan 2999 12:00:00 GMT; Domain=example.com"], url + ) + jar_save.save(file_path=file_path) + data = json.loads(file_path.read_text()) + _, cookies = next(iter(data.items())) + cookies["sid"]["expires_timestamp"] = 0.0 + file_path.write_text(json.dumps(data)) + + jar_load = CookieJar() + jar_load.load(file_path=file_path) + + assert len(jar_load) == 0 + assert "sid" not in jar_load.filter_cookies(url) + + +async def test_save_load_json_preserves_expires_deadline(tmp_path: Path) -> None: + """Verify a future Expires deadline survives a save/load roundtrip.""" + file_path = tmp_path / "expires.json" + url = URL("https://example.com/") + + jar_save = CookieJar() + jar_save.update_cookies_from_headers( + ["sid=x; Expires=Tue, 1 Jan 2999 12:00:00 GMT; Domain=example.com"], url + ) + expirations = dict(jar_save._expirations) + jar_save.save(file_path=file_path) + + jar_load = CookieJar() + jar_load.load(file_path=file_path) + + assert dict(jar_load._expirations) == expirations + assert "sid" in jar_load.filter_cookies(url) + + +async def test_load_json_old_format_without_new_keys(tmp_path: Path) -> None: + """Verify a file written by an older version (no host_only/expires_timestamp) loads.""" + file_path = tmp_path / "old.json" + # Old schema: no host_only, no expires_timestamp; relative max-age morsel attr. + file_path.write_text( + json.dumps( + { + "example.com|/": { + "sid": { + "key": "sid", + "value": "x", + "coded_value": "x", + "domain": "example.com", + "max-age": "3600", + } + } + } + ) + ) + url = URL("https://example.com/") + + jar_load = CookieJar() + # No exception when the new keys are absent. + jar_load.load(file_path=file_path) + + # A host-only cookie saved without Domain by an older version had no domain + # field, so it now loads as a domain cookie (the documented migration loss). + assert "sid" in jar_load.filter_cookies(url) + # max-age is rescheduled from load time rather than an absolute deadline. + assert any(key[2] == "sid" for key in jar_load._expirations) + + async def test_json_format_is_safe(tmp_path: Path) -> None: """Verify the JSON file format cannot execute code on load.""" import json