From e7b729e0d9d6264e85be042b16aa6aee0648fcfd Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Tue, 16 Apr 2002 06:15:28 +0000 Subject: [PATCH] make sure we don't walk past the end of the current SMB buffer when pulling a string this might explain a serious filename corruption bug that Quantum QA spotted (This used to be commit a877eae24becad9e0cd5b33ffe0916a20d5ba227) --- source3/smbd/ipc.c | 2 +- source3/smbd/message.c | 8 ++++---- source3/smbd/nttrans.c | 17 ++++++++-------- source3/smbd/pipes.c | 2 +- source3/smbd/reply.c | 42 ++++++++++++++++++++-------------------- source3/smbd/sesssetup.c | 14 +++++--------- source3/smbd/srvstr.c | 9 +++++++++ 7 files changed, 49 insertions(+), 45 deletions(-) diff --git a/source3/smbd/ipc.c b/source3/smbd/ipc.c index c2f3b7b2f0b..91b221968f2 100644 --- a/source3/smbd/ipc.c +++ b/source3/smbd/ipc.c @@ -375,7 +375,7 @@ int reply_trans(connection_struct *conn, char *inbuf,char *outbuf, int size, int START_PROFILE(SMBtrans); memset(name, '\0',sizeof(name)); - srvstr_pull(inbuf, name, smb_buf(inbuf), sizeof(name), -1, STR_TERMINATE); + srvstr_pull_buf(inbuf, name, smb_buf(inbuf), sizeof(name), STR_TERMINATE); if (dscnt > tdscnt || pscnt > tpscnt) { exit_server("invalid trans parameters"); diff --git a/source3/smbd/message.c b/source3/smbd/message.c index 971834c012c..c2eb16c99e2 100644 --- a/source3/smbd/message.c +++ b/source3/smbd/message.c @@ -118,8 +118,8 @@ int reply_sends(connection_struct *conn, outsize = set_message(outbuf,0,0,True); p = smb_buf(inbuf)+1; - p += srvstr_pull(inbuf, msgfrom, p, sizeof(msgfrom), -1, STR_TERMINATE) + 1; - p += srvstr_pull(inbuf, msgto, p, sizeof(msgto), -1, STR_TERMINATE) + 1; + p += srvstr_pull_buf(inbuf, msgfrom, p, sizeof(msgfrom), STR_TERMINATE) + 1; + p += srvstr_pull_buf(inbuf, msgto, p, sizeof(msgto), STR_TERMINATE) + 1; msg = p; @@ -160,8 +160,8 @@ int reply_sendstrt(connection_struct *conn, msgpos = 0; p = smb_buf(inbuf)+1; - p += srvstr_pull(inbuf, msgfrom, p, sizeof(msgfrom), -1, STR_TERMINATE) + 1; - p += srvstr_pull(inbuf, msgto, p, sizeof(msgto), -1, STR_TERMINATE) + 1; + p += srvstr_pull_buf(inbuf, msgfrom, p, sizeof(msgfrom), STR_TERMINATE) + 1; + p += srvstr_pull_buf(inbuf, msgto, p, sizeof(msgto), STR_TERMINATE) + 1; DEBUG( 3, ( "SMBsendstrt (from %s to %s)\n", msgfrom, msgto ) ); diff --git a/source3/smbd/nttrans.c b/source3/smbd/nttrans.c index edee14513c8..4dec0069f85 100644 --- a/source3/smbd/nttrans.c +++ b/source3/smbd/nttrans.c @@ -502,7 +502,7 @@ static int do_ntcreate_pipe_open(connection_struct *conn, int pnum = -1; char *p = NULL; - srvstr_pull(inbuf, fname, smb_buf(inbuf), sizeof(fname), -1, STR_TERMINATE); + srvstr_pull_buf(inbuf, fname, smb_buf(inbuf), sizeof(fname), STR_TERMINATE); if ((ret = nt_open_pipe(fname, conn, inbuf, outbuf, &pnum)) != 0) return ret; @@ -609,7 +609,7 @@ int reply_ntcreate_and_X(connection_struct *conn, * Check to see if this is a mac fork of some kind. */ - srvstr_pull(inbuf, fname, smb_buf(inbuf), sizeof(fname), -1, STR_TERMINATE); + srvstr_pull_buf(inbuf, fname, smb_buf(inbuf), sizeof(fname), STR_TERMINATE); if( strchr_m(fname, ':')) { END_PROFILE(SMBntcreateX); @@ -635,10 +635,9 @@ int reply_ntcreate_and_X(connection_struct *conn, dir_name_len++; } - srvstr_pull(inbuf, &fname[dir_name_len], smb_buf(inbuf), sizeof(fname)-dir_name_len, - -1, STR_TERMINATE); + srvstr_pull_buf(inbuf, &fname[dir_name_len], smb_buf(inbuf), sizeof(fname)-dir_name_len, STR_TERMINATE); } else { - srvstr_pull(inbuf, fname, smb_buf(inbuf), sizeof(fname), -1, STR_TERMINATE); + srvstr_pull_buf(inbuf, fname, smb_buf(inbuf), sizeof(fname), STR_TERMINATE); } /* @@ -880,7 +879,7 @@ static int do_nt_transact_create_pipe( connection_struct *conn, return ERROR_DOS(ERRDOS,ERRbadaccess); } - srvstr_pull(inbuf, fname, params+53, sizeof(fname), -1, STR_TERMINATE); + srvstr_pull(inbuf, fname, params+53, sizeof(fname), total_parameter_count-53, STR_TERMINATE); if ((ret = nt_open_pipe(fname, conn, inbuf, outbuf, &pnum)) != 0) return ret; @@ -1096,7 +1095,7 @@ static int call_nt_transact_create(connection_struct *conn, * Check to see if this is a mac fork of some kind. */ - srvstr_pull(inbuf, fname, params+53, sizeof(fname), -1, STR_TERMINATE); + srvstr_pull(inbuf, fname, params+53, sizeof(fname), total_parameter_count-53, STR_TERMINATE); if( strchr_m(fname, ':')) { return ERROR_NT(NT_STATUS_OBJECT_PATH_NOT_FOUND); @@ -1122,9 +1121,9 @@ static int call_nt_transact_create(connection_struct *conn, } srvstr_pull(inbuf, &fname[dir_name_len], params+53, sizeof(fname)-dir_name_len, - -1, STR_TERMINATE); + total_parameter_count-53, STR_TERMINATE); } else { - srvstr_pull(inbuf, fname, params+53, sizeof(fname), -1, STR_TERMINATE); + srvstr_pull(inbuf, fname, params+53, sizeof(fname), total_parameter_count-53, STR_TERMINATE); } /* diff --git a/source3/smbd/pipes.c b/source3/smbd/pipes.c index 6c1e6efa735..f7e9c595c13 100644 --- a/source3/smbd/pipes.c +++ b/source3/smbd/pipes.c @@ -50,7 +50,7 @@ int reply_open_pipe_and_X(connection_struct *conn, int i; /* XXXX we need to handle passed times, sattr and flags */ - srvstr_pull(inbuf, pipe_name, smb_buf(inbuf), sizeof(pipe_name), -1, STR_TERMINATE); + srvstr_pull_buf(inbuf, pipe_name, smb_buf(inbuf), sizeof(pipe_name), STR_TERMINATE); /* If the name doesn't start \PIPE\ then this is directed */ /* at a mailslot or something we really, really don't understand, */ diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index 60b1d13417c..0ccdf7c241b 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -163,10 +163,10 @@ int reply_tcon(connection_struct *conn, *service = *password = *dev = 0; p = smb_buf(inbuf)+1; - p += srvstr_pull(inbuf, service, p, sizeof(service), -1, STR_TERMINATE) + 1; - pwlen = srvstr_pull(inbuf, password, p, sizeof(password), -1, STR_TERMINATE) + 1; + p += srvstr_pull_buf(inbuf, service, p, sizeof(service), STR_TERMINATE) + 1; + pwlen = srvstr_pull_buf(inbuf, password, p, sizeof(password), STR_TERMINATE) + 1; p += pwlen; - p += srvstr_pull(inbuf, dev, p, sizeof(dev), -1, STR_TERMINATE) + 1; + p += srvstr_pull_buf(inbuf, dev, p, sizeof(dev), STR_TERMINATE) + 1; p = strrchr_m(service,'\\'); if (p) { @@ -233,7 +233,7 @@ int reply_tcon_and_X(connection_struct *conn, char *inbuf,char *outbuf,int lengt } p = smb_buf(inbuf) + passlen; - p += srvstr_pull(inbuf, path, p, sizeof(path), -1, STR_TERMINATE); + p += srvstr_pull_buf(inbuf, path, p, sizeof(path), STR_TERMINATE); /* * the service name can be either: \\server\share @@ -377,7 +377,7 @@ int reply_chkpth(connection_struct *conn, char *inbuf,char *outbuf, int dum_size SMB_STRUCT_STAT sbuf; START_PROFILE(SMBchkpth); - srvstr_pull(inbuf, name, smb_buf(inbuf) + 1, sizeof(name), -1, STR_TERMINATE); + srvstr_pull_buf(inbuf, name, smb_buf(inbuf) + 1, sizeof(name), STR_TERMINATE); RESOLVE_DFSPATH(name, conn, inbuf, outbuf); @@ -429,7 +429,7 @@ int reply_getatr(connection_struct *conn, char *inbuf,char *outbuf, int dum_size START_PROFILE(SMBgetatr); p = smb_buf(inbuf) + 1; - p += srvstr_pull(inbuf, fname, p, sizeof(fname), -1, STR_TERMINATE); + p += srvstr_pull_buf(inbuf, fname, p, sizeof(fname), STR_TERMINATE); RESOLVE_DFSPATH(fname, conn, inbuf, outbuf); @@ -505,7 +505,7 @@ int reply_setatr(connection_struct *conn, char *inbuf,char *outbuf, int dum_size START_PROFILE(SMBsetatr); p = smb_buf(inbuf) + 1; - p += srvstr_pull(inbuf, fname, p, sizeof(fname), -1, STR_TERMINATE); + p += srvstr_pull_buf(inbuf, fname, p, sizeof(fname), STR_TERMINATE); unix_convert(fname,conn,0,&bad_path,&sbuf); mode = SVAL(inbuf,smb_vwv0); @@ -625,7 +625,7 @@ int reply_search(connection_struct *conn, char *inbuf,char *outbuf, int dum_size maxentries = SVAL(inbuf,smb_vwv0); dirtype = SVAL(inbuf,smb_vwv1); p = smb_buf(inbuf) + 1; - p += srvstr_pull(inbuf, path, p, sizeof(path), -1, STR_TERMINATE); + p += srvstr_pull_buf(inbuf, path, p, sizeof(path), STR_TERMINATE); p++; status_len = SVAL(p, 0); p += 2; @@ -806,7 +806,7 @@ int reply_fclose(connection_struct *conn, char *inbuf,char *outbuf, int dum_size outsize = set_message(outbuf,1,0,True); p = smb_buf(inbuf) + 1; - p += srvstr_pull(inbuf, path, p, sizeof(path), -1, STR_TERMINATE); + p += srvstr_pull_buf(inbuf, path, p, sizeof(path), STR_TERMINATE); p++; status_len = SVAL(p,0); p += 2; @@ -854,7 +854,7 @@ int reply_open(connection_struct *conn, char *inbuf,char *outbuf, int dum_size, share_mode = SVAL(inbuf,smb_vwv0); - srvstr_pull(inbuf, fname, smb_buf(inbuf)+1, sizeof(fname), -1, STR_TERMINATE); + srvstr_pull_buf(inbuf, fname, smb_buf(inbuf)+1, sizeof(fname), STR_TERMINATE); RESOLVE_DFSPATH(fname, conn, inbuf, outbuf); @@ -944,7 +944,7 @@ int reply_open_and_X(connection_struct *conn, char *inbuf,char *outbuf,int lengt } /* XXXX we need to handle passed times, sattr and flags */ - srvstr_pull(inbuf, fname, smb_buf(inbuf), sizeof(fname), -1, STR_TERMINATE); + srvstr_pull_buf(inbuf, fname, smb_buf(inbuf), sizeof(fname), STR_TERMINATE); RESOLVE_DFSPATH(fname, conn, inbuf, outbuf); @@ -1063,7 +1063,7 @@ int reply_mknew(connection_struct *conn, char *inbuf,char *outbuf, int dum_size, com = SVAL(inbuf,smb_com); createmode = SVAL(inbuf,smb_vwv0); - srvstr_pull(inbuf, fname, smb_buf(inbuf) + 1, sizeof(fname), -1, STR_TERMINATE); + srvstr_pull_buf(inbuf, fname, smb_buf(inbuf) + 1, sizeof(fname), STR_TERMINATE); RESOLVE_DFSPATH(fname, conn, inbuf, outbuf); @@ -1135,7 +1135,7 @@ int reply_ctemp(connection_struct *conn, char *inbuf,char *outbuf, int dum_size, START_PROFILE(SMBctemp); createmode = SVAL(inbuf,smb_vwv0); - srvstr_pull(inbuf, fname, smb_buf(inbuf)+1, sizeof(fname), -1, STR_TERMINATE); + srvstr_pull_buf(inbuf, fname, smb_buf(inbuf)+1, sizeof(fname), STR_TERMINATE); pstrcat(fname,"\\TMXXXXXX"); RESOLVE_DFSPATH(fname, conn, inbuf, outbuf); @@ -1393,7 +1393,7 @@ int reply_unlink(connection_struct *conn, char *inbuf,char *outbuf, int dum_size dirtype = SVAL(inbuf,smb_vwv0); - srvstr_pull(inbuf, name, smb_buf(inbuf) + 1, sizeof(name), -1, STR_TERMINATE); + srvstr_pull_buf(inbuf, name, smb_buf(inbuf) + 1, sizeof(name), STR_TERMINATE); RESOLVE_DFSPATH(name, conn, inbuf, outbuf); @@ -2742,7 +2742,7 @@ int reply_mkdir(connection_struct *conn, char *inbuf,char *outbuf, int dum_size, NTSTATUS status; START_PROFILE(SMBmkdir); - srvstr_pull(inbuf, directory, smb_buf(inbuf) + 1, sizeof(directory), -1, STR_TERMINATE); + srvstr_pull_buf(inbuf, directory, smb_buf(inbuf) + 1, sizeof(directory), STR_TERMINATE); status = mkdir_internal(conn, directory); if (!NT_STATUS_IS_OK(status)) @@ -2903,7 +2903,7 @@ int reply_rmdir(connection_struct *conn, char *inbuf,char *outbuf, int dum_size, SMB_STRUCT_STAT sbuf; START_PROFILE(SMBrmdir); - srvstr_pull(inbuf, directory, smb_buf(inbuf) + 1, sizeof(directory), -1, STR_TERMINATE); + srvstr_pull_buf(inbuf, directory, smb_buf(inbuf) + 1, sizeof(directory), STR_TERMINATE); RESOLVE_DFSPATH(directory, conn, inbuf, outbuf) @@ -3264,9 +3264,9 @@ int reply_mv(connection_struct *conn, char *inbuf,char *outbuf, int dum_size, START_PROFILE(SMBmv); p = smb_buf(inbuf) + 1; - p += srvstr_pull(inbuf, name, p, sizeof(name), -1, STR_TERMINATE); + p += srvstr_pull_buf(inbuf, name, p, sizeof(name), STR_TERMINATE); p++; - p += srvstr_pull(inbuf, newname, p, sizeof(newname), -1, STR_TERMINATE); + p += srvstr_pull_buf(inbuf, newname, p, sizeof(newname), STR_TERMINATE); RESOLVE_DFSPATH(name, conn, inbuf, outbuf); RESOLVE_DFSPATH(newname, conn, inbuf, outbuf); @@ -3396,8 +3396,8 @@ int reply_copy(connection_struct *conn, char *inbuf,char *outbuf, int dum_size, *directory = *mask = 0; p = smb_buf(inbuf); - p += srvstr_pull(inbuf, name, p, sizeof(name), -1, STR_TERMINATE); - p += srvstr_pull(inbuf, newname, p, sizeof(newname), -1, STR_TERMINATE); + p += srvstr_pull_buf(inbuf, name, p, sizeof(name), STR_TERMINATE); + p += srvstr_pull_buf(inbuf, newname, p, sizeof(newname), STR_TERMINATE); DEBUG(3,("reply_copy : %s -> %s\n",name,newname)); @@ -3549,7 +3549,7 @@ int reply_setdir(connection_struct *conn, char *inbuf,char *outbuf, int dum_size return ERROR_DOS(ERRDOS,ERRnoaccess); } - srvstr_pull(inbuf, newdir, smb_buf(inbuf) + 1, sizeof(newdir), -1, STR_TERMINATE); + srvstr_pull_buf(inbuf, newdir, smb_buf(inbuf) + 1, sizeof(newdir), STR_TERMINATE); if (strlen(newdir) == 0) { ok = True; diff --git a/source3/smbd/sesssetup.c b/source3/smbd/sesssetup.c index 270a69d96ad..8b9d826067f 100644 --- a/source3/smbd/sesssetup.c +++ b/source3/smbd/sesssetup.c @@ -611,7 +611,7 @@ int reply_sesssetup_and_X(connection_struct *conn, char *inbuf,char *outbuf, plaintext_password.data[passlen1] = 0; } - srvstr_pull(inbuf, user, smb_buf(inbuf)+passlen1, sizeof(user), -1, STR_TERMINATE); + srvstr_pull_buf(inbuf, user, smb_buf(inbuf)+passlen1, sizeof(user), STR_TERMINATE); *domain = 0; } else { @@ -674,14 +674,10 @@ int reply_sesssetup_and_X(connection_struct *conn, char *inbuf,char *outbuf, } p += passlen1 + passlen2; - p += srvstr_pull(inbuf, user, p, sizeof(user), -1, - STR_TERMINATE); - p += srvstr_pull(inbuf, domain, p, sizeof(domain), - -1, STR_TERMINATE); - p += srvstr_pull(inbuf, native_os, p, sizeof(native_os), - -1, STR_TERMINATE); - p += srvstr_pull(inbuf, native_lanman, p, sizeof(native_lanman), - -1, STR_TERMINATE); + p += srvstr_pull_buf(inbuf, user, p, sizeof(user), STR_TERMINATE); + p += srvstr_pull_buf(inbuf, domain, p, sizeof(domain), STR_TERMINATE); + p += srvstr_pull_buf(inbuf, native_os, p, sizeof(native_os), STR_TERMINATE); + p += srvstr_pull_buf(inbuf, native_lanman, p, sizeof(native_lanman), STR_TERMINATE); DEBUG(3,("Domain=[%s] NativeOS=[%s] NativeLanMan=[%s]\n", domain,native_os,native_lanman)); } diff --git a/source3/smbd/srvstr.c b/source3/smbd/srvstr.c index 90da422f133..36fecf5bd2e 100644 --- a/source3/smbd/srvstr.c +++ b/source3/smbd/srvstr.c @@ -30,3 +30,12 @@ int srvstr_pull(void *base_ptr, char *dest, const void *src, int dest_len, int s { return pull_string(base_ptr, dest, src, dest_len, src_len, flags); } + +/* pull a string from the smb_buf part of a packet. In this case the + string can either be null terminated or it can be terminated by the + end of the smbbuf area +*/ +int srvstr_pull_buf(void *inbuf, char *dest, const void *src, int dest_len, int flags) +{ + return pull_string(inbuf, dest, src, dest_len, smb_bufrem(inbuf, src), flags); +} -- 2.34.1