Re: bug in chdir-safer

[James Youngman]

On Thu, Dec 22, 2005 at 03:03:35PM +0000, Eric Blake wrote:
> I like the newly-added chdir-safer.  However, you need to
> provide a dummy #define O_NOFOLLOW 0 for platforms
> like cygwin which have not yet implemented this feature.

I haven't looked at the code recently (and don't have time to do this
today), but my first reaction is that this isn't such a good idea.

If O_NOFOLLOW is not provided by the system, the open() call can
follow a symbolic link which could be (depending on the circumstances)
insecure.  See the "Security Considerations" chapter of the findutils
4.2.x Texinfo documentation for a detailed explanation as to why.

If O_NOFOLLOW isn't available, an implementation should probably use
lstat() to double-check that it has ended up in the subdirectory it
expected to.

An additional wrinkle is that at compile time O_NOFOLLOW may be
provided, but the resulting binary might be run on a system lacking
support for O_NOFOLLOW.  While one might ordinarily just dismiss this
by saying "well, that's not the recommended way to do things" this
issue is security-related, so we should be cautious.  Findutils (in
4.2.x, before the switch to fts()) uses the following check:


/* Determine if we can use O_NOFOLLOW.
 */
#if defined(O_NOFOLLOW)
static boolean 
check_nofollow(void)
{
  struct utsname uts;
  float  release;

  if (0 == uname(&uts))
    {
      /* POSIX requires that atof() ignore "unrecognised suffixes". */
      release = atof(uts.release);
      
      if (0 == strcmp("Linux", uts.sysname))
        {
          /* Linux kernels 2.1.126 and earlier ignore the O_NOFOLLOW flag. */
          return release >= 2.2; /* close enough */
        }
      else if (0 == strcmp("FreeBSD", uts.sysname)) 
        {
          /* FreeBSD 3.0-CURRENT and later support it */
          return release >= 3.1;
        }
    }

  /* Well, O_NOFOLLOW was defined, so we'll try to use it. */
  return true;
}
#endif


Regards,
James.