Re: Keep decoding malformed packet

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

2015-03-23 16:12 GMT+01:00 Victor Xiang <victorxiang28 () gmail com>:

> 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.

___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
Archives:    https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe