ngsniffer: clean up the read process.

Have a routine to read the Sniffer record header, and call that in
ngsniffer_read() and ngsniffer_seek_read().  Only call
ngsniffer_process_record() for frame records that we understand, so that
we only allocate a block for those rather than for records we don't
understand or for EOF records, potentially leaking them.

1 files changed, 112 insertions, 72 deletions

diff --git a/wiretap/ngsniffer.c b/wiretap/ngsniffer.c
index 5340edeaa7..ef757bdb47 100644
--- a/wiretap/ngsniffer.c
+++ b/wiretap/ngsniffer.c
@@ -89,6 +89,13 @@ static const char ngsniffer_magic[] = {
 #define REC_HEADER6	16	/* More broadcast/retransmission counts? */
 #define REC_HEADER7	17	/* ? */
 
+/*
+ * Sniffer record header structure.
+ */
+struct rec_header {
+	guint16	type;		/* record type */
+	guint16	length;		/* record length */
+};
 
 /*
  * Sniffer version record format.
@@ -501,9 +508,11 @@ static gboolean ngsniffer_read(wtap *wth, wtap_rec *rec, Buffer *buf,
     int *err, gchar **err_info, gint64 *data_offset);
 static gboolean ngsniffer_seek_read(wtap *wth, gint64 seek_off,
     wtap_rec *rec, Buffer *buf, int *err, gchar **err_info);
-static int ngsniffer_process_record(wtap *wth, gboolean is_random,
-    guint *padding, wtap_rec *rec, Buffer *buf, int *err,
-    gchar **err_info);
+static gboolean read_rec_header(wtap *wth, gboolean is_random,
+    struct rec_header *hdr, int *err, gchar **err_info);
+static gboolean ngsniffer_process_record(wtap *wth, gboolean is_random,
+    guint *padding, struct rec_header *hdr, wtap_rec *rec, Buffer *buf,
+    int *err, gchar **err_info);
 static void set_metadata_frame2(wtap *wth, wtap_rec *rec,
     struct frame2_rec *frame2);
 static void set_pseudo_header_frame4(union wtap_pseudo_header *pseudo_header,
@@ -1022,7 +1031,7 @@ ngsniffer_read(wtap *wth, wtap_rec *rec, Buffer *buf, int *err,
     gchar **err_info, gint64 *data_offset)
 {
 	ngsniffer_t *ngsniffer;
-	int	ret;
+	struct rec_header hdr;
 	guint	padding;
 
 	ngsniffer = (ngsniffer_t *)wth->priv;
@@ -1034,25 +1043,31 @@ ngsniffer_read(wtap *wth, wtap_rec *rec, Buffer *buf, int *err,
 		*data_offset = ngsniffer->seq.uncomp_offset;
 
 		/*
-		 * Process the record.
+		 * Read the record header.
 		 */
-		ret = ngsniffer_process_record(wth, FALSE, &padding,
-		    rec, buf, err, err_info);
-		if (ret < 0) {
+		if (!read_rec_header(wth, FALSE, &hdr, err, err_info)) {
 			/* Read error or short read */
 			return FALSE;
 		}
 
 		/*
-		 * ret is the record type.
+		 * Process the record.
 		 */
-		switch (ret) {
+		switch (hdr.type) {
 
 		case REC_FRAME2:
 		case REC_FRAME4:
 		case REC_FRAME6:
 			/*
 			 * Packet record.
+			 */
+			if (!ngsniffer_process_record(wth, FALSE, &padding,
+			    &hdr, rec, buf, err, err_info)) {
+				/* Read error, short read, or other error */
+				return FALSE;
+			}
+
+			/*
 			 * Skip any extra data in the record.
 			 */
 			if (padding != 0) {
@@ -1064,8 +1079,15 @@ ngsniffer_read(wtap *wth, wtap_rec *rec, Buffer *buf, int *err,
 
 		case REC_EOF:
 			/*
-			 * End of file.  Return an EOF indication.
+			 * End of file.  Skip past any data (if any),
+			 * the length of which is in hdr.length, and
+			 * return an EOF indication.
 			 */
+			if (hdr.length != 0) {
+				if (!ng_skip_bytes_seq(wth, hdr.length, err,
+				    err_info))
+					return FALSE;
+			}
 			*err = 0;	/* EOF, not error */
 			return FALSE;
 
@@ -1073,11 +1095,11 @@ ngsniffer_read(wtap *wth, wtap_rec *rec, Buffer *buf, int *err,
 			/*
 			 * Well, we don't know what it is, or we know what
 			 * it is but can't handle it.  Skip past the data
-			 * portion, the length of which is in padding,
-			 * and keep looping.
+			 * portion (if any), the length of which is in
+			 * hdr.length, and keep looping.
 			 */
-			if (padding != 0) {
-				if (!ng_skip_bytes_seq(wth, padding, err,
+			if (hdr.length != 0) {
+				if (!ng_skip_bytes_seq(wth, hdr.length, err,
 				    err_info))
 					return FALSE;
 			}
@@ -1090,31 +1112,36 @@ static gboolean
 ngsniffer_seek_read(wtap *wth, gint64 seek_off,
     wtap_rec *rec, Buffer *buf, int *err, gchar **err_info)
 {
-	int	ret;
+	struct rec_header hdr;
 
 	if (!ng_file_seek_rand(wth, seek_off, err, err_info))
 		return FALSE;
 
-	ret = ngsniffer_process_record(wth, TRUE, NULL, rec, buf, err, err_info);
-	if (ret < 0) {
+	if (!read_rec_header(wth, TRUE, &hdr, err, err_info)) {
 		/* Read error or short read */
 		return FALSE;
 	}
 
 	/*
-	 * ret is the record type.
+	 * hdr.type is the record type.
 	 */
-	switch (ret) {
+	switch (hdr.type) {
 
 	case REC_FRAME2:
 	case REC_FRAME4:
 	case REC_FRAME6:
 		/* Packet record */
+		if (!ngsniffer_process_record(wth, TRUE, NULL, &hdr, rec, buf,
+		    err, err_info)) {
+			/* Read error, short read, or other error */
+			return FALSE;
+		}
 		break;
 
 	default:
 		/*
-		 * "Can't happen".
+		 * Other record type, or EOF.
+		 * This "can't happen".
 		 */
 		ws_assert_not_reached();
 		return FALSE;
@@ -1124,18 +1151,57 @@ ngsniffer_seek_read(wtap *wth, gint64 seek_off,
 }
 
 /*
- * Returns -1 on error, REC_EOF on end-of-file, record type on success.
+ * Read the record header.
+ *
+ * Returns TRUE on success, FALSE on error.
+ */
+static gboolean
+read_rec_header(wtap *wth, gboolean is_random, struct rec_header *hdr,
+    int *err, gchar **err_info)
+{
+	char	record_type[2];
+	char	record_length[4]; /* only 1st 2 bytes are length */
+
+	/*
+	 * Read the record type.
+	 */
+	if (!ng_read_bytes_or_eof(wth, record_type, 2, is_random, err, err_info)) {
+		if (*err != 0)
+			return FALSE;
+		/*
+		 * End-of-file; construct a fake EOF record.
+		 * (A file might have an EOF record at the end, or
+		 * it might just come to an end.)
+		 * (XXX - is that true of all Sniffer files?)
+		 */
+		hdr->type = REC_EOF;
+		hdr->length = 0;
+		return TRUE;
+	}
+
+	/*
+	 * Read the record length.
+	 */
+	if (!ng_read_bytes(wth, record_length, 4, is_random, err, err_info))
+		return FALSE;
+
+	hdr->type = pletoh16(record_type);
+	hdr->length = pletoh16(record_length);
+	return TRUE;
+}
+
+/*
+ * Returns TRUE on success, FALSE on error.
  * If padding is non-null, sets *padding to the amount of padding at
  * the end of the record.
  */
-static int
+static gboolean
 ngsniffer_process_record(wtap *wth, gboolean is_random, guint *padding,
-    wtap_rec *rec, Buffer *buf, int *err, gchar **err_info)
+    struct rec_header *hdr, wtap_rec *rec, Buffer *buf, int *err,
+    gchar **err_info)
 {
 	ngsniffer_t *ngsniffer;
-	char	record_type[2];
-	char	record_length[4]; /* only 1st 2 bytes are length */
-	guint	rec_type, rec_length_remaining;
+	guint	rec_length_remaining;
 	struct frame2_rec frame2;
 	struct frame4_rec frame4;
 	struct frame6_rec frame6;
@@ -1143,26 +1209,15 @@ ngsniffer_process_record(wtap *wth, gboolean is_random, guint *padding,
 	guint8	time_high, time_day;
 	guint64 t, tsecs, tpsecs;
 
-	/*
-	 * Read the record header.
-	 */
-	if (!ng_read_bytes_or_eof(wth, record_type, 2, is_random, err, err_info)) {
-		if (*err != 0)
-			return -1;
-		return REC_EOF;
-	}
-	if (!ng_read_bytes(wth, record_length, 4, is_random, err, err_info))
-		return -1;
-
-	rec_type = pletoh16(record_type);
-	rec_length_remaining = pletoh16(record_length);
+	rec_length_remaining = hdr->length;
 
 	/* Initialize - we'll be setting some presence flags below. */
-	rec->presence_flags = 0;
+	rec->rec_type = REC_TYPE_PACKET;
 	rec->block = wtap_block_create(WTAP_BLOCK_PACKET);
+	rec->presence_flags = 0;
 
 	ngsniffer = (ngsniffer_t *)wth->priv;
-	switch (rec_type) {
+	switch (hdr->type) {
 
 	case REC_FRAME2:
 		if (ngsniffer->network == NETWORK_ATM) {
@@ -1172,20 +1227,20 @@ ngsniffer_process_record(wtap *wth, gboolean is_random, guint *padding,
 			 */
 			*err = WTAP_ERR_BAD_FILE;
 			*err_info = g_strdup("ngsniffer: REC_FRAME2 record in an ATM Sniffer file");
-			return -1;
+			return FALSE;
 		}
 
 		/* Do we have an f_frame2_struct worth of data? */
 		if (rec_length_remaining < sizeof frame2) {
 			*err = WTAP_ERR_BAD_FILE;
 			*err_info = g_strdup("ngsniffer: REC_FRAME2 record length is less than record header length");
-			return -1;
+			return FALSE;
 		}
 
 		/* Read the f_frame2_struct */
 		if (!ng_read_bytes(wth, &frame2, (unsigned int)sizeof frame2,
 		   is_random, err, err_info))
-			return -1;
+			return FALSE;
 		time_low = pletoh16(&frame2.time_low);
 		time_med = pletoh16(&frame2.time_med);
 		time_high = frame2.time_high;
@@ -1206,7 +1261,7 @@ ngsniffer_process_record(wtap *wth, gboolean is_random, guint *padding,
 			 */
 			*err = WTAP_ERR_BAD_FILE;
 			*err_info = g_strdup("ngsniffer: REC_FRAME4 record in a non-ATM Sniffer file");
-			return -1;
+			return FALSE;
 		}
 
 		/*
@@ -1223,13 +1278,13 @@ ngsniffer_process_record(wtap *wth, gboolean is_random, guint *padding,
 		if (rec_length_remaining < sizeof frame4) {
 			*err = WTAP_ERR_BAD_FILE;
 			*err_info = g_strdup("ngsniffer: REC_FRAME4 record length is less than record header length");
-			return -1;
+			return FALSE;
 		}
 
 		/* Read the f_frame4_struct */
 		if (!ng_read_bytes(wth, &frame4, (unsigned int)sizeof frame4,
 		    is_random, err, err_info))
-			return -1;
+			return FALSE;
 		time_low = pletoh16(&frame4.time_low);
 		time_med = pletoh16(&frame4.time_med);
 		time_high = frame4.time_high;
@@ -1247,13 +1302,13 @@ ngsniffer_process_record(wtap *wth, gboolean is_random, guint *padding,
 		if (rec_length_remaining < sizeof frame6) {
 			*err = WTAP_ERR_BAD_FILE;
 			*err_info = g_strdup("ngsniffer: REC_FRAME6 record length is less than record header length");
-			return -1;
+			return FALSE;
 		}
 
 		/* Read the f_frame6_struct */
 		if (!ng_read_bytes(wth, &frame6, (unsigned int)sizeof frame6,
 		    is_random, err, err_info))
-			return -1;
+			return FALSE;
 		time_low = pletoh16(&frame6.time_low);
 		time_med = pletoh16(&frame6.time_med);
 		time_high = frame6.time_high;
@@ -1266,26 +1321,12 @@ ngsniffer_process_record(wtap *wth, gboolean is_random, guint *padding,
 		set_pseudo_header_frame6(wth, &rec->rec_header.packet_header.pseudo_header, &frame6);
 		break;
 
-	case REC_EOF:
-		/*
-		 * End of file.  Return an EOF indication.
-		 */
-		*err = 0;	/* EOF, not error */
-		return REC_EOF;
-
 	default:
 		/*
-		 * Unknown record type, or type that's not an EOF or
-		 * a packet record.
+		 * This should never happen.
 		 */
-		if (padding != NULL) {
-			/*
-			 * Treat the entire record as padding, so we
-			 * skip it.
-			 */
-			*padding = rec_length_remaining;
-		}
-		return rec_type;	/* unknown type */
+		ws_assert_not_reached();
+		return FALSE;
 	}
 
 	/*
@@ -1300,14 +1341,14 @@ ngsniffer_process_record(wtap *wth, gboolean is_random, guint *padding,
 		 */
 		*err = WTAP_ERR_BAD_FILE;
 		*err_info = g_strdup("ngsniffer: Record length is less than packet size");
-		return -1;
+		return FALSE;
 	}
+
 	/*
 	 * The maximum value of length is 65535, which is less than
 	 * WTAP_MAX_PACKET_SIZE_STANDARD will ever be, so we don't need to check
 	 * it.
 	 */
-
 	if (padding != NULL) {
 		/*
 		 * Padding, if the frame data size is less than what's
@@ -1316,7 +1357,6 @@ ngsniffer_process_record(wtap *wth, gboolean is_random, guint *padding,
 		*padding = rec_length_remaining - size;
 	}
 
-	rec->rec_type = REC_TYPE_PACKET;
 	rec->presence_flags |= true_size ? WTAP_HAS_TS|WTAP_HAS_CAP_LEN : WTAP_HAS_TS;
 	rec->rec_header.packet_header.len = true_size ? true_size : size;
 	rec->rec_header.packet_header.caplen = size;
@@ -1327,7 +1367,7 @@ ngsniffer_process_record(wtap *wth, gboolean is_random, guint *padding,
 	ws_buffer_assure_space(buf, size);
 	if (!ng_read_bytes(wth, ws_buffer_start_ptr(buf), size, is_random,
 	    err, err_info))
-		return -1;
+		return FALSE;
 
 	rec->rec_header.packet_header.pkt_encap = fix_pseudo_header(wth->file_encap,
 	    buf, size, &rec->rec_header.packet_header.pseudo_header);
@@ -1363,7 +1403,7 @@ ngsniffer_process_record(wtap *wth, gboolean is_random, guint *padding,
 	rec->ts.secs = (time_t)tsecs;
 	rec->ts.nsecs = (int)(tpsecs/1000);	/* psecs to nsecs */
 
-	return rec_type;	/* success */
+	return TRUE;	/* success */
 }
 
 static void