Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Directory enumeration/search permissions mismatch Unix behavior #960

Closed
vector-of-bool opened this issue Mar 4, 2024 · 3 comments
Closed
Labels

Comments

@vector-of-bool
Copy link

Describe the bug
Certain "pathological" directory modes do not match the native POSIX behaviors. A directory that has S_IRUSR can be enumerated, but not necessarily searched (i.e. one can list the files within it, but cannot access those files). A directory that has S_IXUSR can be searched, but not necessarily enumerated (i.e. one cannot list the files of a directory, but can read those files as long as the entry names are known).

How To Reproduce

Test cases
import os
from pathlib import Path
import pytest


def _enumerate_fails():
    # Prep
    d = Path("testdir")
    d.mkdir(exist_ok=True)
    d.chmod(0o777)
    # Set weird mode:
    d.chmod(0b011_101_101)  # -wxr-xr-x
    # We can add the file, even if the directory is not enumerable
    file = d / "file.txt"
    file.write_text("hey")
    # We can search the directory and read files within it:
    assert file.read_text() == "hey"
    # We cannot enumerate the directory
    with pytest.raises(PermissionError):
        list(d.iterdir())


def _search_fails():
    # Prep
    d = Path("testdir")
    d.mkdir(exist_ok=True)
    d.chmod(0o777)
    file = d / "file.txt"
    file.write_text("hey")
    # Set weird mode:
    d.chmod(0b110_101_101)  # rw-r-xr-x
    # We cannot create any files in the directory, because that requires
    # searching it
    another_file = Path("file.txt")
    another_file.write_text("hey")
    with pytest.raises(PermissionError):
        os.link(another_file, d / "link.txt")
    # We can enumerate the directory:
    assert os.listdir(d) == ["file.txt"]
    # We cannot read files inside of the directory (even if we have read access to the file)
    with pytest.raises(PermissionError):
        file.stat()
    with pytest.raises(PermissionError):
        file.read_text()


def test_enumerate_realfs():
    _enumerate_fails()


def test_enumerate_fakefs(fs):
    _enumerate_fails()


def test_search_fails():
    _search_fails()


def test_search_fakefs(fs):
    _search_fails()

Your environment
Please run the following in the environment where the problem happened and
paste the output.

$ python -c "import platform; print(platform.platform())"
Linux-6.7.6-arch1-1-x86_64-with-glibc2.39
$ python -c "import sys; print('Python', sys.version)"
Python 3.11.7 (main, Jan 29 2024, 16:03:57) [GCC 13.2.1 20230801]
$ python -c "from pyfakefs import __version__; print('pyfakefs', __version__)"
pyfakefs 5.3.5
$ python -c "import pytest; print('pytest', pytest.__version__)"
pytest 7.2.0

For directory searching, it looks to be that get_object_from_normpath is responsible for permission checking at each step in path resolution. It incorrectly checks the read bit of the directory objects in _can_read, but it should be checking the execute bit:

@staticmethod
def _can_read(target, owner_can_read):
    if target.st_uid == helpers.get_uid():
        if owner_can_read or target.st_mode & 0o400:
            return True
    if target.st_gid == get_gid():
        if target.st_mode & 0o040:
            return True
    return target.st_mode & 0o004

Additionally, at least to the best of my knowledge, the permission fall-through behavior is not part of POSIX i.e. if get_uid() == st_uid, then only the S_IXUSR bit should be considered, and not the group/other bits.

As for directory enumeration, I believe it would require a separate check since it depends on a different set of mode bits.

@mrbean-bremen
Copy link
Member

It incorrectly checks the read bit of the directory objects in _can_read, but it should be checking the execute bit

Right, that seems to be incorrect.

the permission fall-through behavior is not part of POSIX i.e. if get_uid() == st_uid, then only the S_IXUSR bit should be considered, and not the group/other bits

That's also true, I just checked this for the other issue where it also holds.

I actually forgot that there is already a check for uid and gid, so this looks just like a bug fix instead of a refactoring (which I first thought was needed).

@mrbean-bremen
Copy link
Member

@vector-of-bool - this (and the other issue) shall be fixed now in main. Please try it out, and check if you see any other permission-related problems. I'll wait a bit if anything else comes up before making a new release.

@vector-of-bool
Copy link
Author

Thanks for the quick turnaround! I installed and ran tests on my system and it looks to be behaving properly.

It looks like it may have unveiled issues related to the automatic creation of a /tmp directory, but will require more investigating.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants