Wireshark · Wireshark-commits: [Wireshark-commits] master 2abba7a: csn1: fix csnStreamDissector(): catch unknown CSN_CHOICE values

[Wireshark code review <code-review-do-not-reply@xxxxxxxxxxxxx>]

URL: https://code.wireshark.org/review/gitweb?p=wireshark.git;a=commit;h=2abba7ad629a76ea3afb9bc59a326cba690080a6
Submitter: "Anders Broman <a.broman58@xxxxxxxxx>"
Changed: branch: master
Repository: wireshark

Commits:

2abba7a by Vadim Yanitskiy (axilirator@xxxxxxxxx):

    csn1: fix csnStreamDissector(): catch unknown CSN_CHOICE values
    
    Some CSN.1 definitions may contain so-called unions that usually
    combine two or more choices. The exact element to be chosen is
    determined by the value encoded in one or more bits preceeding
    it. Here is an example of an identity union:
    
      {   0   < Global TFI : < Global TFI IE > >
        | 10  < TLLI / G-RNTI : bit (32) >
        | 110 < TQI : bit (16) > }
    
    So if a given bitstream starts with '0'B, the Global TFI IE follows.
    Otherwise either TLLI / G-RNTI or TQI is to be chosen. But what if
    neither of the choice items matches? For example, what if a given
    bitstream starts with '111'B?
    
    Most likely we should treat the bitstream as malformed, stop further
    decoding and report an error. And that's how Pycrate's [1] CSN.1
    decoder [2] behaves. Hovewer, as it turns out, Wireshark would
    simply skip the whole choice element and start decoding the next
    one from the same bit position.
    
    Here is an example of a malformed packet:
    
      GSM RLC/MAC: PACKET_POLLING_REQUEST (4) (Downlink)
        01.. .... = Payload Type (DL): RLC/MAC block contains an RLC/MAC control block
                                       that does not include the optional octets of
    				   the RLC/MAC control header (1)
        ..00 .... = RRBP: Reserved Block: (N+13) mod 2715648 (0)
        .... 1... = S/P: RRBP field is valid
        .... .001 = USF: 1
        PACKET_POLLING_REQUEST (4) (downlink)
          0001 00.. = MESSAGE_TYPE (DL): PACKET_POLLING_REQUEST (4)
          .... ..11 = PAGE_MODE: Same as before (3)
     ---! ID  <--- This is wrong! '111'B is unknown
          1... .... = CONTROL_ACK_TYPE: PACKET CONTROL ACKNOWLEDGEMENT
                                        message format shall be an RLC/MAC control block
          Padding Bits
            .110 0000  0000 1000  0101 0000  1000 1000 = Padding: 1611157640
            0100 0000  0001 0011  1010 1000  0000 0100 = Padding: 1075030020
            1000 1011  0010 1011  0010 1011  0010 1011 = Padding: 2334862123
            0010 1011  0010 1011  0010 1011  0010 1011 = Padding: 724249387
            0010 1011  0010 1011  0010 1011  0010 1011 = Padding: 724249387
            0010 1011 = Padding: 43
    
    Let's fix this, so after this patch we get:
    
      GSM RLC/MAC: PACKET_POLLING_REQUEST (4) (Downlink)
        01.. .... = Payload Type (DL): RLC/MAC block contains an RLC/MAC control block
                                       that does not include the optional octets of
                                       the RLC/MAC control header (1)
        ..00 .... = RRBP: Reserved Block: (N+13) mod 2715648 (0)
        .... 1... = S/P: RRBP field is valid
        .... .001 = USF: 1
        PACKET_POLLING_REQUEST (4) (downlink)
          0001 00.. = MESSAGE_TYPE (DL): PACKET_POLLING_REQUEST (4)
          .... ..11 = PAGE_MODE: Same as before (3)
          ID
            STREAM NOT SUPPORTED (PacketPollingID)
              [Expert Info (Warning/Protocol): STREAM NOT SUPPORTED (PacketPollingID)]
                [STREAM NOT SUPPORTED (PacketPollingID)]
                [Severity level: Warning]
                [Group: Protocol]
    
    [1] https://github.com/P1sec/pycrate
    [2] https://github.com/P1sec/pycrate/wiki/Using-the-pycrate-csn1-translator-and-runtime
    
    Change-Id: I7096c294e0d04d6afb3414874d3404cbb637fdae
    Reviewed-on: https://code.wireshark.org/review/36077
    Reviewed-by: Pau Espin Pedrol <pespin@xxxxxxxxxxx>
    Petri-Dish: Alexis La Goutte <alexis.lagoutte@xxxxxxxxx>
    Tested-by: Petri Dish Buildbot
    Reviewed-by: Anders Broman <a.broman58@xxxxxxxxx>
    

Actions performed:

    from  22e617d   mptcp: correctly parse v1 ADD_ADDR suboption
     add  2abba7a   csn1: fix csnStreamDissector(): catch unknown CSN_CHOICE values


Summary of changes:
 epan/dissectors/packet-csn1.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)