Doubled TCP SEQ field in ICMP packets

edit

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.