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 42 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
|
This certainly looks like an improvement, but it's still possible for any user to DoS the machine until a reboot with |
Co-authored-by: 🇺🇦 Sviatoslav Sydorenko (Святослав Сидоренко) <wk.cvs.github@sydorenko.org.ua>
Co-authored-by: 🇺🇦 Sviatoslav Sydorenko (Святослав Сидоренко) <wk.cvs.github@sydorenko.org.ua>
Co-authored-by: Windsurf
# Conflicts: # changelog/13669.bugfix.rst
Co-authored-by: Windsurf
for more information, see https://pre-commit.ci
Co-authored-by: Windsurf
for more information, see https://pre-commit.ci
Co-authored-by: Windsurf
# Conflicts: # testing/test_tmpdir.py
for more information, see https://pre-commit.ci
…_dir isolated Co-authored-by: Windsurf
Co-authored-by: Windsurf
for more information, see https://pre-commit.ci
…try_ensure_directory Cover the race-condition branch where `unlink` succeeds but the subsequent `mkdir` still raises an `OSError` (e.g. a concurrent process recreated the path between the two calls). Verifies that `_try_ensure_directory` returns `None` rather than propagating the exception. Co-authored-by: Windsurf
for more information, see https://pre-commit.ci
Great callout Michael! This attack route is now blocked by a safety check before creating the directory: If a non-directory file is blocking the path (e.g. placed by another user), we attempt to remove that file and then retry. |
This cannot work unfortunately: the file is owned by the attacker, so I cannot delete his files from I don't want to sound like an asshole always coming back with new caveats to every approach, but the truth is that this is very old class of vulnerability and all of the ways to exploit it are well-known. (I was not auditing the code, I just saw the fixed path one day and knew immediately that it would be exploitable.) The solution is to use a carefully-created randomly-named path; so if someone proposes a solution that does not involve |
…ytest-dev#13669) Co-authored-by: Windsurf
for more information, see https://pre-commit.ci
Fixed! Previously, TempPathFactory created a fixed, predictable directory ( I've replaced Because |
- Remove blank line before tmppath_result_key assignment - Collapse multi-line Path(tempfile.mkdtemp(...)) calls to single lines - Remove unused `getpass` import and `original_chmod` variable in tests - Add explicit `str` type annotation to `path` variable in symlink test - Add blank line before test_getbasetemp_uses_mkdtemp_rootdir function - Collapse multi-line sorted() call in TestCleanupOldRootdirs to single line Co-authored-by: Windsurf
for more information, see https://pre-commit.ci
…missions instead of no-op Replace the `_noop_chmod` stub (which simply skipped our chmod call) with `_widen_chmod`, which actively sets permissions to 0o755. This makes the test more realistic: `fchmod` must now *correct* an adversarially-widened directory, not merely compensate for a missing tightening step. Co-authored-by: Windsurf
The new approach looks good to me FWIW. mkdtemp() guarantees that the directory will be 0700 and owned by the current user, so the pre-existing chmod/stat safeguards conveniently become redundant if this approach is adopted :) |
TY Michael, and let's leave it to another enthusiastic coder ;) and close the book on this CVE |
Summary
Replace the predictable
pytest-of-<user>rootdir with a randomly-named rootdir created viatempfile.mkdtempto prevent the entire class of predictable-name attacks (symlink races, DoS via pre-created files/dirs). As defense-in-depth, open the rootdir withos.openusingO_NOFOLLOWandO_DIRECTORYflags and perform ownership/permission checks via fd-basedfstat/fchmodto eliminate TOCTOU races.Fixes CVE-2025-71176.
closes #13669
Changes
tempfile.mkdtemp(prefix=f"pytest-of-{user}-", dir=temproot)instead of a fixedpytest-of-{user}directory, making pre-creation attacks infeasible._safe_open_dircontext manager: Opens the rootdir withO_NOFOLLOW | O_DIRECTORY(where available), yields the fd, and guarantees cleanup. Symlinks are rejected at theos.openlevel.fstatandfchmodoperate on the open file descriptor, eliminating the TOCTOU window betweenstat/chmodand the actual directory._cleanup_old_rootdirs: Garbage-collects stale randomly-named rootdirs from previous sessions, respecting the retention count. The current session's rootdir is never removed.O_NOFOLLOW/O_DIRECTORYfallback, predictable-path DoS immunity,_cleanup_old_rootdirsbehavior,_safe_open_dirfd lifecycle, config validation, and session-finish edge cases.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.