From: Stefan Metzmacher <metze@samba.org>
Date: Sun, 9 May 2021 19:16:00 +0000 (+0200)
Subject: libcli/smb: let smb2_negotiate_context_parse() only parse the expected number of... 
X-Git-Tag: talloc-2.3.3~83
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=147dd9d58a429695a3b6c6e45c8b0eaafc67908a;p=thirdparty%2Fsamba.git

libcli/smb: let smb2_negotiate_context_parse() only parse the expected number of contexts

Any garbage at the end needs to be ignored.

This fixes the Negotiate_SMB311_ContextID_NetName test from:
https://github.com/microsoft/WindowsProtocolTestSuites/blob/main/TestSuites/FileServer/src/SMB2/TestSuite/Negotiate/Negotiation.cs#L730

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

Autobuild-User(master): Jeremy Allison <jra@samba.org>
Autobuild-Date(master): Mon Jul 12 21:25:21 UTC 2021 on sn-devel-184
---

diff --git a/libcli/smb/smb2_negotiate_context.c b/libcli/smb/smb2_negotiate_context.c
index b9b8c763a8e..9ec20bc93d0 100644
--- a/libcli/smb/smb2_negotiate_context.c
+++ b/libcli/smb/smb2_negotiate_context.c
@@ -31,12 +31,14 @@ static size_t smb2_negotiate_context_padding(uint32_t offset, size_t n)
   parse a set of SMB2 create contexts
 */
 NTSTATUS smb2_negotiate_context_parse(TALLOC_CTX *mem_ctx, const DATA_BLOB buffer,
+				      uint16_t expected_count,
 				      struct smb2_negotiate_contexts *contexts)
 {
 	const uint8_t *data = buffer.data;
 	uint32_t remaining = buffer.length;
+	uint16_t idx;
 
-	while (true) {
+	for (idx = 0; idx < expected_count; idx++) {
 		uint16_t data_length;
 		uint16_t type;
 		NTSTATUS status;
@@ -63,6 +65,10 @@ NTSTATUS smb2_negotiate_context_parse(TALLOC_CTX *mem_ctx, const DATA_BLOB buffe
 			return status;
 		}
 
+		if (contexts->num_contexts == expected_count) {
+			break;
+		}
+
 		remaining -= next_offset;
 		data += next_offset;
 
@@ -78,6 +84,10 @@ NTSTATUS smb2_negotiate_context_parse(TALLOC_CTX *mem_ctx, const DATA_BLOB buffe
 		data += pad;
 	}
 
+	if (contexts->num_contexts != expected_count) {
+		return NT_STATUS_INVALID_PARAMETER;
+	}
+
 	return NT_STATUS_OK;
 }
 
diff --git a/libcli/smb/smb2_negotiate_context.h b/libcli/smb/smb2_negotiate_context.h
index 1e2e3e8f17d..a671d8a0ebf 100644
--- a/libcli/smb/smb2_negotiate_context.h
+++ b/libcli/smb/smb2_negotiate_context.h
@@ -26,7 +26,7 @@ struct smb2_negotiate_context {
 };
 
 struct smb2_negotiate_contexts {
-	uint32_t num_contexts;
+	uint16_t num_contexts;
 	struct smb2_negotiate_context *contexts;
 };
 
@@ -34,7 +34,8 @@ struct smb2_negotiate_contexts {
   parse a set of SMB2 negotiate contexts
 */
 NTSTATUS smb2_negotiate_context_parse(TALLOC_CTX *mem_ctx, const DATA_BLOB buffer,
-				struct smb2_negotiate_contexts *contexts);
+				      uint16_t expected_count,
+				      struct smb2_negotiate_contexts *contexts);
 
 /*
   negotiate a buffer of a set of negotiate contexts
diff --git a/libcli/smb/smbXcli_base.c b/libcli/smb/smbXcli_base.c
index c38b8afc255..2021d6da584 100644
--- a/libcli/smb/smbXcli_base.c
+++ b/libcli/smb/smbXcli_base.c
@@ -5059,13 +5059,14 @@ static void smbXcli_negprot_smb2_done(struct tevent_req *subreq)
 	negotiate_context_blob.data += ctx_ofs;
 	negotiate_context_blob.length -= ctx_ofs;
 
-	status = smb2_negotiate_context_parse(state, negotiate_context_blob, &c);
-	if (tevent_req_nterror(req, status)) {
-		return;
+	status = smb2_negotiate_context_parse(state,
+					      negotiate_context_blob,
+					      negotiate_context_count,
+					      &c);
+	if (NT_STATUS_EQUAL(status, NT_STATUS_INVALID_PARAMETER)) {
+		status = NT_STATUS_INVALID_NETWORK_RESPONSE;
 	}
-
-	if (negotiate_context_count != c.num_contexts) {
-		tevent_req_nterror(req, NT_STATUS_INVALID_NETWORK_RESPONSE);
+	if (tevent_req_nterror(req, status)) {
 		return;
 	}
 
diff --git a/source3/smbd/smb2_negprot.c b/source3/smbd/smb2_negprot.c
index 414965b75d1..121d5205de8 100644
--- a/source3/smbd/smb2_negprot.c
+++ b/source3/smbd/smb2_negprot.c
@@ -262,15 +262,12 @@ NTSTATUS smbd_smb2_request_process_negprot(struct smbd_smb2_request *req)
 		in_negotiate_context_blob.length -= ofs;
 
 		status = smb2_negotiate_context_parse(req,
-					in_negotiate_context_blob, &in_c);
+						      in_negotiate_context_blob,
+						      in_negotiate_context_count,
+						      &in_c);
 		if (!NT_STATUS_IS_OK(status)) {
 			return smbd_smb2_request_error(req, status);
 		}
-
-		if (in_negotiate_context_count != in_c.num_contexts) {
-			return smbd_smb2_request_error(req,
-					NT_STATUS_INVALID_PARAMETER);
-		}
 	}
 
 	if ((dialect != SMB2_DIALECT_REVISION_2FF) &&