Fix handling of guintvars.

1) A value that fits in a 32-bit unsigned integer may take more than 4
octets - the uppermost bit of the octet is a "more octets follows" flag,
so 4 octets contain only 7*4 - 28 bits of value, so a fifth octet
preceding that with the upper 3 bits zero could result in a value that
fits in 32 bits, and further octets of 0x80 just add further leading
zeroes.

We should, instead, check for *overflow*, meaning that if we add more
bits at the bottom, the result is *less* than the previous value.

2) When the result overflows, we should clamp it a UINT_MAX, rather than
setting it to zero, and should keep accumulating octets, so that we
return the correct octet count.  That prevents infinite loops where the
item's length, and the item itself, are considered zero-length.

This should fix bug 14738.

Bug: 14738
Change-Id: I1d1b60e22f169959c1573b1fcb7e010e027b5132
Reviewed-on: https://code.wireshark.org/review/27986
Reviewed-by: Guy Harris <guy@alum.mit.edu>
This commit is contained in:
Guy Harris 2018-06-03 10:13:20 -07:00
parent f5faa1e12d
commit 5c51008ef0
1 changed files with 7 additions and 7 deletions

View File

@ -31,7 +31,7 @@ guint
tvb_get_guintvar (tvbuff_t *tvb, guint offset,
guint *octetCount, packet_info *pinfo, expert_field *ei)
{
guint value = 0;
guint value = 0, previous_value;
guint octet;
guint counter = 0;
@ -44,15 +44,15 @@ tvb_get_guintvar (tvbuff_t *tvb, guint offset,
octet = tvb_get_guint8 (tvb, offset+counter);
counter++;
if (counter > sizeof(value)) {
proto_tree_add_expert(NULL, pinfo, ei, tvb, offset, counter);
value = 0;
counter = 0;
break;
}
previous_value = value;
value <<= 7; /* Value only exists in 7 of the 8 bits */
value += (octet & 0x7F);
if (value < previous_value) {
/* overflow; clamp the value at UINT_MAX */
proto_tree_add_expert(NULL, pinfo, ei, tvb, offset, counter);
value = UINT_MAX;
}
#ifdef DEBUG
fprintf(stderr,