The branch, master has been updated via 9a3be6f0f8e120797a02fa1be60b51812cfd86f5 (commit) via 738271fc2026b2911b7d20a73496989641714df3 (commit) via 9da3101e449649f0614240f13157ac81e17b2e90 (commit) via 4a322398c5ffaf238eba1e7bfbe47e2d093c7c4d (commit) via 599707c87a739811ba426e44b11189e1ddba078e (commit) from 6a627b440e8b3f42db2a8a27047dd3482bad0d28 (commit)
http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master - Log ----------------------------------------------------------------- commit 9a3be6f0f8e120797a02fa1be60b51812cfd86f5 Author: Volker Lendecke <[EMAIL PROTECTED]> Date: Sat Nov 8 16:48:20 2008 +0100 Move cli_trans_oob to lib/util.c Rename it to trans_oob, it will be used in the server routines. commit 738271fc2026b2911b7d20a73496989641714df3 Author: Volker Lendecke <[EMAIL PROTECTED]> Date: Sat Nov 8 16:14:12 2008 +0100 Remove the variable "size" from reply_nttrans This converts the range checks for the setup[] array to rely on req->wct being set correctly in init_smb_request. As that already verifies the vwv array to be in the range of the smb_request inbuf, we don't have to do overflow checks here anymore. Jeremy, please check thoroughly! :-) Thanks, Volker commit 9da3101e449649f0614240f13157ac81e17b2e90 Author: Volker Lendecke <[EMAIL PROTECTED]> Date: Sat Nov 8 16:14:12 2008 +0100 Remove the variable "size" from reply_trans This converts the range checks for the setup[] array to rely on req->wct being set correctly in init_smb_request. As that already verifies the vwv array to be in the range of the smb_request inbuf, we don't have to do overflow checks here anymore. Jeremy, please check thoroughly! :-) Thanks, Volker commit 4a322398c5ffaf238eba1e7bfbe47e2d093c7c4d Author: Volker Lendecke <[EMAIL PROTECTED]> Date: Sat Nov 8 16:03:07 2008 +0100 Remove an unused variable commit 599707c87a739811ba426e44b11189e1ddba078e Author: Volker Lendecke <[EMAIL PROTECTED]> Date: Sat Nov 8 15:44:20 2008 +0100 Remove two direct inbuf references from reply_sesssetup_and_X_spnego() ----------------------------------------------------------------------- Summary of changes: source3/include/proto.h | 1 + source3/lib/util.c | 19 +++++++++++++++++++ source3/libsmb/clitrans.c | 21 ++++----------------- source3/smbd/ipc.c | 28 ++++++++++++++++------------ source3/smbd/nttrans.c | 23 +++++++++++++---------- source3/smbd/sesssetup.c | 4 ++-- source3/smbd/trans2.c | 2 -- 7 files changed, 55 insertions(+), 43 deletions(-) Changeset truncated at 500 lines: diff --git a/source3/include/proto.h b/source3/include/proto.h index 73be87b..71f12a6 100644 --- a/source3/include/proto.h +++ b/source3/include/proto.h @@ -1251,6 +1251,7 @@ char *procid_str_static(const struct server_id *pid); bool procid_valid(const struct server_id *pid); bool procid_is_local(const struct server_id *pid); int this_is_smp(void); +bool trans_oob(uint32_t bufsize, uint32_t offset, uint32_t length); bool is_offset_safe(const char *buf_base, size_t buf_len, char *ptr, size_t off); char *get_safe_ptr(const char *buf_base, size_t buf_len, char *ptr, size_t off); char *get_safe_str_ptr(const char *buf_base, size_t buf_len, char *ptr, size_t off); diff --git a/source3/lib/util.c b/source3/lib/util.c index 5007fb7..074b523 100644 --- a/source3/lib/util.c +++ b/source3/lib/util.c @@ -2879,6 +2879,25 @@ int this_is_smp(void) } /**************************************************************** + Check if offset/length fit into bufsize. Should probably be + merged with is_offset_safe, but this would require a rewrite + of lanman.c. Later :-) +****************************************************************/ + +bool trans_oob(uint32_t bufsize, uint32_t offset, uint32_t length) +{ + if ((offset + length < offset) || (offset + length < length)) { + /* wrap */ + return true; + } + if ((offset > bufsize) || (offset + length > bufsize)) { + /* overflow */ + return true; + } + return false; +} + +/**************************************************************** Check if an offset into a buffer is safe. If this returns True it's safe to indirect into the byte at pointer ptr+off. diff --git a/source3/libsmb/clitrans.c b/source3/libsmb/clitrans.c index c929f0b..bbdfb75 100644 --- a/source3/libsmb/clitrans.c +++ b/source3/libsmb/clitrans.c @@ -978,19 +978,6 @@ static void cli_trans_ship_rest(struct async_req *req, } } -static bool cli_trans_oob(uint32_t bufsize, uint32_t offset, uint32_t length) -{ - if ((offset + length < offset) || (offset + length < length)) { - /* wrap */ - return true; - } - if ((offset > bufsize) || (offset + length > bufsize)) { - /* overflow */ - return true; - } - return false; -} - static NTSTATUS cli_pull_trans(struct async_req *req, struct cli_request *cli_req, uint8_t smb_cmd, bool expect_first_reply, @@ -1072,10 +1059,10 @@ static NTSTATUS cli_pull_trans(struct async_req *req, * length. Likewise for param_ofs/param_disp. */ - if (cli_trans_oob(smb_len(cli_req->inbuf), param_ofs, *pnum_param) - || cli_trans_oob(*ptotal_param, *pparam_disp, *pnum_param) - || cli_trans_oob(smb_len(cli_req->inbuf), data_ofs, *pnum_data) - || cli_trans_oob(*ptotal_data, *pdata_disp, *pnum_data)) { + if (trans_oob(smb_len(cli_req->inbuf), param_ofs, *pnum_param) + || trans_oob(*ptotal_param, *pparam_disp, *pnum_param) + || trans_oob(smb_len(cli_req->inbuf), data_ofs, *pnum_data) + || trans_oob(*ptotal_data, *pdata_disp, *pnum_data)) { return NT_STATUS_INVALID_NETWORK_RESPONSE; } diff --git a/source3/smbd/ipc.c b/source3/smbd/ipc.c index a617756..bf9b1d8 100644 --- a/source3/smbd/ipc.c +++ b/source3/smbd/ipc.c @@ -503,7 +503,6 @@ void reply_trans(struct smb_request *req) unsigned int pscnt; struct trans_state *state; NTSTATUS result; - unsigned int size; unsigned int av_size; START_PROFILE(SMBtrans); @@ -514,7 +513,6 @@ void reply_trans(struct smb_request *req) return; } - size = smb_len(req->inbuf) + 4; av_size = smb_len(req->inbuf); dsoff = SVAL(req->vwv+12, 0); dscnt = SVAL(req->vwv+11, 0); @@ -624,6 +622,19 @@ void reply_trans(struct smb_request *req) if (state->setup_count) { unsigned int i; + + /* + * No overflow possible here, state->setup_count is an + * unsigned int, being filled by a single byte from + * CVAL(req->vwv+13, 0) above. The cast in the comparison + * below is not necessary, it's here to clarify things. The + * validity of req->vwv and req->wct has been checked in + * init_smb_request already. + */ + if (state->setup_count + 14 > (unsigned int)req->wct) { + goto bad_param; + } + if((state->setup = TALLOC_ARRAY( state, uint16, state->setup_count)) == NULL) { DEBUG(0,("reply_trans: setup malloc fail for %u " @@ -636,17 +647,10 @@ void reply_trans(struct smb_request *req) END_PROFILE(SMBtrans); return; } - if (req->inbuf+smb_vwv14+(state->setup_count*SIZEOFWORD) > - req->inbuf + size) - goto bad_param; - if ((smb_vwv14+(state->setup_count*SIZEOFWORD) < smb_vwv14) || - (smb_vwv14+(state->setup_count*SIZEOFWORD) < - (state->setup_count*SIZEOFWORD))) - goto bad_param; - for (i=0;i<state->setup_count;i++) - state->setup[i] = SVAL(req->inbuf, - smb_vwv14+i*SIZEOFWORD); + for (i=0;i<state->setup_count;i++) { + state->setup[i] = SVAL(req->vwv + 14 + i, 0); + } } state->received_param = pscnt; diff --git a/source3/smbd/nttrans.c b/source3/smbd/nttrans.c index 329ba23..b516f02 100644 --- a/source3/smbd/nttrans.c +++ b/source3/smbd/nttrans.c @@ -2529,7 +2529,6 @@ void reply_nttrans(struct smb_request *req) uint16 function_code; NTSTATUS result; struct trans_state *state; - uint32_t size; uint32_t av_size; START_PROFILE(SMBnttrans); @@ -2540,7 +2539,6 @@ void reply_nttrans(struct smb_request *req) return; } - size = smb_len(req->inbuf) + 4; av_size = smb_len(req->inbuf); pscnt = IVAL(req->vwv+9, 1); psoff = IVAL(req->vwv+11, 1); @@ -2676,6 +2674,19 @@ void reply_nttrans(struct smb_request *req) if(state->setup_count > 0) { DEBUG(10,("reply_nttrans: state->setup_count = %d\n", state->setup_count)); + + /* + * No overflow possible here, state->setup_count is an + * unsigned int, being filled by a single byte from + * CVAL(req->vwv+13, 0) above. The cast in the comparison + * below is not necessary, it's here to clarify things. The + * validity of req->vwv and req->wct has been checked in + * init_smb_request already. + */ + if ((state->setup_count/2) + 19 > (unsigned int)req->wct) { + goto bad_param; + } + state->setup = (uint16 *)TALLOC(state, state->setup_count); if (state->setup == NULL) { DEBUG(0,("reply_nttrans : Out of memory\n")); @@ -2687,14 +2698,6 @@ void reply_nttrans(struct smb_request *req) return; } - if ((smb_nt_SetupStart + state->setup_count < smb_nt_SetupStart) || - (smb_nt_SetupStart + state->setup_count < state->setup_count)) { - goto bad_param; - } - if (smb_nt_SetupStart + state->setup_count > size) { - goto bad_param; - } - memcpy(state->setup, req->vwv+19, state->setup_count); dump_data(10, (uint8 *)state->setup, state->setup_count); } diff --git a/source3/smbd/sesssetup.c b/source3/smbd/sesssetup.c index fde6cdc..24a2010 100644 --- a/source3/smbd/sesssetup.c +++ b/source3/smbd/sesssetup.c @@ -1171,7 +1171,7 @@ static void reply_sesssetup_and_X_spnego(struct smb_request *req) const char *p2; uint16 data_blob_len = SVAL(req->vwv+7, 0); enum remote_arch_types ra_type = get_remote_arch(); - int vuid = SVAL(req->inbuf,smb_uid); + int vuid = req->vuid; user_struct *vuser = NULL; NTSTATUS status = NT_STATUS_OK; uint16 smbpid = req->smbpid; @@ -1203,7 +1203,7 @@ static void reply_sesssetup_and_X_spnego(struct smb_request *req) file_save("negotiate.dat", blob1.data, blob1.length); #endif - p2 = (char *)req->inbuf + smb_vwv13 + data_blob_len; + p2 = (char *)req->buf + data_blob_len; p2 += srvstr_pull_req_talloc(talloc_tos(), req, &tmp, p2, STR_TERMINATE); diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c index 0c63588..3a28bd4 100644 --- a/source3/smbd/trans2.c +++ b/source3/smbd/trans2.c @@ -7533,7 +7533,6 @@ void reply_trans2(struct smb_request *req) unsigned int psoff; unsigned int pscnt; unsigned int tran_call; - unsigned int size; unsigned int av_size; struct trans_state *state; NTSTATUS result; @@ -7551,7 +7550,6 @@ void reply_trans2(struct smb_request *req) psoff = SVAL(req->vwv+10, 0); pscnt = SVAL(req->vwv+9, 0); tran_call = SVAL(req->vwv+14, 0); - size = smb_len(req->inbuf) + 4; av_size = smb_len(req->inbuf); result = allow_new_trans(conn->pending_trans, req->mid); -- Samba Shared Repository