From 59bb3575b0cd41e3d8b7440857eb7634b24e2d33 Mon Sep 17 00:00:00 2001 From: Yuval E Date: Wed, 18 Feb 2026 17:52:58 +0200 Subject: [PATCH 01/13] Security: restrict pickle deserialization in CookieJar.load() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CookieJar.load() previously used pickle.load() without restrictions, allowing arbitrary code execution if an attacker could write to the cookie jar file (e.g. shared volumes, NFS, multi-tenant environments). This commit: - Adds _RestrictedCookieUnpickler that only allows cookie-related types (SimpleCookie, Morsel, defaultdict, and safe builtins) - Replaces pickle.load() with the restricted unpickler in load() - Adds safe JSON-based save_json()/load_json() alternatives - Adds security warnings to the documentation for save()/load() - Adds tests verifying malicious payloads are blocked while legitimate cookies continue to load correctly Existing pickle files with legitimate cookies are fully backward compatible — only payloads containing non-cookie types (os.system, eval, exec, subprocess, etc.) are rejected. Co-Authored-By: Claude Opus 4.6 --- aiohttp/cookiejar.py | 137 +++++++++++++++++++++++++++- docs/client_reference.rst | 40 +++++++- tests/test_cookiejar.py | 186 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 361 insertions(+), 2 deletions(-) diff --git a/aiohttp/cookiejar.py b/aiohttp/cookiejar.py index 792583938f2..8a3c8b5d328 100644 --- a/aiohttp/cookiejar.py +++ b/aiohttp/cookiejar.py @@ -3,6 +3,7 @@ import datetime import heapq import itertools +import json import os # noqa import pathlib import pickle @@ -37,6 +38,41 @@ _SIMPLE_COOKIE = SimpleCookie() +class _RestrictedCookieUnpickler(pickle.Unpickler): + """A restricted unpickler that only allows cookie-related types. + + This prevents arbitrary code execution when loading pickled cookie data + from untrusted sources. Only types that are expected in a serialized + CookieJar are permitted. + + See: https://docs.python.org/3/library/pickle.html#restricting-globals + """ + + _ALLOWED_CLASSES: frozenset[tuple[str, str]] = frozenset( + { + # Core cookie types + ("http.cookies", "SimpleCookie"), + ("http.cookies", "Morsel"), + # Container types used by CookieJar._cookies + ("collections", "defaultdict"), + # builtins that pickle uses for reconstruction + ("builtins", "tuple"), + ("builtins", "set"), + ("builtins", "frozenset"), + ("builtins", "dict"), + } + ) + + def find_class(self, module: str, name: str) -> type: + if (module, name) not in self._ALLOWED_CLASSES: + raise pickle.UnpicklingError( + f"Forbidden class: {module}.{name}. " + "CookieJar.load() only allows cookie-related types for security. " + "See https://docs.python.org/3/library/pickle.html#restricting-globals" + ) + return super().find_class(module, name) + + class CookieJar(AbstractCookieJar): """Implements cookie storage adhering to RFC 6265.""" @@ -117,9 +153,108 @@ def save(self, file_path: PathLike) -> None: pickle.dump(self._cookies, f, pickle.HIGHEST_PROTOCOL) def load(self, file_path: PathLike) -> None: + """Load cookies from a pickled file using a restricted unpickler. + + .. warning:: + + Cookie files loaded from untrusted sources could previously + execute arbitrary code. This method now uses a restricted + unpickler that only allows cookie-related types. + + For new code, consider using :meth:`save_json` and + :meth:`load_json` instead, which use a safe JSON format. + + :param file_path: Path to file from where cookies will be + imported, :class:`str` or :class:`pathlib.Path` instance. + """ file_path = pathlib.Path(file_path) with file_path.open(mode="rb") as f: - self._cookies = pickle.load(f) + self._cookies = _RestrictedCookieUnpickler(f).load() + + def save_json(self, file_path: PathLike) -> None: + """Save cookies to a JSON file (safe alternative to :meth:`save`). + + This method serializes cookies using JSON, which is inherently safe + against deserialization attacks unlike the pickle-based :meth:`save`. + + :param file_path: Path to file where cookies will be serialized, + :class:`str` or :class:`pathlib.Path` instance. + + .. versionadded:: 3.14 + """ + file_path = pathlib.Path(file_path) + data: dict[str, dict[str, dict[str, str]]] = {} + 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] = { + "key": morsel.key, + "value": morsel.value, + "coded_value": morsel.coded_value, + } + # Save all morsel attributes that have values + for attr in morsel._reserved: + attr_val = morsel[attr] + if attr_val: + if isinstance(attr_val, bool): + morsel_data[attr] = "true" + else: + morsel_data[attr] = str(attr_val) + data[key][name] = morsel_data + with file_path.open(mode="w", encoding="utf-8") as f: + json.dump(data, f, indent=2) + + def load_json(self, file_path: PathLike) -> None: + """Load cookies from a JSON file (safe alternative to :meth:`load`). + + This method deserializes cookies from JSON format, which is inherently + safe against code execution attacks. + + :param file_path: Path to file from where cookies will be imported, + :class:`str` or :class:`pathlib.Path` instance. + + .. versionadded:: 3.14 + """ + file_path = pathlib.Path(file_path) + with file_path.open(mode="r", encoding="utf-8") as f: + data = json.load(f) + cookies: defaultdict[tuple[str, str], SimpleCookie] = defaultdict( + SimpleCookie + ) + for compound_key, cookie_data in data.items(): + parts = compound_key.split("|", 1) + domain = parts[0] + path = parts[1] if len(parts) > 1 else "" + key = (domain, path) + for name, morsel_data in cookie_data.items(): + morsel: Morsel[str] = Morsel() + morsel_key = morsel_data.get("key", name) + morsel_value = morsel_data.get("value", "") + morsel_coded_value = morsel_data.get("coded_value", morsel_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, + } + ) + # Restore morsel attributes + for attr in morsel._reserved: + if attr in morsel_data and attr not in ( + "key", + "value", + "coded_value", + ): + attr_val = morsel_data[attr] + if attr in ("secure", "httponly"): + morsel[attr] = True if attr_val == "true" else attr_val # type: ignore[assignment] + else: + morsel[attr] = attr_val + cookies[key][name] = morsel + self._cookies = cookies def clear(self, predicate: ClearCookiePredicate | None = None) -> None: if predicate is None: diff --git a/docs/client_reference.rst b/docs/client_reference.rst index a4018141519..172554f751d 100644 --- a/docs/client_reference.rst +++ b/docs/client_reference.rst @@ -2459,13 +2459,51 @@ Utilities Write a pickled representation of cookies into the file at provided path. + .. warning:: + + The pickle format is kept for backward compatibility but is + inherently unsafe if the cookie file can be modified by an + untrusted party (e.g. shared volumes, NFS mounts, or + multi-tenant environments). Consider using :meth:`save_json` + instead for new code. + :param file_path: Path to file where cookies will be serialized, :class:`str` or :class:`pathlib.Path` instance. .. method:: load(file_path) Load a pickled representation of cookies from the file - at provided path. + at provided path. This method uses a restricted unpickler that + only allows cookie-related types, preventing arbitrary code + execution from malicious pickle payloads. + + .. warning:: + + While :meth:`load` now uses a restricted unpickler for + defense in depth, the pickle format is inherently risky + when loading files from untrusted sources. Consider using + :meth:`load_json` instead for new code. + + :param file_path: Path to file from where cookies will be + imported, :class:`str` or :class:`pathlib.Path` instance. + + .. method:: save_json(file_path) + + Write a JSON representation of cookies into the file at the + provided path. This is a safe alternative to :meth:`save` that + uses a format immune to deserialization attacks. + + .. versionadded:: 3.14 + + :param file_path: Path to file where cookies will be serialized, + :class:`str` or :class:`pathlib.Path` instance. + + .. method:: load_json(file_path) + + Load a JSON representation of cookies from the file at the + provided path. This is a safe alternative to :meth:`load`. + + .. versionadded:: 3.14 :param file_path: Path to file from where cookies will be imported, :class:`str` or :class:`pathlib.Path` instance. diff --git a/tests/test_cookiejar.py b/tests/test_cookiejar.py index 9ca7bacecaf..1d2056b9c9b 100644 --- a/tests/test_cookiejar.py +++ b/tests/test_cookiejar.py @@ -1564,3 +1564,189 @@ async def test_shared_cookie_with_multiple_domains() -> None: # Verify cache is reused efficiently assert ("", "") in jar._morsel_cache assert "universal" in jar._morsel_cache[("", "")] + + +# === Security tests for restricted unpickler and JSON save/load === + + +def test_load_rejects_malicious_pickle(tmp_path: Path) -> None: + """Verify CookieJar.load() blocks arbitrary code execution via pickle. + + A crafted pickle payload using os.system (or any non-cookie class) + must be rejected by the restricted unpickler. + """ + import os + + file_path = tmp_path / "malicious.pkl" + + class RCEPayload: + def __reduce__(self): # type: ignore[override] + return (os.system, ("echo PWNED",)) + + with open(file_path, "wb") as f: + pickle.dump(RCEPayload(), f, pickle.HIGHEST_PROTOCOL) + + jar = CookieJar() + with pytest.raises(pickle.UnpicklingError, match="Forbidden class"): + jar.load(file_path) + + +def test_load_rejects_eval_payload(tmp_path: Path) -> None: + """Verify CookieJar.load() blocks eval-based pickle payloads.""" + file_path = tmp_path / "eval_payload.pkl" + + class EvalPayload: + def __reduce__(self): # type: ignore[override] + return (eval, ("__import__('os').system('echo PWNED')",)) + + with open(file_path, "wb") as f: + pickle.dump(EvalPayload(), f, pickle.HIGHEST_PROTOCOL) + + jar = CookieJar() + with pytest.raises(pickle.UnpicklingError, match="Forbidden class"): + jar.load(file_path) + + +def test_load_rejects_subprocess_payload(tmp_path: Path) -> None: + """Verify CookieJar.load() blocks subprocess-based pickle payloads.""" + import subprocess + + file_path = tmp_path / "subprocess_payload.pkl" + + class SubprocessPayload: + def __reduce__(self): # type: ignore[override] + return (subprocess.call, (["echo", "PWNED"],)) + + with open(file_path, "wb") as f: + pickle.dump(SubprocessPayload(), f, pickle.HIGHEST_PROTOCOL) + + jar = CookieJar() + with pytest.raises(pickle.UnpicklingError, match="Forbidden class"): + jar.load(file_path) + + +def test_load_still_works_with_legitimate_cookies( + tmp_path: Path, + cookies_to_send: SimpleCookie, + cookies_to_receive: SimpleCookie, +) -> None: + """Verify CookieJar.load() still works with legitimate pickled cookies. + + The restricted unpickler must allow SimpleCookie, Morsel, defaultdict, + etc. — all the types that legitimate cookie data uses. + """ + file_path = tmp_path / "legit.pkl" + + jar_save = CookieJar() + jar_save.update_cookies(cookies_to_receive) + jar_save.save(file_path=file_path) + + jar_load = CookieJar() + jar_load.load(file_path=file_path) + + jar_test = SimpleCookie() + for cookie in jar_load: + jar_test[cookie.key] = cookie + + assert jar_test == cookies_to_receive + + +def test_save_load_json_roundtrip( + tmp_path: Path, + cookies_to_receive: SimpleCookie, +) -> None: + """Verify save_json/load_json roundtrip preserves cookies.""" + file_path = tmp_path / "cookies.json" + + jar_save = CookieJar() + jar_save.update_cookies(cookies_to_receive) + jar_save.save_json(file_path=file_path) + + jar_load = CookieJar() + jar_load.load_json(file_path=file_path) + + saved_cookies = SimpleCookie() + for cookie in jar_save: + saved_cookies[cookie.key] = cookie + + loaded_cookies = SimpleCookie() + for cookie in jar_load: + loaded_cookies[cookie.key] = cookie + + assert saved_cookies == loaded_cookies + + +def test_save_load_json_partitioned_cookies(tmp_path: Path) -> None: + """Verify save_json/load_json roundtrip works with partitioned cookies.""" + file_path = tmp_path / "partitioned.json" + + jar_save = CookieJar() + jar_save.update_cookies_from_headers( + ["session=cookie; Partitioned"], URL("https://example.com/") + ) + jar_save.save_json(file_path=file_path) + + jar_load = CookieJar() + jar_load.load_json(file_path=file_path) + + saved_cookies = SimpleCookie() + for cookie in jar_save: + saved_cookies[cookie.key] = cookie + + loaded_cookies = SimpleCookie() + for cookie in jar_load: + loaded_cookies[cookie.key] = cookie + + assert saved_cookies == loaded_cookies + + +def test_json_format_is_safe(tmp_path: Path) -> None: + """Verify the JSON file format cannot execute code on load.""" + import json + + file_path = tmp_path / "safe.json" + + # Write something that might look dangerous but is just data + malicious_data = { + "evil.com|/": { + "session": { + "key": "session", + "value": "__import__('os').system('echo PWNED')", + "coded_value": "__import__('os').system('echo PWNED')", + } + } + } + with open(file_path, "w") as f: + json.dump(malicious_data, f) + + jar = CookieJar() + jar.load_json(file_path=file_path) + + # The "malicious" string is just a cookie value, not executed code + cookies = list(jar) + assert len(cookies) == 1 + assert cookies[0].value == "__import__('os').system('echo PWNED')" + + +def test_save_load_json_secure_cookies(tmp_path: Path) -> None: + """Verify save_json/load_json preserves Secure and HttpOnly flags.""" + file_path = tmp_path / "secure.json" + + jar_save = CookieJar() + jar_save.update_cookies_from_headers( + ["token=abc123; Secure; HttpOnly; Path=/; Domain=example.com"], + URL("https://example.com/"), + ) + jar_save.save_json(file_path=file_path) + + jar_load = CookieJar() + jar_load.load_json(file_path=file_path) + + loaded_cookies = list(jar_load) + assert len(loaded_cookies) == 1 + cookie = loaded_cookies[0] + assert cookie.key == "token" + assert cookie.value == "abc123" + assert cookie["secure"] is True + assert cookie["httponly"] is True + assert cookie["domain"] == "example.com" From 2c899ef56b1f31a22ae6501000dc2e01ec170b94 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 18 Feb 2026 15:54:34 +0000 Subject: [PATCH 02/13] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- aiohttp/cookiejar.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/aiohttp/cookiejar.py b/aiohttp/cookiejar.py index 8a3c8b5d328..8f52cbcec52 100644 --- a/aiohttp/cookiejar.py +++ b/aiohttp/cookiejar.py @@ -219,9 +219,7 @@ def load_json(self, file_path: PathLike) -> None: file_path = pathlib.Path(file_path) with file_path.open(mode="r", encoding="utf-8") as f: data = json.load(f) - cookies: defaultdict[tuple[str, str], SimpleCookie] = defaultdict( - SimpleCookie - ) + cookies: defaultdict[tuple[str, str], SimpleCookie] = defaultdict(SimpleCookie) for compound_key, cookie_data in data.items(): parts = compound_key.split("|", 1) domain = parts[0] From 81d06a0bca8b3869dced3b69b6d40a1266e1833f Mon Sep 17 00:00:00 2001 From: Yuval E Date: Wed, 18 Feb 2026 18:03:28 +0200 Subject: [PATCH 03/13] Fix partitioned cookie handling in load_json() Add "partitioned" to the boolean attribute check in load_json() so it is restored as True (bool) rather than "true" (string) on Python 3.14+ where Morsel._reserved includes partitioned. Co-Authored-By: Claude Opus 4.6 --- aiohttp/cookiejar.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aiohttp/cookiejar.py b/aiohttp/cookiejar.py index 8f52cbcec52..bb527210b6f 100644 --- a/aiohttp/cookiejar.py +++ b/aiohttp/cookiejar.py @@ -247,7 +247,7 @@ def load_json(self, file_path: PathLike) -> None: "coded_value", ): attr_val = morsel_data[attr] - if attr in ("secure", "httponly"): + if attr in ("secure", "httponly", "partitioned"): morsel[attr] = True if attr_val == "true" else attr_val # type: ignore[assignment] else: morsel[attr] = attr_val From 146dd8915af5a5eae6f861fe24ac91440d7239d5 Mon Sep 17 00:00:00 2001 From: Yuval E Date: Wed, 18 Feb 2026 18:07:19 +0200 Subject: [PATCH 04/13] Fix mypy errors and partitioned cookie test - Add type: ignore[no-any-return] on _RestrictedCookieUnpickler.find_class - Add type: ignore[attr-defined] on Morsel._reserved access - Remove unused type: ignore[assignment] in load_json - Fix __reduce__ return type annotations in test payloads - Fix partitioned cookie test to compare attributes directly instead of via SimpleCookie equality (avoids Morsel re-encoding) Co-Authored-By: Claude Opus 4.6 --- aiohttp/cookiejar.py | 8 ++++---- tests/test_cookiejar.py | 24 ++++++++++++------------ 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/aiohttp/cookiejar.py b/aiohttp/cookiejar.py index bb527210b6f..9db09c6a301 100644 --- a/aiohttp/cookiejar.py +++ b/aiohttp/cookiejar.py @@ -70,7 +70,7 @@ def find_class(self, module: str, name: str) -> type: "CookieJar.load() only allows cookie-related types for security. " "See https://docs.python.org/3/library/pickle.html#restricting-globals" ) - return super().find_class(module, name) + return super().find_class(module, name) # type: ignore[no-any-return] class CookieJar(AbstractCookieJar): @@ -194,7 +194,7 @@ def save_json(self, file_path: PathLike) -> None: "coded_value": morsel.coded_value, } # Save all morsel attributes that have values - for attr in morsel._reserved: + for attr in morsel._reserved: # type: ignore[attr-defined] attr_val = morsel[attr] if attr_val: if isinstance(attr_val, bool): @@ -240,7 +240,7 @@ def load_json(self, file_path: PathLike) -> None: } ) # Restore morsel attributes - for attr in morsel._reserved: + for attr in morsel._reserved: # type: ignore[attr-defined] if attr in morsel_data and attr not in ( "key", "value", @@ -248,7 +248,7 @@ def load_json(self, file_path: PathLike) -> None: ): attr_val = morsel_data[attr] if attr in ("secure", "httponly", "partitioned"): - morsel[attr] = True if attr_val == "true" else attr_val # type: ignore[assignment] + morsel[attr] = True if attr_val == "true" else attr_val else: morsel[attr] = attr_val cookies[key][name] = morsel diff --git a/tests/test_cookiejar.py b/tests/test_cookiejar.py index 1d2056b9c9b..91591bafe9d 100644 --- a/tests/test_cookiejar.py +++ b/tests/test_cookiejar.py @@ -1580,7 +1580,7 @@ def test_load_rejects_malicious_pickle(tmp_path: Path) -> None: file_path = tmp_path / "malicious.pkl" class RCEPayload: - def __reduce__(self): # type: ignore[override] + def __reduce__(self) -> tuple[object, ...]: return (os.system, ("echo PWNED",)) with open(file_path, "wb") as f: @@ -1596,7 +1596,7 @@ def test_load_rejects_eval_payload(tmp_path: Path) -> None: file_path = tmp_path / "eval_payload.pkl" class EvalPayload: - def __reduce__(self): # type: ignore[override] + def __reduce__(self) -> tuple[object, ...]: return (eval, ("__import__('os').system('echo PWNED')",)) with open(file_path, "wb") as f: @@ -1614,7 +1614,7 @@ def test_load_rejects_subprocess_payload(tmp_path: Path) -> None: file_path = tmp_path / "subprocess_payload.pkl" class SubprocessPayload: - def __reduce__(self): # type: ignore[override] + def __reduce__(self) -> tuple[object, ...]: return (subprocess.call, (["echo", "PWNED"],)) with open(file_path, "wb") as f: @@ -1689,15 +1689,15 @@ def test_save_load_json_partitioned_cookies(tmp_path: Path) -> None: jar_load = CookieJar() jar_load.load_json(file_path=file_path) - saved_cookies = SimpleCookie() - for cookie in jar_save: - saved_cookies[cookie.key] = cookie - - loaded_cookies = SimpleCookie() - for cookie in jar_load: - loaded_cookies[cookie.key] = cookie - - assert saved_cookies == loaded_cookies + # Compare individual cookie values (same approach as test_save_load_partitioned_cookies) + saved = list(jar_save) + loaded = list(jar_load) + assert len(saved) == len(loaded) + for s, lo in zip(saved, loaded): + assert s.key == lo.key + assert s.value == lo.value + assert s["domain"] == lo["domain"] + assert s["path"] == lo["path"] def test_json_format_is_safe(tmp_path: Path) -> None: From c05f5941969eaa41a12311be89f030560d4d68db Mon Sep 17 00:00:00 2001 From: Yuval E Date: Wed, 18 Feb 2026 18:11:45 +0200 Subject: [PATCH 05/13] Add new technical terms to docs spelling wordlist Add deserialization, NFS, payloads, unpickler to the spelling wordlist for the new CookieJar security documentation. Co-Authored-By: Claude Opus 4.6 --- docs/spelling_wordlist.txt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/spelling_wordlist.txt b/docs/spelling_wordlist.txt index 4e7bab6968c..d390d9bda36 100644 --- a/docs/spelling_wordlist.txt +++ b/docs/spelling_wordlist.txt @@ -100,6 +100,7 @@ deduplicate defs Dependabot deprecations +deserialization DER dev Dev @@ -213,7 +214,8 @@ Multipart musllinux mypy Nagle -Nagle’s +Nagle's +NFS namedtuple nameservers namespace @@ -235,6 +237,7 @@ param params parsers pathlib +payloads peername performant pickleable @@ -355,6 +358,7 @@ unhandled unicode unittest Unittest +unpickler unix unobvious unsets From c447986ad174947193b11046d0ab603c65a6b6f4 Mon Sep 17 00:00:00 2001 From: Yuval E Date: Wed, 18 Feb 2026 18:12:29 +0200 Subject: [PATCH 06/13] Add 'untrusted' to docs spelling wordlist Co-Authored-By: Claude Opus 4.6 --- docs/spelling_wordlist.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/spelling_wordlist.txt b/docs/spelling_wordlist.txt index d390d9bda36..82e1e65487d 100644 --- a/docs/spelling_wordlist.txt +++ b/docs/spelling_wordlist.txt @@ -359,6 +359,7 @@ unicode unittest Unittest unpickler +untrusted unix unobvious unsets From 4928513e086691a2507e282274d990aaa11300b0 Mon Sep 17 00:00:00 2001 From: Yuval E Date: Wed, 18 Feb 2026 18:16:17 +0200 Subject: [PATCH 07/13] Add towncrier changelog fragment for CookieJar security fix Co-Authored-By: Claude Opus 4.6 --- CHANGES/12091.bugfix.rst | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 CHANGES/12091.bugfix.rst diff --git a/CHANGES/12091.bugfix.rst b/CHANGES/12091.bugfix.rst new file mode 100644 index 00000000000..3decd844333 --- /dev/null +++ b/CHANGES/12091.bugfix.rst @@ -0,0 +1,6 @@ +Added a restricted unpickler to :py:meth:`~aiohttp.CookieJar.load` that only allows +cookie-related types (``SimpleCookie``, ``Morsel``, ``defaultdict``, etc.) to be +deserialized, preventing arbitrary code execution via malicious pickle payloads +(CWE-502). Also added :py:meth:`~aiohttp.CookieJar.save_json` and +:py:meth:`~aiohttp.CookieJar.load_json` as safe JSON-based alternatives for +cookie persistence -- by :user:`YuvalElbar6`. From 86177df48328aeef30d8a8cc9fe477a980029b43 Mon Sep 17 00:00:00 2001 From: Yuval E Date: Wed, 18 Feb 2026 18:25:31 +0200 Subject: [PATCH 08/13] Fix spelling in changelog fragment: deserialized -> loaded Co-Authored-By: Claude Opus 4.6 --- CHANGES/12091.bugfix.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES/12091.bugfix.rst b/CHANGES/12091.bugfix.rst index 3decd844333..d316ab76d2b 100644 --- a/CHANGES/12091.bugfix.rst +++ b/CHANGES/12091.bugfix.rst @@ -1,6 +1,6 @@ Added a restricted unpickler to :py:meth:`~aiohttp.CookieJar.load` that only allows cookie-related types (``SimpleCookie``, ``Morsel``, ``defaultdict``, etc.) to be -deserialized, preventing arbitrary code execution via malicious pickle payloads +loaded, preventing arbitrary code execution via malicious pickle payloads (CWE-502). Also added :py:meth:`~aiohttp.CookieJar.save_json` and :py:meth:`~aiohttp.CookieJar.load_json` as safe JSON-based alternatives for cookie persistence -- by :user:`YuvalElbar6`. From cfa864d158d91902cae0677da6c34a7ab278f71b Mon Sep 17 00:00:00 2001 From: Yuval E Date: Thu, 19 Feb 2026 23:45:57 +0200 Subject: [PATCH 09/13] Merge save_json/load_json into save/load per review save() now writes JSON format directly. load() tries JSON first and falls back to restricted pickle for backward compatibility with existing cookie files. Co-Authored-By: Claude Opus 4.6 --- CHANGES/12091.bugfix.rst | 12 ++++---- aiohttp/cookiejar.py | 62 ++++++++++++++------------------------- docs/client_reference.rst | 48 +++++++----------------------- tests/test_cookiejar.py | 34 +++++++++++---------- 4 files changed, 57 insertions(+), 99 deletions(-) diff --git a/CHANGES/12091.bugfix.rst b/CHANGES/12091.bugfix.rst index d316ab76d2b..45ffbc3557f 100644 --- a/CHANGES/12091.bugfix.rst +++ b/CHANGES/12091.bugfix.rst @@ -1,6 +1,6 @@ -Added a restricted unpickler to :py:meth:`~aiohttp.CookieJar.load` that only allows -cookie-related types (``SimpleCookie``, ``Morsel``, ``defaultdict``, etc.) to be -loaded, preventing arbitrary code execution via malicious pickle payloads -(CWE-502). Also added :py:meth:`~aiohttp.CookieJar.save_json` and -:py:meth:`~aiohttp.CookieJar.load_json` as safe JSON-based alternatives for -cookie persistence -- by :user:`YuvalElbar6`. +Switched :py:meth:`~aiohttp.CookieJar.save` to use JSON format and +:py:meth:`~aiohttp.CookieJar.load` to try JSON first with a fallback to +a restricted pickle unpickler that only allows cookie-related types +(``SimpleCookie``, ``Morsel``, ``defaultdict``, etc.), preventing +arbitrary code execution via malicious pickle payloads +(CWE-502) -- by :user:`YuvalElbar6`. diff --git a/aiohttp/cookiejar.py b/aiohttp/cookiejar.py index 9db09c6a301..1d1b1d1ffdd 100644 --- a/aiohttp/cookiejar.py +++ b/aiohttp/cookiejar.py @@ -148,39 +148,10 @@ def quote_cookie(self) -> bool: return self._quote_cookie def save(self, file_path: PathLike) -> None: - file_path = pathlib.Path(file_path) - with file_path.open(mode="wb") as f: - pickle.dump(self._cookies, f, pickle.HIGHEST_PROTOCOL) - - def load(self, file_path: PathLike) -> None: - """Load cookies from a pickled file using a restricted unpickler. - - .. warning:: - - Cookie files loaded from untrusted sources could previously - execute arbitrary code. This method now uses a restricted - unpickler that only allows cookie-related types. - - For new code, consider using :meth:`save_json` and - :meth:`load_json` instead, which use a safe JSON format. - - :param file_path: Path to file from where cookies will be - imported, :class:`str` or :class:`pathlib.Path` instance. - """ - file_path = pathlib.Path(file_path) - with file_path.open(mode="rb") as f: - self._cookies = _RestrictedCookieUnpickler(f).load() - - def save_json(self, file_path: PathLike) -> None: - """Save cookies to a JSON file (safe alternative to :meth:`save`). - - This method serializes cookies using JSON, which is inherently safe - against deserialization attacks unlike the pickle-based :meth:`save`. + """Save cookies to a file using JSON format. :param file_path: Path to file where cookies will be serialized, :class:`str` or :class:`pathlib.Path` instance. - - .. versionadded:: 3.14 """ file_path = pathlib.Path(file_path) data: dict[str, dict[str, dict[str, str]]] = {} @@ -205,20 +176,31 @@ def save_json(self, file_path: PathLike) -> None: with file_path.open(mode="w", encoding="utf-8") as f: json.dump(data, f, indent=2) - def load_json(self, file_path: PathLike) -> None: - """Load cookies from a JSON file (safe alternative to :meth:`load`). - - This method deserializes cookies from JSON format, which is inherently - safe against code execution attacks. + def load(self, file_path: PathLike) -> None: + """Load cookies from a file. - :param file_path: Path to file from where cookies will be imported, - :class:`str` or :class:`pathlib.Path` instance. + Tries to load JSON format first. Falls back to loading legacy + pickle format (using a restricted unpickler) for backward + compatibility with existing cookie files. - .. versionadded:: 3.14 + :param file_path: Path to file from where cookies will be + imported, :class:`str` or :class:`pathlib.Path` instance. """ file_path = pathlib.Path(file_path) - with file_path.open(mode="r", encoding="utf-8") as f: - data = json.load(f) + # Try JSON format first + try: + with file_path.open(mode="r", encoding="utf-8") as f: + data = json.load(f) + self._load_json_data(data) + return + except (json.JSONDecodeError, UnicodeDecodeError, ValueError): + pass + # 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]]]) -> None: + """Load cookies from parsed JSON data.""" cookies: defaultdict[tuple[str, str], SimpleCookie] = defaultdict(SimpleCookie) for compound_key, cookie_data in data.items(): parts = compound_key.split("|", 1) diff --git a/docs/client_reference.rst b/docs/client_reference.rst index 172554f751d..842710674d3 100644 --- a/docs/client_reference.rst +++ b/docs/client_reference.rst @@ -2456,54 +2456,28 @@ Utilities .. method:: save(file_path) - Write a pickled representation of cookies into the file + Write a JSON representation of cookies into the file at provided path. - .. warning:: + .. versionchanged:: 3.14 - The pickle format is kept for backward compatibility but is - inherently unsafe if the cookie file can be modified by an - untrusted party (e.g. shared volumes, NFS mounts, or - multi-tenant environments). Consider using :meth:`save_json` - instead for new code. + Previously used pickle format. Now uses JSON for safe + serialization. :param file_path: Path to file where cookies will be serialized, :class:`str` or :class:`pathlib.Path` instance. .. method:: load(file_path) - Load a pickled representation of cookies from the file - at provided path. This method uses a restricted unpickler that - only allows cookie-related types, preventing arbitrary code - execution from malicious pickle payloads. - - .. warning:: - - While :meth:`load` now uses a restricted unpickler for - defense in depth, the pickle format is inherently risky - when loading files from untrusted sources. Consider using - :meth:`load_json` instead for new code. - - :param file_path: Path to file from where cookies will be - imported, :class:`str` or :class:`pathlib.Path` instance. - - .. method:: save_json(file_path) - - Write a JSON representation of cookies into the file at the - provided path. This is a safe alternative to :meth:`save` that - uses a format immune to deserialization attacks. - - .. versionadded:: 3.14 - - :param file_path: Path to file where cookies will be serialized, - :class:`str` or :class:`pathlib.Path` instance. - - .. method:: load_json(file_path) + Load cookies from the file at provided path. Tries JSON format + first, then falls back to legacy pickle format (using a restricted + unpickler that only allows cookie-related types) for backward + compatibility with existing cookie files. - Load a JSON representation of cookies from the file at the - provided path. This is a safe alternative to :meth:`load`. + .. versionchanged:: 3.14 - .. versionadded:: 3.14 + Now loads JSON format by default. Falls back to restricted + pickle for files saved by older versions. :param file_path: Path to file from where cookies will be imported, :class:`str` or :class:`pathlib.Path` instance. diff --git a/tests/test_cookiejar.py b/tests/test_cookiejar.py index 91591bafe9d..ed69355a69a 100644 --- a/tests/test_cookiejar.py +++ b/tests/test_cookiejar.py @@ -1625,21 +1625,23 @@ def __reduce__(self) -> tuple[object, ...]: jar.load(file_path) -def test_load_still_works_with_legitimate_cookies( +def test_load_falls_back_to_pickle( tmp_path: Path, - cookies_to_send: SimpleCookie, cookies_to_receive: SimpleCookie, ) -> None: - """Verify CookieJar.load() still works with legitimate pickled cookies. + """Verify load() falls back to restricted pickle for legacy cookie files. - The restricted unpickler must allow SimpleCookie, Morsel, defaultdict, - etc. — all the types that legitimate cookie data uses. + Existing cookie files saved with older versions of aiohttp used pickle. + load() should detect that the file is not JSON and fall back to the + restricted pickle unpickler for backward compatibility. """ file_path = tmp_path / "legit.pkl" + # Write a legacy pickle file directly (as old aiohttp save() would) jar_save = CookieJar() jar_save.update_cookies(cookies_to_receive) - jar_save.save(file_path=file_path) + with file_path.open(mode="wb") as f: + pickle.dump(jar_save._cookies, f, pickle.HIGHEST_PROTOCOL) jar_load = CookieJar() jar_load.load(file_path=file_path) @@ -1655,15 +1657,15 @@ def test_save_load_json_roundtrip( tmp_path: Path, cookies_to_receive: SimpleCookie, ) -> None: - """Verify save_json/load_json roundtrip preserves cookies.""" + """Verify save/load roundtrip preserves cookies via JSON format.""" file_path = tmp_path / "cookies.json" jar_save = CookieJar() jar_save.update_cookies(cookies_to_receive) - jar_save.save_json(file_path=file_path) + jar_save.save(file_path=file_path) jar_load = CookieJar() - jar_load.load_json(file_path=file_path) + jar_load.load(file_path=file_path) saved_cookies = SimpleCookie() for cookie in jar_save: @@ -1677,17 +1679,17 @@ def test_save_load_json_roundtrip( def test_save_load_json_partitioned_cookies(tmp_path: Path) -> None: - """Verify save_json/load_json roundtrip works with partitioned cookies.""" + """Verify save/load roundtrip works with partitioned cookies.""" file_path = tmp_path / "partitioned.json" jar_save = CookieJar() jar_save.update_cookies_from_headers( ["session=cookie; Partitioned"], URL("https://example.com/") ) - jar_save.save_json(file_path=file_path) + jar_save.save(file_path=file_path) jar_load = CookieJar() - jar_load.load_json(file_path=file_path) + jar_load.load(file_path=file_path) # Compare individual cookie values (same approach as test_save_load_partitioned_cookies) saved = list(jar_save) @@ -1720,7 +1722,7 @@ def test_json_format_is_safe(tmp_path: Path) -> None: json.dump(malicious_data, f) jar = CookieJar() - jar.load_json(file_path=file_path) + jar.load(file_path=file_path) # The "malicious" string is just a cookie value, not executed code cookies = list(jar) @@ -1729,7 +1731,7 @@ def test_json_format_is_safe(tmp_path: Path) -> None: def test_save_load_json_secure_cookies(tmp_path: Path) -> None: - """Verify save_json/load_json preserves Secure and HttpOnly flags.""" + """Verify save/load preserves Secure and HttpOnly flags.""" file_path = tmp_path / "secure.json" jar_save = CookieJar() @@ -1737,10 +1739,10 @@ def test_save_load_json_secure_cookies(tmp_path: Path) -> None: ["token=abc123; Secure; HttpOnly; Path=/; Domain=example.com"], URL("https://example.com/"), ) - jar_save.save_json(file_path=file_path) + jar_save.save(file_path=file_path) jar_load = CookieJar() - jar_load.load_json(file_path=file_path) + jar_load.load(file_path=file_path) loaded_cookies = list(jar_load) assert len(loaded_cookies) == 1 From a7dd300feb79aa07ad7add991c15123e2785b9d3 Mon Sep 17 00:00:00 2001 From: Yuval Elbar <41901908+YuvalElbar6@users.noreply.github.com> Date: Fri, 20 Feb 2026 00:53:57 +0200 Subject: [PATCH 10/13] Update aiohttp/cookiejar.py Co-authored-by: Sam Bull --- aiohttp/cookiejar.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aiohttp/cookiejar.py b/aiohttp/cookiejar.py index 1d1b1d1ffdd..b1a8a2d720d 100644 --- a/aiohttp/cookiejar.py +++ b/aiohttp/cookiejar.py @@ -191,7 +191,7 @@ def load(self, file_path: PathLike) -> None: try: with file_path.open(mode="r", encoding="utf-8") as f: data = json.load(f) - self._load_json_data(data) + self._cookies = self._load_json_data(data) return except (json.JSONDecodeError, UnicodeDecodeError, ValueError): pass From 6a233431ee8fc2fbe7e852316c41ad2aae12d17b Mon Sep 17 00:00:00 2001 From: Yuval E Date: Fri, 20 Feb 2026 00:59:02 +0200 Subject: [PATCH 11/13] Address review: remove defensive defaults, str(), fix return pattern - Remove str() conversion for morsel attributes (already strings) - _load_json_data() returns cookies instead of assigning internally - Remove defensive else/defaults in JSON parsing (let invalid data fail) - Use tuple unpacking for domain|path split Co-Authored-By: Claude Opus 4.6 --- aiohttp/cookiejar.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/aiohttp/cookiejar.py b/aiohttp/cookiejar.py index b1a8a2d720d..18b60566952 100644 --- a/aiohttp/cookiejar.py +++ b/aiohttp/cookiejar.py @@ -171,7 +171,7 @@ def save(self, file_path: PathLike) -> None: if isinstance(attr_val, bool): morsel_data[attr] = "true" else: - morsel_data[attr] = str(attr_val) + morsel_data[attr] = attr_val data[key][name] = morsel_data with file_path.open(mode="w", encoding="utf-8") as f: json.dump(data, f, indent=2) @@ -199,19 +199,19 @@ def load(self, file_path: PathLike) -> None: 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]]]) -> None: + def _load_json_data( + self, data: dict[str, dict[str, dict[str, str]]] + ) -> defaultdict[tuple[str, str], SimpleCookie]: """Load cookies from parsed JSON data.""" cookies: defaultdict[tuple[str, str], SimpleCookie] = defaultdict(SimpleCookie) for compound_key, cookie_data in data.items(): - parts = compound_key.split("|", 1) - domain = parts[0] - path = parts[1] if len(parts) > 1 else "" + 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.get("key", name) - morsel_value = morsel_data.get("value", "") - morsel_coded_value = morsel_data.get("coded_value", morsel_value) + 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] @@ -234,7 +234,7 @@ def _load_json_data(self, data: dict[str, dict[str, dict[str, str]]]) -> None: else: morsel[attr] = attr_val cookies[key][name] = morsel - self._cookies = cookies + return cookies def clear(self, predicate: ClearCookiePredicate | None = None) -> None: if predicate is None: From 34b7c90bacf1376ede177e7b8fbc9b8784d8501c Mon Sep 17 00:00:00 2001 From: Yuval E Date: Fri, 20 Feb 2026 08:15:08 +0200 Subject: [PATCH 12/13] Store boolean cookie attrs as native JSON booleans Instead of converting bool to "true" string for JSON storage and back on load, just store the native bool values directly. JSON handles booleans natively, so no conversion is needed. Co-Authored-By: Claude Opus 4.6 --- aiohttp/cookiejar.py | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/aiohttp/cookiejar.py b/aiohttp/cookiejar.py index 18b60566952..b3cefcf24eb 100644 --- a/aiohttp/cookiejar.py +++ b/aiohttp/cookiejar.py @@ -154,12 +154,12 @@ def save(self, file_path: PathLike) -> None: :class:`str` or :class:`pathlib.Path` instance. """ file_path = pathlib.Path(file_path) - data: dict[str, dict[str, dict[str, str]]] = {} + data: dict[str, dict[str, dict[str, str | bool]]] = {} 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] = { + morsel_data: dict[str, str | bool] = { "key": morsel.key, "value": morsel.value, "coded_value": morsel.coded_value, @@ -168,10 +168,7 @@ def save(self, file_path: PathLike) -> None: for attr in morsel._reserved: # type: ignore[attr-defined] attr_val = morsel[attr] if attr_val: - if isinstance(attr_val, bool): - morsel_data[attr] = "true" - else: - morsel_data[attr] = attr_val + morsel_data[attr] = attr_val data[key][name] = morsel_data with file_path.open(mode="w", encoding="utf-8") as f: json.dump(data, f, indent=2) @@ -200,7 +197,7 @@ def load(self, file_path: PathLike) -> None: self._cookies = _RestrictedCookieUnpickler(f).load() def _load_json_data( - self, data: dict[str, dict[str, dict[str, str]]] + 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) @@ -228,11 +225,7 @@ def _load_json_data( "value", "coded_value", ): - attr_val = morsel_data[attr] - if attr in ("secure", "httponly", "partitioned"): - morsel[attr] = True if attr_val == "true" else attr_val - else: - morsel[attr] = attr_val + morsel[attr] = morsel_data[attr] cookies[key][name] = morsel return cookies From 6b691eec931e1da6a224ddc3a3594c9af3acc732 Mon Sep 17 00:00:00 2001 From: Sam Bull Date: Fri, 20 Feb 2026 17:56:48 +0000 Subject: [PATCH 13/13] Update cookiejar.py --- aiohttp/cookiejar.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/aiohttp/cookiejar.py b/aiohttp/cookiejar.py index b3cefcf24eb..3ba7de6869b 100644 --- a/aiohttp/cookiejar.py +++ b/aiohttp/cookiejar.py @@ -189,12 +189,10 @@ def load(self, file_path: PathLike) -> None: with file_path.open(mode="r", encoding="utf-8") as f: data = json.load(f) self._cookies = self._load_json_data(data) - return except (json.JSONDecodeError, UnicodeDecodeError, ValueError): - pass - # Fall back to legacy pickle format with restricted unpickler - with file_path.open(mode="rb") as f: - self._cookies = _RestrictedCookieUnpickler(f).load() + # 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]]]