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.
This commit is contained in:
Ran Benita 2021-03-06 23:17:46 +02:00
parent 1278f8b97e
commit c49100cef8
3 changed files with 49 additions and 0 deletions

View File

@ -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 information written by tests in temporary directories (such as those created by
the ``tmp_path``/``tmpdir`` fixture). Now the directories are created with the ``tmp_path``/``tmpdir`` fixture). Now the directories are created with
private permissions. 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.

View File

@ -124,6 +124,25 @@ class TempPathFactory:
# getuser() likely returned illegal characters for the platform, use unknown back off mechanism # getuser() likely returned illegal characters for the platform, use unknown back off mechanism
rootdir = temproot.joinpath("pytest-of-unknown") rootdir = temproot.joinpath("pytest-of-unknown")
rootdir.mkdir(mode=0o700, exist_ok=True) 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( basetemp = make_numbered_dir_with_cleanup(
prefix="pytest-", prefix="pytest-",
root=rootdir, root=rootdir,

View File

@ -470,3 +470,28 @@ def test_tmp_path_factory_create_directory_with_safe_permissions(
assert (basetemp.stat().st_mode & 0o077) == 0 assert (basetemp.stat().st_mode & 0o077) == 0
# Parent too (pytest-of-foo). # Parent too (pytest-of-foo).
assert (basetemp.parent.stat().st_mode & 0o077) == 0 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: 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