Re: BCD Decoding

[Pascal Quantin <pascal.quantin () gmail com>]

2013/12/18 Pascal Quantin <pascal.quantin () gmail com>

> Le 18 déc. 2013 à 00:55, Evan Huus <eapache () gmail com> a écrit :
>
> > On Tue, Dec 17, 2013 at 6:13 PM, Pascal Quantin
> > <pascal.quantin () gmail com> 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 () gmail com>
> > > > 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 () wireshark org>
> > > > Archives:    http://www.wireshark.org/lists/wireshark-dev
> > > > Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
> > > >
> > > > mailto:wireshark-dev-request () wireshark org?subject=unsubscribe
> ___________________________________________________________________________
> > > Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
> > > Archives:    http://www.wireshark.org/lists/wireshark-dev
> > > Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
> > >             mailto:wireshark-dev-request () wireshark org
> ?subject=unsubscribe
> >
> ___________________________________________________________________________
> > Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
> > Archives:    http://www.wireshark.org/lists/wireshark-dev
> > Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
> >             mailto:wireshark-dev-request () wireshark org
> ?subject=unsubscribe
___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
Archives:    http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe