Create directories that contain cache files so they are only owner accessible#359
Open
samdoran wants to merge 1 commit intograntjenks:masterfrom
Open
Create directories that contain cache files so they are only owner accessible#359samdoran wants to merge 1 commit intograntjenks:masterfrom
samdoran wants to merge 1 commit intograntjenks:masterfrom
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This aims to address CVE-2025-69872 by ensuring that parent directories containing the cache are not world readable. This is not a perfect solution but adds further mitigation.
The parent cache directory is created with
tempfile.mkdtemp()which is only accessible by the creating user. The change in this PR only adds marginal additional security by ensuring the last intermediate directory created is accessible only by the creating user ID.The core problem is that pickle should not be used when the source of the data is untrusted.
I experimented with a solution that created a hash of the pickle file then verified against that hash before unpicking, but that only seemed to move the problem. The hash needs to be stored somewhere and an attacker could update the hash at the same time the pickle itself is modified.
I would be happy to add clarification to the documentation if we feel that will help.
Changing the default to
JSONDiskmay be another possible solution, but that would be a breaking change.Fixes #357