1 files changed, 81 insertions, 110 deletions

diff --git a/source3/smbd/ipc.c b/source3/smbd/ipc.c
index b9460e5211..649ead4682 100644
--- a/source3/smbd/ipc.c
+++ b/source3/smbd/ipc.c
@@ -91,7 +91,6 @@ void send_trans_reply(connection_struct *conn,
 	int tot_data_sent = 0;
 	int tot_param_sent = 0;
 	int align;
-	char *outbuf;
 
 	int ldata  = rdata  ? rdata_len : 0;
 	int lparam = rparam ? rparam_len : 0;
@@ -104,38 +103,43 @@ void send_trans_reply(connection_struct *conn,
 
 	align = ((this_lparam)%4);
 
-	if (!create_outbuf(talloc_tos(), (char *)req->inbuf, &outbuf,
-			   10, 1+align+this_ldata+this_lparam)) {
-		smb_panic("could not allocate outbuf");
-	}
+	reply_outbuf(req, 10, 1+align+this_ldata+this_lparam);
+
+	/*
+	 * We might have SMBtranss in req which was transferred to the outbuf,
+	 * fix that.
+	 */
+	SCVAL(req->outbuf, smb_com, SMBtrans);
 
-	copy_trans_params_and_data(outbuf, align,
+	copy_trans_params_and_data((char *)req->outbuf, align,
 				rparam, tot_param_sent, this_lparam,
 				rdata, tot_data_sent, this_ldata);
 
-	SSVAL(outbuf,smb_vwv0,lparam);
-	SSVAL(outbuf,smb_vwv1,ldata);
-	SSVAL(outbuf,smb_vwv3,this_lparam);
-	SSVAL(outbuf,smb_vwv4,smb_offset(smb_buf(outbuf)+1,outbuf));
-	SSVAL(outbuf,smb_vwv5,0);
-	SSVAL(outbuf,smb_vwv6,this_ldata);
-	SSVAL(outbuf,smb_vwv7,smb_offset(smb_buf(outbuf)+1+this_lparam+align,
-					 outbuf));
-	SSVAL(outbuf,smb_vwv8,0);
-	SSVAL(outbuf,smb_vwv9,0);
+	SSVAL(req->outbuf,smb_vwv0,lparam);
+	SSVAL(req->outbuf,smb_vwv1,ldata);
+	SSVAL(req->outbuf,smb_vwv3,this_lparam);
+	SSVAL(req->outbuf,smb_vwv4,
+	      smb_offset(smb_buf(req->outbuf)+1, req->outbuf));
+	SSVAL(req->outbuf,smb_vwv5,0);
+	SSVAL(req->outbuf,smb_vwv6,this_ldata);
+	SSVAL(req->outbuf,smb_vwv7,
+	      smb_offset(smb_buf(req->outbuf)+1+this_lparam+align,
+			 req->outbuf));
+	SSVAL(req->outbuf,smb_vwv8,0);
+	SSVAL(req->outbuf,smb_vwv9,0);
 
 	if (buffer_too_large) {
-		error_packet_set((char *)outbuf, ERRDOS, ERRmoredata,
+		error_packet_set((char *)req->outbuf, ERRDOS, ERRmoredata,
 				 STATUS_BUFFER_OVERFLOW, __LINE__, __FILE__);
 	}
 
-	show_msg(outbuf);
-	if (!srv_send_smb(smbd_server_fd(), (char *)outbuf,
+	show_msg((char *)req->outbuf);
+	if (!srv_send_smb(smbd_server_fd(), (char *)req->outbuf,
 			  IS_CONN_ENCRYPTED(conn))) {
 		exit_server_cleanly("send_trans_reply: srv_send_smb failed.");
 	}
 
-	TALLOC_FREE(outbuf);
+	TALLOC_FREE(req->outbuf);
 
 	tot_data_sent = this_ldata;
 	tot_param_sent = this_lparam;
@@ -155,39 +159,45 @@ void send_trans_reply(connection_struct *conn,
 
 		align = (this_lparam%4);
 
-		if (!create_outbuf(talloc_tos(), (char *)req->inbuf, &outbuf,
-				   10, 1+align+this_ldata+this_lparam)) {
-			smb_panic("could not allocate outbuf");
-		}
+		reply_outbuf(req, 10, 1+align+this_ldata+this_lparam);
+
+		/*
+		 * We might have SMBtranss in req which was transferred to the
+		 * outbuf, fix that.
+		 */
+		SCVAL(req->outbuf, smb_com, SMBtrans);
 
-		copy_trans_params_and_data(outbuf, align,
+		copy_trans_params_and_data((char *)req->outbuf, align,
 					   rparam, tot_param_sent, this_lparam,
 					   rdata, tot_data_sent, this_ldata);
 
-		SSVAL(outbuf,smb_vwv3,this_lparam);
-		SSVAL(outbuf,smb_vwv4,smb_offset(smb_buf(outbuf)+1,outbuf));
-		SSVAL(outbuf,smb_vwv5,tot_param_sent);
-		SSVAL(outbuf,smb_vwv6,this_ldata);
-		SSVAL(outbuf,smb_vwv7,
-		      smb_offset(smb_buf(outbuf)+1+this_lparam+align, outbuf));
-		SSVAL(outbuf,smb_vwv8,tot_data_sent);
-		SSVAL(outbuf,smb_vwv9,0);
+		SSVAL(req->outbuf,smb_vwv3,this_lparam);
+		SSVAL(req->outbuf,smb_vwv4,
+		      smb_offset(smb_buf(req->outbuf)+1,req->outbuf));
+		SSVAL(req->outbuf,smb_vwv5,tot_param_sent);
+		SSVAL(req->outbuf,smb_vwv6,this_ldata);
+		SSVAL(req->outbuf,smb_vwv7,
+		      smb_offset(smb_buf(req->outbuf)+1+this_lparam+align,
+				 req->outbuf));
+		SSVAL(req->outbuf,smb_vwv8,tot_data_sent);
+		SSVAL(req->outbuf,smb_vwv9,0);
 
 		if (buffer_too_large) {
-			error_packet_set(outbuf, ERRDOS, ERRmoredata,
+			error_packet_set((char *)req->outbuf,
+					 ERRDOS, ERRmoredata,
 					 STATUS_BUFFER_OVERFLOW,
 					 __LINE__, __FILE__);
 		}
 
-		show_msg(outbuf);
-		if (!srv_send_smb(smbd_server_fd(), outbuf,
+		show_msg((char *)req->outbuf);
+		if (!srv_send_smb(smbd_server_fd(), (char *)req->outbuf,
 				  IS_CONN_ENCRYPTED(conn)))
 			exit_server_cleanly("send_trans_reply: srv_send_smb "
 					    "failed.");
 
 		tot_data_sent  += this_ldata;
 		tot_param_sent += this_lparam;
-		TALLOC_FREE(outbuf);
+		TALLOC_FREE(req->outbuf);
 	}
 }
 
@@ -493,8 +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);
 
@@ -504,8 +512,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);
 	psoff = SVAL(req->vwv+10, 0);
@@ -551,6 +557,12 @@ void reply_trans(struct smb_request *req)
 		goto bad_param;
 
 	if (state->total_data)  {
+
+		if (trans_oob(state->total_data, 0, dscnt)
+		    || trans_oob(smb_len(req->inbuf), dsoff, dscnt)) {
+			goto bad_param;
+		}
+
 		/* Can't use talloc here, the core routines do realloc on the
 		 * params and data. Out of paranoia, 100 bytes too many. */
 		state->data = (char *)SMB_MALLOC(state->total_data+100);
@@ -565,21 +577,16 @@ void reply_trans(struct smb_request *req)
 		/* null-terminate the slack space */
 		memset(&state->data[state->total_data], 0, 100);
 
-		if (dscnt > state->total_data ||
-				dsoff+dscnt < dsoff) {
-			goto bad_param;
-		}
-
-		if (dsoff > av_size ||
-				dscnt > av_size ||
-				dsoff+dscnt > av_size) {
-			goto bad_param;
-		}
-
 		memcpy(state->data,smb_base(req->inbuf)+dsoff,dscnt);
 	}
 
 	if (state->total_param) {
+
+		if (trans_oob(state->total_param, 0, pscnt)
+		    || trans_oob(smb_len(req->inbuf), psoff, pscnt)) {
+			goto bad_param;
+		}
+
 		/* Can't use talloc here, the core routines do realloc on the
 		 * params and data. Out of paranoia, 100 bytes too many */
 		state->param = (char *)SMB_MALLOC(state->total_param+100);
@@ -595,17 +602,6 @@ void reply_trans(struct smb_request *req)
 		/* null-terminate the slack space */
 		memset(&state->param[state->total_param], 0, 100);
 
-		if (pscnt > state->total_param ||
-				psoff+pscnt < psoff) {
-			goto bad_param;
-		}
-
-		if (psoff > av_size ||
-				pscnt > av_size ||
-				psoff+pscnt > av_size) {
-			goto bad_param;
-		}
-
 		memcpy(state->param,smb_base(req->inbuf)+psoff,pscnt);
 	}
 
@@ -614,6 +610,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 "
@@ -626,17 +635,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;
@@ -682,7 +684,6 @@ void reply_transs(struct smb_request *req)
 	connection_struct *conn = req->conn;
 	unsigned int pcnt,poff,dcnt,doff,pdisp,ddisp;
 	struct trans_state *state;
-	unsigned int av_size;
 
 	START_PROFILE(SMBtranss);
 
@@ -715,8 +716,6 @@ void reply_transs(struct smb_request *req)
 	if (SVAL(req->vwv+1, 0) < state->total_data)
 		state->total_data = SVAL(req->vwv+1, 0);
 
-	av_size = smb_len(req->inbuf);
-
 	pcnt = SVAL(req->vwv+2, 0);
 	poff = SVAL(req->vwv+3, 0);
 	pdisp = SVAL(req->vwv+4, 0);
@@ -733,41 +732,19 @@ void reply_transs(struct smb_request *req)
 		goto bad_param;
 
 	if (pcnt) {
-		if (pdisp > state->total_param ||
-				pcnt > state->total_param ||
-				pdisp+pcnt > state->total_param ||
-				pdisp+pcnt < pdisp) {
-			goto bad_param;
-		}
-
-		if (poff > av_size ||
-				pcnt > av_size ||
-				poff+pcnt > av_size ||
-				poff+pcnt < poff) {
+		if (trans_oob(state->total_param, pdisp, pcnt)
+		    || trans_oob(smb_len(req->inbuf), poff, pcnt)) {
 			goto bad_param;
 		}
-
-		memcpy(state->param+pdisp,smb_base(req->inbuf)+poff,
-		       pcnt);
+		memcpy(state->param+pdisp,smb_base(req->inbuf)+poff,pcnt);
 	}
 
 	if (dcnt) {
-		if (ddisp > state->total_data ||
-				dcnt > state->total_data ||
-				ddisp+dcnt > state->total_data ||
-				ddisp+dcnt < ddisp) {
+		if (trans_oob(state->total_data, ddisp, dcnt)
+		    || trans_oob(smb_len(req->inbuf), doff, dcnt)) {
 			goto bad_param;
 		}
-
-		if (ddisp > av_size ||
-				dcnt > av_size ||
-				ddisp+dcnt > av_size ||
-				ddisp+dcnt < ddisp) {
-			goto bad_param;
-		}
-
-		memcpy(state->data+ddisp, smb_base(req->inbuf)+doff,
-		       dcnt);
+		memcpy(state->data+ddisp, smb_base(req->inbuf)+doff,dcnt);
 	}
 
 	if ((state->received_param < state->total_param) ||
@@ -776,12 +753,6 @@ void reply_transs(struct smb_request *req)
 		return;
 	}
 
-        /*
-	 * construct_reply_common will copy smb_com from inbuf to
-	 * outbuf. SMBtranss is wrong here.
-         */
-        SCVAL(req->inbuf,smb_com,SMBtrans);
-
 	handle_trans(conn, req, state);
 
 	DLIST_REMOVE(conn->pending_trans, state);