Skip to content

Commit 59bb357

Browse files
YuvalElbar6claude
andcommitted
Security: restrict pickle deserialization in CookieJar.load()
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 <[email protected]>
1 parent eaeba86 commit 59bb357

3 files changed

Lines changed: 361 additions & 2 deletions

File tree

aiohttp/cookiejar.py

Lines changed: 136 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import datetime
44
import heapq
55
import itertools
6+
import json
67
import os # noqa
78
import pathlib
89
import pickle
@@ -37,6 +38,41 @@
3738
_SIMPLE_COOKIE = SimpleCookie()
3839

3940

41+
class _RestrictedCookieUnpickler(pickle.Unpickler):
42+
"""A restricted unpickler that only allows cookie-related types.
43+
44+
This prevents arbitrary code execution when loading pickled cookie data
45+
from untrusted sources. Only types that are expected in a serialized
46+
CookieJar are permitted.
47+
48+
See: https://docs.python.org/3/library/pickle.html#restricting-globals
49+
"""
50+
51+
_ALLOWED_CLASSES: frozenset[tuple[str, str]] = frozenset(
52+
{
53+
# Core cookie types
54+
("http.cookies", "SimpleCookie"),
55+
("http.cookies", "Morsel"),
56+
# Container types used by CookieJar._cookies
57+
("collections", "defaultdict"),
58+
# builtins that pickle uses for reconstruction
59+
("builtins", "tuple"),
60+
("builtins", "set"),
61+
("builtins", "frozenset"),
62+
("builtins", "dict"),
63+
}
64+
)
65+
66+
def find_class(self, module: str, name: str) -> type:
67+
if (module, name) not in self._ALLOWED_CLASSES:
68+
raise pickle.UnpicklingError(
69+
f"Forbidden class: {module}.{name}. "
70+
"CookieJar.load() only allows cookie-related types for security. "
71+
"See https://docs.python.org/3/library/pickle.html#restricting-globals"
72+
)
73+
return super().find_class(module, name)
74+
75+
4076
class CookieJar(AbstractCookieJar):
4177
"""Implements cookie storage adhering to RFC 6265."""
4278

@@ -117,9 +153,108 @@ def save(self, file_path: PathLike) -> None:
117153
pickle.dump(self._cookies, f, pickle.HIGHEST_PROTOCOL)
118154

119155
def load(self, file_path: PathLike) -> None:
156+
"""Load cookies from a pickled file using a restricted unpickler.
157+
158+
.. warning::
159+
160+
Cookie files loaded from untrusted sources could previously
161+
execute arbitrary code. This method now uses a restricted
162+
unpickler that only allows cookie-related types.
163+
164+
For new code, consider using :meth:`save_json` and
165+
:meth:`load_json` instead, which use a safe JSON format.
166+
167+
:param file_path: Path to file from where cookies will be
168+
imported, :class:`str` or :class:`pathlib.Path` instance.
169+
"""
120170
file_path = pathlib.Path(file_path)
121171
with file_path.open(mode="rb") as f:
122-
self._cookies = pickle.load(f)
172+
self._cookies = _RestrictedCookieUnpickler(f).load()
173+
174+
def save_json(self, file_path: PathLike) -> None:
175+
"""Save cookies to a JSON file (safe alternative to :meth:`save`).
176+
177+
This method serializes cookies using JSON, which is inherently safe
178+
against deserialization attacks unlike the pickle-based :meth:`save`.
179+
180+
:param file_path: Path to file where cookies will be serialized,
181+
:class:`str` or :class:`pathlib.Path` instance.
182+
183+
.. versionadded:: 3.14
184+
"""
185+
file_path = pathlib.Path(file_path)
186+
data: dict[str, dict[str, dict[str, str]]] = {}
187+
for (domain, path), cookie in self._cookies.items():
188+
key = f"{domain}|{path}"
189+
data[key] = {}
190+
for name, morsel in cookie.items():
191+
morsel_data: dict[str, str] = {
192+
"key": morsel.key,
193+
"value": morsel.value,
194+
"coded_value": morsel.coded_value,
195+
}
196+
# Save all morsel attributes that have values
197+
for attr in morsel._reserved:
198+
attr_val = morsel[attr]
199+
if attr_val:
200+
if isinstance(attr_val, bool):
201+
morsel_data[attr] = "true"
202+
else:
203+
morsel_data[attr] = str(attr_val)
204+
data[key][name] = morsel_data
205+
with file_path.open(mode="w", encoding="utf-8") as f:
206+
json.dump(data, f, indent=2)
207+
208+
def load_json(self, file_path: PathLike) -> None:
209+
"""Load cookies from a JSON file (safe alternative to :meth:`load`).
210+
211+
This method deserializes cookies from JSON format, which is inherently
212+
safe against code execution attacks.
213+
214+
:param file_path: Path to file from where cookies will be imported,
215+
:class:`str` or :class:`pathlib.Path` instance.
216+
217+
.. versionadded:: 3.14
218+
"""
219+
file_path = pathlib.Path(file_path)
220+
with file_path.open(mode="r", encoding="utf-8") as f:
221+
data = json.load(f)
222+
cookies: defaultdict[tuple[str, str], SimpleCookie] = defaultdict(
223+
SimpleCookie
224+
)
225+
for compound_key, cookie_data in data.items():
226+
parts = compound_key.split("|", 1)
227+
domain = parts[0]
228+
path = parts[1] if len(parts) > 1 else ""
229+
key = (domain, path)
230+
for name, morsel_data in cookie_data.items():
231+
morsel: Morsel[str] = Morsel()
232+
morsel_key = morsel_data.get("key", name)
233+
morsel_value = morsel_data.get("value", "")
234+
morsel_coded_value = morsel_data.get("coded_value", morsel_value)
235+
# Use __setstate__ to bypass validation, same pattern
236+
# used in _build_morsel and _cookie_helpers.
237+
morsel.__setstate__( # type: ignore[attr-defined]
238+
{
239+
"key": morsel_key,
240+
"value": morsel_value,
241+
"coded_value": morsel_coded_value,
242+
}
243+
)
244+
# Restore morsel attributes
245+
for attr in morsel._reserved:
246+
if attr in morsel_data and attr not in (
247+
"key",
248+
"value",
249+
"coded_value",
250+
):
251+
attr_val = morsel_data[attr]
252+
if attr in ("secure", "httponly"):
253+
morsel[attr] = True if attr_val == "true" else attr_val # type: ignore[assignment]
254+
else:
255+
morsel[attr] = attr_val
256+
cookies[key][name] = morsel
257+
self._cookies = cookies
123258

124259
def clear(self, predicate: ClearCookiePredicate | None = None) -> None:
125260
if predicate is None:

docs/client_reference.rst

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2459,13 +2459,51 @@ Utilities
24592459
Write a pickled representation of cookies into the file
24602460
at provided path.
24612461

2462+
.. warning::
2463+
2464+
The pickle format is kept for backward compatibility but is
2465+
inherently unsafe if the cookie file can be modified by an
2466+
untrusted party (e.g. shared volumes, NFS mounts, or
2467+
multi-tenant environments). Consider using :meth:`save_json`
2468+
instead for new code.
2469+
24622470
:param file_path: Path to file where cookies will be serialized,
24632471
:class:`str` or :class:`pathlib.Path` instance.
24642472

24652473
.. method:: load(file_path)
24662474

24672475
Load a pickled representation of cookies from the file
2468-
at provided path.
2476+
at provided path. This method uses a restricted unpickler that
2477+
only allows cookie-related types, preventing arbitrary code
2478+
execution from malicious pickle payloads.
2479+
2480+
.. warning::
2481+
2482+
While :meth:`load` now uses a restricted unpickler for
2483+
defense in depth, the pickle format is inherently risky
2484+
when loading files from untrusted sources. Consider using
2485+
:meth:`load_json` instead for new code.
2486+
2487+
:param file_path: Path to file from where cookies will be
2488+
imported, :class:`str` or :class:`pathlib.Path` instance.
2489+
2490+
.. method:: save_json(file_path)
2491+
2492+
Write a JSON representation of cookies into the file at the
2493+
provided path. This is a safe alternative to :meth:`save` that
2494+
uses a format immune to deserialization attacks.
2495+
2496+
.. versionadded:: 3.14
2497+
2498+
:param file_path: Path to file where cookies will be serialized,
2499+
:class:`str` or :class:`pathlib.Path` instance.
2500+
2501+
.. method:: load_json(file_path)
2502+
2503+
Load a JSON representation of cookies from the file at the
2504+
provided path. This is a safe alternative to :meth:`load`.
2505+
2506+
.. versionadded:: 3.14
24692507

24702508
:param file_path: Path to file from where cookies will be
24712509
imported, :class:`str` or :class:`pathlib.Path` instance.

0 commit comments

Comments
 (0)