From e601c0259e9e6a48e04ce3e0ff793cb564a89716 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 16 Mar 2000 20:55:37 +0000 Subject: Fixes to add "paranoid" option to popen. Checks some basic things. Jeremy (This used to be commit 3b8cbb10de322fd7a1063fb5b681790b10d24ab0) --- source3/include/proto.h | 2 +- source3/lib/system.c | 57 ++++++++++++++++++++++++++++++++++++++++++- source3/param/loadparm.c | 20 ++++++++++++++- source3/printing/print_svid.c | 2 +- source3/smbd/dfree.c | 2 +- 5 files changed, 78 insertions(+), 5 deletions(-) diff --git a/source3/include/proto.h b/source3/include/proto.h index d96d2f77c0..27d3de49b8 100644 --- a/source3/include/proto.h +++ b/source3/include/proto.h @@ -233,7 +233,7 @@ DIR *wsys_opendir(const smb_ucs2_t *wfname); smb_ucs2_t *wsys_getwd(smb_ucs2_t *s); int wsys_chown(const smb_ucs2_t *wfname, uid_t uid, gid_t gid); int wsys_chroot(const smb_ucs2_t *wfname); -FILE *sys_popen(const char *command, const char *mode); +FILE *sys_popen(const char *command, const char *mode, BOOL paranoid); int sys_pclose( FILE *fp); /*The following definitions come from lib/talloc.c */ diff --git a/source3/lib/system.c b/source3/lib/system.c index 9ef0af494f..8ac07e26a5 100644 --- a/source3/lib/system.c +++ b/source3/lib/system.c @@ -963,7 +963,7 @@ typedef struct _popen_list static popen_list *popen_chain; -FILE *sys_popen(const char *command, const char *mode) +FILE *sys_popen(const char *command, const char *mode, BOOL paranoid) { int parent_end, child_end; int pipe_fds[2]; @@ -999,6 +999,61 @@ FILE *sys_popen(const char *command, const char *mode) if(!(argl = extract_args(command))) goto err_exit; + if(paranoid) { + /* + * Do some basic paranioa checks. Do a stat on the parent + * directory and ensure it's not world writable. Do a stat + * on the file itself and ensure it's owned by root and not + * world writable. Note this does *not* prevent symlink races, + * but is a generic "don't let the admin screw themselves" + * check. + */ + + SMB_STRUCT_STAT st; + pstring dir_name; + char *ptr = strrchr(argl[0], '/'); + + if(sys_stat(argl[0], &st) != 0) + goto err_exit; + + if((st.st_uid != (uid_t)0) || (st.st_mode & S_IWOTH)) { + errno = EACCES; + goto err_exit; + } + + if(!ptr) { + /* + * No '/' in name - use current directory. + */ + pstrcpy(dir_name, "."); + } else { + + /* + * Copy into a pstring and do the checks + * again (in case we were length tuncated). + */ + + pstrcpy(dir_name, argl[0]); + ptr = strrchr(dir_name, '/'); + if(!ptr) { + errno = EINVAL; + goto err_exit; + } + if(strcmp(dir_name, "/") != 0) + *ptr = '\0'; + if(!dir_name[0]) + pstrcpy(dir_name, "."); + } + + if(sys_stat(argl[0], &st) != 0) + goto err_exit; + + if(!S_ISDIR(st.st_mode) || (st.st_mode & S_IWOTH)) { + errno = EACCES; + goto err_exit; + } + } + entry->child_pid = fork(); if (entry->child_pid == -1) { diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c index 48dc6cf123..4f6178a569 100644 --- a/source3/param/loadparm.c +++ b/source3/param/loadparm.c @@ -2039,7 +2039,7 @@ static BOOL handle_source_env(char *pszParmValue,char **ptr) DEBUG(4, ("handle_source_env: source env from pipe\n")); p++; - if ((env = sys_popen(p, "r")) == NULL) { + if ((env = sys_popen(p, "r", True)) == NULL) { DEBUG(0,("handle_source_env: Failed to popen %s. Error was %s\n", p, strerror(errno) )); return(False); } @@ -2050,11 +2050,29 @@ static BOOL handle_source_env(char *pszParmValue,char **ptr) } else { + SMB_STRUCT_STAT st; + DEBUG(4, ("handle_source_env: source env from file %s\n", fname)); if ((env = sys_fopen(fname, "r")) == NULL) { DEBUG(0,("handle_source_env: Failed to open file %s, Error was %s\n", fname, strerror(errno) )); return(False); } + + /* + * Ensure this file is owned by root and not writable by world. + */ + if(fstat(fileno(env), &st) != 0) { + DEBUG(0,("handle_source_env: Failed to stat file %s, Error was %s\n", fname, strerror(errno) )); + fclose(env); + return False; + } + + if((st.st_uid != (uid_t)0) || (st.st_mode & S_IWOTH)) { + DEBUG(0,("handle_source_env: unsafe to source env file %s. Not owned by root or world writable\n", fname )); + fclose(env); + return False; + } + result=source_env(env); fclose(env); } diff --git a/source3/printing/print_svid.c b/source3/printing/print_svid.c index 9891e158a2..21e8eeb643 100644 --- a/source3/printing/print_svid.c +++ b/source3/printing/print_svid.c @@ -49,7 +49,7 @@ static void populate_printers(void) { FILE *fp; - if ((fp = sys_popen("/usr/bin/lpstat -v", "r")) != NULL) { + if ((fp = sys_popen("/usr/bin/lpstat -v", "r", False)) != NULL) { char buf[BUFSIZ]; while (fgets(buf, sizeof (buf), fp) != NULL) { diff --git a/source3/smbd/dfree.c b/source3/smbd/dfree.c index 7866d60277..0a892bad05 100644 --- a/source3/smbd/dfree.c +++ b/source3/smbd/dfree.c @@ -217,7 +217,7 @@ static SMB_BIG_UINT disk_free(char *path, BOOL small_query, slprintf (line, sizeof(pstring) - 1, "%s %s", dfree_command, path); DEBUG (3, ("disk_free: Running command %s\n", line)); - pp = sys_popen(line, "r"); + pp = sys_popen(line, "r", False); if (pp) { fgets(line, sizeof(pstring), pp); line[sizeof(pstring)-1] = '\0'; -- cgit