[prev in list] [next in list] [prev in thread] [next in thread] 

List:       coreutils-bug
Subject:    Re: chdir-safer can go?
From:       Paul Eggert <eggert () CS ! UCLA ! EDU>
Date:       2006-07-19 23:43:39
Message-ID: 874pxd2mac.fsf () penguin ! cs ! ucla ! edu
[Download RAW message or body]

Jim Meyering <jim@meyering.net> writes:

> True, even with chdir_no_follow, there is a race in that an attacker
> can replace a just-created empty directory with a different _directory_.
> But that poses very little risk.

Yes, I see no way to fix that race short of new system calls.

There is another race you didn't mention: when you are executing
something like "mkdir -m 0000 FOO", an attacker can hard-link an
unreadable non-directory in place of a newly-created FOO before mkdir
chmods it.  But this is an unavoidable race, since "mkdir -m 0000 FOO"
should never let FOO be readable, even temporarily.  And the old code
suffered from this race even worse, since it used lchown/lchmod even
in the normal case like "mkdir -m 0755 FOO" where the directory is
user-readable, so this is not a regression.

> I'd prefer an implementation that provides protection against the
> replace-just-created-dir-with-symlink race condition.

Me too, and I thought I had done that, but now that you mention it I
missed a race.  I was worried only about the case where an attacker
replaces a newly created directory with a symlink to a non-directory,
since I assumed that the problem you're most worried about is creating
setuid/setgid executables.  But you are also worried about when an
attacker replaces a newly created directory with a symlink to a
directory; this won't create a setuid/setgid executable but it still
might lead to other problems.  I will look into this and fix the bug
as soon as I can.  (And if my analysis here is wrong please let me
know....)

----

In case you'd like to check my reasoning, here's my "proof" that
current CVS handles any race involving creating symlinks to
non-directories.  (It's a tricky area, so kibitizing is welcome....)

There are two cases.  First, current CVS never uses dangerous
operations like chown or chmod on a newly-created ancestor directory
A.  The only operations it applies to A involve resolving file names
like A/FOO, where A must be a directory or nothing changes.  So the
race cannot occur here.

Second, when mkdir and install create a non-ancestor directory DIR,
they always do so via mkdir (DIR, MODE) and then invoke dirchownmod
(DIR, MODE, ...); this in turn either opens DIR with O_NOFOLLOW |
O_DIRECTORY and uses fchown/fchmod or, if the open fails, uses
lchown/lchmod.  So the race condition cannot occur here either.

There are also races if the operating system does not support
O_NOFOLLOW, lchown, or lchmod, but this is not a regression either.
(If O_DIRECTORY is not present, the code defends itself by statting
the open file and checking whether it's a directory, so that's not an
issue.)



[prev in list] [next in list] [prev in thread] [next in thread]