Bug#508867: stale nfs filehandle

[Tim Connors <reportbug@rather.puzzling.org>]

severity grave
tags 508867 security
thanks

On Thu, 29 Jan 2009, Tim Connors wrote:

> Hmmm, Trond reckons this race condition always has existed in the kernel,
> implying that since userland can work around it, it should:
> http://bugzilla.kernel.org/show_bug.cgi?id=12557
>
> Would it be possible to, upon an ESTALE return result from the acccess()
> call, to open() the file instead, and then perform an access() if indeed
> the latter is then still necessary?  That might be sufficient to
> invalidate the cache (since catting the Xauthority file or running xauth
> list seems to invalidate the cache).

As a test, I commented out the access() calls in AuGetAddr.c that are
commented as saying "checks REAL id", and this bug disappears.  But now
that I know the purpose of the access() call, I presume this is because
suid programs like xterm link against libxau6, and thus need to check the
access permissions of $XAUTHORITY using the real uid/gid rather than the
effective uid/gid.

Unfortunately, as per access(2):

       Warning: Using access() to check if a user is authorized to, for
       example, open a file before actually doing so using open(2)
       creates a security hole, because the user might exploit the
       short time interval between checking and opening the file to
       manipulate it.  For this reason, the use of this system call
       should be avoided.

So, it looks like to my untrained eye that one could set
XAUTHORITY=/tmp/hack/wtmp, where /tmp/hack is a symlink to a directory
called /tmp/hack.1, on the same filesystem as /var/log.  Then get libxau6
to read the contents of /tmp/hack/wtmp, then racily change the symlink of
/tmp/hack to point to /var/log before libxau6 writes the new $XAUTHORITY
back out to disk, and voila, corrupt the contents of /var/log/wtmp or any
other file of gid of /usr/bin/xterm.


Would it be better to temporarily drop priveleges and just do an open()
instead which will also have the side benefit of closing this bug and the
alleged security bug above?  As it stands, the access() presumably isn't
adding much security as per above, so should be removed altogether, also
closing this bug (just leaving the security bug that exists anyway, but
/var/log/wtmp is not that critical anyway).

-- 
TimC
Entropy isn't what it used to be.