Wireshark-bugs: [Wireshark-bugs] [Bug 6045] Dissector for the Apple USB Multiplexing (USBMUX) pr

Date: Fri, 1 Jul 2011 05:44:32 -0700 (PDT)
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=6045

--- Comment #7 from Chris Maynard <christopher.maynard@xxxxxxxxx> 2011-07-01 05:44:29 PDT ---
(In reply to comment #6)
> Created an attachment (id=6557)
 --> (https://bugs.wireshark.org/bugzilla/attachment.cgi?id=6557) [details]
> This version of the dissector adds a lookup table for message types
> 
> This version of the dissector adds a message types lookup table containing the
> most commonly encountered USBMUX packet types. However, only type 6 (TCP)
> packets are supported at present.

>From usbmuxd-proto.h, the usbmuxd_header is defined as:

struct usbmuxd_header {
    uint32_t length;    // length of message, including header
    uint32_t version;   // protocol version
    uint32_t message;   // message type
    uint32_t tag;       // responses to this query will echo back this tag
} __attribute__((__packed__));

Are you attempting to look into the 3rd field of the usbmuxd_header, i.e., the
message field, to use that as your "magic" field for heuristics?  If so, you're
looking in the wrong place, and I think you're comparing the wrong data since
the multi-byte fields are little-endian and it looks like you're doing the
comparison assuming big-endian.  But in general, I think those heuristics are
going to be far too weak.  You should probably check that there is at least
enough data for a usbmuxd_header, that the length is correct (if you can), and
that you have a known/valid message type.

Once inside dissect_usbmux(), I think the overall layout could be improved as
well.  In other words, have some common code to add the header, then follow
that up with something like a switch() statement for the message type and call
a function appropriate for each type.

Your offsets look wrong in your proto_tree_add_item() functions.  It looks like
you skipped a byte.  It looks like all your lengths are wrong too.  You add
each header field to the tree with length 2, but they should be of length 4. 
And your hf_* entries should be FT_UINT32, not FT_UINT8.  You're also adding
them in the reverse order of how they appear in the header.

Where does {6, "TCP Packet"}, come from if not from usbmuxd-proto.h?

Maybe you could add a link to usbmux somewhere at the top of the dissector?
(http://marcansoft.com/blog/iphonelinux/usbmuxd/)

I'd also rather see some of the other types added to the dissector first as
well.  From the usbmuxd-proto.h, it looks like usbmuxd_result_msg (and
value_string for the result), usbmuxd_connect_request, usbmuxd_listen_request
and usbmuxd_device_record.

Lastly, I'm sensing that all these USB heuristic dissectors are going to be a
problem as more of them are added and heuristics ultimately fail/conflict. 
Unfortunately, I don't have a solution to this problem.

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