Use faccessat, not access, when checking file permissions.

This fixes a bug that has been present in Emacs since its creation.
It was reported by Chris Torek in 1983 even before GNU Emacs existed,
which must set some sort of record.  (Torek's bug report was against
a predecessor of GNU Emacs, but GNU Emacs happened to have the
same common flaw.)  See Torek's Usenet posting
"setuid/setgid programs & Emacs" Article-I.D.: sri-arpa.858
Posted: Fri Apr  8 14:18:56 1983.
* .bzrignore: Add lib/fcntl.h.
* configure.ac (euidaccess): Remove check; gnulib does this for us now.
(gl_FCNTL_O_FLAGS): Define a dummy version.
* lib/at-func.c, lib/euidaccess.c, lib/faccessat.c, lib/fcntl.in.h:
* lib/getgroups.c, lib/group-member.c, lib/root-uid.h:
* lib/xalloc-oversized.h, m4/euidaccess.m4, m4/faccessat.m4:
* m4/fcntl_h.m4, m4/getgroups.m4, m4/group-member.m4:
New files, from gnulib.
* lib/gnulib.mk, m4/gnulib-comp.m4: Regenerate.
* admin/merge-gnulib (GNULIB_MODULES): Add faccessat.
(GNULIB_TOOL_FLAGS): Avoid at-internal, fchdir, malloc-posix,
openat-die, openat-h, save-cwd.  Do not avoid fcntl-h.
Omit gnulib's m4/fcntl-o.m4.
* nt/inc/ms-w32.h (AT_FDCWD, AT_EACCESS): New symbols.
(access): Remove.
(faccessat): New macro.
* src/Makefile.in (LIB_EACCESS): New macro.
(LIBES): Use it.
* src/callproc.c (init_callproc):
* src/charset.c (init_charset):
* src/fileio.c (check_existing, check_executable, check_writable)
(Ffile_readable_p):
* src/lread.c (openp, load_path_check):
* src/process.c (allocate_pty):
* src/xrdb.c (file_p):
Use effective UID when checking permissions, not real UID.
* src/callproc.c (init_callproc):
* src/charset.c (init_charset):
* src/lread.c (load_path_check, init_lread):
Test whether directories are accessible, not merely whether they exist.
* src/conf_post.h (GNULIB_SUPPORT_ONLY_AT_FDCWD): New macro.
* src/fileio.c (check_existing, check_executable, check_writable)
(Ffile_readable_p):
Use symbolic names instead of integers for the flags, as they're
portable now.
(check_writable): New arg AMODE.  All uses changed.
Set errno on failure.
(Ffile_readable_p): Use faccessat, not stat + open + close.
(Ffile_writable_p): No need to call check_existing + check_writable.
Just call check_writable and then look at errno.  This saves a syscall.
dir should never be nil; replace an unnecessary runtime check
with an eassert.  When checking the parent directory of a nonexistent
file, check that the directory is searchable as well as writable, as
we can't create files in unsearchable directories.
(file_directory_p): New function, which uses 'stat' on most platforms
but faccessat with D_OK (for efficiency) if WINDOWSNT.
(Ffile_directory_p, Fset_file_times): Use it.
(file_accessible_directory_p): New function, which uses a single
syscall for efficiency.
(Ffile_accessible_directory_p): Use it.
* src/xrdb.c (file_p): Use file_directory_p.
* src/lisp.h (file_directory_p, file_accessible_directory_p): New decls.
* src/lread.c (openp): When opening a file, use fstat rather than
stat, as that avoids a permissions race.  When not opening a file,
use file_directory_p rather than stat.
(dir_warning): First arg is now a usage string, not a format.
Use errno.  All uses changed.
* src/nsterm.m (ns_term_init): Remove unnecessary call to file-readable
that merely introduced a race.
* src/process.c, src/sysdep.c, src/term.c: All uses of '#ifdef O_NONBLOCK'
changed to '#if O_NONBLOCK', to accommodate gnulib O_* style,
and similarly for the other O_* flags.
* src/w32.c (sys_faccessat): Rename from sys_access and switch to
faccessat's API.  All uses changed.
* src/xrdb.c: Do not include <sys/stat.h>; no longer needed.
(magic_db): Rename from magic_file_p.
(magic_db, search_magic_path): Return an XrmDatabase rather than a
char *, so that we don't have to test for file existence
separately from opening the file for reading.  This removes a race
fixes a permission-checking problem, and simplifies the code.
All uses changed.
(file_p): Remove; no longer needed.

Fixes: debbugs:12632

1 files changed, 39 insertions, 62 deletions

diff --git a/src/xrdb.c b/src/xrdb.c
index 9d056a607e4..59b0876ebf8 100644
--- a/src/xrdb.c
+++ b/src/xrdb.c
@@ -41,7 +41,6 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 #ifdef HAVE_PWD_H
 #include <pwd.h>
 #endif
-#include <sys/stat.h>
 
 #ifdef USE_MOTIF
 /* For Vdouble_click_time.  */
@@ -50,7 +49,6 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 
 char *x_get_string_resource (XrmDatabase rdb, const char *name,
 			     const char *class);
-static int file_p (const char *filename);
 
 
 /* X file search path processing.  */
@@ -108,7 +106,7 @@ x_get_customization_string (XrmDatabase db, const char *name,
 		database associated with display.
 		(This is x_customization_string.)
 
-   Return the expanded file name if it exists and is readable, and
+   Return the resource database if its file was read successfully, and
    refers to %L only when the LANG environment variable is set, or
    otherwise provided by X.
 
@@ -117,10 +115,11 @@ x_get_customization_string (XrmDatabase db, const char *name,
 
    Return NULL otherwise.  */
 
-static char *
-magic_file_p (const char *string, ptrdiff_t string_len, const char *class,
-	      const char *escaped_suffix)
+static XrmDatabase
+magic_db (const char *string, ptrdiff_t string_len, const char *class,
+	  const char *escaped_suffix)
 {
+  XrmDatabase db;
   char *lang = getenv ("LANG");
 
   ptrdiff_t path_size = 100;
@@ -217,14 +216,9 @@ magic_file_p (const char *string, ptrdiff_t string_len, const char *class,
     }
 
   path[path_len] = '\0';
-
-  if (! file_p (path))
-    {
-      xfree (path);
-      return NULL;
-    }
-
-  return path;
+  db = XrmGetFileDatabase (path);
+  xfree (path);
+  return db;
 }
 
 
@@ -258,22 +252,11 @@ gethomedir (void)
 }
 
 
-static int
-file_p (const char *filename)
-{
-  struct stat status;
-
-  return (access (filename, 4) == 0             /* exists and is readable */
-	  && stat (filename, &status) == 0	/* get the status */
-	  && (S_ISDIR (status.st_mode)) == 0);	/* not a directory */
-}
-
-
 /* Find the first element of SEARCH_PATH which exists and is readable,
    after expanding the %-escapes.  Return 0 if we didn't find any, and
    the path name of the one we found otherwise.  */
 
-static char *
+static XrmDatabase
 search_magic_path (const char *search_path, const char *class,
 		   const char *escaped_suffix)
 {
@@ -286,18 +269,16 @@ search_magic_path (const char *search_path, const char *class,
 
       if (p > s)
 	{
-	  char *path = magic_file_p (s, p - s, class, escaped_suffix);
-	  if (path)
-	    return path;
+	  XrmDatabase db = magic_db (s, p - s, class, escaped_suffix);
+	  if (db)
+	    return db;
 	}
       else if (*p == ':')
 	{
-	  char *path;
-
-	  s = "%N%S";
-	  path = magic_file_p (s, strlen (s), class, escaped_suffix);
-	  if (path)
-	    return path;
+	  static char const ns[] = "%N%S";
+	  XrmDatabase db = magic_db (ns, strlen (ns), class, escaped_suffix);
+	  if (db)
+	    return db;
 	}
 
       if (*p == ':')
@@ -312,21 +293,12 @@ search_magic_path (const char *search_path, const char *class,
 static XrmDatabase
 get_system_app (const char *class)
 {
-  XrmDatabase db = NULL;
   const char *path;
-  char *p;
 
   path = getenv ("XFILESEARCHPATH");
   if (! path) path = PATH_X_DEFAULTS;
 
-  p = search_magic_path (path, class, 0);
-  if (p)
-    {
-      db = XrmGetFileDatabase (p);
-      xfree (p);
-    }
-
-  return db;
+  return search_magic_path (path, class, 0);
 }
 
 
@@ -340,35 +312,40 @@ get_fallback (Display *display)
 static XrmDatabase
 get_user_app (const char *class)
 {
+  XrmDatabase db = 0;
   const char *path;
-  char *file = 0;
-  char *free_it = 0;
 
   /* Check for XUSERFILESEARCHPATH.  It is a path of complete file
      names, not directories.  */
-  if (((path = getenv ("XUSERFILESEARCHPATH"))
-       && (file = search_magic_path (path, class, 0)))
+  path = getenv ("XUSERFILESEARCHPATH");
+  if (path)
+    db = search_magic_path (path, class, 0);
 
+  if (! db)
+    {
       /* Check for APPLRESDIR; it is a path of directories.  In each,
 	 we have to search for LANG/CLASS and then CLASS.  */
-      || ((path = getenv ("XAPPLRESDIR"))
-	  && ((file = search_magic_path (path, class, "/%L/%N"))
-	      || (file = search_magic_path (path, class, "/%N"))))
+      path = getenv ("XAPPLRESDIR");
+      if (path)
+	{
+	  db = search_magic_path (path, class, "/%L/%N");
+	  if (!db)
+	    db = search_magic_path (path, class, "/%N");
+	}
+    }
 
+  if (! db)
+    {
       /* Check in the home directory.  This is a bit of a hack; let's
 	 hope one's home directory doesn't contain any %-escapes.  */
-      || (free_it = gethomedir (),
-	  ((file = search_magic_path (free_it, class, "%L/%N"))
-	   || (file = search_magic_path (free_it, class, "%N")))))
-    {
-      XrmDatabase db = XrmGetFileDatabase (file);
-      xfree (file);
-      xfree (free_it);
-      return db;
+      char *home = gethomedir ();
+      db = search_magic_path (home, class, "%L/%N");
+      if (! db)
+	db = search_magic_path (home, class, "%N");
+      xfree (home);
     }
 
-  xfree (free_it);
-  return NULL;
+  return db;
 }