From f5b33585f0d4ef8a8ecdce0a85ce5d5367d8f99e Mon Sep 17 00:00:00 2001 From: guy Date: Fri, 25 Mar 2005 19:52:51 +0000 Subject: [PATCH] In the cases fixed by the two previous fixes, check to make sure the items don't run past the length left in the option, and, if they do, put an indication into the protocol tree that they did. The length returned by "tvb_strsize()" includes the terminating null character. git-svn-id: http://anonsvn.wireshark.org/wireshark/trunk@13900 f5534014-38df-0310-8fa8-9805f1628bb7 --- epan/dissectors/packet-ppp.c | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/epan/dissectors/packet-ppp.c b/epan/dissectors/packet-ppp.c index 62d45f8e62..333e2949a7 100644 --- a/epan/dissectors/packet-ppp.c +++ b/epan/dissectors/packet-ppp.c @@ -2384,8 +2384,7 @@ dissect_cbcp_callback_opt(const ip_tcp_opt *optp, tvbuff_t *tvb, offset += 3; length -= 3; - /* XXX - Should we check for a maximum length instead of using a cast? */ - while ((gint) length > 0) { + while (length > 0) { ta = proto_tree_add_text(field_tree, tvb, offset, length, "Callback Address"); addr_type = tvb_get_guint8(tvb, offset); @@ -2396,11 +2395,16 @@ dissect_cbcp_callback_opt(const ip_tcp_opt *optp, tvbuff_t *tvb, offset++; length--; addr_len = tvb_strsize(tvb, offset); + if (addr_len > length) { + proto_tree_add_text(addr_tree, tvb, offset, length, + "Address: (runs past end of option)"); + break; + } proto_tree_add_text(addr_tree, tvb, offset, addr_len, "Address: %s", tvb_format_text(tvb, offset, addr_len - 1)); - offset += (addr_len + 1); - length -= (addr_len + 1); + offset += addr_len; + length -= addr_len; } } @@ -2457,25 +2461,30 @@ dissect_bap_phone_delta_opt(const ip_tcp_opt *optp, tvbuff_t *tvb, offset += 2; length -= 2; - /* XXX - Should we check for a maximum length instead of using a cast? */ - while (((gint) length) > 0) { + while (length > 0) { subopt_type = tvb_get_guint8(tvb, offset); subopt_len = tvb_get_guint8(tvb, offset + 1); ti = proto_tree_add_text(field_tree, tvb, offset, subopt_len, - "Sub-Option (%d byte%s)", + "Sub-Option (%u byte%s)", subopt_len, plurality(subopt_len, "", "s")); suboption_tree = proto_item_add_subtree(ti, ett_bap_phone_delta_subopt); - if (subopt_len < 1) { - proto_tree_add_text(suboption_tree, tvb, offset + 1, 1, - "Invalid suboption length: %u", subopt_len); - return; - } proto_tree_add_text(suboption_tree, tvb, offset, 1, "Sub-Option Type: %s (%u)", val_to_str(subopt_type, bap_phone_delta_subopt_vals, "Unknown"), subopt_type); + if (subopt_len < 1) { + proto_tree_add_text(suboption_tree, tvb, offset + 1, 1, + "Sub-Option Length: %u (invalid, must be >= 1)", subopt_len); + return; + } + if (subopt_len > length) { + proto_tree_add_text(suboption_tree, tvb, offset + 1, 1, + "Sub-Option Length: %u (invalid, must be <= length remaining in option %u)", subopt_len, length); + return; + } + proto_tree_add_text(suboption_tree, tvb, offset + 1, 1, "Sub-Option Length: %u", subopt_len); -- 2.34.1