From 0fba3bef558036952f59846a7e7b48e1f5b9e479 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Thu, 14 Jan 2010 00:14:06 +0000
Subject: [PATCH] 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.
---
src/port/exec.c | 86 +++++--------------------------------------------
1 file changed, 8 insertions(+), 78 deletions(-)
diff --git a/src/port/exec.c b/src/port/exec.c
index 7f41e57a715..a4f8b16419f 100644
--- 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)
@@ -106,62 +84,18 @@ validate_exec(const char *path)
if (!S_ISREG(buf.st_mode))
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)
--
GitLab