This is a static archive of our old Q&A Site. Please post any new questions and answers at ask.wireshark.org.

Custom dissector - Handling of zero length in proto_tree_add_item

0

Hi,

I have a question regarding the usage of proto_tree_add_item() in a custom dissector. I encountered a scenario wherein the length parameter to the proto_tree_add_item() function was zero resulting in a malformed packet error (Trying to fetch an unsigned integer with length 0).

My question is, should the custom dissector always handle such scenarios (length 0 or -1) before calling the proto_tree_add_item() function? Or is there some other way to handle such cases?

Any help is greatly appreciated! Thanks!

asked 07 Nov '16, 10:18

sherlock_000's gravatar image

sherlock_000
11458
accept rate: 0%

edited 07 Nov '16, 10:47

I don't get why the size of a source field which is assumed to represent an uint should ever be 0. Either the field in question is optional for the protocol, so it should not be added to the tree at all if it is not present in the tvb, or it is mandatory and thus expected at that point of the tvb, and in such case, the packet is really malformed if the tvb ends right before the expected occurrence of the field. Another possibility is that the field is optional and has no value, so its type is not uint.

Can you elaborate?

(07 Nov '16, 11:01) sindy

Or the field is variable-length and doesn't use the binary encoding that proto_tree_add_item() expects all FT_UINTn values to have, where there are, for octet-aligned values, somewhere between 1 and 8 octets of value. What is the encoding of the field in question?

(07 Nov '16, 13:01) Guy Harris ♦♦

Thanks Sindy and Guy Harris for your comments. As you mentioned, it might be better if I explain my situation with an example.

I have a UINT16 field (Little Endian encoding) which might be part of the protocol I am dissecting if the previous byte was set. The following is the logic I am trying to achieve

1) Check whether the previous byte was set

2) If so, check the number of remaining bytes in the tvb buffer

3) If the number of remaining bytes is less than 2 (as the field is UINT16), I would like to add the field to the tree with the remaining bytes and also add an Expert info to this field indicating that there is an issue with this.

The problem that I mentioned in my initial post occurs during step 3 when I try to add the field using proto_tree_add_item() with remaining bytes(in this case 0, because the tvb buffer did not have any bytes remaining). I was expecting the field to be created the same way as it works for a FT_BYTES field with length 0, but unfortunately malformed packet error occurs for the FT_UINT16 field.

So to go back to my initial question, should the dissector take care that at least 1 byte is available before calling proto_tree_add_item for a FT_UINTn field?

(08 Nov '16, 00:44) sherlock_000

What are the protocols over which this protocol runs?

In particular, does it run over a byte-stream protocol such as TCP or TLS-over-TCP, or does it run over a protocol that exposes packet boundaries to protocols running atop it?

(08 Nov '16, 00:50) Guy Harris ♦♦

It runs over either TCP or UDP

(08 Nov '16, 00:50) sherlock_000

If it runs over UDP, does a single packet for the protocol correspond to a single UDP packet, so that you don't have multiple packets for the protocol in a single UDP packet and you don't have multiple UDP packets being reassembled into a single packet for the protocol?

If it runs over TCP, there has to be some mechanism to indicate where the packet boundaries are in the byte stream; does each packet begin with a length field that indicates how long the packet is, or is some other mechanism used?

(08 Nov '16, 00:55) Guy Harris ♦♦

A single TCP/UDP payload might contain more than one packet of the custom protocol. As of now, there is no reassembly of multiple TCP/UDP packets necessary, but it might be helpful for me to know how to handle such cases too.

To your question regarding the length of the protocol packet, yes, there is a header within each protocol packet that has the length information of that particular packet. There could be cases(rare) wherein the length field might be wrong. Will that cause a problem?

(08 Nov '16, 01:06) sherlock_000
showing 5 of 7 show 2 more comments

One Answer:

0

OK, so if the flag byte has a non-zero value, but there are fewer than 2 bytes left in the packet after it, that probably means that the protocol's packet is not entirely contained within the TCP segment (or UDP packet?), so that you will be able to correctly dissect the 2-byte integer field only if you do reassembly.

I.e., yes, there IS reassembly of multiple TCP packets necessary to solve your problem, and, if packets for the custom protocol can be split between UDP packets, there is also reassembly of multiple UDP packets necessary to solve your problem.

For TCP, use tcp_dissect_pdus(); see section 2.7.1 of the README.dissector file.

For UDP, if there are never packets for the custom protocol that are split between UDP packets - i.e., a single UDP packet can contain more than one packet for the custom protocol, but no packet for the custom protocol is ever split between UDP packets - you can just have a loop that processes the UDP payload a packet at a time. If there are packets for the custom protocol that are split between UDP packets, then you'll have to write your own reassembly code.

answered 08 Nov '16, 01:27

Guy%20Harris's gravatar image

Guy Harris ♦♦
17.4k335196
accept rate: 19%

Thanks for the answer. I am currently having a loop similar to what you mentioned to handle multiple packets within a single TCP/UDP payload.

The reason I didn't use tcp_dissect_pdus() is because there might be packets where the message length might be incorrect. What will happen if the packet length extracted from the header is invalid(for e.g. higher than the actual reported length, or smaller than the length expected for the current protocol packet)? How will tcp_dissect_pdus() react in such a case?

You had also mentioned that if there are fewer than 2 bytes left in the overall UDP packet, there could be a possibility of reassembly. But I am also interested in a situation when the protocol was not implemented properly in a device and hence that optional field of 2 bytes was not sent. I was thinking I could add the field using proto_tree_add_item and then include the expert info to that field. Is it never possible for a dissector to add a FT_UINTn field with 0 bytes?

(08 Nov '16, 01:48) sherlock_000
1

What will happen if the packet length extracted from the header is invalid(for e.g. higher than the actual reported length, or smaller than the length expected for the current protocol packet)?

Then your protocol won't actually work, and if Wireshark mis-dissects it, that's a feature not a bug! It will show what the receiving machine will think the packet will look like.

You had also mentioned that if there are fewer than 2 bytes left in the overall UDP packet, there could be a possibility of reassembly.

No, I said that if the flag byte means "there is a 2-byte integer following the flag byte", but there aren't 2 bytes remaining after that in the TCP segment or UDP datagram, then the packet for your protocol is malformed.

But I am also interested in a situation when the protocol was not implemented properly in a device and hence that optional field of 2 bytes was not sent.

Then, for TCP, Wireshark will try to grab the next 2 bytes from the data stream and add it to the packet, and dissect that. If that doesn't look right, that's an excellent indication that the sending machine is buggy and needs to be fixed, because the receiving machine will also try to grab the next to bytes from the data stream and treat it as part of the packet.

Is it never possible for a dissector to add a FT_UINTn field with 0 bytes?

No, because that would not make any sense at all. An FT_UINT16 field with 1 byte is rarely used - if the field is always supposed to be 1 byte long, that'd be an FT_UINT8 field, and if it's 1 byte in some packets and 2 bytes in other packets, it's convenient to use FT_UINT16 (although the main difference between FT_UINT8 and FT_UINT16 is whether, when displayed in hex or octal, it shows 8 or 16 bits worth of hex or octal digits - for decimal, we don't provide leading zeroes, so that doesn't make a difference).

But at least there is a value if there's only 1 byte. With 0 bytes, there are no bytes to provide a value, so there is no possible way in which an FT_UINTn field with 0 bytes could be sanely implemented.

And, if you really don't want to do reassembly, because the senders of these packets are developed by not-very-competent programmers and they screw up the packet length that much, then, if there aren't 2 bytes present, you should just put up an expert info saying "either this packet crosses TCP segment boundaries or the person who developed the code that sent it needs adult supervision".

(08 Nov '16, 02:15) Guy Harris ♦♦

Thanks a lot for your detailed explanation.

When I asked the question, I was under the impression that a field with FT_BYTES or FT_UINTn should behave in a same way when the length parameter is 0. Because I remember seeing a FT_BYTES field displaying <MISSING> when the length parameter for proto_tree_add_item() was 0. So I was expecting the same behavior for a FT_UINTn field too.

(08 Nov '16, 03:05) sherlock_000