Wireshark-bugs: [Wireshark-bugs] [Bug 1124] Application level protocol PDUs not dissected proper

Date: Wed, 27 Sep 2006 04:48:08 +0000 (GMT)
http://bugs.wireshark.org/bugzilla/show_bug.cgi?id=1124


jhoger@xxxxxxxxx changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ronniesahlberg@xxxxxxxxx




------- Comment #3 from jhoger@xxxxxxxxx  2006-09-27 04:48 GMT -------
DOH! maybe BitTorrent wasn't such a good idea as an example. BitTorrent's
pdu_length measurement function is broken. It asks for minimum header length of
4 bytes but tries to check the type byte (right after the header) and defaults
to returning the length of the tvb if it ain't there. This is not correct... it
should just return the length from the header + 4 bytes for the header itself.
In the case I am testing here, the pdu length function is being called with
only 4 bytes on a tvb of length 4. There are no additional bytes available to
use in determining the length. In general, it seems that pdu length functions
should only look at as many bytes as asked for in the call to tcp_dissect_pdus.

The code which follows does it properly. Recompiling, I find Wireshark is still
unable to desegment the second PIECE packet. So this bug is still open, valid.
I don't know if the fix that follows is good for BitTorrent generally, since
the dissector made reference to something called "Continuations" that do not
follow the header format. So it is possible that for the general case,
tcp_dissect_pdus will not work unmodified for the BitTorrent protocol. From my
observations if "Continuations" are valid, there are probably no continuations
after the initial handshake. So if the dissector could be made stateful so that
it switches to tcp_dissect_pdus only after the inital handshake, that would be
a more correct fix than mine.

But this fix is completely sufficient to properly repro the bug given my
capture file that I originally attached to this bug report.


static guint get_bittorrent_pdu_length(tvbuff_t *tvb, int offset)
{
   guint8 type;
   guint32 length;

   if (tvb_get_guint8(tvb, offset) == 19 &&
      tvb_memeql(tvb, offset + 1, "BitTorrent protocol", 19) == 0) {
      /* Return the length of a Handshake message */
      return 1 + /* pstrlen */
         19 +    /* pstr */
         8 +     /* reserved */
         20 +    /* SHA1 hash of the info key */
         20;     /* peer id */
   } else {
      length = tvb_get_ntohl(tvb, offset);
      return (length + BITTORRENT_HEADER_LENGTH);
  }
}


-- 
Configure bugmail: http://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.