Wireshark is a network protocol analyzer, used for viewing the contents of network traffic. To do this, it comes bundled with “dissectors”, each of which decodes a specific type of traffic. These are often written in C, which makes them vulnerable to a very wide variety of programming errors.

Even “simple” crash bugs in dissectors are still serious vulnerabilities, since the dissectors run inside the wireshark process; an attacker can make use of such bugs to prevent analysis of their network traffic.

Modern fuzzer tools are pretty good at automatically finding problems with such code, but even when provided with good templates to build upon, they can still miss some basic mistakes if they don’t manage to construct input which reaches the relevant path. I manually reviewed the source code for some of these dissectors, and came up with a few such bugs. To confirm them, I wrote custom python scripts which created a pcap (packet capture) file which could be sufficiently parsed by the dissector to trigger the bug in question, and then opened the file in wireshark.

The first bug was in a NFS (network filesystem) dissector; the NFS dissectors have an optional (disabled by default) feature to record filenames when files are opened, so that they can be displayed in the user interface for later packets (which only provide the handle). This is important when analyzing NFS traffic, since it allows you to see the paths/names of files which are being read/modified. When this optional was enabled, the mount dissector would allocate memory to store the string containing the path from a mount packet. To prevent a crash due to attempting to allocate too much memory, it would ignore the packet if presented with an excessively large length value:

int len;
len = tvb_get_ntohl(tvb, offset); // read len from packet
if (len >= ITEM_LABEL_LENGTH) // signed(!) bounds check
  THROW(ReportedBoundsError);
name = g_malloc(strlen(host)+1+len+1+200);

Unfortunately, “len” is declared as a signed integer here, so a large value in the length field of a packet is interpreted as a negative number, which bypasses the sanity check. I filed this bug along with an example pcap and a patch (which just modifies “len” to be an unsigned integer), and my patch was committed within a day. This was assigned CVE-2013-2481.

The websocket dissector also contained a similar bug, where the value passed to malloc would not be sanity-checked due to the use of signed variables. I filed a bug with a provisional patch, and it got committed with some minor changes. Unfortunately, this didn’t get backported to the 1.8 branch due to an oversight; although it was fixed a few days later after their infrastructure automatically reported the bug on the 1.8 branch, it meant that it was assigned CVE-2013-3562 without crediting me. :-)

Another bug was in the RTPS dissector, which maintains an indentation string on the stack. This is used to pad out entries into the tree, by inserting null terminators into it at an index based on the depth of the parsed tree. However, when inserting these values, it doesn’t check whether it is still in-bounds, and so an attacker can cause a null byte to be inserted out-of-bounds, and crash wireshark. I constructed a malicious RTPS packet which nested bytecodes excessively (my example contained 20 nested unions) and filed a bug; someone wrote a fix and checked it in, within a day. This was CVE-2013-2480.

Finally, I found a much simpler problem in the ACN dissector (which was, however, disabled by default); it would divide by zero (resulting in a crash) if provided with a zero field in some circumstances. I filed a bug with a patch and it was committed within a day. This was CVE-2013-2483.