CVE-2016-5356: Improper Restriction of Operations within the Bounds of a Memory Buffer (CWE-119)
value: wiretap/cosine.c in the CoSine 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 performs operations on a memory buffer, but it reads from or writes to a memory location outside the buffer's intended boundary. This may result in read or write operations on unexpected memory locations that could be linked to other variables, data structures, or internal program data.
CVSS v3 base score: 5.9
Published: 2016-08-07T16:59Z
Repository: https://github.com/wireshark/wireshark
Language: C
Files changed: wiretap/cosine.c
Commit date: 2016-05-01 23:23:29 +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: 12395
Change-Id: I43b458a73b0934e9a5c2c89d34eac5a8f21a7455
Reviewed-on: https://code.wireshark.org/review/15223
Reviewed-by: Guy Harris <guy@alum.mit.edu>

Diff: diff --git a/wiretap/cosine.c b/wiretap/cosine.c
index 5d48b912e83..c7f59522942 100644
--- a/wiretap/cosine.c
+++ b/wiretap/cosine.c
@@ -328,12 +328,11 @@ static gboolean
 parse_cosine_packet(FILE_T fh, struct wtap_pkthdr *phdr, Buffer *buf,
     char *line, int *err, gchar **err_info)
 {
 	union wtap_pseudo_header *pseudo_header = &phdr->pseudo_header;
 	int	num_items_scanned;
-	int	yy, mm, dd, hr, min, sec, csec;
-	guint	pkt_len;
+	int	yy, mm, dd, hr, min, sec, csec, pkt_len;
 	int	pro, off, pri, rm, error;
 	guint	code1, code2;
 	char	if_name[COSINE_MAX_IF_NAME_LEN] = "", direction[6] = "";
 	struct	tm tm;
 	guint8 *pd;
@@ -341,11 +340,11 @@ parse_cosine_packet(FILE_T fh, struct wtap_pkthdr *phdr, Buffer *buf,
 
 	if (sscanf(line, "%4d-%2d-%2d,%2d:%2d:%2d.%9d:",
 		   &yy, &mm, &dd, &hr, &min, &sec, &csec) == 7) {
 		/* appears to be output to a control blade */
 		num_items_scanned = sscanf(line,
-		   "%4d-%2d-%2d,%2d:%2d:%2d.%9d: %5s (%127[A-Za-z0-9/:]), Length:%9u, Pro:%9d, Off:%9d, Pri:%9d, RM:%9d, Err:%9d [%8x, %8x]",
+		   "%4d-%2d-%2d,%2d:%2d:%2d.%9d: %5s (%127[A-Za-z0-9/:]), Length:%9d, Pro:%9d, Off:%9d, Pri:%9d, RM:%9d, Err:%9d [%8x, %8x]",
 			&yy, &mm, &dd, &hr, &min, &sec, &csec,
 				   direction, if_name, &pkt_len,
 				   &pro, &off, &pri, &rm, &error,
 				   &code1, &code2);
 
@@ -355,11 +354,11 @@ parse_cosine_packet(FILE_T fh, struct wtap_pkthdr *phdr, Buffer *buf,
 			return FALSE;
 		}
 	} else {
 		/* appears to be output to PE */
 		num_items_scanned = sscanf(line,
-		   "%5s (%127[A-Za-z0-9/:]), Length:%9u, Pro:%9d, Off:%9d, Pri:%9d, RM:%9d, Err:%9d [%8x, %8x]",
+		   "%5s (%127[A-Za-z0-9/:]), Length:%9d, Pro:%9d, Off:%9d, Pri:%9d, RM:%9d, Err:%9d [%8x, %8x]",
 				   direction, if_name, &pkt_len,
 				   &pro, &off, &pri, &rm, &error,
 				   &code1, &code2);
 
 		if (num_items_scanned != 10) {
@@ -367,10 +366,15 @@ parse_cosine_packet(FILE_T fh, struct wtap_pkthdr *phdr, Buffer *buf,
 			*err_info = g_strdup("cosine: header line is neither control blade nor PE output");
 			return FALSE;
 		}
 		yy = mm = dd = hr = min = sec = csec = 0;
 	}
+	if (pkt_len < 0) {
+		*err = WTAP_ERR_BAD_FILE;
+		*err_info = g_strdup("cosine: 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: 	int	yy, mm, dd, hr, min, sec, csec;
	guint	pkt_len;
		   "%4d-%2d-%2d,%2d:%2d:%2d.%9d: %5s (%127[A-Za-z0-9/:]), Length:%9u, Pro:%9d, Off:%9d, Pri:%9d, RM:%9d, Err:%9d [%8x, %8x]",
		   "%5s (%127[A-Za-z0-9/:]), Length:%9u, Pro:%9d, Off:%9d, Pri:%9d, RM:%9d, Err:%9d [%8x, %8x]",
Fixed code: 	int	yy, mm, dd, hr, min, sec, csec, pkt_len;
		   "%4d-%2d-%2d,%2d:%2d:%2d.%9d: %5s (%127[A-Za-z0-9/:]), Length:%9d, Pro:%9d, Off:%9d, Pri:%9d, RM:%9d, Err:%9d [%8x, %8x]",
		   "%5s (%127[A-Za-z0-9/:]), Length:%9d, Pro:%9d, Off:%9d, Pri:%9d, RM:%9d, Err:%9d [%8x, %8x]",
	if (pkt_len < 0) {
		*err = WTAP_ERR_BAD_FILE;
		*err_info = g_strdup("cosine: packet header has a negative packet length");
		return FALSE;
	}