Restrict pickle deserialization to safe types (CVE-2025-69872)#364
Restrict pickle deserialization to safe types (CVE-2025-69872)#364a2811057970 wants to merge 1 commit into
Conversation
BREAKING CHANGE: Pickle deserialization now only permits safe built-in types (builtins, collections, datetime, decimal, fractions, uuid). Arbitrary objects can no longer be deserialized from cache, preventing code execution via crafted pickle payloads. Users caching custom types should migrate to JSONDisk or a custom Disk subclass. There is no opt-out mechanism by design. - Add SafeUnpickler with allowlist-based find_class override - Add UnpicklingError (inherits pickle.UnpicklingError) for downstream compatibility with libraries catching pickle.PickleError - Support pickle protocols 0-5 via __builtin__, copy_reg, and _codecs allowlist entries - Use frozenset values in SAFE_PICKLE_CLASSES to prevent runtime bypass - Bump version to 6.0.0 (breaking change per semver) This takes a different approach to PR grantjenks#361 (HMAC envelope). The HMAC approach still allows arbitrary deserialization once the signature is verified, meaning an attacker with read+write access to the cache directory can read the auto-generated key file and forge valid payloads. The allowlist approach blocks dangerous types regardless of filesystem access. Fixes: CVE-2025-69872 Closes: grantjenks#357, grantjenks#360, grantjenks#362
|
@grantjenks would appreciate a review when you get a chance. This is a minimal alternative to #361 for CVE-2025-69872 — 150 lines of additive code vs 1000+ lines, with stronger security guarantees (blocks dangerous types even if the attacker has full cache directory access). |
|
Thanks @a2811057970 for this fix! While waiting for this to be merged and released, we created a fork with the patch applied. uv (overrides the transitive dep — no direct dependency needed): [tool.uv]
override-dependencies = ["diskcache>=6.0.0"]
[tool.uv.sources]
diskcache = { git = "https://github.com/mapped/python-diskcache", tag = "v6.0.2" }Poetry (only works if [tool.poetry.dependencies]
diskcache = { git = "https://github.com/mapped/python-diskcache", tag = "v6.0.2" }Poetry cannot override transitive dependencies by package name — if |
|
Thanks for putting this together. I don’t intend to merge this PR as written. The central problem is that the change restricts deserialization but not serialization. DiskCache will still successfully write an unsupported object and then fail when the same version tries to read it. That is a particularly surprising failure mode for a cache. This also breaks existing uses of the Django backend involving functions, model instances, querysets, HttpResponse, and cookies. The linked fork handles those failures by changing the existing tests to expect UnpicklingError; that documents the regression but does not resolve it. Successful testing with one DVC pipeline is useful, but it does not establish general compatibility. A safe-by-default major release may be reasonable, but it would need at least synchronous rejection of unsupported writes, an explicit trusted-pickle option, honest migration guidance, and complete integration testing. I’m not comfortable adopting a no-opt-out policy or redefining DiskCache’s supported data model through this PR. |
Summary
Mitigates CVE-2025-69872 by restricting pickle deserialization to a hardcoded allowlist of safe built-in types. Arbitrary objects can no longer be deserialized from cache, preventing code execution via crafted pickle payloads.
Approach
This uses a
SafeUnpicklerwith an allowlist-basedfind_classoverride — a fundamentally different approach to #361 (HMAC envelope). The key difference:What's allowed
int,float,str,bytes,bytearray,list,dict,tuple,set,frozenset,complex,range,slice,object,bool,None,collections.OrderedDict,collections.defaultdict,collections.deque,datetime.*,decimal.Decimal,fractions.Fraction,uuid.UUIDEverything else raises
UnpicklingErroron read.Breaking change
This is intentionally a breaking change (version bumped to 6.0.0). Users caching custom types have two migration paths:
JSONDiskfor JSON-serializable dataDiskand overrideget()/fetch()with a custom serializerThere is no opt-out by design — an escape hatch would just be cargo-culted by every downstream that hits an error.
Downstream compatibility
UnpicklingErrorinherits frompickle.UnpicklingErrorso libraries that catchpickle.PickleError(e.g. dvc-data'stranslate_pickle_errordecorator) handle it gracefully.Tests
isortandbluepassFixes CVE-2025-69872
Closes #357, #360, #362