Restrict pickle deserialization in CookieJar.load()

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
+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`.

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)  # type: ignore[no-any-return]
+
+
 class CookieJar(AbstractCookieJar):
     """Implements cookie storage adhering to RFC 6265."""
 
@@ -117,9 +153,106 @@ 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:  # type: ignore[attr-defined]
+                    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:  # type: ignore[attr-defined]
+                    if attr in morsel_data and attr not in (
+                        "key",
+                        "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
+                cookies[key][name] = morsel
+        self._cookies = cookies
 
     def clear(self, predicate: ClearCookiePredicate | None = None) -> None:
         if predicate is None:

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.

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,8 @@ unhandled
 unicode
 unittest
 Unittest
+unpickler
+untrusted
 unix
 unobvious
 unsets