From 1b73d0cc22c7cd1a35cdaa702221bc8dfe7c0f31 Mon Sep 17 00:00:00 2001
From: Gordon Ross <gordon.w.ross@gmail.com>
Date: Thu, 26 May 2016 00:52:32 -0400
Subject: [PATCH] SMB TreeConnectAndX response improvement

The file system type string can be decoded in either the
three word seven word formats.  While I'm here, comment
the various formats an simplify a bit.

Bug: 12479
Change-Id: Ie5554068bef9d9c916c6c9862da00529639863b3
Reviewed-on: https://code.wireshark.org/review/15580
Reviewed-by: Anders Broman <a.broman58@gmail.com>
---
 epan/dissectors/packet-smb.c | 81 +++++++++++++++---------------------
 1 file changed, 34 insertions(+), 47 deletions(-)

diff --git a/epan/dissectors/packet-smb.c b/epan/dissectors/packet-smb.c
index a576f8ed67..e132b52e4a 100644
--- a/epan/dissectors/packet-smb.c
+++ b/epan/dissectors/packet-smb.c
@@ -7585,20 +7585,16 @@ dissect_tree_connect_andx_request(tvbuff_t *tvb, packet_info *pinfo, proto_tree
 static int
 dissect_tree_connect_andx_response(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, int offset, proto_tree *smb_tree, smb_info_t *si)
 {
-	guint8      wc, wleft, cmd = 0xff;
+	guint8      wc, cmd = 0xff;
 	guint16     andxoffset     = 0;
 	guint16     bc;
 	int         an_len;
-	int         count          = 0;
-	proto_tree *tr             = NULL;
 	const char *an;
 
 	DISSECTOR_ASSERT(si);
 
 	WORD_COUNT;
 
-	wleft = wc;	/* this is at least 1 */
-
 	/* next smb command */
 	cmd = tvb_get_guint8(tvb, offset);
 	if (cmd != 0xff) {
@@ -7612,29 +7608,26 @@ dissect_tree_connect_andx_response(tvbuff_t *tvb, packet_info *pinfo, proto_tree
 	proto_tree_add_item(tree, hf_smb_reserved, tvb, offset, 1, ENC_NA);
 	offset += 1;
 
-	wleft--;
-	if (wleft == 0)
-		goto bytecount;
-
 	/* andxoffset */
 	andxoffset = tvb_get_letohs(tvb, offset);
 	proto_tree_add_uint(tree, hf_smb_andxoffset, tvb, offset, 2, andxoffset);
 	offset += 2;
-	wleft--;
-	if (wleft == 0)
-		goto bytecount;
 
-	/* flags */
-	offset = dissect_connect_support_bits(tvb, tree, offset);
-	wleft--;
+	/* There are three valid formats of tree connect response.
+	   All have the first two words: andx_cmd, andx_off,
+	   and then have additional words as follows:
+		wc=2: (ancient LanMan -- no more words)
+		wc=3: (NT, non-ext) opt_support
+		wc=7: (NT, extended) opt_support,
+			tree_access(2w), guest_access(2w)
+	   byte_count follows those words as usual */
 
-	/* XXX - I've seen captures where this is 7, but I have no
-	   idea how to dissect it.  I'm guessing the third word
-	   contains connect support bits, which looks plausible
-	   from the values I've seen. */
+	if (wc >= 3) {
+		/* flags */
+		offset = dissect_connect_support_bits(tvb, tree, offset);
+	}
 
-	/* MaximalShareAccessRights and GuestMaximalShareAccessRights */
-	while (wleft != 0) {
+	if (wc == 7) {
 		/*
 		 * Refer to [MS-SMB] - v20100711
 		 * When a server returns extended information, the response
@@ -7642,17 +7635,14 @@ dissect_tree_connect_andx_response(tvbuff_t *tvb, packet_info *pinfo, proto_tree
 		 * MaximalShareAccessRights, and GuestMaximalShareAccessRights fields
 		 * has added.
 		 */
-		if (count == 0) {
-			tr = proto_tree_add_subtree(tree, tvb, offset, 4,
-				ett_smb_nt_access_mask, NULL, "Maximal Share Access Rights");
-		} else {
-			tr = proto_tree_add_subtree(tree, tvb, offset, 4,
-				ett_smb_nt_access_mask, NULL, "Guest Maximal Share Access Rights");
-		}
-
+		proto_tree *tr;
+		tr = proto_tree_add_subtree(tree, tvb, offset, 4,
+			ett_smb_nt_access_mask, NULL, "Maximal Share Access Rights");
+		offset = dissect_smb_access_mask(tvb, tr, offset);
+
+		tr = proto_tree_add_subtree(tree, tvb, offset, 4,
+			ett_smb_nt_access_mask, NULL, "Guest Maximal Share Access Rights");
 		offset = dissect_smb_access_mask(tvb, tr, offset);
-		wleft -= 2;
-		count++;
 	}
 
 	BYTE_COUNT;
@@ -7700,23 +7690,20 @@ dissect_tree_connect_andx_response(tvbuff_t *tvb, packet_info *pinfo, proto_tree
 		}
 	}
 
+	if (bc != 0) {
+		/*
+		 * Sometimes this isn't present.
+		 */
 
-	if (wc == 3) {
-		if (bc != 0) {
-			/*
-			 * Sometimes this isn't present.
-			 */
-
-			/* Native FS */
-			an = get_unicode_or_ascii_string(tvb, &offset,
-				si->unicode, &an_len, /*TRUE*/FALSE, FALSE,
-				&bc);
-			if (an == NULL)
-				goto endofcommand;
-			proto_tree_add_string(tree, hf_smb_fs, tvb,
-				offset, an_len, an);
-			COUNT_BYTES(an_len);
-		}
+		/* Native FS */
+		an = get_unicode_or_ascii_string(tvb, &offset,
+			si->unicode, &an_len, /*TRUE*/FALSE, FALSE,
+			&bc);
+		if (an == NULL)
+			goto endofcommand;
+		proto_tree_add_string(tree, hf_smb_fs, tvb,
+			offset, an_len, an);
+		COUNT_BYTES(an_len);
 	}
 
 	END_OF_SMB