Fixes to add "paranoid" option to popen. Checks some basic things.

Jeremy

diff --git a/source/include/proto.h b/source/include/proto.h
index d96d2f77c0bac606c4137a36ac14ce977c19fc76..27d3de49b8890ad758d0f4eee93afdbd1d742927 100644 (file)
--- a/source/include/proto.h
+++ b/source/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/source/lib/system.c b/source/lib/system.c
index 9ef0af494f36cc3ec31d598a276818e7a38c4c69..8ac07e26a5f4cc6d564f11425194327dd809a577 100644 (file)
--- a/source/lib/system.c
+++ b/source/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/source/param/loadparm.c b/source/param/loadparm.c
index 48dc6cf123a984c68946c2ee83d0bdb613efa50e..4f6178a569007b85d03892da247243fa4cc1354e 100644 (file)
--- a/source/param/loadparm.c
+++ b/source/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/source/printing/print_svid.c b/source/printing/print_svid.c
index 9891e158a27c10f435d3270f4f980c099fba6dfa..21e8eeb643973d71ee2b2fda3ba0a35d55d9405c 100644 (file)
--- a/source/printing/print_svid.c
+++ b/source/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/source/smbd/dfree.c b/source/smbd/dfree.c
index 7866d60277d650478b6464a89df925522e404baa..0a892bad05e34118759b2c2a5eab6d4fc49fefc4 100644 (file)
--- a/source/smbd/dfree.c
+++ b/source/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';