CVE-2016-5354: NULL Pointer Dereference (CWE-476)
value: The USB subsystem in Wireshark 1.12.x before 1.12.12 and 2.x before 2.0.4 mishandles class types, which allows remote attackers to cause a denial of service (application crash) via a crafted packet.
CWE description: The product dereferences a pointer that it expects to be valid but is NULL.
CVSS v3 base score: 5.9
Published: 2016-08-07T16:59Z
Repository: https://github.com/wireshark/wireshark
Language: C
Files changed: epan/dissectors/packet-u3v.c, epan/dissectors/packet-usb-audio.c, epan/dissectors/packet-usb-masstorage.c, epan/dissectors/packet-usb-video.c, epan/dissectors/packet-usb.h
Commit date: 2016-05-01 20:16:15 +0000
Commit message: Make class "type" for USB conversations.

USB dissectors can't assume that only their class type has been passed around in the conversation.  Make explicit check that class type expected matches the dissector and stop/prevent dissection if there isn't a match.

Bug: 12356
Change-Id: Ib23973a4ebd0fbb51952ffc118daf95e3389a209
Reviewed-on: https://code.wireshark.org/review/15212
Petri-Dish: Michael Mann <mmann78@netscape.net>
Reviewed-by: Martin Kaiser <wireshark@kaiser.cx>
Petri-Dish: Martin Kaiser <wireshark@kaiser.cx>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Michael Mann <mmann78@netscape.net>

Diff: diff --git a/epan/dissectors/packet-u3v.c b/epan/dissectors/packet-u3v.c
index 3359766a74b..8e1a979eddc 100644
--- a/epan/dissectors/packet-u3v.c
+++ b/epan/dissectors/packet-u3v.c
@@ -1681,10 +1681,14 @@ dissect_u3v(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* data)
     u3v_conv_info = (u3v_conv_info_t *)usb_conv_info->class_data;
 
     if (!u3v_conv_info) {
         u3v_conv_info = wmem_new0(wmem_file_scope(), u3v_conv_info_t);
         usb_conv_info->class_data = u3v_conv_info;
+        usb_conv_info->class_data_type = USB_CONV_U3V;
+    } else if (usb_conv_info->class_data_type != USB_CONV_U3V) {
+        /* Don't dissect if another USB type is in the conversation */
+        return 0;
     }
 
     prefix = tvb_get_letohl(tvb, 0);
     if ((tvb_reported_length(tvb) >= 4) && ( ( U3V_CONTROL_PREFIX == prefix ) || ( U3V_EVENT_PREFIX == prefix ) ) ) {
         control_detected = TRUE;
diff --git a/epan/dissectors/packet-usb-audio.c b/epan/dissectors/packet-usb-audio.c
index 90c8e74aa3a..df6e9ef7dff 100644
--- a/epan/dissectors/packet-usb-audio.c
+++ b/epan/dissectors/packet-usb-audio.c
@@ -433,12 +433,16 @@ dissect_ac_if_hdr_body(tvbuff_t *tvb, gint offset, packet_info *pinfo _U_,
             tvb, offset, 2, ver, "%2.2f", ver);
     audio_conv_info = (audio_conv_info_t *)usb_conv_info->class_data;
     if(!audio_conv_info) {
         audio_conv_info = wmem_new(wmem_file_scope(), audio_conv_info_t);
         usb_conv_info->class_data = audio_conv_info;
+        usb_conv_info->class_data_type = USB_CONV_AUDIO;
         /* XXX - set reasonable default values for all components
            that are not filled in by this function */
+    } else if (usb_conv_info->class_data_type != USB_CONV_AUDIO) {
+        /* Don't dissect if another USB type is in the conversation */
+        return 0;
     }
     audio_conv_info->ver_major = ver_major;
     offset += 2;
 
     /* version 1 refers to the Basic Audio Device specification,
diff --git a/epan/dissectors/packet-usb-masstorage.c b/epan/dissectors/packet-usb-masstorage.c
index 743775be72b..919f7ef3643 100644
--- a/epan/dissectors/packet-usb-masstorage.c
+++ b/epan/dissectors/packet-usb-masstorage.c
@@ -197,13 +197,16 @@ dissect_usb_ms_bulk(tvbuff_t *tvb, packet_info *pinfo, proto_tree *parent_tree,
     if(!usb_ms_conv_info){
         usb_ms_conv_info=wmem_new(wmem_file_scope(), usb_ms_conv_info_t);
         usb_ms_conv_info->itl=wmem_tree_new(wmem_file_scope());
         usb_ms_conv_info->itlq=wmem_tree_new(wmem_file_scope());
         usb_conv_info->class_data=usb_ms_conv_info;
+        usb_conv_info->class_data_type = USB_CONV_MASS_STORAGE;
+    } else if (usb_conv_info->class_data_type != USB_CONV_MASS_STORAGE) {
+        /* Don't dissect if another USB type is in the conversation */
+        return 0;
     }
 
-
     is_request=(pinfo->srcport==NO_ENDPOINT);
 
     col_set_str(pinfo->cinfo, COL_PROTOCOL, "USBMS");
 
     col_clear(pinfo->cinfo, COL_INFO);
diff --git a/epan/dissectors/packet-usb-video.c b/epan/dissectors/packet-usb-video.c
index 0bc292000f4..3bf08f6156f 100644
--- a/epan/dissectors/packet-usb-video.c
+++ b/epan/dissectors/packet-usb-video.c
@@ -1033,10 +1033,14 @@ dissect_usb_video_control_interface_descriptor(proto_tree *parent_tree, tvbuff_t
         if (!video_conv_info)
         {
             video_conv_info = wmem_new(wmem_file_scope(), video_conv_info_t);
             video_conv_info->entities = wmem_tree_new(wmem_file_scope());
             usb_conv_info->class_data = video_conv_info;
+            usb_conv_info->class_data_type = USB_CONV_VIDEO;
+        } else if (usb_conv_info->class_data_type != USB_CONV_VIDEO) {
+            /* Stop dissection if another USB type is in the conversation */
+            return descriptor_len;
         }
 
         entity = (video_entity_t*) wmem_tree_lookup32(video_conv_info->entities, entity_id);
         if (!entity)
         {
diff --git a/epan/dissectors/packet-usb.h b/epan/dissectors/packet-usb.h
index 20ecc80b04c..261592db5d3 100644
--- a/epan/dissectors/packet-usb.h
+++ b/epan/dissectors/packet-usb.h
@@ -88,10 +88,12 @@ typedef struct _usb_trans_info_t {
     usb_conv_info_t *interface_info;
 
     guint64 usb_id;
 } usb_trans_info_t;
 
+enum usb_conv_class_data_type {USB_CONV_UNKNOWN = 0, USB_CONV_U3V, USB_CONV_AUDIO, USB_CONV_VIDEO, USB_CONV_MASS_STORAGE};
+
 /* Conversation Structure
  * there is one such structure for each device/endpoint conversation */
 struct _usb_conv_info_t {
     guint16  bus_id;
     guint16  device_address;
@@ -111,11 +113,12 @@ struct _usb_conv_info_t {
     guint16 deviceVendor;       /* Device    Descriptor - USB Vendor  ID */
     guint32 deviceProduct;      /* Device    Descriptor - USB Product ID - MSBs only for encoding unknown */
     wmem_tree_t *transactions;
     usb_trans_info_t *usb_trans_info; /* pointer to the current transaction */
 
-    void *class_data;	/* private class/id decode data */
+    void *class_data;           /* private class/id decode data */
+    enum usb_conv_class_data_type class_data_type;
 
     wmem_array_t *alt_settings;
 };
 
 /* This is what a tap will tap */

Vulnerable code: 
    void *class_data;	/* private class/id decode data */
Fixed code:         usb_conv_info->class_data_type = USB_CONV_U3V;
    } else if (usb_conv_info->class_data_type != USB_CONV_U3V) {
        /* Don't dissect if another USB type is in the conversation */
        return 0;
        usb_conv_info->class_data_type = USB_CONV_AUDIO;
    } else if (usb_conv_info->class_data_type != USB_CONV_AUDIO) {
        /* Don't dissect if another USB type is in the conversation */
        return 0;
        usb_conv_info->class_data_type = USB_CONV_MASS_STORAGE;
    } else if (usb_conv_info->class_data_type != USB_CONV_MASS_STORAGE) {
        /* Don't dissect if another USB type is in the conversation */
        return 0;
            usb_conv_info->class_data_type = USB_CONV_VIDEO;
        } else if (usb_conv_info->class_data_type != USB_CONV_VIDEO) {
            /* Stop dissection if another USB type is in the conversation */
            return descriptor_len;
enum usb_conv_class_data_type {USB_CONV_UNKNOWN = 0, USB_CONV_U3V, USB_CONV_AUDIO, USB_CONV_VIDEO, USB_CONV_MASS_STORAGE};

    void *class_data;           /* private class/id decode data */
    enum usb_conv_class_data_type class_data_type;