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

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 b9b8c763a8ed9fb3e813b490ade621566f98e7c5..9ec20bc93d04d21ac76b84f9dbbafd2dda75f276 100644 (file)
--- 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 1e2e3e8f17d603e41ff9f9b0b584e0b2d9831666..a671d8a0ebfc5f5f6930500cf99cf6d49020ef63 100644 (file)
--- 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 c38b8afc25592aa023bb1886242a354b70ba1b73..2021d6da5844139779adecae6c88de1bd37a5e4f 100644 (file)
--- 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 414965b75d18ee352f97171ce725ce763ad18ca3..121d5205de8955a6eac8bab4bfe9d6917133351e 100644 (file)
--- 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) &&