bug#12632: file permissions checking mishandled when setuid

[Eli Zaretskii]

> Date: Sat, 13 Oct 2012 23:16:44 -0700
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: 12632@debbugs.gnu.org
> 
> > How will the new code work if 'dir' is nil?
> 
> 'dir' can't be nil there.

file-name-directory can return nil, so 'dir' can be nil if the
function is called with a name of a non-exiting file that has no
leading directories.

> > Also, what about lread.c:openp, around line 1555: doesn't it want
> > 'euidaccess' as well, rather than 'stat'?
> 
> Sure, we can do that.  Done in the revised patch.
> [...]
> +
> +           /* Check that we can access or open it.  */
> +           if (NATNUMP (predicate))
> +             fd = (((XFASTINT (predicate) & ~INT_MAX) == 0
> +                    && euidaccess (pfn, XFASTINT (predicate)) == 0
> +                    && ! file_directory_p (pfn))
> +                   ? 1 : -1);
> +           else

This won't compile on Windows, since there's no 'euidaccess' (yet).

>        if (STRINGP (dirfile))
>          {
>            dirfile = Fdirectory_file_name (dirfile);
> -          if (access (SSDATA (dirfile), 0) < 0)
> +          if (euidaccess (SSDATA (dirfile), F_OK) != 0)
>              dir_warning ("Warning: Lisp directory `%s' does not exist.\n",
>                           XCAR (path_tail));
>          }

Same here.

> > I don't understand why is it a good idea to use 'euidaccess' in
> > check_existing.  Isn't the fact of the mere existence of a file
> > independent of user's access rights?
> 
> No, because you cannot even stat a file that's in a directory that you
> can't search.  Using 'access' rather than 'euidaccess' might
> let a setuid Emacs search directories that it shouldn't be able
> to search, or vice versa.

But that's not a concern for check_existing, I think.  That's a
concern for check_writable, file-accessible-directory, etc.

IOW, Emacs should be able to test whether a file exists even if it
will be unable to access it later.

In any case, using 'euidaccess' here subtly changes the semantics of
file-exists-p, so if we decide to do that, it should be documented as
an incompatible change.