tmpdir: prevent symlink attacks and TOCTOU races (CVE-2025-71176)#14279
tmpdir: prevent symlink attacks and TOCTOU races (CVE-2025-71176)#14279laurac8r wants to merge 12 commits intopytest-dev:mainfrom
Conversation
Open the base temporary directory using `os.open` with `O_NOFOLLOW` and `O_DIRECTORY` flags to prevent symlink attacks. Use the resulting file descriptor for `fstat` and `fchmod` operations to eliminate Time-of-Check Time-of-Use (TOCTOU) races. Co-authored-by: Windsurf, Gemini
Co-authored-by: Windsurf, Gemini
Co-authored-by Windsurf
Co-authored-by Windsurf, Gemini
for more information, see https://pre-commit.ci
Co-authored-by: Windsurf
for more information, see https://pre-commit.ci
Co-authored-by: Windsurf
| @@ -0,0 +1,3 @@ | |||
| Fixed a symlink attack vulnerability (CVE-2025-71176) in the :fixture:`tmp_path` fixture's base directory handling. | |||
|
|
|||
| 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. | |||
There was a problem hiding this comment.
Mind linking this abbrebiation to the definition?
|
|
||
| # Figure out what rootdir name pytest would use, then replace it | ||
| # with a symlink pointing to the attacker-controlled directory. | ||
| import getpass |
There was a problem hiding this comment.
Why is the import happening inside a function?
| ) -> None: | ||
| """Cover the branch where basetemp is set but the directory no longer | ||
| exists when pytest_sessionfinish runs (314->320 partial branch).""" | ||
| from _pytest.tmpdir import pytest_sessionfinish |
There was a problem hiding this comment.
Misplaced imports are normally discouraged. Any justification?
| @@ -0,0 +1,3 @@ | |||
| Fixed a symlink attack vulnerability (CVE-2025-71176) in the :fixture:`tmp_path` fixture's base directory handling. | |||
There was a problem hiding this comment.
There's a :cve: role that the sphinx-issues extension provides. Plus, feel free to take the credit:
| Fixed a symlink attack vulnerability (CVE-2025-71176) in the :fixture:`tmp_path` fixture's base directory handling. | |
| Fixed a symlink attack vulnerability (:cve:`2025-71176`) in the :fixture:`tmp_path` fixture's base directory handling -- by :user:`laurac8r`. |
There was a problem hiding this comment.
It'd be useful to symlink 14279.bugfix.rst to 13669.bugfix.rst so that both are linked @ https://pytest--14279.org.readthedocs.build/en/14279/changelog.html#bug-fixes (you can also use this link to preview the change note render after pushing updates to this PR).
There was a problem hiding this comment.
Some of this should probably go under a @contextlib.contextmanager for maintainability.
| assert (basetemp.parent.stat().st_mode & 0o077) == 0 | ||
|
|
||
|
|
||
| @pytest.mark.skipif(not hasattr(os, "getuid"), reason="checks unix permissions") |
There was a problem hiding this comment.
You can assign this to a variable and then reuse it in all these tests like so:
skip_if_no_getuid = pytest.mark.skipif(not hasattr(os, "getuid"), reason="checks unix permissions")| @pytest.mark.skipif(not hasattr(os, "getuid"), reason="checks unix permissions") | |
| @skip_if_no_getuid |
| 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).""" |
There was a problem hiding this comment.
These line numbers will become wrong over time.
|
|
||
| # 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) |
There was a problem hiding this comment.
Feels like this should be isolated, no?
|
This certainly looks like an improvement, but it's still possible for any user to DoS the machine until a reboot with |
Open the base temporary directory using
os.openwithO_NOFOLLOWandO_DIRECTORYflags to prevent symlink attacks. Use the resulting file descriptor forfstatandfchmodoperations to eliminate Time-of-Check Time-of-Use (TOCTOU) races.closes #13669
If this change fixes an issue, please:
closes #XYZWto the PR description and/or commits (whereXYZWis the issue number). See the github docs for more information.Important
Unsupervised agentic contributions are not accepted. See our AI/LLM-Assisted Contributions Policy.
Co-authored-bycommit trailers.Unless your change is trivial or a small documentation fix (e.g., a typo or reword of a small section) please:
Create a new changelog file in the
changelogdirectory, with a name like<ISSUE NUMBER>.<TYPE>.rst. See changelog/README.rst for details.Write sentences in the past or present tense, examples:
Also make sure to end the sentence with a
..Add yourself to
AUTHORSin alphabetical order.