Ask Your Question
0

Doubled TCP SEQ field in ICMP packets

asked 2019-02-25 15:06:29 +0000

updated 2019-02-25 15:08:42 +0000

Hi all,

Just spotted an interesting thing: in ICMP "Destination Unreachable" packets "original TCP Datagram" part contains two sequence fields.

image description

therefore I have 2 identical SEQ values in a column too. Is it a bug?

Wireshark vesion Version 3.1.0rc0-163-g7a482205, Windows x64.

PCAP

edit retag flag offensive close merge delete

1 Answer

Sort by » oldest newest most voted
0

answered 2019-02-25 15:57:37 +0000

cmaynard gravatar image

This definitely looks like a bug to me. For some reason, the TCP sequence number is being added to the tree if the TCP header is being dissected from within an ICMP packet, as is the case here. However, the TCP dissector then adds it again anyway, so you end up with two of them as you see here.

From packet-tcp.c:

6019         /*  If we're dissecting the headers of a TCP packet in an ICMP packet
6020          *  then go ahead and put the sequence numbers in the tree now (because
6021          *  they won't be put in later because the ICMP packet only contains up
6022          *  to the sequence number).
6023          *  We should only need to do this for IPv4 since IPv6 will hopefully
6024          *  carry enough TCP payload for this dissector to put the sequence
6025          *  numbers in via the regular code path.
6026          */
6027         {
6028             wmem_list_frame_t *frame;
6029             frame = wmem_list_frame_prev(wmem_list_tail(pinfo->layers));
6030             if (proto_ip == (gint) GPOINTER_TO_UINT(wmem_list_frame_data(frame))) {
6031                 frame = wmem_list_frame_prev(frame);
6032                 if (proto_icmp == (gint) GPOINTER_TO_UINT(wmem_list_frame_data(frame))) {
6033                     proto_tree_add_item(tcp_tree, hf_tcp_seq, tvb, offset + 4, 4, ENC_BIG_ENDIAN);
6034                 }
6035             }
6036         }
6037     }

… then later on:

6235     if(tcp_relative_seq) {
6236         proto_tree_add_uint_format_value(tcp_tree, hf_tcp_seq, tvb, offset + 4, 4, tcph->th_seq, "%u    (relative sequence number)", tcph->th_seq);
6237     } else {
6238         proto_tree_add_uint(tcp_tree, hf_tcp_seq, tvb, offset + 4, 4, tcph->th_seq);
6239     }

I would recommend filing a Wireshark bug report for this. There's only a single TCP sequence number field present so there should only be one instance of the field added to the tree, regardless of whether the TCP header is carried in an ICMP packet or not. Well, at least that's my assessment. Maybe there's a good reason for this, but I can't think of one.

edit flag offensive delete link more

Comments

The comment in the first blob of code makes a false claim - "the ICMP packet only contains up to the sequence number". The ICMP packet in question does contain data past the sequence number in the TCP header.

Yes, this is a bug. Filing a Wireshark bug report means that:

  • there's a record that it's broken (ask.wireshark.org questions don't constitute a bug database that can be queried in a simple fashion to find what's broken and needs to be fixed);
  • when we fix it, we can report it as fixed in the release notes.
Guy Harris gravatar imageGuy Harris ( 2019-02-25 20:23:49 +0000 )edit

Thanks,

Bug 15533 is reported.

Packet_vlad gravatar imagePacket_vlad ( 2019-02-26 07:27:53 +0000 )edit

Your Answer

Please start posting anonymously - your entry will be published after you log in or create a new account.

Add Answer

Question Tools

1 follower

Stats

Asked: 2019-02-25 15:06:29 +0000

Seen: 43 times

Last updated: Feb 25