diff --git a/AUTHORS b/AUTHORS index 6885ec6e793..8329ddf9288 100644 --- a/AUTHORS +++ b/AUTHORS @@ -264,6 +264,7 @@ Kojo Idrissa Kostis Anagnostopoulos Kristoffer Nordström Kyle Altendorf +Laura Kaminskiy Lawrence Mitchell Lee Kamentsky Leonardus Chen diff --git a/changelog/13669.bugfix.rst b/changelog/13669.bugfix.rst new file mode 100644 index 00000000000..81f443bf7dc --- /dev/null +++ b/changelog/13669.bugfix.rst @@ -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-`` 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. diff --git a/src/_pytest/tmpdir.py b/src/_pytest/tmpdir.py index 855ad273ecf..121e37e80fb 100644 --- a/src/_pytest/tmpdir.py +++ b/src/_pytest/tmpdir.py @@ -172,14 +172,31 @@ def getbasetemp(self) -> Path: # just error out on this, at least for a while. uid = get_user_id() if uid is not None: - rootdir_stat = rootdir.stat() - if rootdir_stat.st_uid != uid: + # Open the directory without following symlinks to prevent + # symlink attacks (CVE-2025-71176). Using a file descriptor + # for fstat/fchmod also eliminates TOCTOU races. + open_flags = os.O_RDONLY + for _flag in ("O_NOFOLLOW", "O_DIRECTORY"): + open_flags |= getattr(os, _flag, 0) + try: + dir_fd = os.open(str(rootdir), open_flags) + except OSError as e: raise OSError( - f"The temporary directory {rootdir} is not owned by the current user. " - "Fix this and try again." - ) - if (rootdir_stat.st_mode & 0o077) != 0: - os.chmod(rootdir, rootdir_stat.st_mode & ~0o077) + f"The temporary directory {rootdir} could not be " + "safely opened (it may be a symlink). " + "Remove the symlink or directory and try again." + ) from e + try: + rootdir_stat = os.fstat(dir_fd) + if rootdir_stat.st_uid != uid: + raise OSError( + f"The temporary directory {rootdir} is not owned by the current user. " + "Fix this and try again." + ) + if (rootdir_stat.st_mode & 0o077) != 0: + os.fchmod(dir_fd, rootdir_stat.st_mode & ~0o077) + finally: + os.close(dir_fd) keep = self._retention_count if self._retention_policy == "none": keep = 0 diff --git a/testing/test_tmpdir.py b/testing/test_tmpdir.py index 12891d81488..c994b8a916f 100644 --- a/testing/test_tmpdir.py +++ b/testing/test_tmpdir.py @@ -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") +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- 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 + + 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).""" + from _pytest.tmpdir import pytest_sessionfinish + + 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)