Check before setting the length of a protocol item.

Don't assume that 8 + {32-bit unsigned integer} won't overflow.  Use
tvb_ensure_bytes_exist() to ensure that the data in question is present;
it also checks for overflows.

Also, set the length after we've succeeded in dissecting the item - if
we throw an exception, it's because we don't have all the data, so the
tvb_ensure_bytes_exist() would have failed, but this way we at least get
to dissect what data we *do* have.

Change-Id: If27a2e3ed7978c2051ccb2ddba0d498255d0e350
Reviewed-on: https://code.wireshark.org/review/20840
Reviewed-by: Guy Harris <guy@alum.mit.edu>
This commit is contained in:
Guy Harris 2017-04-01 12:28:05 -07:00
parent 38bc42b00b
commit de032c63e0
1 changed files with 6 additions and 1 deletions

View File

@ -880,7 +880,6 @@ AddAttribute(packet_info *pinfo, tvbuff_t *tvb, proto_tree *tree, guint offset,
tvb, offset, 4, ENC_BIG_ENDIAN);
offset +=4;
proto_item_set_len(attr_item, 8+len);
proto_item_append_text(attr_item, ": %s", val_to_str_ext_const(tag, &isns_attribute_tags_ext, "Unknown"));
/* it seems that an empty attribute is always valid, the original code had a similar statement */
@ -890,6 +889,7 @@ AddAttribute(packet_info *pinfo, tvbuff_t *tvb, proto_tree *tree, guint offset,
/* 5.6.5.1 */
proto_tree_add_uint_format_value(tree, hf_isns_portal_group_tag, tvb, offset, 8, 0, "<NULL>");
}
proto_item_set_len(attr_item, 8+len);
return offset;
}
@ -1139,6 +1139,11 @@ AddAttribute(packet_info *pinfo, tvbuff_t *tvb, proto_tree *tree, guint offset,
proto_tree_add_item(attr_tree, hf_isns_not_decoded_yet, tvb, offset, len, ENC_NA);
}
/* Make sure the data is all there - and that the length won't overflow */
tvb_ensure_bytes_exist(tvb, offset, len);
/* Set the length of the item to cover only the actual item length */
proto_item_set_len(attr_item, 8+len);
offset += len;
return offset;
}