CVE-2016-5357: Improper Input Validation (CWE-20)
value: wiretap/netscreen.c in the NetScreen 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/netscreen.c
Commit date: 2016-05-01 23:14:25 +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: 12396
Change-Id: I54fe8f61f42c32b5ef33da633ece51bbcda8c95f
Reviewed-on: https://code.wireshark.org/review/15220
Reviewed-by: Guy Harris <guy@alum.mit.edu>

Diff: diff --git a/wiretap/netscreen.c b/wiretap/netscreen.c
index e10b1d9fe3a..d0ed5c732be 100644
--- a/wiretap/netscreen.c
+++ b/wiretap/netscreen.c
@@ -261,32 +261,37 @@ netscreen_seek_read(wtap *wth, gint64 seek_off,
  */
 static gboolean
 parse_netscreen_packet(FILE_T fh, struct wtap_pkthdr *phdr, Buffer* buf,
     char *line, int *err, gchar **err_info)
 {
+	int		pkt_len;
 	int		sec;
 	int		dsec;
 	char		cap_int[NETSCREEN_MAX_INT_NAME_LENGTH];
 	char		direction[2];
-	guint		pkt_len;
 	char		cap_src[13];
 	char		cap_dst[13];
 	guint8		*pd;
 	gchar		*p;
 	int		n, i = 0;
-	guint		offset = 0;
+	int		offset = 0;
 	gchar		dststr[13];
 
 	phdr->rec_type = REC_TYPE_PACKET;
 	phdr->presence_flags = WTAP_HAS_TS|WTAP_HAS_CAP_LEN;
 
-	if (sscanf(line, "%9d.%9d: %15[a-z0-9/:.-](%1[io]) len=%9u:%12s->%12s/",
+	if (sscanf(line, "%9d.%9d: %15[a-z0-9/:.-](%1[io]) len=%9d:%12s->%12s/",
 		   &sec, &dsec, cap_int, direction, &pkt_len, cap_src, cap_dst) < 5) {
 		*err = WTAP_ERR_BAD_FILE;
 		*err_info = g_strdup("netscreen: Can't parse packet-header");
 		return -1;
 	}
+	if (pkt_len < 0) {
+		*err = WTAP_ERR_BAD_FILE;
+		*err_info = g_strdup("netscreen: 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;
	guint		offset = 0;
	if (sscanf(line, "%9d.%9d: %15[a-z0-9/:.-](%1[io]) len=%9u:%12s->%12s/",
Fixed code: 	int		pkt_len;
	int		offset = 0;
	if (sscanf(line, "%9d.%9d: %15[a-z0-9/:.-](%1[io]) len=%9d:%12s->%12s/",
	if (pkt_len < 0) {
		*err = WTAP_ERR_BAD_FILE;
		*err_info = g_strdup("netscreen: packet header has a negative packet length");
		return FALSE;
	}