[SECURITY] Re: PATCH: rm -rf and readdir bug in coreutils-6.7

[Mikulas Patocka]

BTW. looking at the code leads me to another observation --- all those 
security checks against 'rm'-vs-'mv' race break apart if the attacker is 
able to create directories with colliding inode number (which can happen 
on 64-bit filesystems that can store more than 2^32 files --- for example 
NFS v3, OCFS or my SPADFS filesystem)

Imagine this:
user creates directory /home/user/foo with i_ino colliding with home
user creates directory /home/user/foo/boo with i_ino colliding with user
user creates directory /home/user/foo/boo/bla with a lot of content
user persuades the root to delete /home/user/foo
while deleting, he moves /home/user/foo/boo/bla to /home/user/bla
--- rm will then carelessly remove all home directories.

Utilities like rm, cp or mv shouldn't probably execute open("..") or 
chdir("..") at all. The best implementation would be walk the whole path 
from initial file again when finished processing directory and keep cache 
of about 3 to 5 open handles to directories up to initial directory, so 
that walking the whole path can be avoided for most common cases (for 
performance reasons).

Another possibility to fix it (without rewriting the whole algorithm) 
would be to compare not only st_ino and st_dev but also st_uid and st_gid 
in macro SAME_INODE --- this way the user cannot arrange directories to 
delete other user's files. Although I'm still not sure it this is 100% 
safe.

Mikulas

> Hi
>
> I have a system that precaches directory content at opendir call and I found 
> that coreutils-6.7 rm -r command doesn't work on it (it used to work fine in 
> coreutils 5).
>
> The problem is this: when walking up to the root in directory tree, rm opens 
> parent directory with opendir, then deletes its subdirectory with rmdir and 
> then starts reading the parent with readdir --- readdir reads just deleted 
> entry, rm tries to delete it again and fails.
>
> opendir caching is allowed by standard 
> http://www.opengroup.org/onlinepubs/007908799/xsh/readdir.html ("If a file is 
> removed from or added to the directory after the most recent call to 
> opendir() or rewinddir(), whether a subsequent call to readdir() returns an 
> entry for that file is unspecified."), so the system behaves correctly and rm 
> has a bug.
>
> Here is a trace when I tried rm -rf /A and there was four directories 
> /A/B/C/D --- it correctly deletes D, but reads entry for 'D' again and fails.
>
> I made a fix --- it modifies AD_pop_and_chdir so that it only returns fd of a 
> directory but doesn't try to do fdopendir and do fdopendir in remove_dir 
> after the subdirectory is deleted - it seems to work but it makes the file 
> even more messy.
>
> Mikulas
>
> The trace of rm -rf /A, I added printfs to opendir, readdir and closedir to 
> see what happens:
>
> opendir '/A' -> BFFEF950
> readdir BFFEF950, pos 0: name '.', dt 4
> readdir BFFEF950, pos 1: name '..', dt 4
> readdir BFFEF950, pos 2: name 'B', dt 4
> opendir '/A/B' -> BFFEF9F0
> closedir: BFFEF950
> readdir BFFEF9F0, pos 0: name '.', dt 4
> readdir BFFEF9F0, pos 1: name '..', dt 4
> readdir BFFEF9F0, pos 2: name 'C', dt 4
> opendir '/A/B/C' -> BFFEF980
> closedir: BFFEF9F0
> readdir BFFEF980, pos 0: name '.', dt 4
> readdir BFFEF980, pos 1: name '..', dt 4
> readdir BFFEF980, pos 2: name 'D', dt 4
> opendir '/A/B/C/D' -> BFFEF9C0
> closedir: BFFEF980
> readdir BFFEF9C0, pos 0: name '.', dt 4
> readdir BFFEF9C0, pos 1: name '..', dt 4
> closedir: BFFEF9C0
>         --- note here: it opens the parent directory and then deletes the
>         subdirectory --- the subsequent readdir will read just deleted
>         entry
> opendir '/A/B/C' -> BFFEBE00
> removed directory: `A/B/C/D'
> readdir BFFEBE00, pos 0: name '.', dt 4
> readdir BFFEBE00, pos 1: name '..', dt 4
>         --- here: entry 'D' no longer exists, but it is read from cached
>         information on opendir
> readdir BFFEBE00, pos 2: name 'D', dt 4
> closedir: BFFEBE00
> opendir '/A/B' -> BFFEBE00
> readdir BFFEBE00, pos 0: name '.', dt 4
> readdir BFFEBE00, pos 1: name '..', dt 4
> readdir BFFEBE00, pos 2: name 'C', dt 4
> closedir: BFFEBE00
> opendir '/A' -> BFFEBE00
> readdir BFFEBE00, pos 0: name '.', dt 4
> readdir BFFEBE00, pos 1: name '..', dt 4
> readdir BFFEBE00, pos 2: name 'B', dt 4
> closedir: BFFEBE00