CVE-2016-5355: Improper Input Validation (CWE-20)
value: wiretap/toshiba.c in the Toshiba file parser in Wireshark 1.12.x before 1.12.12 and 2.x before 2.0.4 mishandles sscanf unsigned-integer processing, which allows remote attackers to cause a denial of service (application crash) via a crafted file.
CWE description: The product receives input or data, but it does
        not validate or incorrectly validates that the input has the
        properties that are required to process the data safely and
        correctly.
CVSS v3 base score: 5.9
Published: 2016-08-07T16:59Z
Repository: https://github.com/wireshark/wireshark
Language: C
Files changed: wiretap/toshiba.c
Commit date: 2016-04-29 22:20:26 +0000
Commit message: Fix packet length handling.

Treat the packet length as unsigned - it shouldn't be negative in the
file.  If it is, that'll probably cause the sscanf to fail, so we'll
report the file as bad.

Check it against WTAP_MAX_PACKET_SIZE to make sure we don't try to
allocate a huge amount of memory, just as we do in other file readers.

Use the now-validated packet size as the length in
ws_buffer_assure_space(), so we are certain to have enough space, and
don't allocate too much space.

Bug: 12394
Change-Id: Ifa023ce70f7a2697bf151009b035a6e6cf8d5d90
Reviewed-on: https://code.wireshark.org/review/15169
Reviewed-by: Guy Harris <guy@alum.mit.edu>

Diff: diff --git a/wiretap/toshiba.c b/wiretap/toshiba.c
index 091eb1e4c13..9a066811892 100644
--- a/wiretap/toshiba.c
+++ b/wiretap/toshiba.c
@@ -96,15 +96,10 @@ static const char toshiba_hdr_magic[]  =
 
 /* Magic text for start of packet */
 static const char toshiba_rec_magic[]  = { '[', 'N', 'o', '.' };
 #define TOSHIBA_REC_MAGIC_SIZE  (sizeof toshiba_rec_magic  / sizeof toshiba_rec_magic[0])
 
-/*
- * XXX - is this the biggest packet we can get?
- */
-#define TOSHIBA_MAX_PACKET_LEN	16384
-
 static gboolean toshiba_read(wtap *wth, int *err, gchar **err_info,
 	gint64 *data_offset);
 static gboolean toshiba_seek_read(wtap *wth, gint64 seek_off,
 	struct wtap_pkthdr *phdr, Buffer *buf, int *err, gchar **err_info);
 static gboolean parse_single_hex_dump_line(char* rec, guint8 *buf,
@@ -251,11 +246,12 @@ parse_toshiba_packet(FILE_T fh, struct wtap_pkthdr *phdr, Buffer *buf,
     int *err, gchar **err_info)
 {
 	union wtap_pseudo_header *pseudo_header = &phdr->pseudo_header;
 	char	line[TOSHIBA_LINE_LENGTH];
 	int	num_items_scanned;
-	int	pkt_len, pktnum, hr, min, sec, csec;
+	guint	pkt_len;
+	int	pktnum, hr, min, sec, csec;
 	char	channel[10], direction[10];
 	int	i, hex_lines;
 	guint8	*pd;
 
 	/* Our file pointer should be on the line containing the
@@ -303,16 +299,26 @@ parse_toshiba_packet(FILE_T fh, struct wtap_pkthdr *phdr, Buffer *buf,
 		/* Check for "OFFSET 0001-0203" at beginning of line */
 		line[16] = '\0';
 
 	} while (strcmp(line, "OFFSET 0001-0203") != 0);
 
-	num_items_scanned = sscanf(line+64, "LEN=%9d", &pkt_len);
+	num_items_scanned = sscanf(line+64, "LEN=%9u", &pkt_len);
 	if (num_items_scanned != 1) {
 		*err = WTAP_ERR_BAD_FILE;
 		*err_info = g_strdup("toshiba: OFFSET line doesn't have valid LEN item");
 		return FALSE;
 	}
+	if (pkt_len > WTAP_MAX_PACKET_SIZE) {
+		/*
+		 * Probably a corrupt capture file; don't blow up trying
+		 * to allocate space for an immensely-large packet.
+		 */
+		*err = WTAP_ERR_BAD_FILE;
+		*err_info = g_strdup_printf("toshiba: File has %u-byte packet, bigger than maximum of %u",
+		    pkt_len, WTAP_MAX_PACKET_SIZE);
+		return FALSE;
+	}
 
 	phdr->rec_type = REC_TYPE_PACKET;
 	phdr->presence_flags = WTAP_HAS_TS|WTAP_HAS_CAP_LEN;
 	phdr->ts.secs = hr * 3600 + min * 60 + sec;
 	phdr->ts.nsecs = csec * 10000000;
@@ -339,11 +345,11 @@ parse_toshiba_packet(FILE_T fh, struct wtap_pkthdr *phdr, Buffer *buf,
 			pseudo_header->eth.fcs_len = -1;
 			break;
 	}
 
 	/* Make sure we have enough room for the packet */
-	ws_buffer_assure_space(buf, TOSHIBA_MAX_PACKET_LEN);
+	ws_buffer_assure_space(buf, pkt_len);
 	pd = ws_buffer_start_ptr(buf);
 
 	/* Calculate the number of hex dump lines, each
 	 * containing 16 bytes of data */
 	hex_lines = pkt_len / 16 + ((pkt_len % 16) ? 1 : 0);

Vulnerable code: /*
 * XXX - is this the biggest packet we can get?
 */
#define TOSHIBA_MAX_PACKET_LEN	16384

	int	pkt_len, pktnum, hr, min, sec, csec;
	num_items_scanned = sscanf(line+64, "LEN=%9d", &pkt_len);
	ws_buffer_assure_space(buf, TOSHIBA_MAX_PACKET_LEN);
Fixed code: 	guint	pkt_len;
	int	pktnum, hr, min, sec, csec;
	num_items_scanned = sscanf(line+64, "LEN=%9u", &pkt_len);
	if (pkt_len > WTAP_MAX_PACKET_SIZE) {
		/*
		 * Probably a corrupt capture file; don't blow up trying
		 * to allocate space for an immensely-large packet.
		 */
		*err = WTAP_ERR_BAD_FILE;
		*err_info = g_strdup_printf("toshiba: File has %u-byte packet, bigger than maximum of %u",
		    pkt_len, WTAP_MAX_PACKET_SIZE);
		return FALSE;
	}
	ws_buffer_assure_space(buf, pkt_len);