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

DirEntry.stat() of os.scandir() has no dir_fd parameter #90636

Closed
vstinner opened this issue Jan 23, 2022 · 2 comments
Closed

DirEntry.stat() of os.scandir() has no dir_fd parameter #90636

vstinner opened this issue Jan 23, 2022 · 2 comments
Labels
3.11 only security fixes stdlib Python modules in the Lib dir

Comments

@vstinner
Copy link
Member

BPO 46478
Nosy @vstinner, @serhiy-storchaka, @corona10

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2022-01-23.04:29:57.130>
created_at = <Date 2022-01-23.03:38:48.909>
labels = ['invalid', 'library', '3.11']
title = 'DirEntry.stat() of os.scandir() has no dir_fd parameter'
updated_at = <Date 2022-01-23.04:29:57.129>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2022-01-23.04:29:57.129>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2022-01-23.04:29:57.130>
closer = 'vstinner'
components = ['Library (Lib)']
creation = <Date 2022-01-23.03:38:48.909>
creator = 'vstinner'
dependencies = []
files = []
hgrepos = []
issue_num = 46478
keywords = []
message_count = 2.0
messages = ['411338', '411341']
nosy_count = 3.0
nosy_names = ['vstinner', 'serhiy.storchaka', 'corona10']
pr_nums = []
priority = 'normal'
resolution = 'not a bug'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue46478'
versions = ['Python 3.11']

@vstinner
Copy link
Member Author

I read the Rust CVE-2022-21658 vulnerability of std::fs::remove_dir_all:
https://blog.rust-lang.org/2022/01/20/cve-2022-21658.html

It's a race condition if an attacker replaces a directory with a symlink while Rust is removing the parent directory, Rust follows the symlink rather than just removing the symlink.

shutil._rmtree_safe_fd() uses os.scandir(). If entry.is_dir(follow_symlinks=False) is true, it calls entry.stat(follow_symlinks=False). It opens the directory as a file to remove the directory. It checks os.path.samestat(orig_st, os.fstat(dirfd)): if it's false, it raises an exception:

try:
# This can only happen if someone replaces
# a directory with a symlink after the call to
# os.scandir or stat.S_ISDIR above.
raise OSError("Cannot call rmtree on a symbolic "
"link")
except OSError:
onerror(os.path.islink, fullname, sys.exc_info())

I understand that this check is in place to detect the Rust CVE-2022-21658 vulnerability.

I noticed that the first entry.is_dir(follow_symlinks=False) call does a stat() syscall, but it doesn't pass the directory file descriptor. It would be even safer to pass it, just in case if the parent directory has been modified in the meanwhile as well.

@vstinner vstinner added 3.11 only security fixes stdlib Python modules in the Lib dir labels Jan 23, 2022
@vstinner
Copy link
Member Author

Oh, I didn't test os.scandir() properly. If you pass a file descriptor to os.scandir(), it yields DirEntry entries with contains the directory FD. Ignore my request, Python works as expected :-D

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.11 only security fixes stdlib Python modules in the Lib dir
Projects
None yet
Development

No branches or pull requests

1 participant