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-05-01 23:31:51 +0000
Commit message: Don't treat the packet length as unsigned.

The scanf family of functions are as annoyingly bad at handling unsigned
numbers as strtoul() is - both of them are perfectly willing to accept a
value beginning with a negative sign as an unsigned value.  When using
strtoul(), you can compensate for this by explicitly checking for a '-'
as the first character of the string, but you can't do that with
sscanf().

So revert to having pkt_len be signed, and scanning it with %d, but
check for a negative value and fail if we see a negative value.

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

Diff: diff --git a/wiretap/toshiba.c b/wiretap/toshiba.c
index 9a066811892..091b7908840 100644
--- a/wiretap/toshiba.c
+++ b/wiretap/toshiba.c
@@ -246,12 +246,11 @@ 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;
-	guint	pkt_len;
-	int	pktnum, hr, min, sec, csec;
+	int	pkt_len, 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
@@ -299,16 +298,21 @@ 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=%9u", &pkt_len);
+	num_items_scanned = sscanf(line+64, "LEN=%9d", &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 < 0) {
+		*err = WTAP_ERR_BAD_FILE;
+		*err_info = g_strdup("toshiba: packet header has a negative packet length");
+		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.
 		 */

Vulnerable code: 	guint	pkt_len;
	int	pktnum, hr, min, sec, csec;
	num_items_scanned = sscanf(line+64, "LEN=%9u", &pkt_len);
Fixed code: 	int	pkt_len, pktnum, hr, min, sec, csec;
	num_items_scanned = sscanf(line+64, "LEN=%9d", &pkt_len);
	if (pkt_len < 0) {
		*err = WTAP_ERR_BAD_FILE;
		*err_info = g_strdup("toshiba: packet header has a negative packet length");
		return FALSE;
	}