From d70bd4489feed4035c3a35cd2e76cdf877b3f485 Mon Sep 17 00:00:00 2001 From: Gerald Combs 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 Petri-Dish: Gerald Combs Tested-by: Petri Dish Buildbot Reviewed-by: Anders Broman --- 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