From 822686e880b3757977e9d56470e00dcd391371f2 Mon Sep 17 00:00:00 2001
From: Ran Benita <ran@unusedvar.com>
Date: Sat, 6 Mar 2021 23:17:46 +0200
Subject: [PATCH] tmpdir: prevent using a non-private root temp directory

pytest uses a root temp directory named `/tmp/pytest-of-<username>`. The
name is predictable, and the directory might already exists from a
previous run, so that's allowed.

This makes it possible for my_user to pre-create
`/tmp/pytest-of-another_user`, thus giving my_user control of
another_user's tempdir.

Prevent this scenario by adding a couple of safety checks. I believe
they are sufficient.

Testing the first check requires changing the owner, which requires
root permissions, so can't be unit-tested easily, but I checked it
manually.
---
 changelog/8414.bugfix.rst |  5 +++++
 src/_pytest/tmpdir.py     | 19 +++++++++++++++++++
 testing/test_tmpdir.py    | 25 +++++++++++++++++++++++++
 3 files changed, 49 insertions(+)

diff --git a/changelog/8414.bugfix.rst b/changelog/8414.bugfix.rst
index a8b791ae210..73c3068a839 100644
--- a/changelog/8414.bugfix.rst
+++ b/changelog/8414.bugfix.rst
@@ -3,3 +3,8 @@ permissions. This means that any user in the system was able to read
 information written by tests in temporary directories (such as those created by
 the ``tmp_path``/``tmpdir`` fixture). Now the directories are created with
 private permissions.
+
+pytest used silenty use a pre-existing ``/tmp/pytest-of-<username>`` directory,
+even if owned by another user. This means another user could pre-create such a
+directory and gain control of another user's temporary directory. Now such a
+condition results in an error.
diff --git a/src/_pytest/tmpdir.py b/src/_pytest/tmpdir.py
index 53fc327964f..3fe17583788 100644
--- a/src/_pytest/tmpdir.py
+++ b/src/_pytest/tmpdir.py
@@ -115,6 +115,25 @@ def getbasetemp(self) -> Path:
             # make_numbered_dir() call
             rootdir = temproot.joinpath(f"pytest-of-{user}")
             rootdir.mkdir(mode=0o700, exist_ok=True)
+            # Because we use exist_ok=True with a predictable name, make sure
+            # we are the owners, to prevent any funny business (on unix, where
+            # temproot is usually shared).
+            # Also, to keep things private, fixup any world-readable temp
+            # rootdir's permissions. Historically 0o755 was used, so we can't
+            # just error out on this, at least for a while.
+            if hasattr(os, "getuid"):
+                rootdir_stat = rootdir.stat()
+                uid = os.getuid()
+                # getuid shouldn't fail, but cpython defines such a case.
+                # Let's hope for the best.
+                if uid != -1:
+                    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.chmod(rootdir, rootdir_stat.st_mode & ~0o077)
             basetemp = make_numbered_dir_with_cleanup(
                 prefix="pytest-",
                 root=rootdir,
diff --git a/testing/test_tmpdir.py b/testing/test_tmpdir.py
index fc2a62ea1c3..8b3b65010d4 100644
--- a/testing/test_tmpdir.py
+++ b/testing/test_tmpdir.py
@@ -461,3 +461,28 @@ def test_tmp_path_factory_create_directory_with_safe_permissions(
     assert (basetemp.stat().st_mode & 0o077) == 0
     # Parent too (pytest-of-foo).
     assert (basetemp.parent.stat().st_mode & 0o077) == 0
+
+
+@pytest.mark.skipif(not hasattr(os, "getuid"), reason="checks unix permissions")
+def test_tmp_path_factory_fixes_up_world_readable_permissions(
+    tmp_path: Path, monkeypatch,
+) -> None:
+    """Verify that if a /tmp/pytest-of-foo directory already exists with
+    world-readable permissions, it is fixed.
+
+    pytest used to mkdir with such permissions, that's why we fix it up.
+    """
+    # Use the test's tmp_path as the system temproot (/tmp).
+    monkeypatch.setenv("PYTEST_DEBUG_TEMPROOT", str(tmp_path))
+    tmp_factory = TempPathFactory(None, lambda *args: None, _ispytest=True)
+    basetemp = tmp_factory.getbasetemp()
+
+    # Before - simulate bad perms.
+    os.chmod(basetemp.parent, 0o777)
+    assert (basetemp.parent.stat().st_mode & 0o077) != 0
+
+    tmp_factory = TempPathFactory(None, lambda *args: None, _ispytest=True)
+    basetemp = tmp_factory.getbasetemp()
+
+    # After - fixed.
+    assert (basetemp.parent.stat().st_mode & 0o077) == 0