- changed the umask handling. We now set the umask to 0 and explicitly

set the mode on all created files. I think this is a better policy.

- change the debug levels on some items

- fix a charset handling bug which affected foreign and extended
charset users

- no longer switch back to the original directory when idle, instead
switch to / as the original directory may not be readable by ordinary
users.

- fix some bugs where the create mode of files was not being
explicitly set (it was relying on the umask and using fopen). Not a
big bug as it only affected obscure commands like the messaging ops.

- got rid of the lock code in the lpq cache as its no longer needed

- rewrote smbrun to be faster and to remove the security hole. We now
don't actually need a external smbrun binary, its all done by smbd.

- add a more explicit warning about uids and gids of -1 or 65535

source/client/client.c

@@ -1521,7 +1521,7 @@ static void do_get(char *rname,char *lname,file_info *finfo1)
     get_total_time_ms += this_time;
     get_total_size += finfo.size;
 
-    DEBUG(2,("(%g kb/s) (average %g kb/s)\n",
+    DEBUG(1,("(%g kb/s) (average %g kb/s)\n",
 	     finfo.size / (1.024*this_time + 1.0e-4),
 	     get_total_size / (1.024*get_total_time_ms)));
   }
@@ -2022,7 +2022,7 @@ static void do_put(char *rname,char *lname,file_info *finfo)
     put_total_time_ms += this_time;
     put_total_size += finfo->size;
 
-    DEBUG(2,("(%g kb/s) (average %g kb/s)\n",
+    DEBUG(1,("(%g kb/s) (average %g kb/s)\n",
 	     finfo->size / (1.024*this_time + 1.0e-4),
 	     put_total_size / (1.024*put_total_time_ms)));
   }

source/include/local.h

@@ -154,4 +154,8 @@
 #define LONG_CONNECT_TIMEOUT 30
 #define SHORT_CONNECT_TIMEOUT 5
 
+
+/* the directory to sit in when idle */
+#define IDLE_DIR "/"
+
 #endif

source/include/proto.h

@@ -805,7 +805,7 @@ void init_uid(void);
 BOOL become_guest(void);
 BOOL become_user(int cnum, int uid);
 BOOL unbecome_user(void );
-int smbrun(char *cmd,char *outfile);
+int smbrun(char *cmd,char *outfile,BOOL shared);
 
 /*The following definitions come from  username.c  */
 

source/lib/charset.c

@@ -40,6 +40,8 @@ static void add_dos_char(int lower, int upper)
   if (lower && upper) {
     lower_char_map[(char)upper] = (char)lower;
     upper_char_map[(char)lower] = (char)upper;
+    lower_char_map[(char)lower] = (char)lower;
+    upper_char_map[(char)upper] = (char)upper;
   }
 }
 

source/lib/util.c

@@ -123,6 +123,7 @@ void reopen_logs(void)
 
       if (!strcsequal(fname,debugf) || !dbf || !file_exist(debugf,NULL))
 	{
+	  int oldumask = umask(022);
 	  strcpy(debugf,fname);
 	  if (dbf) fclose(dbf);
 	  if (append_log)
@@ -130,6 +131,7 @@ void reopen_logs(void)
 	  else
 	    dbf = fopen(debugf,"w");
 	  if (dbf) setbuf(dbf,NULL);
+	  umask(oldumask);
 	}
     }
   else
@@ -205,7 +207,9 @@ va_dcl
     {
       if (!dbf) 
 	{
+	  int oldumask = umask(022);
       	  dbf = fopen(debugf,"w");
+	  umask(oldumask);
 	  if (dbf)
 	    setbuf(dbf,NULL);
 	  else
@@ -2883,7 +2887,7 @@ connect_again:
   }
 
   if (ret < 0 && (errno == EINPROGRESS || errno == EALREADY)) {
-      DEBUG(2,("timeout connecting to %s:%d\n",inet_ntoa(*addr),port));
+      DEBUG(1,("timeout connecting to %s:%d\n",inet_ntoa(*addr),port));
       close(res);
       return -1;
   }
@@ -2896,7 +2900,7 @@ connect_again:
 #endif
 
   if (ret < 0) {
-    DEBUG(2,("error connecting to %s:%d (%s)\n",
+    DEBUG(1,("error connecting to %s:%d (%s)\n",
 	     inet_ntoa(*addr),port,strerror(errno)));
     return -1;
   }

source/locking/shmem.c

@@ -75,17 +75,13 @@ static int shm_times_locked = 0;
 
 static BOOL shm_register_process(char *processreg_file, pid_t pid, BOOL *other_processes)
 {
-   int old_umask;
    int shm_processes_fd = -1;
    int nb_read;
    pid_t other_pid;
    int free_slot = -1;
    int erased_slot;   
    
-   
-   old_umask = umask(0);
    shm_processes_fd = open(processreg_file, O_RDWR | O_CREAT, 0666);
-   umask(old_umask);
    if ( shm_processes_fd < 0 )
    {
       DEBUG(0,("ERROR shm_register_process : processreg_file open failed with code %d\n",errno));

source/passdb/smbpass.c

@@ -60,7 +60,7 @@ do_pw_lock(int fd, int waitsecs, int type)
 
 int pw_file_lock(char *name, int type, int secs)
 {
-	int             fd = open(name, O_RDWR | O_CREAT, 0666);
+	int fd = open(name, O_RDWR | O_CREAT, 0600);
 	if (fd < 0)
 		return (-1);
 	if (do_pw_lock(fd, secs, type)) {

source/printing/printing.c

@@ -130,7 +130,7 @@ void print_file(int fnum)
   tempstr = build_print_command(cnum, PRINTCOMMAND(snum), syscmd, Files[fnum].name);
   if (tempstr != NULL)
     {
-      int ret = smbrun(syscmd,NULL);
+      int ret = smbrun(syscmd,NULL,False);
       DEBUG(3,("Running the command `%s' gave %d\n",syscmd,ret));
     }
   else
@@ -923,7 +923,6 @@ int get_printqueue(int snum,int cnum,print_queue_struct **queue,
   struct stat sbuf;
   BOOL dorun=True;
   int cachetime = lp_lpqcachetime();
-  int lfd = -1;
 
   *line = 0;
   check_lpq_cache(snum);
@@ -954,20 +953,10 @@ int get_printqueue(int snum,int cnum,print_queue_struct **queue,
 	DEBUG(3,("Using cached lpq output\n"));
 	dorun = False;
       }
-
-      if (dorun) {
-	lfd = file_lock(outfile,LPQ_LOCK_TIMEOUT);
-	if (lfd<0 || 
-	    (!fstat(lfd,&sbuf) && (time(NULL) - sbuf.st_mtime)<cachetime)) {
-	  DEBUG(3,("Using cached lpq output\n"));
-	  dorun = False;
-	  file_unlock(lfd); lfd = -1;
-	}
-      }
     }
 
   if (dorun) {
-    ret = smbrun(syscmd,outfile);
+    ret = smbrun(syscmd,outfile,True);
     DEBUG(3,("Running the command `%s' gave %d\n",syscmd,ret));
   }
 
@@ -975,7 +964,6 @@ int get_printqueue(int snum,int cnum,print_queue_struct **queue,
 
   f = fopen(outfile,"r");
   if (!f) {
-    if (lfd >= 0) file_unlock(lfd);
     return(0);
   }
 
@@ -1006,12 +994,13 @@ int get_printqueue(int snum,int cnum,print_queue_struct **queue,
 
   fclose(f);
 
-  if (lfd >= 0) file_unlock(lfd);
-
-  if (!cachetime) 
+  if (!cachetime) {
     unlink(outfile);
-  else
+  } else {
+    /* we only expect this to succeed on trapdoor systems, on normal systems
+     the file is owned by root */
     chmod(outfile,0666);
+  }
   return(count);
 }
 
@@ -1047,7 +1036,7 @@ void del_printqueue(int cnum,int snum,int jobid)
   string_sub(syscmd,"%j",jobstr);
   standard_sub(cnum,syscmd);
 
-  ret = smbrun(syscmd,NULL);
+  ret = smbrun(syscmd,NULL,False);
   DEBUG(3,("Running the command `%s' gave %d\n",syscmd,ret));  
   lpq_reset(snum); /* queue has changed */
 }
@@ -1085,7 +1074,7 @@ void status_printjob(int cnum,int snum,int jobid,int status)
   string_sub(syscmd,"%j",jobstr);
   standard_sub(cnum,syscmd);
 
-  ret = smbrun(syscmd,NULL);
+  ret = smbrun(syscmd,NULL,False);
   DEBUG(3,("Running the command `%s' gave %d\n",syscmd,ret));  
   lpq_reset(snum); /* queue has changed */
 }

source/smbd/message.c

@@ -42,8 +42,8 @@ static void msg_deliver(void)
 {
   pstring s;
   fstring name;
-  FILE *f;
   int i;
+  int fd;
 
   if (! (*lp_msg_command()))
     {
@@ -56,21 +56,19 @@ static void msg_deliver(void)
   sprintf(s,"/tmp/msg.XXXXXX");
   strcpy(name,(char *)mktemp(s));
 
-  f = fopen(name,"w");
-  if (!f)
-    {
+  fd = open(name,O_WRONLY|O_CREAT|O_TRUNC|O_EXCL,0600);
+  if (fd == -1) {
     DEBUG(1,("can't open message file %s\n",name));
     return;
   }
 
-  for (i=0;i<msgpos;)
-    {
-      if (msgbuf[i]=='\r' && i<(msgpos-1) && msgbuf[i+1]=='\n')
-	i++;
-      fputc(msgbuf[i++],f);
+  for (i=0;i<msgpos;) {
+    if (msgbuf[i]=='\r' && i<(msgpos-1) && msgbuf[i+1]=='\n') {
+      i++; continue;      
     }
-
-  fclose(f);
+    write(fd,&msgbuf[i++],1);
+  }
+  close(fd);
 
 
   /* run the command */
@@ -81,7 +79,7 @@ static void msg_deliver(void)
       string_sub(s,"%f",msgfrom);
       string_sub(s,"%t",msgto);
       standard_sub(-1,s);
-      smbrun(s,NULL);
+      smbrun(s,NULL,False);
     }
 
   msgpos = 0;

source/smbd/server.c

@@ -567,7 +567,7 @@ int disk_free(char *path,int *bsize,int *dfree,int *dsize)
       sprintf(syscmd,"%s %s",df_command,path);
       standard_sub_basic(syscmd);
 
-      ret = smbrun(syscmd,outfile);
+      ret = smbrun(syscmd,outfile,False);
       DEBUG(3,("Running the command `%s' gave %d\n",syscmd,ret));
 	  
       {
@@ -923,7 +923,7 @@ static void check_magic(int fnum,int cnum)
       sprintf(magic_output,"%s.out",fname);
 
     chmod(fname,0755);
-    ret = smbrun(fname,magic_output);
+    ret = smbrun(fname,magic_output,False);
     DEBUG(3,("Invoking magic command %s gave %d\n",fname,ret));
     unlink(fname);
   }
@@ -2096,7 +2096,7 @@ int make_connection(char *service,char *user,char *password, int pwlen, char *de
       strcpy(cmd,lp_rootpreexec(SNUM(cnum)));
       standard_sub(cnum,cmd);
       DEBUG(5,("cmd=%s\n",cmd));
-      smbrun(cmd,NULL);
+      smbrun(cmd,NULL,False);
     }
 
   if (!become_user(cnum,pcon->uid))
@@ -2149,7 +2149,7 @@ int make_connection(char *service,char *user,char *password, int pwlen, char *de
       pstring cmd;
       strcpy(cmd,lp_preexec(SNUM(cnum)));
       standard_sub(cnum,cmd);
-      smbrun(cmd,NULL);
+      smbrun(cmd,NULL,False);
     }
   
   /* we've finished with the sensitive stuff */
@@ -2629,7 +2629,7 @@ void close_cnum(int cnum, int uid)
       pstring cmd;
       strcpy(cmd,lp_postexec(SNUM(cnum)));
       standard_sub(cnum,cmd);
-      smbrun(cmd,NULL);
+      smbrun(cmd,NULL,False);
       unbecome_user();
     }
 
@@ -2640,7 +2640,7 @@ void close_cnum(int cnum, int uid)
       pstring cmd;
       strcpy(cmd,lp_rootpostexec(SNUM(cnum)));
       standard_sub(cnum,cmd);
-      smbrun(cmd,NULL);
+      smbrun(cmd,NULL,False);
     }
 
   Connections[cnum].open = False;
@@ -2764,8 +2764,10 @@ BOOL claim_connection(int cnum,char *name,int max_connections,BOOL Clear)
 
   if (!file_exist(fname,NULL))
     {
+      int oldmask = umask(022);
       f = fopen(fname,"w");
       if (f) fclose(f);
+      umask(oldmask);
     }
 
   total_recs = file_size(fname) / sizeof(crec);
@@ -3617,7 +3619,9 @@ static void usage(char *pname)
 
   fault_setup(exit_server);
 
-  umask(0777 & ~DEF_CREATE_MASK);
+  /* we want total control over the permissions on created files,
+     so set our umask to 0 */
+  umask(0);
 
   init_uid();
 

source/smbd/uid.c

@@ -27,9 +27,6 @@ extern connection_struct Connections[];
 
 static int initial_uid;
 static int initial_gid;
-static int old_umask = 022;
-
-static pstring OriginalDir;
 
 /* what user is current? */
 struct current_user current_user;
@@ -57,7 +54,7 @@ void init_uid(void)
 
   current_user.cnum = -1;
 
-  GetWd(OriginalDir);
+  ChDir(IDLE_DIR);
 }
 
 
@@ -69,6 +66,10 @@ static BOOL become_uid(int uid)
   if (initial_uid != 0)
     return(True);
 
+  if (uid == -1 || uid == 65535) {
+    DEBUG(1,("WARNING: using uid %d is a security risk\n",uid));    
+  }
+
 #ifdef AIX
   {
     /* AIX 3 stuff - inspired by a code fragment in wu-ftpd */
@@ -119,6 +120,10 @@ static BOOL become_gid(int gid)
   if (initial_uid != 0)
     return(True);
 
+  if (gid == -1 || gid == 65535) {
+    DEBUG(1,("WARNING: using gid %d is a security risk\n",gid));    
+  }
+  
 #ifdef USE_SETRES 
   if (setresgid(-1,gid,-1) != 0)
 #elif defined(USE_SETFS)
@@ -199,7 +204,6 @@ static BOOL check_user_ok(int cnum,user_struct *vuser,int snum)
 ****************************************************************************/
 BOOL become_user(int cnum, int uid)
 {
-  int new_umask;
   user_struct *vuser;
   int snum,gid;
   int id = uid;
@@ -259,14 +263,11 @@ BOOL become_user(int cnum, int uid)
 	return(False);
     }
 
-  new_umask = 0777 & ~CREATE_MODE(cnum);
-  old_umask = umask(new_umask);
-
   current_user.cnum = cnum;
   current_user.id = id;
   
-  DEBUG(5,("become_user uid=(%d,%d) gid=(%d,%d) new_umask=0%o\n",
-	   getuid(),geteuid(),getgid(),getegid(),new_umask));
+  DEBUG(5,("become_user uid=(%d,%d) gid=(%d,%d)\n",
+	   getuid(),geteuid(),getgid(),getegid()));
   
   return(True);
 }
@@ -279,9 +280,7 @@ BOOL unbecome_user(void )
   if (current_user.cnum == -1)
     return(False);
 
-  ChDir(OriginalDir);
-
-  umask(old_umask);
+  ChDir(IDLE_DIR);
 
   if (initial_uid == 0)
     {
@@ -318,9 +317,9 @@ BOOL unbecome_user(void )
   current_user.uid = initial_uid;
   current_user.gid = initial_gid;
   
-  if (ChDir(OriginalDir) != 0)
+  if (ChDir(IDLE_DIR) != 0)
     DEBUG(0,("%s chdir(%s) failed in unbecome_user\n",
-	     timestring(),OriginalDir));  
+	     timestring(),IDLE_DIR));
 
   DEBUG(5,("unbecome_user now uid=(%d,%d) gid=(%d,%d)\n",
 	getuid(),geteuid(),getgid(),getegid()));
@@ -332,14 +331,69 @@ BOOL unbecome_user(void )
 
 
 /****************************************************************************
-run a command via system() using smbrun, being careful about uid/gid handling
+This is a utility function of smbrun(). It must be called only from
+the child as it may leave the caller in a privilaged state.
 ****************************************************************************/
-int smbrun(char *cmd,char *outfile)
+static BOOL setup_stdout_file(char *outfile,BOOL shared)
 {  
+  int fd;
+  mode_t mode = S_IWUSR|S_IRUSR|S_IRGRP|S_IROTH;
+
+  close(1);
+
+  if (shared) {
+    /* become root - unprivilaged users can't delete these files */
+#ifdef USE_SETRES
+    setresgid(0,0,0);
+    setresuid(0,0,0);
+#else      
+    setuid(0);
+    seteuid(0);
+#endif
+  }
+
+  /* now create the file with O_EXCL set */
+  unlink(outfile);
+  fd = open(outfile,O_RDWR|O_CREAT|O_TRUNC|O_EXCL,mode);
+
+  if (fd == -1) return False;
+
+  if (fd != 1) {
+    if (dup2(fd,1) != 0) {
+      DEBUG(2,("Failed to create stdout file descriptor\n"));
+      close(fd);
+      return False;
+    }
+    close(fd);
+  }
+  return True;
+}
+
+
+/****************************************************************************
+run a command being careful about uid/gid handling and putting the output in
+outfile (or discard it if outfile is NULL).
+
+if shared is True then ensure the file will be writeable by all users
+but created such that its owned by root. This overcomes a security hole.
+
+if shared is not set then open the file with O_EXCL set
+****************************************************************************/
+int smbrun(char *cmd,char *outfile,BOOL shared)
+{
+  int fd,pid;
+  int uid = current_user.uid;
+  int gid = current_user.gid;
+
+#if USE_SYSTEM
   int ret;
   pstring syscmd;  
   char *path = lp_smbrun();
 
+  /* in the old method we use system() to execute smbrun which then
+     executes the command (using system() again!). This involves lots
+     of shell launches and is very slow. It also suffers from a
+     potential security hole */
   if (!file_exist(path,NULL))
     {
       DEBUG(0,("SMBRUN ERROR: Can't find %s. Installation problem?\n",path));
@@ -347,13 +401,69 @@ int smbrun(char *cmd,char *outfile)
     }
 
   sprintf(syscmd,"%s %d %d \"(%s 2>&1) > %s\"",
-	  path,current_user.uid,current_user.gid,cmd,
+	  path,uid,gid,cmd,
 	  outfile?outfile:"/dev/null");
 
   DEBUG(5,("smbrun - running %s ",syscmd));
   ret = system(syscmd);
   DEBUG(5,("gave %d\n",ret));
   return(ret);
+#else
+  /* in this newer method we will exec /bin/sh with the correct
+     arguments, after first setting stdout to point at the file */
+
+  if ((pid=fork())) {
+    int status=0;
+    /* the parent just waits for the child to exit */
+    if (waitpid(pid,&status,0) != pid) {
+      DEBUG(2,("waitpid(%d) : %s\n",pid,strerror(errno)));
+      return -1;
+    }
+    return status;
+  }
+
+
+  /* we are in the child. we exec /bin/sh to do the work for us. we
+     don't directly exec the command we want because it may be a
+     pipeline or anything else the config file specifies */
+
+  /* point our stdout at the file we want output to go into */
+  if (outfile && !setup_stdout_file(outfile,shared)) {
+    exit(80);
+  }
+
+  /* now completely lose our privilages. This is a fairly paranoid
+     way of doing it, but it does work on all systems that I know of */
+#ifdef USE_SETRES
+  setresgid(0,0,0);
+  setresuid(0,0,0);
+  setresgid(gid,gid,gid);
+  setresuid(uid,uid,uid);      
+#else      
+  setuid(0);
+  seteuid(0);
+  setgid(gid);
+  setegid(gid);
+  setuid(uid);
+  seteuid(uid);
+#endif
+
+  if (getuid() != uid || geteuid() != uid ||
+      getgid() != gid || getegid() != gid) {
+    /* we failed to lose our privilages - do not execute the command */
+    exit(81); /* we can't print stuff at this stage, instead use exit codes
+		 for debugging */
+  }
+
+  /* close all other file descriptors, leaving only 0, 1 and 2. 0 and
+     2 point to /dev/null from the startup code */
+  for (fd=3;fd<256;fd++) close(fd);
+
+  execl("/bin/sh","sh","-c",cmd,NULL);  
+
+  /* not reached */
+  exit(82);
+#endif
 }
 
 

source/utils/testprns.c

@@ -53,10 +53,9 @@ int main(int argc, char *argv[])
    else
    {
       dbf = fopen("test.log", "w");
-      if (dbf == NULL)
+      if (dbf == NULL) {
          printf("Unable to open logfile.\n");
-      else
-      {
+      } else {
          DEBUGLEVEL = 3;
          pszTemp = (argc < 3) ? PRINTCAP_NAME : argv[2];
          printf("Looking for printer %s in printcap file %s\n",