Re: Race condition in "rm -r"

[glynn () SENSEI CO UK (Glynn Clements)]

Alex Belits wrote:

> > Use a statically-linked "rm" and "chroot /tmp" first.
> >
> > >   Maybe stat "." after chdir to verify that we ended up the
> > >   expected place?
>
>   More like;y getcwd() will be useful

A literal getcwd() would seem to be excessive; AFAICT, you only need
to check the parent-child relationship. Repeatedly checking the rest
of the path would seem wasteful. Wouldn't stat("..") suffice?

> -- there is nothing in stat that can
> tell us if we followed a link, and inode comparison may be unreliable.

Can you see any flaws in the following logic?

Even in the case where lstat() indicates a directory, but an attacker
replaces it with a symlink before the chdir(), a stat() on "." will
only (AFAIK) give the expected device/inode pair if the directory has
just been created.

If this is the case, then rm can't realistically be tricked into
deleting anything useful. Admittedly there is the theoretical
possibility of deleting a new directory, if the attacker knows exactly
when and where it is going to appear, but the window would appear to
be incredibly small. Not only would you have to win the lstat/chdir
race, but the right circumstances (a new directory is created at a
known location whilst root is running "rm" on an unsafe directory)
have to exist at the time.

However, just checking ".." would be both simpler and more reliable.

> > My "rm" (GNU fileutils 4.0) does this:
> >
> >     getdents(3, /* 45 entries */, 3933)     = 924
> >     lstat("Imakefile", {st_mode=S_IFREG|0644, st_size=2842, ...}) = 0
> >     unlink("Imakefile")                     = 0
> >     lstat("pixmaps", {st_mode=S_IFDIR|0755, st_size=1024, ...}) = 0
> >     chdir("pixmaps")                        = 0
> >     close(3)                                = 0
> > 1>  open(".", O_RDONLY|O_NONBLOCK)          = 3
> >     fcntl(3, F_SETFD, FD_CLOEXEC)           = 0
> > 2>  fstat(3, {st_mode=S_IFDIR|0755, st_size=1024, ...}) = 0
> >     lseek(3, 0, SEEK_CUR)                   = 0
> >     getdents(3, /* 49 entries */, 3933)     = 1112
> >     lstat("about.xpm", {st_mode=S_IFREG|0644, st_size=43055, ...}) = 0
> >     unlink("about.xpm")                     = 0
> >     lstat("apple.xpm", {st_mode=S_IFREG|0644, st_size=927, ...}) = 0
> >     unlink("apple.xpm")                     = 0
> >
> > Any suggestions as to why it is doing the fstat() in (2) if it isn't
> > checking for symlink games? [Note: I'm not saying that it *is*
> > checking, just that it seems odd if it isn't.]
>
>   Relevant piece from glibc 2.1.1 opendir() (other libraries probably do
> something similar):

OK; thanks for the info.

--
Glynn Clements <glynn () sensei co uk>