From e23f2b9cef8428bda51b413642d9720ba5c590d5 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Fri, 4 Oct 1996 09:31:07 +0000 Subject: - 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 (This used to be commit 5aa735c940ccdb6acae5f28449d484181c912e49) --- source3/smbd/message.c | 28 +++++----- source3/smbd/server.c | 18 +++--- source3/smbd/uid.c | 146 +++++++++++++++++++++++++++++++++++++++++++------ 3 files changed, 152 insertions(+), 40 deletions(-) (limited to 'source3/smbd') diff --git a/source3/smbd/message.c b/source3/smbd/message.c index b26a6605ed..22523aad3b 100644 --- a/source3/smbd/message.c +++ b/source3/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) - { - DEBUG(1,("can't open message file %s\n",name)); - return; - } + 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;iuid)) @@ -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(); diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c index 555cd457e7..7274c18478 100644 --- a/source3/smbd/uid.c +++ b/source3/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 */ @@ -118,6 +119,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) @@ -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 } -- cgit