Strengthen the I4B heuristics.

Check some more field values, and fix some tests to check against the
maximum possible value given in the i4b_trace.h file rather than against
that value + 1.  (> max, or >= max+1, are both reasonable, but > max+1
isn't.)

Check the first few packets, not just the first packet.

Make some header fields unsigned, as that's how we treat them in most
cases; that way we treat them that way by default.

Change-Id: I8c2d28af048c676a3dbae367bbb49c886e0dc566
Ping-Bug: 16031
Reviewed-on: https://code.wireshark.org/review/34432
Petri-Dish: Guy Harris <guy@alum.mit.edu>
Tested-by: Petri Dish Buildbot
Reviewed-by: Guy Harris <guy@alum.mit.edu>

2 files changed, 89 insertions, 19 deletions

diff --git a/wiretap/i4b_trace.h b/wiretap/i4b_trace.h
index 174278f3a1..ffbb25c2ab 100644
--- a/wiretap/i4b_trace.h
+++ b/wiretap/i4b_trace.h
@@ -15,8 +15,8 @@
  *---------------------------------------------------------------------------*/
 typedef struct {
 	guint32 length;		/* length of the following mbuf		*/
-	gint32 unit;		/* controller unit number		*/
-	gint32 type;		/* type of channel			*/
+	guint32 unit;		/* controller unit number		*/
+	guint32 type;		/* type of channel			*/
 #define TRC_CH_I	0		/* Layer 1 INFO's		*/
 #define TRC_CH_D	1		/* D channel			*/
 #define TRC_CH_B1	2		/* B1 channel			*/
@@ -24,7 +24,7 @@ typedef struct {
 	gint32 dir;		/* direction				*/
 #define FROM_TE	0			/* user -> network		*/
 #define FROM_NT 1			/* network -> user		*/
-	gint32 trunc;		/* # of truncated bytes (frame > MCLBYTES) */
+	guint32 trunc;		/* # of truncated bytes (frame > MCLBYTES) */
 	guint32 count;		/* frame count for this unit/type	*/
 	guint32 ts_sec;		/* timestamp seconds */
 	guint32 ts_usec;	/* timestamp microseconds */
diff --git a/wiretap/i4btrace.c b/wiretap/i4btrace.c
index 35cb3f30cb..69de8859bd 100644
--- a/wiretap/i4btrace.c
+++ b/wiretap/i4btrace.c
@@ -28,12 +28,36 @@ static int i4b_read_rec(wtap *wth, FILE_T fh, wtap_rec *rec,
     Buffer *buf, int *err, gchar **err_info);
 
 /*
+ * Byte-swap the header.
+ */
+#define I4B_BYTESWAP_HEADER(hdr) \
+	{ \
+		hdr.length = GUINT32_SWAP_LE_BE(hdr.length); \
+		hdr.unit = GUINT32_SWAP_LE_BE(hdr.unit); \
+		hdr.type = GUINT32_SWAP_LE_BE(hdr.type); \
+		hdr.dir = GUINT32_SWAP_LE_BE(hdr.dir); \
+		hdr.trunc = GUINT32_SWAP_LE_BE(hdr.trunc); \
+		hdr.count = GUINT32_SWAP_LE_BE(hdr.count); \
+		hdr.ts_sec = GUINT32_SWAP_LE_BE(hdr.ts_sec); \
+		hdr.ts_usec = GUINT32_SWAP_LE_BE(hdr.ts_usec); \
+	}
+
+/*
  * Test some fields in the header to see if they make sense.
  */
 #define	I4B_HDR_IS_OK(hdr) \
-	(!((unsigned int)hdr.length < 3 || (unsigned int)hdr.length > 16384 || \
-	    (unsigned int)hdr.unit > 4 || (unsigned int)hdr.type > 4 || \
-	    (unsigned int)hdr.dir > 2 || (unsigned int)hdr.trunc > 2048))
+	(!(hdr.length < sizeof(hdr) || \
+	    hdr.length > 16384 || \
+	    hdr.unit > 4 || \
+	    hdr.type > TRC_CH_B2 || \
+	    hdr.dir > FROM_NT || \
+	    hdr.trunc > 2048 || \
+	    hdr.ts_usec >= 1000000))
+
+/*
+ * Number of packets to try reading.
+ */
+#define PACKETS_TO_CHECK	5
 
 wtap_open_return_val i4btrace_open(wtap *wth, int *err, gchar **err_info)
 {
@@ -53,11 +77,7 @@ wtap_open_return_val i4btrace_open(wtap *wth, int *err, gchar **err_info)
 		/*
 		 * OK, try byte-swapping the header fields.
 		 */
-		hdr.length = GUINT32_SWAP_LE_BE(hdr.length);
-		hdr.unit = GUINT32_SWAP_LE_BE(hdr.unit);
-		hdr.type = GUINT32_SWAP_LE_BE(hdr.type);
-		hdr.dir = GUINT32_SWAP_LE_BE(hdr.dir);
-		hdr.trunc = GUINT32_SWAP_LE_BE(hdr.trunc);
+		I4B_BYTESWAP_HEADER(hdr);
 		if (!I4B_HDR_IS_OK(hdr)) {
 			/*
 			 * It doesn't look valid in either byte order.
@@ -72,6 +92,63 @@ wtap_open_return_val i4btrace_open(wtap *wth, int *err, gchar **err_info)
 		byte_swapped = TRUE;
 	}
 
+	/*
+	 * Now try to read past the packet bytes; if that fails with
+	 * a short read, we don't fail, so that we can report
+	 * the file as a truncated I4B file.
+	 */
+	if (!wtap_read_bytes(wth->fh, NULL, hdr.length - (guint32)sizeof(hdr),
+	    err, err_info)) {
+		if (*err != WTAP_ERR_SHORT_READ)
+			return WTAP_OPEN_ERROR;
+	}
+
+	/*
+	 * Now try reading a few more packets.
+	 */
+	for (int i = 1; i < PACKETS_TO_CHECK; i++) {
+		/*
+		 * Read and check the file header; we've already
+		 * decided whether this would be a byte-swapped file
+		 * or not, so we swap iff we decided it was.
+		 */
+		if (!wtap_read_bytes_or_eof(wth->fh, &hdr, sizeof(hdr), err,
+		    err_info)) {
+			if (*err == 0) {
+				/* EOF; no more packets to try. */
+				break;
+			}
+			if (*err != WTAP_ERR_SHORT_READ)
+				return WTAP_OPEN_ERROR;
+			return WTAP_OPEN_NOT_MINE;
+		}
+
+		if (byte_swapped)
+			I4B_BYTESWAP_HEADER(hdr);
+		if (!I4B_HDR_IS_OK(hdr)) {
+			/*
+			 * It doesn't look valid in either byte order.
+			 */
+			return WTAP_OPEN_NOT_MINE;
+		}
+
+		/*
+		 * Now try to read past the packet bytes; if that fails with
+		 * a short read, we don't fail, so that we can report
+		 * the file as a truncated I4B file.
+		 */
+		if (!wtap_read_bytes(wth->fh, NULL, hdr.length - (guint32)sizeof(hdr),
+		    err, err_info)) {
+			if (*err != WTAP_ERR_SHORT_READ)
+				return WTAP_OPEN_ERROR;
+
+			/*
+			 * Probably a truncated file, so just quit.
+			 */
+			break;
+		}
+	}
+
 	if (file_seek(wth->fh, 0, SEEK_SET, err) == -1)
 		return WTAP_OPEN_ERROR;
 
@@ -134,14 +211,7 @@ i4b_read_rec(wtap *wth, FILE_T fh, wtap_rec *rec, Buffer *buf,
 		/*
 		 * Byte-swap the header.
 		 */
-		hdr.length = GUINT32_SWAP_LE_BE(hdr.length);
-		hdr.unit = GUINT32_SWAP_LE_BE(hdr.unit);
-		hdr.type = GUINT32_SWAP_LE_BE(hdr.type);
-		hdr.dir = GUINT32_SWAP_LE_BE(hdr.dir);
-		hdr.trunc = GUINT32_SWAP_LE_BE(hdr.trunc);
-		hdr.count = GUINT32_SWAP_LE_BE(hdr.count);
-		hdr.ts_sec = GUINT32_SWAP_LE_BE(hdr.ts_sec);
-		hdr.ts_usec = GUINT32_SWAP_LE_BE(hdr.ts_usec);
+		I4B_BYTESWAP_HEADER(hdr);
 	}
 
 	if (hdr.length < sizeof(hdr)) {