sin/samba-mirror
1
0
Fork 0
mirror of https://github.com/samba-team/samba.git synced 2025-03-19 18:50:24 +03:00
Code

libcli/smb: make sure we have a body size of 0x31 before dereferencing an ioctl response

Browse Source 
Found by valgrind, reported by Noel Power <nopower@suse.com>:

==7913== Invalid read of size 1
==7913==    at 0xC4F23EE: smb2cli_ioctl_done (smb2cli_ioctl.c:245)
==7913==    by 0x747A744: _tevent_req_notify_callback (tevent_req.c:112)
==7913==    by 0x747A817: tevent_req_finish (tevent_req.c:149)
==7913==    by 0x747A93C: tevent_req_trigger (tevent_req.c:206)
==7913==    by 0x7479B2B: tevent_common_loop_immediate
(tevent_immediate.c:135)
==7913==    by 0xA9CB4BE: run_events_poll (events.c:192)
==7913==    by 0xA9CBB32: s3_event_loop_once (events.c:303)
==7913==    by 0x7478C72: _tevent_loop_once (tevent.c:533)
==7913==    by 0x747AACD: tevent_req_poll (tevent_req.c:256)
==7913==    by 0x505315D: tevent_req_poll_ntstatus (tevent_ntstatus.c:109)
==7913==    by 0xA7201F2: cli_tree_connect (cliconnect.c:2764)
==7913==    by 0x165FF7: cm_prepare_connection (winbindd_cm.c:1276)
==7913==  Address 0x16ce24ec is 764 bytes inside a block of size 813 alloc'd
==7913==    at 0x4C29110: malloc (in
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==7913==    by 0x768A0C1: __talloc_with_prefix (talloc.c:668)
==7913==    by 0x768A27E: _talloc_pool (talloc.c:721)
==7913==    by 0x768A41E: _talloc_pooled_object (talloc.c:790)
==7913==    by 0x747A594: _tevent_req_create (tevent_req.c:66)
==7913==    by 0xCF6E2FA: read_packet_send (async_sock.c:414)
==7913==    by 0xCF6EB54: read_smb_send (read_smb.c:54)
==7913==    by 0xC4DA146: smbXcli_conn_receive_next (smbXcli_base.c:1027)
==7913==    by 0xC4DA02D: smbXcli_req_set_pending (smbXcli_base.c:978)
==7913==    by 0xC4DF776: smb2cli_req_compound_submit (smbXcli_base.c:3166)
==7913==    by 0xC4DFC1D: smb2cli_req_send (smbXcli_base.c:3268)
==7913==    by 0xC4F2210: smb2cli_ioctl_send (smb2cli_ioctl.c:149)
==7913==

BUG: https://bugzilla.samba.org/show_bug.cgi?id=11622

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
(cherry picked from commit 91e12e04fc05a0b09b70ca2986aab9b96a8a035c)
This commit is contained in:
Stefan Metzmacher 2015-11-27 17:31:04 +01:00 committed by Karolin Seeger
parent cd143a4b7f
commit af6bd5c4b9
1 changed files with 47 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

84
libcli/smb/smb2cli_ioctl.c
View File

@ -22,8 +22,6 @@
#include "lib/util/tevent_ntstatus.h"
#include "smb_common.h"
#include "smbXcli_base.h"
#include "librpc/ndr/libndr.h"
#include "librpc/gen_ndr/ioctl.h"
struct smb2cli_ioctl_state {
uint8_t fixed[0x38];
@ -31,6 +29,7 @@ struct smb2cli_ioctl_state {
uint32_t max_input_length;
uint32_t max_output_length;
struct iovec *recv_iov;
bool out_valid;
DATA_BLOB out_input_buffer;
DATA_BLOB out_output_buffer;
uint32_t ctl_code;
@ -161,32 +160,6 @@ struct tevent_req *smb2cli_ioctl_send(TALLOC_CTX *mem_ctx,
return req;
}
/*
* 3.3.4.4 Sending an Error Response
* An error code other than one of the following indicates a failure:
*/
static bool smb2cli_ioctl_is_failure(uint32_t ctl_code, NTSTATUS status,
size_t data_size)
{
if (NT_STATUS_IS_OK(status)) {
return false;
}
/*
* STATUS_INVALID_PARAMETER in a FSCTL_SRV_COPYCHUNK or
* FSCTL_SRV_COPYCHUNK_WRITE Response, when returning an
* SRV_COPYCHUNK_RESPONSE as described in section 2.2.32.1.
*/
if (NT_STATUS_EQUAL(status, NT_STATUS_INVALID_PARAMETER) &&
(ctl_code == FSCTL_SRV_COPYCHUNK ||
ctl_code == FSCTL_SRV_COPYCHUNK_WRITE) &&
data_size == sizeof(struct srv_copychunk_rsp)) {
return false;
}
return true;
}
static void smb2cli_ioctl_done(struct tevent_req *subreq)
{
struct tevent_req *req =
@ -225,6 +198,16 @@ static void smb2cli_ioctl_done(struct tevent_req *subreq)
.body_size = 0x09,
},
{
/*
* a normal error
*/
.status = NT_STATUS_INVALID_PARAMETER,
.body_size = 0x09
},
{
/*
* a special case for FSCTL_SRV_COPYCHUNK_*
*/
.status = NT_STATUS_INVALID_PARAMETER,
.body_size = 0x31
},
@ -233,10 +216,35 @@ static void smb2cli_ioctl_done(struct tevent_req *subreq)
status = smb2cli_req_recv(subreq, state, &iov,
expected, ARRAY_SIZE(expected));
TALLOC_FREE(subreq);
if (iov == NULL && tevent_req_nterror(req, status)) {
return;
if (NT_STATUS_EQUAL(status, NT_STATUS_INVALID_PARAMETER)) {
switch (state->ctl_code) {
case FSCTL_SRV_COPYCHUNK:
case FSCTL_SRV_COPYCHUNK_WRITE:
break;
default:
tevent_req_nterror(req, status);
return;
}
if (iov[1].iov_len != 0x30) {
tevent_req_nterror(req,
NT_STATUS_INVALID_NETWORK_RESPONSE);
return;
}
} else if (NT_STATUS_EQUAL(status, STATUS_BUFFER_OVERFLOW)) {
/* no error */
} else {
if (tevent_req_nterror(req, status)) {
return;
}
}
/*
* At this stage we're sure that got a body size of 0x31,
* either with NT_STATUS_OK, STATUS_BUFFER_OVERFLOW or
* NT_STATUS_INVALID_PARAMETER.
*/
state->recv_iov = iov;
fixed = (uint8_t *)iov[1].iov_base;
dyn = (uint8_t *)iov[2].iov_base;
@ -247,11 +255,6 @@ static void smb2cli_ioctl_done(struct tevent_req *subreq)
output_buffer_offset = IVAL(fixed, 0x20);
output_buffer_length = IVAL(fixed, 0x24);
if (smb2cli_ioctl_is_failure(state->ctl_code, status, output_buffer_length) &&
tevent_req_nterror(req, status)) {
return;
}
if ((input_buffer_offset > 0) && (input_buffer_length > 0)) {
uint32_t ofs;
@ -332,6 +335,8 @@ static void smb2cli_ioctl_done(struct tevent_req *subreq)
state->out_output_buffer.length = output_buffer_length;
}
state->out_valid = true;
if (tevent_req_nterror(req, status)) {
return;
}
@ -349,8 +354,13 @@ NTSTATUS smb2cli_ioctl_recv(struct tevent_req *req,
struct smb2cli_ioctl_state);
NTSTATUS status = NT_STATUS_OK;
if (tevent_req_is_nterror(req, &status) &&
smb2cli_ioctl_is_failure(state->ctl_code, status, state->out_output_buffer.length)) {
if (tevent_req_is_nterror(req, &status) && !state->out_valid) {
if (out_input_buffer) {
*out_input_buffer = data_blob_null;
}
if (out_output_buffer) {
*out_output_buffer = data_blob_null;
}
tevent_req_received(req);
return status;
}