Hi, Wireshark team,
Recently when I supported parsing new message by wireshark, I found bit-alignment by API proto_tree_add_int_bits_format_value is always reversal. After deeper investigation to code, I think there's bug in below code:
static guint64
_tvb_get_bits64(tvbuff_t *tvb, guint bit_offset, const gint total_no_of_bits)
{
....
if(required_bits_in_first_octet > total_no_of_bits)
{
/* the required bits don't extend to the end of the first octet */
guint8 right_shift = required_bits_in_first_octet - total_no_of_bits;
value = (tvb_get_guint8(tvb, octet_offset) >> right_shift) & bit_mask8[total_no_of_bits % 8];
}
Here right_shift looks wrong to me, here it's always (7 - bit_offset % 8), but I think what's correct way is directly shift ( bit_offset % 8 ).
Another questionable place is code below:
static guint64
_tvb_get_bits64(tvbuff_t *tvb, guint bit_offset, const gint total_no_of_bits)
{
...
else
{
guint8 remaining_bit_length = total_no_of_bits;
/* get the bits up to the first octet boundary */
value = 0;
required_bits_in_first_octet %= 8;
if(required_bits_in_first_octet != 0)
{
value = tvb_get_guint8(tvb, octet_offset) & bit_mask8[required_bits_in_first_octet];
remaining_bit_length -= required_bits_in_first_octet;
octet_offset ++;
}
Here looks bit_offset is missed when getting value, I think the correct way is -
value = (tvb_get_guint8(tvb, octet_offset) >> bit_offset % 8) & bit_mask8[required_bits_in_first_octet]
Regards,
Li Du