From ca270e59cd5eb8dd4cf6784b94b9e280f3efb1f7 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 29 Jun 2021 15:24:13 +0200 Subject: [PATCH 1/2] libcli/smb: make smb2cli_ioctl_parse_buffer() available as smb2cli_parse_dyn_buffer() It will be used in smb2cli_read.c soon... BUG: https://bugzilla.samba.org/show_bug.cgi?id=14607 Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison (cherry picked from commit 1faf15b3d0f41fa8a94b76d1616a4460ce0c6fa4) --- libcli/smb/smb2cli_ioctl.c | 123 +++++-------------------------------- libcli/smb/smbXcli_base.c | 91 +++++++++++++++++++++++++++ libcli/smb/smbXcli_base.h | 9 +++ 3 files changed, 116 insertions(+), 107 deletions(-) diff --git a/libcli/smb/smb2cli_ioctl.c b/libcli/smb/smb2cli_ioctl.c index f9abcc57bab3..d638b2816780 100644 --- a/libcli/smb/smb2cli_ioctl.c +++ b/libcli/smb/smb2cli_ioctl.c @@ -160,97 +160,6 @@ struct tevent_req *smb2cli_ioctl_send(TALLOC_CTX *mem_ctx, return req; } -static NTSTATUS smb2cli_ioctl_parse_buffer(uint32_t dyn_offset, - const DATA_BLOB dyn_buffer, - uint32_t min_offset, - uint32_t buffer_offset, - uint32_t buffer_length, - uint32_t max_length, - uint32_t *next_offset, - DATA_BLOB *buffer) -{ - uint32_t offset; - bool oob; - - *buffer = data_blob_null; - *next_offset = dyn_offset; - - if (buffer_offset == 0) { - /* - * If the offset is 0, we better ignore - * the buffer_length field. - */ - return NT_STATUS_OK; - } - - if (buffer_length == 0) { - /* - * If the length is 0, we better ignore - * the buffer_offset field. - */ - return NT_STATUS_OK; - } - - if ((buffer_offset % 8) != 0) { - /* - * The offset needs to be 8 byte aligned. - */ - return NT_STATUS_INVALID_NETWORK_RESPONSE; - } - - /* - * We used to enforce buffer_offset to be - * an exact match of the expected minimum, - * but the NetApp Ontap 7.3.7 SMB server - * gets the padding wrong and aligns the - * input_buffer_offset by a value of 8. - * - * So we just enforce that the offset is - * not lower than the expected value. - */ - SMB_ASSERT(min_offset >= dyn_offset); - if (buffer_offset < min_offset) { - return NT_STATUS_INVALID_NETWORK_RESPONSE; - } - - /* - * Make [input|output]_buffer_offset relative to "dyn_buffer" - */ - offset = buffer_offset - dyn_offset; - oob = smb_buffer_oob(dyn_buffer.length, offset, buffer_length); - if (oob) { - return NT_STATUS_INVALID_NETWORK_RESPONSE; - } - - /* - * Give the caller a hint what we consumed, - * the caller may need to add possible padding. - */ - *next_offset = buffer_offset + buffer_length; - - if (max_length == 0) { - /* - * If max_input_length is 0 we ignore the - * input_buffer_length, because Windows 2008 echos the - * DCERPC request from the requested input_buffer to - * the response input_buffer. - * - * We just use the same logic also for max_output_length... - */ - buffer_length = 0; - } - - if (buffer_length > max_length) { - return NT_STATUS_INVALID_NETWORK_RESPONSE; - } - - *buffer = (DATA_BLOB) { - .data = dyn_buffer.data + offset, - .length = buffer_length, - }; - return NT_STATUS_OK; -} - static void smb2cli_ioctl_done(struct tevent_req *subreq) { struct tevent_req *req = @@ -352,14 +261,14 @@ static void smb2cli_ioctl_done(struct tevent_req *subreq) input_min_offset = dyn_ofs; input_next_offset = dyn_ofs; - error = smb2cli_ioctl_parse_buffer(dyn_ofs, - dyn_buffer, - input_min_offset, - input_buffer_offset, - input_buffer_length, - state->max_input_length, - &input_next_offset, - &state->out_input_buffer); + error = smb2cli_parse_dyn_buffer(dyn_ofs, + dyn_buffer, + input_min_offset, + input_buffer_offset, + input_buffer_length, + state->max_input_length, + &input_next_offset, + &state->out_input_buffer); if (tevent_req_nterror(req, error)) { return; } @@ -370,14 +279,14 @@ static void smb2cli_ioctl_done(struct tevent_req *subreq) */ output_min_offset = NDR_ROUND(input_next_offset, 8); output_next_offset = 0; /* this variable is completely ignored */ - error = smb2cli_ioctl_parse_buffer(dyn_ofs, - dyn_buffer, - output_min_offset, - output_buffer_offset, - output_buffer_length, - state->max_output_length, - &output_next_offset, - &state->out_output_buffer); + error = smb2cli_parse_dyn_buffer(dyn_ofs, + dyn_buffer, + output_min_offset, + output_buffer_offset, + output_buffer_length, + state->max_output_length, + &output_next_offset, + &state->out_output_buffer); if (tevent_req_nterror(req, error)) { return; } diff --git a/libcli/smb/smbXcli_base.c b/libcli/smb/smbXcli_base.c index e71f82456b2e..2811f45ed311 100644 --- a/libcli/smb/smbXcli_base.c +++ b/libcli/smb/smbXcli_base.c @@ -6668,3 +6668,94 @@ uint64_t smb2cli_conn_get_mid(struct smbXcli_conn *conn) { return conn->smb2.mid; } + +NTSTATUS smb2cli_parse_dyn_buffer(uint32_t dyn_offset, + const DATA_BLOB dyn_buffer, + uint32_t min_offset, + uint32_t buffer_offset, + uint32_t buffer_length, + uint32_t max_length, + uint32_t *next_offset, + DATA_BLOB *buffer) +{ + uint32_t offset; + bool oob; + + *buffer = data_blob_null; + *next_offset = dyn_offset; + + if (buffer_offset == 0) { + /* + * If the offset is 0, we better ignore + * the buffer_length field. + */ + return NT_STATUS_OK; + } + + if (buffer_length == 0) { + /* + * If the length is 0, we better ignore + * the buffer_offset field. + */ + return NT_STATUS_OK; + } + + if ((buffer_offset % 8) != 0) { + /* + * The offset needs to be 8 byte aligned. + */ + return NT_STATUS_INVALID_NETWORK_RESPONSE; + } + + /* + * We used to enforce buffer_offset to be + * an exact match of the expected minimum, + * but the NetApp Ontap 7.3.7 SMB server + * gets the padding wrong and aligns the + * input_buffer_offset by a value of 8. + * + * So we just enforce that the offset is + * not lower than the expected value. + */ + SMB_ASSERT(min_offset >= dyn_offset); + if (buffer_offset < min_offset) { + return NT_STATUS_INVALID_NETWORK_RESPONSE; + } + + /* + * Make [input|output]_buffer_offset relative to "dyn_buffer" + */ + offset = buffer_offset - dyn_offset; + oob = smb_buffer_oob(dyn_buffer.length, offset, buffer_length); + if (oob) { + return NT_STATUS_INVALID_NETWORK_RESPONSE; + } + + /* + * Give the caller a hint what we consumed, + * the caller may need to add possible padding. + */ + *next_offset = buffer_offset + buffer_length; + + if (max_length == 0) { + /* + * If max_input_length is 0 we ignore the + * input_buffer_length, because Windows 2008 echos the + * DCERPC request from the requested input_buffer to + * the response input_buffer. + * + * We just use the same logic also for max_output_length... + */ + buffer_length = 0; + } + + if (buffer_length > max_length) { + return NT_STATUS_INVALID_NETWORK_RESPONSE; + } + + *buffer = (DATA_BLOB) { + .data = dyn_buffer.data + offset, + .length = buffer_length, + }; + return NT_STATUS_OK; +} diff --git a/libcli/smb/smbXcli_base.h b/libcli/smb/smbXcli_base.h index 2afc7165cd97..4452cd808ea1 100644 --- a/libcli/smb/smbXcli_base.h +++ b/libcli/smb/smbXcli_base.h @@ -390,6 +390,15 @@ void smb2cli_conn_set_cc_max_chunks(struct smbXcli_conn *conn, void smb2cli_conn_set_mid(struct smbXcli_conn *conn, uint64_t mid); uint64_t smb2cli_conn_get_mid(struct smbXcli_conn *conn); +NTSTATUS smb2cli_parse_dyn_buffer(uint32_t dyn_offset, + const DATA_BLOB dyn_buffer, + uint32_t min_offset, + uint32_t buffer_offset, + uint32_t buffer_length, + uint32_t max_length, + uint32_t *next_offset, + DATA_BLOB *buffer); + struct tevent_req *smb2cli_req_create(TALLOC_CTX *mem_ctx, struct tevent_context *ev, struct smbXcli_conn *conn, -- 2.25.1 From 2a7725514d85848ed1a0552a25cc3a40106fe1b7 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 29 Jun 2021 15:42:56 +0200 Subject: [PATCH 2/2] libcli/smb: allow unexpected padding in SMB2 READ responses Make use of smb2cli_parse_dyn_buffer() in smb2cli_read_done() as it was exactly introduced for a similar problem see: commit 4c6c71e1378401d66bf2ed230544a75f7b04376f Author: Stefan Metzmacher AuthorDate: Thu Jan 14 17:32:15 2021 +0100 Commit: Volker Lendecke CommitDate: Fri Jan 15 08:36:34 2021 +0000 libcli/smb: allow unexpected padding in SMB2 IOCTL responses A NetApp Ontap 7.3.7 SMB server add 8 padding bytes to an offset that's already 8 byte aligned. RN: Work around special SMB2 IOCTL response behavior of NetApp Ontap 7.3.7 BUG: https://bugzilla.samba.org/show_bug.cgi?id=14607 Pair-Programmed-With: Volker Lendecke Signed-off-by: Stefan Metzmacher Signed-off-by: Volker Lendecke Autobuild-User(master): Volker Lendecke Autobuild-Date(master): Fri Jan 15 08:36:34 UTC 2021 on sn-devel-184 RN: Work around special SMB2 READ response behavior of NetApp Ontap 7.3.7 BUG: https://bugzilla.samba.org/show_bug.cgi?id=14607 Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Thu Jul 15 23:53:55 UTC 2021 on sn-devel-184 (cherry picked from commit 155348cda65b441a6c4db1ed84dbf1682d02973c) --- libcli/smb/smb2cli_read.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/libcli/smb/smb2cli_read.c b/libcli/smb/smb2cli_read.c index 8110b65d4322..c7f48741b875 100644 --- a/libcli/smb/smb2cli_read.c +++ b/libcli/smb/smb2cli_read.c @@ -90,8 +90,13 @@ static void smb2cli_read_done(struct tevent_req *subreq) tevent_req_data(req, struct smb2cli_read_state); NTSTATUS status; + NTSTATUS error; struct iovec *iov; + const uint8_t dyn_ofs = SMB2_HDR_BODY + 0x10; + DATA_BLOB dyn_buffer = data_blob_null; uint8_t data_offset; + DATA_BLOB data_buffer = data_blob_null; + uint32_t next_offset = 0; /* this variable is completely ignored */ static const struct smb2cli_req_expected_response expected[] = { { .status = STATUS_BUFFER_OVERFLOW, @@ -117,14 +122,23 @@ static void smb2cli_read_done(struct tevent_req *subreq) data_offset = CVAL(iov[1].iov_base, 2); state->data_length = IVAL(iov[1].iov_base, 4); - if ((data_offset != SMB2_HDR_BODY + 16) || - (state->data_length > iov[2].iov_len)) { - tevent_req_nterror(req, NT_STATUS_INVALID_NETWORK_RESPONSE); + dyn_buffer = data_blob_const((uint8_t *)iov[2].iov_base, + iov[2].iov_len); + + error = smb2cli_parse_dyn_buffer(dyn_ofs, + dyn_buffer, + dyn_ofs, /* min_offset */ + data_offset, + state->data_length, + dyn_buffer.length, /* max_length */ + &next_offset, + &data_buffer); + if (tevent_req_nterror(req, error)) { return; } state->recv_iov = iov; - state->data = (uint8_t *)iov[2].iov_base; + state->data = data_buffer.data; state->out_valid = true; -- 2.25.1