From d70bd4489feed4035c3a35cd2e76cdf877b3f485 Mon Sep 17 00:00:00 2001
From: Gerald Combs <gerald@wireshark.org>
Date: Mon, 29 Jun 2020 15:19:19 -0700
Subject: [PATCH] GVCP: Fix an infinite loop.

Remove an "if(tree)" test in order to ensure that our offset always
advances.

Bug: 16029
Change-Id: I5bb38f2eccfbf3c44a06682a17aafcba9d8fa0c6
Reviewed-on: https://code.wireshark.org/review/37611
Reviewed-by: Gerald Combs <gerald@wireshark.org>
Petri-Dish: Gerald Combs <gerald@wireshark.org>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
---
 epan/dissectors/packet-gvcp.c | 101 +++++++++++++++++-----------------
 1 file changed, 51 insertions(+), 50 deletions(-)

diff --git a/epan/dissectors/packet-gvcp.c b/epan/dissectors/packet-gvcp.c
index 4b5d950..3a58cfa 100644
--- a/epan/dissectors/packet-gvcp.c
+++ b/epan/dissectors/packet-gvcp.c
@@ -1803,66 +1803,67 @@ static void dissect_eventdata_cmd(proto_tree *gvcp_telegram_tree, tvbuff_t *tvb,
 	/* fill in Info column in Wireshark GUI */
 	col_append_fstr(pinfo->cinfo, COL_INFO, "[ID: 0x%04X]", eventid);
 
-	if (gvcp_telegram_tree != NULL)
+	/* If extended ID, then we have event_size here (2.1) */
+	if (extendedblockids)
 	{
-		/* If extended ID, then we have event_size here (2.1) */
-		if (extendedblockids)
-		{
-			proto_tree_add_item(gvcp_telegram_tree, hf_gvcp_eventcmd_extid_length, tvb, offset, 2, ENC_BIG_ENDIAN);
-			data_length = tvb_get_ntohs(tvb, offset); // We get the data length here 
-		}
-
-		/* skip reserved field */
-		offset += 2;
-
-		/* Use range to determine type of event */
-		if ((eventid >= 0x0000) && (eventid <= 0x8000))
-		{
-			/* Standard ID */
-			proto_tree_add_item(gvcp_telegram_tree, hf_gvcp_eventcmd_id, tvb, offset, 2, ENC_BIG_ENDIAN);
-		}
-		else if ((eventid >= 0x8001) && (eventid <= 0x8FFF))
-		{
-			/* Error */
-			proto_tree_add_item(gvcp_telegram_tree, hf_gvcp_eventcmd_error_id, tvb, offset, 2, ENC_BIG_ENDIAN);
-		}
-		else if ((eventid >= 0x9000) && (eventid <= 0xFFFF))
-		{
-			/* Device specific */
-			proto_tree_add_item(gvcp_telegram_tree, hf_gvcp_eventcmd_device_specific_id, tvb, offset, 2, ENC_BIG_ENDIAN);
-		}
+		proto_tree_add_item(gvcp_telegram_tree, hf_gvcp_eventcmd_extid_length, tvb, offset, 2, ENC_BIG_ENDIAN);
+		data_length = tvb_get_ntohs(tvb, offset); // We get the data length here
+	}
+	/* skip reserved field */
+	offset += 2;
+	 /* Use range to determine type of event */
+	if ((eventid >= 0x0000) && (eventid <= 0x8000))
+	{
+		/* Standard ID */
+		proto_tree_add_item(gvcp_telegram_tree, hf_gvcp_eventcmd_id, tvb, offset, 2, ENC_BIG_ENDIAN);
+	}
+	else if ((eventid >= 0x8001) && (eventid <= 0x8FFF))
+	{
+		/* Error */
+		proto_tree_add_item(gvcp_telegram_tree, hf_gvcp_eventcmd_error_id, tvb, offset, 2, ENC_BIG_ENDIAN);
+	}
+	else if ((eventid >= 0x9000) && (eventid <= 0xFFFF))
+	{
+		/* Device specific */
+		proto_tree_add_item(gvcp_telegram_tree, hf_gvcp_eventcmd_device_specific_id, tvb, offset, 2, ENC_BIG_ENDIAN);
+	}
+	offset += 2;
+	
+	/* Stream channel (possibly) associated with event */
+	proto_tree_add_item(gvcp_telegram_tree, hf_gvcp_eventcmd_stream_channel_index, tvb, offset, 2, ENC_BIG_ENDIAN);
+	if (extendedblockids == 0)
+	{
+		/* Block id (16 bit) associated with event */
+		proto_tree_add_item(gvcp_telegram_tree, hf_gvcp_eventcmd_block_id, tvb, offset, 2, ENC_BIG_ENDIAN);
 		offset += 2;
-
-		/* Stream channel (possibly) associated with event */
-		proto_tree_add_item(gvcp_telegram_tree, hf_gvcp_eventcmd_stream_channel_index, tvb, offset, 2, ENC_BIG_ENDIAN);
+	}
+	else
+	{
 		offset += 2;
-
-		if (extendedblockids == 0)
-		{
-			/* Block id (16 bit) associated with event */
-			proto_tree_add_item(gvcp_telegram_tree, hf_gvcp_eventcmd_block_id, tvb, offset, 2, ENC_BIG_ENDIAN);
-			offset += 2;
-		}
-		else
-		{
-			offset += 2;
-			/* Block id (64 bit) only if reported by gvcp flag */
-			proto_tree_add_item(gvcp_telegram_tree, hf_gvcp_eventcmd_block_id_64bit_v2_0, tvb, offset, 8, ENC_BIG_ENDIAN);
-			offset += 8;
-		}
-
-		/* Timestamp (64 bit) associated with event */
-		proto_tree_add_item(gvcp_telegram_tree, hf_gvcp_eventcmd_timestamp, tvb, offset, 8, ENC_BIG_ENDIAN);
+		/* Block id (64 bit) only if reportedby gvcp flag */
+		proto_tree_add_item(gvcp_telegram_tree, hf_gvcp_eventcmd_block_id_64bit_v2_0, tvb, offset, 8, ENC_BIG_ENDIAN);
 		offset += 8;
+	}
 
-		if ((data_length == 24) && (extendedblockids))
+	/* Timestamp (64 bit) associated with event */
+	proto_tree_add_item(gvcp_telegram_tree, hf_gvcp_eventcmd_timestamp, tvb, offset, 8, ENC_BIG_ENDIAN);
+	offset += 8;
+
+	if (extendedblockids)
+	{
+		if (data_length > 24)
 		{
-			/* "no data" this is an ok case for extended id, eventcmd to be deprecated */
-			return;
+			/* Data */
+			 proto_tree_add_item(gvcp_telegram_tree, hf_gvcp_eventcmd_data, tvb, offset, data_length - 24, ENC_NA);
+			 offset += data_length - 24;
 		}
 
+	}
+	else
+	{
 		/* Data */
 		proto_tree_add_item(gvcp_telegram_tree, hf_gvcp_eventcmd_data, tvb, offset, -1, ENC_NA);
+		return;
 	}
 }
 
-- 
2.23.0