Simplify validate_exec() by using access(2) to check file permissions,

rather than trying to implement the equivalent logic by hand.  The motivation
for the original coding appears to have been to check with the effective uid's
permissions not the real uid's; but there is no longer any difference, because
we don't run the postmaster setuid (indeed, main.c enforces that they're the
same).  Using access() means we will get it right in situations the original
coding failed to handle, such as ACL-based permissions.  Besides it's a lot
shorter, cleaner, and more thread-safe.  Per bug #5275 from James Bellinger.

diff --git a/src/port/exec.c b/src/port/exec.c
index 7f41e57..a4f8b16 100644 (file)
--- a/src/port/exec.c
+++ b/src/port/exec.c
@@ -9,7 +9,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/port/exec.c,v 1.66 2010/01/02 16:58:13 momjian Exp $
+ *       $PostgreSQL: pgsql/src/port/exec.c,v 1.67 2010/01/14 00:14:06 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -20,25 +20,11 @@
 #include "postgres_fe.h"
 #endif
 
-#include <grp.h>
-#include <pwd.h>
 #include <signal.h>
 #include <sys/stat.h>
 #include <sys/wait.h>
 #include <unistd.h>
 
-#ifndef S_IRUSR                                        /* XXX [TRH] should be in a header */
-#define S_IRUSR                 S_IREAD
-#define S_IWUSR                 S_IWRITE
-#define S_IXUSR                 S_IEXEC
-#define S_IRGRP                 ((S_IRUSR)>>3)
-#define S_IWGRP                 ((S_IWUSR)>>3)
-#define S_IXGRP                 ((S_IXUSR)>>3)
-#define S_IROTH                 ((S_IRUSR)>>6)
-#define S_IWOTH                 ((S_IWUSR)>>6)
-#define S_IXOTH                 ((S_IXUSR)>>6)
-#endif
-
 #ifndef FRONTEND
 /* We use only 3-parameter elog calls in this file, for simplicity */
 /* NOTE: caller must provide gettext call around str! */
@@ -70,20 +56,12 @@ static int
 validate_exec(const char *path)
 {
        struct stat buf;
-
-#ifndef WIN32
-       uid_t           euid;
-       struct group *gp;
-       struct passwd *pwp;
-       int                     i;
-       int                     in_grp = 0;
-#else
-       char            path_exe[MAXPGPATH + sizeof(".exe") - 1];
-#endif
        int                     is_r;
        int                     is_x;
 
 #ifdef WIN32
+       char            path_exe[MAXPGPATH + sizeof(".exe") - 1];
+
        /* Win32 requires a .exe suffix for stat() */
        if (strlen(path) >= strlen(".exe") &&
                pg_strcasecmp(path + strlen(path) - strlen(".exe"), ".exe") != 0)
@@ -107,61 +85,17 @@ validate_exec(const char *path)
                return -1;
 
        /*
-        * Ensure that we are using an authorized executable.
-        */
-
-       /*
         * Ensure that the file is both executable and readable (required for
         * dynamic loading).
         */
-#ifdef WIN32
+#ifndef WIN32
+       is_r = (access(path, R_OK) == 0);
+       is_x = (access(path, X_OK) == 0);
+#else
        is_r = buf.st_mode & S_IRUSR;
        is_x = buf.st_mode & S_IXUSR;
-       return is_x ? (is_r ? 0 : -2) : -1;
-#else
-       euid = geteuid();
-
-       /* If owned by us, just check owner bits */
-       if (euid == buf.st_uid)
-       {
-               is_r = buf.st_mode & S_IRUSR;
-               is_x = buf.st_mode & S_IXUSR;
-               return is_x ? (is_r ? 0 : -2) : -1;
-       }
-
-       /* OK, check group bits */
-
-       pwp = getpwuid(euid);           /* not thread-safe */
-       if (pwp)
-       {
-               if (pwp->pw_gid == buf.st_gid)  /* my primary group? */
-                       ++in_grp;
-               else if (pwp->pw_name &&
-                                (gp = getgrgid(buf.st_gid)) != NULL && /* not thread-safe */
-                                gp->gr_mem != NULL)
-               {                                               /* try list of member groups */
-                       for (i = 0; gp->gr_mem[i]; ++i)
-                       {
-                               if (!strcmp(gp->gr_mem[i], pwp->pw_name))
-                               {
-                                       ++in_grp;
-                                       break;
-                               }
-                       }
-               }
-               if (in_grp)
-               {
-                       is_r = buf.st_mode & S_IRGRP;
-                       is_x = buf.st_mode & S_IXGRP;
-                       return is_x ? (is_r ? 0 : -2) : -1;
-               }
-       }
-
-       /* Check "other" bits */
-       is_r = buf.st_mode & S_IROTH;
-       is_x = buf.st_mode & S_IXOTH;
-       return is_x ? (is_r ? 0 : -2) : -1;
 #endif
+       return is_x ? (is_r ? 0 : -2) : -1;
 }
 
 
@@ -178,10 +112,6 @@ validate_exec(const char *path)
  * path because we will later change working directory.  Finally, we want
  * a true path not a symlink location, so that we can locate other files
  * that are part of our installation relative to the executable.
- *
- * This function is not thread-safe because it calls validate_exec(),
- * which calls getgrgid().     This function should be used only in
- * non-threaded binaries, not in library routines.
  */
 int
 find_my_exec(const char *argv0, char *retpath)