From ef9c79ae81b00a63aa8638076ec81dc9482972e9 Mon Sep 17 00:00:00 2001 From: John Thacker Date: Thu, 10 Aug 2023 05:29:09 -0400 Subject: [PATCH] btsdp: Keep offset advancing hf_data_element_value is a FT_NONE, so we can add the item with the expected length and get_hfi_length() will adjust the length without throwing an exception. There's no need to add it with zero length and call proto_item_set_len. Also, don't increment the offset by 0 instead of the real length when there isn't enough data in the packet, as that can lead to failing to advance the offset. When dissecting a sequence type (sequence or alternative) and recursing into the sequence member, instead of using the main packet tvb directly, create a subset using the indicated length of the sequence. That will properly throw an exception if a contained item is larger than the containing sequence, instead of dissecting the same bytes as several different items (inside the sequence recursively, as well in the outer loop.) Fix #19258 Upstream-Status: Backport [https://gitlab.com/wireshark/wireshark/-/commit/ef9c79ae81b00a63aa8638076ec81dc9482972e9] CVE: CVE-2023-4511 Signed-off-by: Vijay Anusuri --- epan/dissectors/packet-btsdp.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/epan/dissectors/packet-btsdp.c b/epan/dissectors/packet-btsdp.c index 529bb71..f18d531 100644 --- a/epan/dissectors/packet-btsdp.c +++ b/epan/dissectors/packet-btsdp.c @@ -1925,13 +1925,11 @@ dissect_data_element(proto_tree *tree, proto_tree **next_tree, offset += len - length; } - pitem = proto_tree_add_item(ptree, hf_data_element_value, tvb, offset, 0, ENC_NA); + pitem = proto_tree_add_item(ptree, hf_data_element_value, tvb, offset, length, ENC_NA); if (length > tvb_reported_length_remaining(tvb, offset)) { expert_add_info(pinfo, pitem, &ei_data_element_value_large); - length = 0; - } - proto_item_set_len(pitem, length); - if (length == 0) + proto_item_append_text(pitem, ": MISSING"); + } else if (length == 0) proto_item_append_text(pitem, ": MISSING"); if (next_tree) *next_tree = proto_item_add_subtree(pitem, ett_btsdp_data_element_value); @@ -3523,6 +3521,8 @@ dissect_sdp_type(proto_tree *tree, packet_info *pinfo, tvbuff_t *tvb, gint bytes_to_go = size; gint first = 1; wmem_strbuf_t *substr; + tvbuff_t *next_tvb = tvb_new_subset_length(tvb, offset, size); + gint next_offset = 0; ti = proto_tree_add_item(next_tree, (type == 6) ? hf_data_element_value_sequence : hf_data_element_value_alternative, tvb, offset, size, ENC_NA); @@ -3537,14 +3537,15 @@ dissect_sdp_type(proto_tree *tree, packet_info *pinfo, tvbuff_t *tvb, first = 0; } - size = dissect_sdp_type(st, pinfo, tvb, offset, attribute, service_uuid, + size = dissect_sdp_type(st, pinfo, next_tvb, next_offset, + attribute, service_uuid, service_did_vendor_id, service_did_vendor_id_source, service_hdp_data_exchange_specification, service_info, &substr); if (size < 1) { break; } wmem_strbuf_append_printf(info_buf, "%s ", wmem_strbuf_get_str(substr)); - offset += size ; + next_offset += size; bytes_to_go -= size; } -- 2.25.1