[issue4489] shutil.rmtree is vulnerable to a symlink attack

[Charles-François Natali]

Charles-François Natali <neologix at free.fr> added the comment:

There's a race:
"""
--- Lib/shutil.py       2011-11-05 00:11:05.745221315 +0100
+++ Lib/shutil.py.new   2011-11-05 00:11:01.445220324 +0100
@@ -307,6 +307,7 @@
        try:
            mode = os.fstatat(dirfd, name, os.AT_SYMLINK_NOFOLLOW).st_mode
         except os.error:
             mode = 0
         if stat.S_ISDIR(mode):
+            input("press enter")
             newfd = os.openat(dirfd, name, os.O_RDONLY)
             _rmtree_safe(newfd, ignore_errors, onerror)
             try:
"""

$ rm -rf /tmp/target
$ mkdir -p /tmp/target/etc
$ ./python -c "import shutil; shutil.rmtree('/tmp/target')"
press enter^Z
[1]+  Stopped                 ./python -c "import shutil; shutil.rmtree('/tmp/target')"
$ rm -r /tmp/target/etc; ln -s /etc /tmp/target/
$ fg
./python -c "import shutil; shutil.rmtree('/tmp/target')"

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/home/cf/python/cpython/Lib/shutil.py", line 290, in rmtree
    _rmtree_safe(fd, ignore_errors, onerror)
  File "/home/cf/python/cpython/Lib/shutil.py", line 314, in _rmtree_safe
    _rmtree_safe(newfd, ignore_errors, onerror)
  File "/home/cf/python/cpython/Lib/shutil.py", line 323, in _rmtree_safe
    onerror(os.unlinkat, (dirfd, name), sys.exc_info())
  File "/home/cf/python/cpython/Lib/shutil.py", line 321, in _rmtree_safe
    os.unlinkat(dirfd, name)
PermissionError: [Errno 13] Permission denied
[52334 refs]

"""
openat(3, "etc", O_RDONLY|O_LARGEFILE)  = 4
dup(4)                                  = 5
fstat64(5, {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
fcntl64(5, F_GETFL)                     = 0x8000 (flags O_RDONLY|O_LARGEFILE)
fcntl64(5, F_SETFD, FD_CLOEXEC)         = 0
getdents64(5, /* 162 entries */, 32768) = 5176
getdents64(5, /* 0 entries */, 32768)   = 0
close(5)                                = 0
fstatat64(4, "passwd", {st_mode=S_IFREG|0644, st_size=980, ...}, AT_SYMLINK_NOFOLLOW) = 0
unlinkat(4, "passwd", 0)                = -1 EACCES (Permission denied)
"""

You should use the lstat/open/fstat idiom.
Also, here:
"""
            mode1 = os.lstat(path).st_mode
            if stat.S_ISLNK(mode1):
                raise OSError("Cannot call rmtree on a symbolic link")
        except OSError:
            onerror(os.lstat, path, sys.exc_info())
            # can't continue even if onerror hook returns
            return
        fd = os.open(path, os.O_RDONLY)
        try:
            mode2 = os.fstat(fd).st_mode
            if mode1 != mode2:
                raise OSError("Target changed")
"""

You check that path is not a symlink, then you open it, perform fstat on it, and check that the mode is the same.
But if someone replaces path by a symlink to a directory with the same mode, then rmtree won't catch this. You should also compare st_dev and st_ino to make sure we're dealing with the same file.

One more thing :-)
"""
        fd = os.open(path, os.O_RDONLY)
        try:
            mode2 = os.fstat(fd).st_mode
            if mode1 != mode2:
                raise OSError("Target changed")
        except OSError:
            onerror(os.fstat, fd, sys.exc_info())
            # can't continue if target has changed
            return
"""

Here `fd` is not closed (there might be other places leaking FD).

Finally, since writting a such code is tricky, what do you - all - think of making this a generic walker method that would take as argument the methods to call on a directory and on a file (or link), so that we could reuse it to write chmodtree(), chowntree() and friends?

----------
nosy: +neologix

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue4489>
_______________________________________