Wireshark-dev: [Wireshark-dev] [PATCH 14/16] NFS: Fix dissecting NFS4 CREATE_SESSION cb_sec_par

From: Pali Rohár <pali@xxxxxxxxxx>
Date: Fri, 13 Sep 2024 23:08:49 +0200
Show individual flavors in array.

Dissect number of AUTH_UNIX aux gids.

Show AUTH_UNIX stamp in hex notation (like in RPC).

Throw an fatal error when cannot continue parsing (this is because
cb_sec_params does not contain length of the buffer, it has to be
calculated based on the known data).
---
 epan/dissectors/packet-nfs.c | 35 +++++++++++++++++++++++++++++++----
 1 file changed, 31 insertions(+), 4 deletions(-)

diff --git a/epan/dissectors/packet-nfs.c b/epan/dissectors/packet-nfs.c
index e34b5b3f053b..9bb59ff18d96 100644
--- a/epan/dissectors/packet-nfs.c
+++ b/epan/dissectors/packet-nfs.c
@@ -893,6 +893,9 @@ static int ett_nfs4_maxops;
 static int ett_nfs4_maxreqs;
 static int ett_nfs4_streamchanattrs;
 static int ett_nfs4_rdmachanattrs;
+static int ett_nfs4_sec_parms_array;
+static int ett_nfs4_sec_parms_item;
+static int ett_nfs4_gids;
 static int ett_nfs4_machinename;
 static int ett_nfs4_flavor;
 static int ett_nfs4_stamp;
@@ -9701,28 +9704,42 @@ dissect_rpc_nfs_impl_id4(tvbuff_t *tvb, int offset, proto_tree *tree, const char
 
 
 static int
-dissect_rpc_secparms4(tvbuff_t *tvb, int offset, proto_tree *tree)
+dissect_rpc_secparms4(tvbuff_t *tvb, int offset, proto_tree *parent_tree)
 {
 	unsigned count, i;
+	int array_offset;
+	proto_tree *array_tree;
+	proto_item *array_item;
+
+	array_offset = offset;
+	array_tree = proto_tree_add_subtree(parent_tree, tvb, offset, 0, ett_nfs4_sec_parms_array, &array_item, "cb_sec_parms");
 
 	count = tvb_get_ntohl(tvb, offset);
 	offset += 4;
 
 	for (i = 0; i < count; i++) {
+		proto_item *item;
+		int item_offset = offset;
+		proto_tree *tree = proto_tree_add_subtree(array_tree, tvb, offset, 0, ett_nfs4_sec_parms_item, &item, "entry");
 		unsigned j, flavor = tvb_get_ntohl(tvb, offset);
 		offset = dissect_rpc_uint32(tvb, tree, hf_nfs4_flavor, offset);
 
 		switch (flavor) {
+		case AUTH_NULL:
+			break;
 		case AUTH_UNIX: {
 			unsigned count2;
+			proto_tree *tree2;
 			offset = dissect_rpc_uint32(tvb, tree, hf_nfs4_stamp, offset);
 			offset = dissect_nfs_utf8string(tvb, offset, tree, hf_nfs4_machinename, NULL);
 			offset = dissect_rpc_uint32(tvb, tree, hf_nfs4_uid, offset);
 			offset = dissect_rpc_uint32(tvb, tree, hf_nfs4_gid, offset);
 			count2 = tvb_get_ntohl(tvb, offset);
+			tree2 = proto_tree_add_subtree_format(tree, tvb, offset,
+					4+count2*4, ett_nfs4_gids, NULL, "auxiliary gids (%u)", count2);
 			offset += 4;
 			for (j = 0; j < count2; j++) {
-				offset = dissect_rpc_uint32(tvb, tree, hf_nfs4_gid, offset);
+				offset = dissect_rpc_uint32(tvb, tree2, hf_nfs4_gid, offset);
 			}
 			break;
 		}
@@ -9734,9 +9751,16 @@ dissect_rpc_secparms4(tvbuff_t *tvb, int offset, proto_tree *tree)
 			offset = dissect_nfsdata(tvb, offset, tree, hf_nfs_data);
 			break;
 		default:
-			break;
+			/* Report fatal error as it is not possible to figure out item length and therefore
+			 * not possible to continue parsing next item or continue parsing next packet. */
+			THROW(ReportedBoundsError);
 		}
+
+		proto_item_set_len(item, offset - item_offset);
 	}
+
+	proto_item_set_len(array_item, offset - array_offset);
+
 	return offset;
 }
 
@@ -14086,7 +14110,7 @@ proto_register_nfs(void)
 			VALS(rpc_auth_flavor), 0, NULL, HFILL }},
 
 		{ &hf_nfs4_stamp, {
-			"stamp", "nfs.stamp4", FT_UINT32, BASE_DEC,
+			"stamp", "nfs.stamp4", FT_UINT32, BASE_HEX,
 			NULL, 0, NULL, HFILL }},
 
 		{ &hf_nfs4_uid, {
@@ -15021,6 +15045,9 @@ proto_register_nfs(void)
 		&ett_nfs4_maxreqs,
 		&ett_nfs4_streamchanattrs,
 		&ett_nfs4_rdmachanattrs,
+		&ett_nfs4_sec_parms_array,
+		&ett_nfs4_sec_parms_item,
+		&ett_nfs4_gids,
 		&ett_nfs4_machinename,
 		&ett_nfs4_flavor,
 		&ett_nfs4_stamp,
-- 
2.20.1