Re: euidaccess

[Bruno Haible]

Jim Meyering wrote:
> For some of the same reasons, the euidaccess function/module should be
> avoided, simply because it cannot be used robustly -- by its very nature.
> Use faccessat instead.

Here's a proposed doc update and additional warnings for the use of
'access' and 'euidaccess' (guarded by GNULIB_POSIXCHECK):


2010-03-06  Bruno Haible  <address@hidden>

        Clarify access, euidaccess, faccessat.
        * doc/posix-functions/faccessat.texi: Mention security problem under
        "Other problems", not "Portability problems".
        * doc/posix-functions/access.texi: Likewise. Mention a related security
        problem.
        * doc/glibc-functions/euidaccess.texi: Mention security problems.
        * lib/euidaccess.c: Add comments about platforms.
        * lib/unistd.in.h (access, euidaccess): Add warnings.

--- doc/posix-functions/faccessat.texi.orig     Sat Mar  6 16:03:05 2010
+++ doc/posix-functions/faccessat.texi  Sat Mar  6 15:57:12 2010
@@ -19,8 +19,12 @@
 
 Portability problems not fixed by Gnulib:
 @itemize
address@hidden itemize
+
+Other problems of this function:
address@hidden
 @item
 There is an inherent race between calling this function and performing
-some action based on the results; you should think twice before
-trusting this function in a set-uid or set-gid program.
+some action based on the results; you should think twice before trusting
+this function, especially in a set-uid or set-gid program.
 @end itemize
--- doc/glibc-functions/euidaccess.texi.orig    Sat Mar  6 16:03:05 2010
+++ doc/glibc-functions/euidaccess.texi Sat Mar  6 16:01:48 2010
@@ -15,3 +15,15 @@
 Portability problems not fixed by Gnulib:
 @itemize
 @end itemize
+
+Other problems of this function:
address@hidden
address@hidden
+There is an inherent race between calling this function and performing
+some action based on the results; you should think twice before trusting
+this function, especially in a set-uid or set-gid program.
address@hidden
+This function does not have an option for not following symbolic links
+(like @code{stat} versus @code{lstat}).  If you need this option, use
+the Gnulib module @code{faccessat} with the @code{AT_EACCESS} flag.
address@hidden itemize
--- doc/posix-functions/access.texi.orig        Sat Mar  6 16:03:05 2010
+++ doc/posix-functions/access.texi     Sat Mar  6 16:01:52 2010
@@ -16,8 +16,16 @@
 This function uses the effective id instead of the real id on some
 platforms:
 Cygwin 1.5.x.
address@hidden itemize
+
+Other problems of this function:
address@hidden
 @item
 There is an inherent race between calling this function and performing
-some action based on the results; you should think twice before
-trusting this function in a set-uid or set-gid program.
+some action based on the results; you should think twice before trusting
+this function, especially in a set-uid or set-gid program.
address@hidden
+This function does not have an option for not following symbolic links
+(like @code{stat} versus @code{lstat}).  If you need this option, use
+the Gnulib module @code{faccessat}.
 @end itemize
--- lib/euidaccess.c.orig       Sat Mar  6 16:03:05 2010
+++ lib/euidaccess.c    Sat Mar  6 15:40:05 2010
@@ -78,15 +78,15 @@
 int
 euidaccess (const char *file, int mode)
 {
-#if HAVE_FACCESSAT
+#if HAVE_FACCESSAT                      /* glibc */
   return faccessat (AT_FDCWD, file, mode, AT_EACCESS);
-#elif defined EFF_ONLY_OK
+#elif defined EFF_ONLY_OK               /* IRIX, OSF/1, Interix */
   return access (file, mode | EFF_ONLY_OK);
-#elif defined ACC_SELF
+#elif defined ACC_SELF                  /* AIX */
   return accessx (file, mode, ACC_SELF);
-#elif HAVE_EACCESS
+#elif HAVE_EACCESS                      /* FreeBSD */
   return eaccess (file, mode);
-#else
+#else       /* MacOS X, NetBSD, OpenBSD, HP-UX, Solaris, Cygwin, mingw, BeOS */
 
   uid_t uid = getuid ();
   gid_t gid = getgid ();
--- lib/unistd.in.h.orig        Sat Mar  6 16:03:05 2010
+++ lib/unistd.in.h     Sat Mar  6 15:50:25 2010
@@ -146,6 +146,13 @@
 #endif
 
 
+#if defined GNULIB_POSIXCHECK
+/* The access() function is a security risk.  */
+_GL_WARN_ON_USE (access, "the access function is a security risk - "
+                 "use the gnulib module faccessat instead");
+#endif
+
+
 #if @GNULIB_CHOWN@
 # if @REPLACE_CHOWN@
 #  undef chown
@@ -269,6 +276,11 @@
    the current process.  */
 extern int euidaccess (const char *filename, int mode) _GL_ARG_NONNULL ((1));
 # endif
+# if defined GNULIB_POSIXCHECK
+/* Like access(), this function is a security risk.  */
+_GL_WARN_ON_USE (euidaccess, "the euidaccess function is a security risk - "
+                 "use the gnulib module faccessat instead");
+# endif
 #elif defined GNULIB_POSIXCHECK
 # undef euidaccess
 # if HAVE_RAW_DECL_EUIDACCESS