-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
tmpdir: prevent symlink attacks and TOCTOU races (CVE-2025-71176) #14279
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from 12 commits
Commits
Show all changes
90 commits
Select commit
Hold shift + click to select a range
f601e56
tmpdir: prevent symlink attacks and TOCTOU races (CVE-2025-71176)
laurac8r ec18caa
docs: added a bugfix changelog entry
laurac8r b3cb812
chore: added name to `AUTHORS` file
laurac8r 5894e25
chore: adding test coverage
laurac8r 7f93f0a
chore: Add tests for tmp_path retention configuration validation
laurac8r e232f12
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] fe0832b
chore: improve coide coverage for edge case
laurac8r 23000e8
Merge remote-tracking branch 'origin/hotfix/cve' into hotfix/cve
laurac8r 09bd0ed
Merge branch 'pytest-dev:main' into hotfix/cve
laurac8r 068fd4e
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] a724939
chore: remove dead code
laurac8r 9a4451a
Merge branch 'pytest-dev:main' into hotfix/cve
laurac8r 95f39ee
Merge branch 'pytest-dev:main' into hotfix/cve
laurac8r ed4a728
Apply suggestion from @webknjaz
laurac8r 206731a
Apply suggestion from @webknjaz
laurac8r 975b944
Merge branch 'pytest-dev:main' into hotfix/cve
laurac8r d456ad4
docs: enhance CVE-2025-71176 changelog entry with hyperlinks
a624cc1
Merge remote-tracking branch 'origin/hotfix/cve' into hotfix/cve
c025681
refactor(tmpdir): extract _safe_open_dir into a reusable context manager
f2c6f23
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] d58ba2a
docs: update docstring
9d501ec
Merge remote-tracking branch 'origin/hotfix/cve' into hotfix/cve
45bdce9
refactor(testing): consolidate imports in test_tmpdir.py
879767b
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] ab3d9e4
refactor: consolidate imports and hoist getpass to module level
40e8fdd
Merge remote-tracking branch 'origin/hotfix/cve' into hotfix/cve
64aa0f1
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] e1ab060
test(tmpdir): make test_pytest_sessionfinish_handles_missing_basetemp…
c8be3c5
Merge remote-tracking branch 'origin/hotfix/cve' into hotfix/cve
3a27865
hotfix: mitigate DoS when a non-directory file blocks pytest-of-<user>
e403fbf
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 0e3581c
test(tmpdir): add regression test for mkdir failure after unlink in _…
f9918cf
Merge remote-tracking branch 'origin/hotfix/cve' into hotfix/cve
d1d6cae
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 24003ce
Merge branch 'main' into hotfix/cve
064b26e
tmpdir: replace predictable rootdir with mkdtemp-based random suffix …
124027e
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 654e3dd
refactor: style: minor code formatting cleanup in tmpdir and test_tmpdir
9a586f6
Merge remote-tracking branch 'origin/hotfix/cve' into hotfix/cve
43aa2c8
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] e696db0
test(tmpdir): strengthen fchmod defense-in-depth test by widening per…
65fc036
Merge branch 'main' into hotfix/cve
laurac8r 6437860
Merge branch 'main' into hotfix/cve
laurac8r 1d1cf8e
Merge branch 'main' into hotfix/cve
laurac8r a0c8ace
Merge branch 'main' into hotfix/cve
laurac8r 1d4fee4
test(tmpdir): add tests for safe_rmtree to handle symlinks and direct…
laurac8r 81de30b
Merge remote-tracking branch 'origin/hotfix/cve' into hotfix/cve
laurac8r 9f82169
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 9a7726e
refactor: replace rmtree with safe_rmtree for improved directory remo…
laurac8r a8b8456
Merge remote-tracking branch 'origin/hotfix/cve' into hotfix/cve
laurac8r 04baaa3
feat: add safe_rmtree function with symlink attack protection
laurac8r bb1371b
fix: enhance symlink protection in _cleanup_old_rootdirs to prevent C…
laurac8r eb1179e
Merge branch 'main' into hotfix/cve
laurac8r 991a5dd
refactor: rename _safe_open_dir to _verify_ownership_and_tighten_perm…
laurac8r e83a0ca
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 29c866c
refactor: rename _safe_open_dir to _verify_ownership_and_tighten_perm…
laurac8r 02e492d
fix: update tmp_path directory creation to prevent symlink attacks an…
laurac8r 76a9dd0
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 56df2c4
hotfix: unicode encoding
laurac8r f4b54ae
chore: remove changelog entry
laurac8r 8ab71db
fix: improve docstring for safe_rmtree to clarify symlink attack prot…
laurac8r f6847a0
docs: move functionality comments from docstring to body
laurac8r ea4c00e
docs: move functionality comments from docstring to body
laurac8r af8b534
chore: move comment
laurac8r 5c6a6cd
chore: reformatting
laurac8r e369678
fix: update assertion for symlink warnings in test_tmpdir
laurac8r 727304d
docs: update docstring to include issue reference for TestSafeRmtree
laurac8r 2df369b
Merge branch 'main' into hotfix/cve
laurac8r 7137411
fix: remove redundant import of shutil in test_tmpdir
laurac8r 3bcfa95
add symlink
laurac8r cd34048
hotfix: make Windows tests pass
laurac8r 5ee872a
fix: enhance symlink safety checks and add create_symlink option in m…
laurac8r ab08a78
fix: disable symlink creation in make_numbered_dir by default
laurac8r 8df59d5
test: add test for make_numbered_dir with create_symlink=False
laurac8r 53efdc4
chore: linting
laurac8r 12e90a9
enhance: prevent warning spam in cleanup loops
laurac8r e5941d2
docs: move comment
laurac8r 079664e
docs: Rewrote comment to accurately describe best-effort behavior
laurac8r 9177048
hotfix: Per-entry OSError handling in _cleanup_old_rootdirs via a _mt…
laurac8r 0c94285
feat:
laurac8r 140ca06
hotfix: Removed redundant path-based os.chmod; fd-based fchmod now ha…
laurac8r aba08fc
refactor: Added stacklevel keyword param to _check_symlink_attack_saf…
laurac8r 671867f
docs: Documented POSIX-only O_NOFOLLOW limitation in _verify_ownershi…
laurac8r 24d7bb5
hotfix: Improve symlink attack defense in _cleanup_old_rootdirs by re…
laurac8r 7af0a46
chore: remove unnecessary comment
laurac8r 99cce96
hotfix: Move ensure_extended_length_path call after _check_symlink_at…
laurac8r a2a2db5
hotfix: Simplify warnings in safe_rmtree tests by removing unnecessar…
laurac8r 03e0931
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] bc9dc77
hotfix: Introduce _safe_open_dir context manager to enhance symlink s…
laurac8r 479a5a6
hotfix: Update test_warns_once_when_avoids_symlink_attacks to ensure …
laurac8r File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| Fixed a symlink attack vulnerability (CVE-2025-71176) in the :fixture:`tmp_path` fixture's base directory handling. | ||
laurac8r marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| The ``pytest-of-<user>`` directory under the system temp root is now opened with ``O_NOFOLLOW`` and verified using file-descriptor-based ``fstat``/``fchmod``, preventing symlink attacks and TOCTOU races. | ||
laurac8r marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
laurac8r marked this conversation as resolved.
Show resolved
Hide resolved
|
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -619,3 +619,149 @@ def test_tmp_path_factory_fixes_up_world_readable_permissions( | |
|
|
||
| # After - fixed. | ||
| assert (basetemp.parent.stat().st_mode & 0o077) == 0 | ||
|
|
||
|
|
||
| @pytest.mark.skipif(not hasattr(os, "getuid"), reason="checks unix permissions") | ||
laurac8r marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| def test_tmp_path_factory_rejects_symlink_rootdir( | ||
| tmp_path: Path, monkeypatch: MonkeyPatch | ||
| ) -> None: | ||
| """CVE-2025-71176: verify that a symlink at the pytest-of-<user> location | ||
| is rejected to prevent symlink attacks.""" | ||
| monkeypatch.setenv("PYTEST_DEBUG_TEMPROOT", str(tmp_path)) | ||
|
|
||
| # Pre-create a target directory that the symlink will point to. | ||
| attacker_dir = tmp_path / "attacker-controlled" | ||
| attacker_dir.mkdir(mode=0o700) | ||
|
|
||
| # Figure out what rootdir name pytest would use, then replace it | ||
| # with a symlink pointing to the attacker-controlled directory. | ||
| import getpass | ||
laurac8r marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| user = getpass.getuser() | ||
| rootdir = tmp_path / f"pytest-of-{user}" | ||
| # Ensure the real dir exists so the cleanup branch is exercised. | ||
| rootdir.mkdir(mode=0o700, exist_ok=True) | ||
| rootdir.rmdir() | ||
| rootdir.symlink_to(attacker_dir) | ||
|
|
||
| tmp_factory = TempPathFactory(None, 3, "all", lambda *args: None, _ispytest=True) | ||
| with pytest.raises(OSError, match="could not be safely opened"): | ||
| tmp_factory.getbasetemp() | ||
|
|
||
|
|
||
| @pytest.mark.skipif(not hasattr(os, "getuid"), reason="checks unix permissions") | ||
| def test_tmp_path_factory_rejects_wrong_owner( | ||
| tmp_path: Path, monkeypatch: MonkeyPatch | ||
| ) -> None: | ||
| """CVE-2025-71176: verify that a rootdir owned by a different user is | ||
| rejected (covers the fstat uid mismatch branch).""" | ||
| monkeypatch.setenv("PYTEST_DEBUG_TEMPROOT", str(tmp_path)) | ||
|
|
||
| # Make get_user_id() return a uid that won't match the directory owner. | ||
| monkeypatch.setattr("_pytest.tmpdir.get_user_id", lambda: os.getuid() + 1) | ||
|
|
||
| tmp_factory = TempPathFactory(None, 3, "all", lambda *args: None, _ispytest=True) | ||
| with pytest.raises(OSError, match="not owned by the current user"): | ||
| tmp_factory.getbasetemp() | ||
|
|
||
|
|
||
| @pytest.mark.skipif(not hasattr(os, "getuid"), reason="checks unix permissions") | ||
| def test_tmp_path_factory_nofollow_flag_missing( | ||
| tmp_path: Path, monkeypatch: MonkeyPatch | ||
| ) -> None: | ||
| """CVE-2025-71176: verify that the code still works when O_NOFOLLOW or | ||
| O_DIRECTORY flags are not available on the platform.""" | ||
| monkeypatch.setenv("PYTEST_DEBUG_TEMPROOT", str(tmp_path)) | ||
| monkeypatch.delattr(os, "O_NOFOLLOW", raising=False) | ||
| monkeypatch.delattr(os, "O_DIRECTORY", raising=False) | ||
|
|
||
| tmp_factory = TempPathFactory(None, 3, "all", lambda *args: None, _ispytest=True) | ||
| basetemp = tmp_factory.getbasetemp() | ||
|
|
||
| # Should still create the directory with safe permissions. | ||
| assert basetemp.is_dir() | ||
| assert (basetemp.parent.stat().st_mode & 0o077) == 0 | ||
|
|
||
|
|
||
| def test_tmp_path_factory_from_config_rejects_negative_count( | ||
|
||
| tmp_path: Path, | ||
| ) -> None: | ||
| """Verify that a negative tmp_path_retention_count raises ValueError.""" | ||
|
|
||
| @dataclasses.dataclass | ||
| class BadCountConfig: | ||
| basetemp: str | Path = "" | ||
|
|
||
| def getini(self, name): | ||
| if name == "tmp_path_retention_count": | ||
| return -1 | ||
| assert False | ||
|
|
||
| config = cast(Config, BadCountConfig(tmp_path)) | ||
| with pytest.raises(ValueError, match="tmp_path_retention_count must be >= 0"): | ||
| TempPathFactory.from_config(config, _ispytest=True) | ||
|
|
||
|
|
||
| def test_tmp_path_factory_from_config_rejects_invalid_policy( | ||
| tmp_path: Path, | ||
| ) -> None: | ||
| """Verify that an invalid tmp_path_retention_policy raises ValueError.""" | ||
|
|
||
| @dataclasses.dataclass | ||
| class BadPolicyConfig: | ||
| basetemp: str | Path = "" | ||
|
|
||
| def getini(self, name): | ||
| if name == "tmp_path_retention_count": | ||
| return 3 | ||
| elif name == "tmp_path_retention_policy": | ||
| return "invalid_policy" | ||
| assert False | ||
|
|
||
| config = cast(Config, BadPolicyConfig(tmp_path)) | ||
| with pytest.raises(ValueError, match="tmp_path_retention_policy must be either"): | ||
| TempPathFactory.from_config(config, _ispytest=True) | ||
|
|
||
|
|
||
| def test_tmp_path_factory_none_policy_sets_keep_zero( | ||
| tmp_path: Path, monkeypatch: MonkeyPatch | ||
| ) -> None: | ||
| """Verify that retention_policy='none' sets keep=0.""" | ||
| monkeypatch.setenv("PYTEST_DEBUG_TEMPROOT", str(tmp_path)) | ||
| tmp_factory = TempPathFactory(None, 3, "none", lambda *args: None, _ispytest=True) | ||
| basetemp = tmp_factory.getbasetemp() | ||
| assert basetemp.is_dir() | ||
|
|
||
|
|
||
| def test_pytest_sessionfinish_noop_when_no_basetemp( | ||
| pytester: Pytester, | ||
| ) -> None: | ||
| """Verify that pytest_sessionfinish returns early when basetemp is None.""" | ||
| p = pytester.makepyfile( | ||
| """ | ||
| def test_no_tmp(): | ||
| pass | ||
| """ | ||
| ) | ||
| result = pytester.runpytest(p) | ||
| result.assert_outcomes(passed=1) | ||
|
|
||
|
|
||
| def test_pytest_sessionfinish_handles_missing_basetemp_dir( | ||
| tmp_path: Path, | ||
| ) -> None: | ||
| """Cover the branch where basetemp is set but the directory no longer | ||
| exists when pytest_sessionfinish runs (314->320 partial branch).""" | ||
laurac8r marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| from _pytest.tmpdir import pytest_sessionfinish | ||
laurac8r marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| factory = TempPathFactory(None, 3, "failed", lambda *args: None, _ispytest=True) | ||
| # Point _basetemp at a path that does not exist on disk. | ||
| factory._basetemp = tmp_path / "already-gone" | ||
|
|
||
| class FakeSession: | ||
| class config: | ||
| _tmp_path_factory = factory | ||
|
|
||
| # exitstatus=0 + policy="failed" + _given_basetemp=None enters the | ||
| # cleanup block; basetemp.is_dir() is False so rmtree is skipped. | ||
| pytest_sessionfinish(FakeSession, exitstatus=0) | ||
laurac8r marked this conversation as resolved.
Show resolved
Hide resolved
|
||
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.
Uh oh!
There was an error while loading. Please reload this page.