[Pascal Quantin <pascal.quantin@xxxxxxxxx>]

2013/12/18 Pascal Quantin <pascal.quantin@xxxxxxxxx>

Le 18 déc. 2013 à 00:55, Evan Huus <eapache@xxxxxxxxx> a écrit :

> On Tue, Dec 17, 2013 at 6:13 PM, Pascal Quantin
> <pascal.quantin@xxxxxxxxx> wrote:
>> Hi Even,
>>
>> in 3GPP world BCD encoding starts with the least significant nibble. That's
>> why tvb_bcd_dig_to_wmwm_packet_str() behaves like this. Changing it to
>> decode the most significant nibble first would break all the dissectors
>> currently using this function.
>
> OK, just wondering.
>
>> The "stop condition" for the most significant nibble set to 0xf is just to
>> detect the filler digit in case you have an odd number of digits. In case of
>> even number, the length itself is sufficient and you do not need a filler,
>> so no "stop condition" is required.
>
> In that case, what should the decoder do if it encounters a 0xf nibble
> embedded in a value (ie due to a malformed packet instead of
> indicating a stop condition)? Currently our behaviour is rather
> undefined:
> - if 0xf is in the high nibble, decoding stops even though the whole
> length has not yet been decoded (ie if we pass a len of 12 but the
> very first nibble is 0xf then we don't decode anything at all)
> - if 0xf is in the low nibble, we read past the end of the digit array
> (dgt_set_t) and decode it as a garbage value
>
> Throwing an exception seems a little extreme, but I'm not sure what else to do.

What about adding a new entry to the dht_set_t array and display a '?' like what we do for 0xa-0xe values?

BTW we have exactly the same bug in my_dgt_tbcd_unpack() function located in packet-gsm_a_common.c file, as it is almost a copy/paste.

>
>> 2013/12/17 Evan Huus <eapache@xxxxxxxxx>
>>>
>>> Alexis's ASAN build recently caught an error in
>>> tvb_bcd_dig_to_wmem_packet_str in which it appears that if the least
>>> significant nibble of the decoded byte is 0xf then we read one element
>>> past the end of the 14-element digit array.
>>>
>>> If the most significant nibble is 0xf we treat that as a stop
>>> condition. Is the correct approach to treat a least significant nibble
>>> of 0xf as a stop condition also?
>>>
>>> While in the neighbourhood - shouldn't we be decoding the more
>>> significant nibble first, not second? Wiki states that most BCD
>>> implementations are big-endian...
>>>
>>> Cheers,
>>> Evan
>>>
>>> ___________________________________________________________________________
>>> Sent via:    Wireshark-dev mailing list <wireshark-dev@xxxxxxxxxxxxx>
>>> Archives:    http://www.wireshark.org/lists/wireshark-dev
>>> Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
>>>
>>> mailto:wireshark-dev-request@xxxxxxxxxxxxx?subject=unsubscribe
>>
>>
>>
>> ___________________________________________________________________________
>> Sent via:    Wireshark-dev mailing list <wireshark-dev@xxxxxxxxxxxxx>
>> Archives:    http://www.wireshark.org/lists/wireshark-dev
>> Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
>>             mailto:wireshark-dev-request@xxxxxxxxxxxxx?subject=unsubscribe
> ___________________________________________________________________________
> Sent via:    Wireshark-dev mailing list <wireshark-dev@xxxxxxxxxxxxx>
> Archives:    http://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
>             mailto:wireshark-dev-request@xxxxxxxxxxxxx?subject=unsubscribe