diff options
author | guy <guy@f5534014-38df-0310-8fa8-9805f1628bb7> | 2005-03-25 19:52:51 +0000 |
---|---|---|
committer | guy <guy@f5534014-38df-0310-8fa8-9805f1628bb7> | 2005-03-25 19:52:51 +0000 |
commit | f5b33585f0d4ef8a8ecdce0a85ce5d5367d8f99e (patch) | |
tree | a9959aa979184b06c9564a25b3eebdacb4e3e336 /epan/dissectors/packet-ppp.c | |
parent | d3a7adedc9c2df4d0940945f76ba4316af629113 (diff) |
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
Diffstat (limited to 'epan/dissectors/packet-ppp.c')
-rw-r--r-- | epan/dissectors/packet-ppp.c | 33 |
1 files 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); |