Wireshark-dev: Re: [Wireshark-dev] Keep decoding malformed packet

From: Pascal Quantin <pascal.quantin@xxxxxxxxx>
Date: Mon, 23 Mar 2015 16:53:56 +0100
2015-03-23 16:12 GMT+01:00 Victor Xiang <victorxiang28@xxxxxxxxx>:

I have a dissector written with ASN1. At some point in the packet I have a D-BL-ACK element with the following structure:

 

D-BL-ACK ::= SEQUENCE

{

      nr INTEGER(0..1),

      tl-sdu D-MLE-PDU

}

 

In a frame there can be many PDUs.

 

The problem is that the D-BL-ACK doesn’t always have  a tl-sdu. So the packets that does have a tl-sdu the dissector is decoding well and in the packets that don’t have a tl-sdu, it crashes as it is expecting that field and says Malformed Packet in the tree (The tvb of the PDU is overflowing). After crashing, it stops decoding that packet even if there are more PDUs to decode in that packet.

 

The only way to know if there is or not a tl-sdu is to see if there are any more bits in the PDU.

 

I  would like to know if there is any way I can tell it to keep on decoding the next PDU in the frame even if it crashes in the previous one.

 

The D-BL-ACK element is not modified yet in the CNF file.

 

 

The autogenerated code of the dissector is:

 

static const per_sequence_t D_BL_ACK_sequence[] = {

  { &hf_tetra_nr   , ASN1_NO_EXTENSIONS     , ASN1_NOT_OPTIONAL, dissect_tetra_INTEGER_0_1 },

  { &hf_tetra_tl_sdu_01, ASN1_NO_EXTENSIONS     , ASN1_NOT_OPTIONAL, dissect_tetra_D_MLE_PDU },

  { NULL, 0, 0, NULL }

};

 

static int

dissect_tetra_D_BL_ACK(tvbuff_t *tvb _U_, int offset _U_, asn1_ctx_t *actx _U_, proto_tree *tree _U_, int hf_index _U_) {

  offset = dissect_per_sequence(tvb, offset, actx, tree, hf_index,

                                   ett_tetra_D_BL_ACK, D_BL_ACK_sequence);

 

  return offset;

}

 

Thanks in advance

 


Hi Victor,

from your ASN.1 definition, tl-sdu should always be present (it is not marked as optional), so Wireshark is perfectly right to mark the packet as malformed. I would have expected something like:
D-BL-ACK ::= SEQUENCE
{
    nr INTEGER(0..1),
    tl-sdu D-MLE-PDU OPTIONAL
}
But this would change the bitstring in unaligned PER (the ASN.1 variant being used by the TETRA dissector): you would have a 1 bit flag before the encoding of the nr parameter to indicate whether tl-sdu parameter is present or not.

So even before modifying Wireshark, the real question is: how do you know that this field is present or not? Is there a typo error in the ASN.1 definition used to generate the dissector (missing OPTIONAL argument), or is there a trick allowing to skip this field (even if this would be a not valid ASN.1 encoding)?
If you can tell us what's the criteria to know the PDU presence, we can probably cook some code in the .cnf file to handle this.

It would be even better if you were filling a bug to https://bugs.wiresahrk.org as we are talking about a dissector that is part of the official Wireshark distribution. This way we could ensure that it is fixed in newer releases.

To tell you the truth, I have always been very surprised by this ASN.1 code. It seems to be manually written while the ETSI EN 300 392-2 specifications does not make reference to it. So using a given description language (lie, ASN.1) for a protocol not supposed to use it can lead to surprises.

Regards,
Pascal.