Re: some other problems with chmod-safer.c, chown.c, etc.

[Jim Meyering]

> I noticed a problem with chown.c (when CHOWN_MODIFIES_SYMLINK) and
> chmod-safer.c.  If the file being chmod'ed or chown'ed is a special
> file, opening it can cause undesirable side effects.  For example, it
> might cause a tape drive to be rewound.

[ In case it's not obvious to the readers, here we're talking about
  lib/chown.c.  It is used only on crufty old systems for which chown(2)
  affects symlinks (it should dereference them). ]

As for lib/chown.c, these days, I don't think it's worth spending time
worrying about security on systems that are so old.

As for the use of chmod_safer in mknod.c, bearing in mind that it is
called only for a device that has just been created, does it matter
if there might be such a side effect?

> Also, the current implementations of mkdir, mkfifo, and mknod arrange
> for files to be temporarily readable to the owner, even when the
> requested mode is that the file not be readable to the owner.  In the
> traditional Unix model this doesn't introduce a security-related race
> condition, since an attacker with owner (or root) privileges can set
> the file to be readable anyway.  But in the ACL world I'm not sure
> this holds: that is, isn't it possible that granting the owner
> temporary read permission can cause a real hole?

I don't know, but even if it can, I suspect it's one of
those theoretical-only possibilities.  Anyone who's concerned
about restricting access via ACLs would do well not to leave
a file owned by a potential attacker.

> Also, if a directory is temporarily mode 400, that can cause problems
> with other processes running 'find'.  Surely the temporary mode (if we
> want to continue to use it) should be 400 OR'ed in with the desired
> mode, not 400.

Sure.  That sounds better, in case some application finds
it useful to create a directory without owner-read access.

> It's not clear to me that these are the only problems with the code;
> there are some other fairly subtle issues involved, which I don't
> entirely follow.

I'm not surprised.
I've seen too many subtle bugs appear in code I once thought solid.
Give us details :-)

> A bigger issue: it's not clear to me how much safety chmod_safer adds.
> For example, if /tmp is world-writeable and is not sticky, then "mkdir
> -m 755 /tmp/a/b" is unsafe even with chmod_safer in place, since the
> attacker can replace /tmp/a with a symlink.  Another example: "mkdir
> -m 755 /tmp/a" is unsafe if the attacker can rename /tmp/a and rename
> a victim directory to /tmp/a before mkdir gets around to invoking
> chmod on what it thinks is the newly created /tmp/a.

We won't be able to avoid this latter scenario until there is a kernel
interface that defines a mkdir+open-like function that returns a
file descriptor corresponding to a just-created directory.
Such a function would be good not just for race avoidance,
but also for efficiency.

I'm not worried about someone replacing a just-created directory with
another one, because that's an unavoidable race.  The same holds for
the replace-existing-parent-with-symlink case.  The race that we *can*
avoid is the one involving a symlink attack, whereby someone replaces a
just-created directory or device with a symlink to some other directory.
I think avoiding that sort of race condition, even as it appeared in
mkdir, mknod and mkfifo is worth the price of a little more code.

Hmm... this made me realize there's another avoidable race:
when someone anticipates the second or subsequent mkdir call,
and creates a symlink in place of the target directory.  That's
easy to avoid.  Patch coming separately.

> I suspect that users who share world-writeable directories with
> untrusted users are fundamentally at risk, and that we cannot fix
> coreutils to be "secure" in that situation.  If so, perhaps we
> shouldn't bother to try, if trying causes problems in other areas.

I see your point, but some of the more predictable races *can*
(and should, imho) be avoided.