From d357f8b33594472ffa78d0a112accccc2a8b1fe7 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 20 Jun 2006 02:38:28 +0000 Subject: [PATCH] r16397: Fix Klocwork #11767 and drasticly simplify the logic in smbd/process.c. All interested (Volker, Jerry, James etc). PLEASE REVIEW THIS CHANGE. The logic should be identical but *much* easier to follow and change (and shouldn't confuse Klockwork :-). Jeremy. --- source/smbd/message.c | 305 ++++++++++++++++++++-------------------- source/smbd/negprot.c | 1 + source/smbd/nttrans.c | 1 + source/smbd/process.c | 74 +++++----- source/smbd/reply.c | 7 + source/smbd/sesssetup.c | 1 + 6 files changed, 198 insertions(+), 191 deletions(-) diff --git a/source/smbd/message.c b/source/smbd/message.c index fd28df0d801..31dab458443 100644 --- a/source/smbd/message.c +++ b/source/smbd/message.c @@ -34,211 +34,210 @@ static fstring msgfrom; static fstring msgto; /**************************************************************************** -deliver the message + Deliver the message. ****************************************************************************/ + static void msg_deliver(void) { - pstring name; - int i; - int fd; - char *msg; - int len; - ssize_t sz; - - if (! (*lp_msg_command())) - { - DEBUG(1,("no messaging command specified\n")); - msgpos = 0; - return; - } - - /* put it in a temporary file */ - slprintf(name,sizeof(name)-1, "%s/msg.XXXXXX",tmpdir()); - fd = smb_mkstemp(name); - - if (fd == -1) { - DEBUG(1,("can't open message file %s\n",name)); - return; - } - - /* - * Incoming message is in DOS codepage format. Convert to UNIX. - */ + pstring name; + int i; + int fd; + char *msg; + int len; + ssize_t sz; + + if (! (*lp_msg_command())) { + DEBUG(1,("no messaging command specified\n")); + msgpos = 0; + return; + } + + /* put it in a temporary file */ + slprintf(name,sizeof(name)-1, "%s/msg.XXXXXX",tmpdir()); + fd = smb_mkstemp(name); + + if (fd == -1) { + DEBUG(1,("can't open message file %s\n",name)); + return; + } + + /* + * Incoming message is in DOS codepage format. Convert to UNIX. + */ - if ((len = (int)convert_string_allocate(NULL,CH_DOS, CH_UNIX, msgbuf, msgpos, (void **)(void *)&msg, True)) < 0 || !msg) { - DEBUG(3,("Conversion failed, delivering message in DOS codepage format\n")); - for (i = 0; i < msgpos;) { - if (msgbuf[i] == '\r' && i < (msgpos-1) && msgbuf[i+1] == '\n') { - i++; continue; - } - sz = write(fd, &msgbuf[i++], 1); - if ( sz != 1 ) { - DEBUG(0,("Write error to fd %d: %ld(%d)\n",fd, (long)sz, errno )); - } - } - } else { - for (i = 0; i < len;) { - if (msg[i] == '\r' && i < (len-1) && msg[i+1] == '\n') { - i++; continue; - } - sz = write(fd, &msg[i++],1); - if ( sz != 1 ) { - DEBUG(0,("Write error to fd %d: %ld(%d)\n",fd, (long)sz, errno )); - } - } - SAFE_FREE(msg); - } - close(fd); - - - /* run the command */ - if (*lp_msg_command()) - { - fstring alpha_msgfrom; - fstring alpha_msgto; - pstring s; - - pstrcpy(s,lp_msg_command()); - pstring_sub(s,"%f",alpha_strcpy(alpha_msgfrom,msgfrom,NULL,sizeof(alpha_msgfrom))); - pstring_sub(s,"%t",alpha_strcpy(alpha_msgto,msgto,NULL,sizeof(alpha_msgto))); - standard_sub_basic(current_user_info.smb_name, s, sizeof(s)); - pstring_sub(s,"%s",name); - smbrun(s,NULL); - } - - msgpos = 0; + if ((len = (int)convert_string_allocate(NULL,CH_DOS, CH_UNIX, msgbuf, msgpos, (void **)(void *)&msg, True)) < 0 || !msg) { + DEBUG(3,("Conversion failed, delivering message in DOS codepage format\n")); + for (i = 0; i < msgpos;) { + if (msgbuf[i] == '\r' && i < (msgpos-1) && msgbuf[i+1] == '\n') { + i++; + continue; + } + sz = write(fd, &msgbuf[i++], 1); + if ( sz != 1 ) { + DEBUG(0,("Write error to fd %d: %ld(%d)\n",fd, (long)sz, errno )); + } + } + } else { + for (i = 0; i < len;) { + if (msg[i] == '\r' && i < (len-1) && msg[i+1] == '\n') { + i++; + continue; + } + sz = write(fd, &msg[i++],1); + if ( sz != 1 ) { + DEBUG(0,("Write error to fd %d: %ld(%d)\n",fd, (long)sz, errno )); + } + } + SAFE_FREE(msg); + } + close(fd); + + /* run the command */ + if (*lp_msg_command()) { + fstring alpha_msgfrom; + fstring alpha_msgto; + pstring s; + + pstrcpy(s,lp_msg_command()); + pstring_sub(s,"%f",alpha_strcpy(alpha_msgfrom,msgfrom,NULL,sizeof(alpha_msgfrom))); + pstring_sub(s,"%t",alpha_strcpy(alpha_msgto,msgto,NULL,sizeof(alpha_msgto))); + standard_sub_basic(current_user_info.smb_name, s, sizeof(s)); + pstring_sub(s,"%s",name); + smbrun(s,NULL); + } + + msgpos = 0; } - - /**************************************************************************** - reply to a sends + Reply to a sends. + conn POINTER CAN BE NULL HERE ! ****************************************************************************/ -int reply_sends(connection_struct *conn, - char *inbuf,char *outbuf, int dum_size, int dum_buffsize) + +int reply_sends(connection_struct *conn, char *inbuf,char *outbuf, int dum_size, int dum_buffsize) { - int len; - char *msg; - int outsize = 0; - char *p; + int len; + char *msg; + int outsize = 0; + char *p; - START_PROFILE(SMBsends); + START_PROFILE(SMBsends); - msgpos = 0; + msgpos = 0; - if (! (*lp_msg_command())) { - END_PROFILE(SMBsends); - return(ERROR_DOS(ERRSRV,ERRmsgoff)); - } + if (! (*lp_msg_command())) { + END_PROFILE(SMBsends); + return(ERROR_DOS(ERRSRV,ERRmsgoff)); + } - outsize = set_message(outbuf,0,0,True); + outsize = set_message(outbuf,0,0,True); - p = smb_buf(inbuf)+1; - p += srvstr_pull_buf(inbuf, msgfrom, p, sizeof(msgfrom), STR_ASCII|STR_TERMINATE) + 1; - p += srvstr_pull_buf(inbuf, msgto, p, sizeof(msgto), STR_ASCII|STR_TERMINATE) + 1; + p = smb_buf(inbuf)+1; + p += srvstr_pull_buf(inbuf, msgfrom, p, sizeof(msgfrom), STR_ASCII|STR_TERMINATE) + 1; + p += srvstr_pull_buf(inbuf, msgto, p, sizeof(msgto), STR_ASCII|STR_TERMINATE) + 1; - msg = p; + msg = p; - len = SVAL(msg,0); - len = MIN(len,sizeof(msgbuf)-msgpos); + len = SVAL(msg,0); + len = MIN(len,sizeof(msgbuf)-msgpos); - memset(msgbuf,'\0',sizeof(msgbuf)); + memset(msgbuf,'\0',sizeof(msgbuf)); - memcpy(&msgbuf[msgpos],msg+2,len); - msgpos += len; + memcpy(&msgbuf[msgpos],msg+2,len); + msgpos += len; - msg_deliver(); + msg_deliver(); - END_PROFILE(SMBsends); - return(outsize); + END_PROFILE(SMBsends); + return(outsize); } - /**************************************************************************** - reply to a sendstrt + Reply to a sendstrt. + conn POINTER CAN BE NULL HERE ! ****************************************************************************/ -int reply_sendstrt(connection_struct *conn, - char *inbuf,char *outbuf, int dum_size, int dum_buffsize) + +int reply_sendstrt(connection_struct *conn, char *inbuf,char *outbuf, int dum_size, int dum_buffsize) { - int outsize = 0; - char *p; + int outsize = 0; + char *p; - START_PROFILE(SMBsendstrt); + START_PROFILE(SMBsendstrt); - if (! (*lp_msg_command())) { - END_PROFILE(SMBsendstrt); - return(ERROR_DOS(ERRSRV,ERRmsgoff)); - } + if (! (*lp_msg_command())) { + END_PROFILE(SMBsendstrt); + return(ERROR_DOS(ERRSRV,ERRmsgoff)); + } - outsize = set_message(outbuf,1,0,True); + outsize = set_message(outbuf,1,0,True); - memset(msgbuf,'\0',sizeof(msgbuf)); - msgpos = 0; + memset(msgbuf,'\0',sizeof(msgbuf)); + msgpos = 0; - p = smb_buf(inbuf)+1; - p += srvstr_pull_buf(inbuf, msgfrom, p, sizeof(msgfrom), STR_ASCII|STR_TERMINATE) + 1; - p += srvstr_pull_buf(inbuf, msgto, p, sizeof(msgto), STR_ASCII|STR_TERMINATE) + 1; + p = smb_buf(inbuf)+1; + p += srvstr_pull_buf(inbuf, msgfrom, p, sizeof(msgfrom), STR_ASCII|STR_TERMINATE) + 1; + p += srvstr_pull_buf(inbuf, msgto, p, sizeof(msgto), STR_ASCII|STR_TERMINATE) + 1; - DEBUG( 3, ( "SMBsendstrt (from %s to %s)\n", msgfrom, msgto ) ); + DEBUG( 3, ( "SMBsendstrt (from %s to %s)\n", msgfrom, msgto ) ); - END_PROFILE(SMBsendstrt); - return(outsize); + END_PROFILE(SMBsendstrt); + return(outsize); } - /**************************************************************************** - reply to a sendtxt + Reply to a sendtxt. + conn POINTER CAN BE NULL HERE ! ****************************************************************************/ -int reply_sendtxt(connection_struct *conn, - char *inbuf,char *outbuf, int dum_size, int dum_buffsize) + +int reply_sendtxt(connection_struct *conn, char *inbuf,char *outbuf, int dum_size, int dum_buffsize) { - int len; - int outsize = 0; - char *msg; - START_PROFILE(SMBsendtxt); + int len; + int outsize = 0; + char *msg; + START_PROFILE(SMBsendtxt); - if (! (*lp_msg_command())) { - END_PROFILE(SMBsendtxt); - return(ERROR_DOS(ERRSRV,ERRmsgoff)); - } + if (! (*lp_msg_command())) { + END_PROFILE(SMBsendtxt); + return(ERROR_DOS(ERRSRV,ERRmsgoff)); + } - outsize = set_message(outbuf,0,0,True); + outsize = set_message(outbuf,0,0,True); - msg = smb_buf(inbuf) + 1; + msg = smb_buf(inbuf) + 1; - len = SVAL(msg,0); - len = MIN(len,sizeof(msgbuf)-msgpos); + len = SVAL(msg,0); + len = MIN(len,sizeof(msgbuf)-msgpos); - memcpy(&msgbuf[msgpos],msg+2,len); - msgpos += len; + memcpy(&msgbuf[msgpos],msg+2,len); + msgpos += len; - DEBUG( 3, ( "SMBsendtxt\n" ) ); + DEBUG( 3, ( "SMBsendtxt\n" ) ); - END_PROFILE(SMBsendtxt); - return(outsize); + END_PROFILE(SMBsendtxt); + return(outsize); } - /**************************************************************************** - reply to a sendend + Reply to a sendend. + conn POINTER CAN BE NULL HERE ! ****************************************************************************/ -int reply_sendend(connection_struct *conn, - char *inbuf,char *outbuf, int dum_size, int dum_buffsize) + +int reply_sendend(connection_struct *conn, char *inbuf,char *outbuf, int dum_size, int dum_buffsize) { - int outsize = 0; - START_PROFILE(SMBsendend); + int outsize = 0; + START_PROFILE(SMBsendend); - if (! (*lp_msg_command())) { - END_PROFILE(SMBsendend); - return(ERROR_DOS(ERRSRV,ERRmsgoff)); - } + if (! (*lp_msg_command())) { + END_PROFILE(SMBsendend); + return(ERROR_DOS(ERRSRV,ERRmsgoff)); + } - outsize = set_message(outbuf,0,0,True); + outsize = set_message(outbuf,0,0,True); - DEBUG(3,("SMBsendend\n")); + DEBUG(3,("SMBsendend\n")); - msg_deliver(); + msg_deliver(); - END_PROFILE(SMBsendend); - return(outsize); + END_PROFILE(SMBsendend); + return(outsize); } diff --git a/source/smbd/negprot.c b/source/smbd/negprot.c index 5d2ed6a10d1..3347008cdf8 100644 --- a/source/smbd/negprot.c +++ b/source/smbd/negprot.c @@ -456,6 +456,7 @@ static const struct { /**************************************************************************** Reply to a negprot. + conn POINTER CAN BE NULL HERE ! ****************************************************************************/ int reply_negprot(connection_struct *conn, diff --git a/source/smbd/nttrans.c b/source/smbd/nttrans.c index e397139d2ef..aa6f79e1657 100644 --- a/source/smbd/nttrans.c +++ b/source/smbd/nttrans.c @@ -1545,6 +1545,7 @@ static int call_nt_transact_create(connection_struct *conn, char *inbuf, char *o /**************************************************************************** Reply to a NT CANCEL request. + conn POINTER CAN BE NULL HERE ! ****************************************************************************/ int reply_ntcancel(connection_struct *conn, diff --git a/source/smbd/process.c b/source/smbd/process.c index 440d0ac0a50..b3ce49360d8 100644 --- a/source/smbd/process.c +++ b/source/smbd/process.c @@ -567,10 +567,10 @@ are used by some brain-dead clients when printing, and I don't want to force write permissions on print services. */ #define AS_USER (1<<0) -#define NEED_WRITE (1<<1) +#define NEED_WRITE (1<<1) /* Must be paired with AS_USER */ #define TIME_INIT (1<<2) -#define CAN_IPC (1<<3) -#define AS_GUEST (1<<5) +#define CAN_IPC (1<<3) /* Must be paired with AS_USER */ +#define AS_GUEST (1<<5) /* Must *NOT* be paired with AS_USER */ #define DO_CHDIR (1<<6) /* @@ -932,48 +932,46 @@ static int switch_message(int type,char *inbuf,char *outbuf,int size,int bufsize user_struct *vuser = NULL; last_session_tag = session_tag; - if(session_tag != UID_FIELD_INVALID) + if(session_tag != UID_FIELD_INVALID) { vuser = get_valid_user_struct(session_tag); - if(vuser != NULL) - set_current_user_info(&vuser->user); - } - - /* does this protocol need to be run as root? */ - if (!(flags & AS_USER)) - change_to_root_user(); - - /* does this protocol need a valid tree connection? */ - if ((flags & AS_USER) && !conn) { - /* Amazingly, the error code depends on the command (from Samba4). */ - if (type == SMBntcreateX) { - return ERROR_NT(NT_STATUS_INVALID_HANDLE); - } else { - return ERROR_DOS(ERRSRV, ERRinvnid); + if (vuser) { + set_current_user_info(&vuser->user); + } } } + /* Does this call need to be run as the connected user? */ + if (flags & AS_USER) { + + /* Does this call need a valid tree connection? */ + if (!conn) { + /* Amazingly, the error code depends on the command (from Samba4). */ + if (type == SMBntcreateX) { + return ERROR_NT(NT_STATUS_INVALID_HANDLE); + } else { + return ERROR_DOS(ERRSRV, ERRinvnid); + } + } - /* does this protocol need to be run as the connected user? */ - if ((flags & AS_USER) && !change_to_user(conn,session_tag)) { - if (flags & AS_GUEST) - flags &= ~AS_USER; - else + if (!change_to_user(conn,session_tag)) { return(ERROR_FORCE_DOS(ERRSRV,ERRbaduid)); - } + } - /* this code is to work around a bug is MS client 3 without - introducing a security hole - it needs to be able to do - print queue checks as guest if it isn't logged in properly */ - if (flags & AS_USER) - flags &= ~AS_GUEST; + /* All NEED_WRITE and CAN_IPC flags must also have AS_USER. */ - /* does it need write permission? */ - if ((flags & NEED_WRITE) && !CAN_WRITE(conn)) - return(ERROR_DOS(ERRSRV,ERRaccess)); + /* Does it need write permission? */ + if ((flags & NEED_WRITE) && !CAN_WRITE(conn)) { + return(ERROR_DOS(ERRSRV,ERRaccess)); + } - /* ipc services are limited */ - if (IS_IPC(conn) && (flags & AS_USER) && !(flags & CAN_IPC)) - return(ERROR_DOS(ERRSRV,ERRaccess)); + /* IPC services are limited */ + if (IS_IPC(conn) && !(flags & CAN_IPC)) { + return(ERROR_DOS(ERRSRV,ERRaccess)); + } + } else { + /* This call needs to be run as root */ + change_to_root_user(); + } /* load service specific parameters */ if (conn) { @@ -985,8 +983,9 @@ static int switch_message(int type,char *inbuf,char *outbuf,int size,int bufsize /* does this protocol need to be run as guest? */ if ((flags & AS_GUEST) && (!change_to_guest() || - !check_access(smbd_server_fd(), lp_hostsallow(-1), lp_hostsdeny(-1)))) + !check_access(smbd_server_fd(), lp_hostsallow(-1), lp_hostsdeny(-1)))) { return(ERROR_DOS(ERRSRV,ERRaccess)); + } current_inbuf = inbuf; /* In case we need to defer this message in open... */ outsize = smb_messages[type].fn(conn, inbuf,outbuf,size,bufsize); @@ -997,7 +996,6 @@ static int switch_message(int type,char *inbuf,char *outbuf,int size,int bufsize return(outsize); } - /**************************************************************************** Construct a reply to the incoming packet. ****************************************************************************/ diff --git a/source/smbd/reply.c b/source/smbd/reply.c index d333ebf32eb..e68e8662d74 100644 --- a/source/smbd/reply.c +++ b/source/smbd/reply.c @@ -547,6 +547,7 @@ int reply_special(char *inbuf,char *outbuf) /**************************************************************************** Reply to a tcon. + conn POINTER CAN BE NULL HERE ! ****************************************************************************/ int reply_tcon(connection_struct *conn, @@ -605,6 +606,7 @@ int reply_tcon(connection_struct *conn, /**************************************************************************** Reply to a tcon and X. + conn POINTER CAN BE NULL HERE ! ****************************************************************************/ int reply_tcon_and_X(connection_struct *conn, char *inbuf,char *outbuf,int length,int bufsize) @@ -738,6 +740,7 @@ int reply_unknown(char *inbuf,char *outbuf) /**************************************************************************** Reply to an ioctl. + conn POINTER CAN BE NULL HERE ! ****************************************************************************/ int reply_ioctl(connection_struct *conn, @@ -1591,6 +1594,7 @@ int reply_open_and_X(connection_struct *conn, char *inbuf,char *outbuf,int lengt /**************************************************************************** Reply to a SMBulogoffX. + conn POINTER CAN BE NULL HERE ! ****************************************************************************/ int reply_ulogoffX(connection_struct *conn, char *inbuf,char *outbuf,int length,int bufsize) @@ -3236,6 +3240,7 @@ int reply_flush(connection_struct *conn, char *inbuf,char *outbuf, int size, int /**************************************************************************** Reply to a exit. + conn POINTER CAN BE NULL HERE ! ****************************************************************************/ int reply_exit(connection_struct *conn, @@ -3511,6 +3516,7 @@ int reply_unlock(connection_struct *conn, char *inbuf,char *outbuf, int size, /**************************************************************************** Reply to a tdis. + conn POINTER CAN BE NULL HERE ! ****************************************************************************/ int reply_tdis(connection_struct *conn, @@ -3538,6 +3544,7 @@ int reply_tdis(connection_struct *conn, /**************************************************************************** Reply to a echo. + conn POINTER CAN BE NULL HERE ! ****************************************************************************/ int reply_echo(connection_struct *conn, diff --git a/source/smbd/sesssetup.c b/source/smbd/sesssetup.c index 46acb20bdad..fb579707cae 100644 --- a/source/smbd/sesssetup.c +++ b/source/smbd/sesssetup.c @@ -635,6 +635,7 @@ static int reply_spnego_auth(connection_struct *conn, char *inbuf, char *outbuf, /**************************************************************************** Reply to a session setup command. + conn POINTER CAN BE NULL HERE ! ****************************************************************************/ static int reply_sesssetup_and_X_spnego(connection_struct *conn, char *inbuf, -- 2.34.1