Make sure no read routine process more than INT_MAX packets.

Some read routines don't read a single bufferful of packets and process
just those packets; if packets continue to be made available, they could
conceivably process an arbitrary number of packets.

That would mean that the packet count overflows; either that makes it
look like a negative number, making it look as if an error occurred, or
makes it look like a too-small positive number.

This can't be fixed by making the count 64-bit, as it ultimately gets
returned by pcap_dispatch(), which is defined to return an int.

Instead, if the maximum packet count argument to those routines is a
value that means "no maximum", we set the maximum to INT_MAX.  Those
routines are *not* defined to loop forever, so this isn't an issue.

This should fix issue #1087.

Author: guyharris

dlpisubs.c

@@ -146,7 +146,12 @@ pcap_process_pkts(pcap_t *p, pcap_handler callback, u_char *user,
 #endif
 #endif
 
-	/* Loop through packets */
+	/*
+	 * Loop through packets.
+	 *
+	 * This assumes that a single buffer of packets will have
+	 * <= INT_MAX packets, so the packet count doesn't overflow.
+	 */
 	ep = bufp + len;
 	n = 0;
 

pcap-airpcap.c

@@ -627,6 +627,9 @@ airpcap_read(pcap_t *p, int cnt, pcap_handler callback, u_char *user)
 
 	/*
 	 * Loop through each packet.
+	 *
+	 * This assumes that a single buffer of packets will have
+	 * <= INT_MAX packets, so the packet count doesn't overflow.
 	 */
 #define bhp ((AirpcapBpfHeader *)bp)
 	n = 0;

pcap-bpf.c

@@ -1177,6 +1177,9 @@ pcap_read_bpf(pcap_t *p, int cnt, pcap_handler callback, u_char *user)
 
 	/*
 	 * Loop through each packet.
+	 *
+	 * This assumes that a single buffer of packets will have
+	 * <= INT_MAX packets, so the packet count doesn't overflow.
 	 */
 #ifdef BIOCSTSTAMP
 #define bhp ((struct bpf_xhdr *)bp)

pcap-dag.c

@@ -391,7 +391,12 @@ dag_read(pcap_t *p, int cnt, pcap_handler callback, u_char *user)
 
 	}
 
-	/* Process the packets. */
+	/*
+	 * Process the packets.
+	 *
+	 * This assumes that a single buffer of packets will have
+	 * <= INT_MAX packets, so the packet count doesn't overflow.
+	 */
 	while (pd->dag_mem_top - pd->dag_mem_bottom >= dag_record_size) {
 
 		unsigned short packet_len = 0;

pcap-dos.c

@@ -12,6 +12,7 @@
 #include <signal.h>
 #include <float.h>
 #include <fcntl.h>
+#include <limits.h> /* for INT_MAX */
 #include <io.h>
 
 #if defined(USE_32BIT_DRIVERS)
@@ -355,7 +356,22 @@ pcap_read_dos (pcap_t *p, int cnt, pcap_handler callback, u_char *data)
 {
   int rc, num = 0;
 
-  while (num <= cnt || PACKET_COUNT_IS_UNLIMITED(cnt))
+  /*
+   * This can conceivably process more than INT_MAX packets,
+   * which would overflow the packet count, causing it either
+   * to look like a negative number, and thus cause us to
+   * return a value that looks like an error, or overflow
+   * back into positive territory, and thus cause us to
+   * return a too-low count.
+   *
+   * Therefore, if the packet count is unlimited, we clip
+   * it at INT_MAX; this routine is not expected to
+   * process packets indefinitely, so that's not an issue.
+   */
+  if (PACKET_COUNT_IS_UNLIMITED(cnt))
+    cnt = INT_MAX;
+
+  while (num <= cnt)
   {
     if (p->fd <= 0)
        return (-1);

pcap-dpdk.c

@@ -85,6 +85,7 @@ env DPDK_CFG="--log-level=debug -l0 -dlibrte_pmd_e1000.so -dlibrte_pmd_ixgbe.so
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
+#include <limits.h> /* for INT_MAX */
 #include <time.h>
 
 #include <sys/time.h>
@@ -331,13 +332,28 @@ static int pcap_dpdk_dispatch(pcap_t *p, int max_cnt, pcap_handler cb, u_char *c
 	u_char *large_buffer=NULL;
 	int timeout_ms = p->opt.timeout;
 
-	if ( !PACKET_COUNT_IS_UNLIMITED(max_cnt) && max_cnt < MAX_PKT_BURST){
+	/*
+	 * This can conceivably process more than INT_MAX packets,
+	 * which would overflow the packet count, causing it either
+	 * to look like a negative number, and thus cause us to
+	 * return a value that looks like an error, or overflow
+	 * back into positive territory, and thus cause us to
+	 * return a too-low count.
+	 *
+	 * Therefore, if the packet count is unlimited, we clip
+	 * it at INT_MAX; this routine is not expected to
+	 * process packets indefinitely, so that's not an issue.
+	 */
+	if (PACKET_COUNT_IS_UNLIMITED(max_cnt))
+		max_cnt = INT_MAX;
+
+	if (max_cnt < MAX_PKT_BURST){
 		burst_cnt = max_cnt;
 	}else{
 		burst_cnt = MAX_PKT_BURST;
 	}
 
-	while( PACKET_COUNT_IS_UNLIMITED(max_cnt) || pkt_cnt < max_cnt){
+	while( pkt_cnt < max_cnt){
 		if (p->break_loop){
 			p->break_loop = 0;
 			return PCAP_ERROR_BREAK;

pcap-linux.c

@@ -4064,9 +4064,22 @@ pcap_read_linux_mmap_v2(pcap_t *handle, int max_packets, pcap_handler callback,
 		}
 	}
 
-	/* non-positive values of max_packets are used to require all
-	 * packets currently available in the ring */
-	while ((pkts < max_packets) || PACKET_COUNT_IS_UNLIMITED(max_packets)) {
+	/*
+	 * This can conceivably process more than INT_MAX packets,
+	 * which would overflow the packet count, causing it either
+	 * to look like a negative number, and thus cause us to
+	 * return a value that looks like an error, or overflow
+	 * back into positive territory, and thus cause us to
+	 * return a too-low count.
+	 *
+	 * Therefore, if the packet count is unlimited, we clip
+	 * it at INT_MAX; this routine is not expected to
+	 * process packets indefinitely, so that's not an issue.
+	 */
+	if (PACKET_COUNT_IS_UNLIMITED(max_packets))
+		max_packets = INT_MAX;
+
+	while (pkts < max_packets) {
 		/*
 		 * Get the current ring buffer frame, and break if
 		 * it's still owned by the kernel.
@@ -4159,9 +4172,22 @@ pcap_read_linux_mmap_v3(pcap_t *handle, int max_packets, pcap_handler callback,
 		return pkts;
 	}
 
-	/* non-positive values of max_packets are used to require all
-	 * packets currently available in the ring */
-	while ((pkts < max_packets) || PACKET_COUNT_IS_UNLIMITED(max_packets)) {
+	/*
+	 * This can conceivably process more than INT_MAX packets,
+	 * which would overflow the packet count, causing it either
+	 * to look like a negative number, and thus cause us to
+	 * return a value that looks like an error, or overflow
+	 * back into positive territory, and thus cause us to
+	 * return a too-low count.
+	 *
+	 * Therefore, if the packet count is unlimited, we clip
+	 * it at INT_MAX; this routine is not expected to
+	 * process packets indefinitely, so that's not an issue.
+	 */
+	if (PACKET_COUNT_IS_UNLIMITED(max_packets))
+		max_packets = INT_MAX;
+
+	while (pkts < max_packets) {
 		int packets_to_read;
 
 		if (handlep->current_packet == NULL) {
@@ -4174,12 +4200,12 @@ pcap_read_linux_mmap_v3(pcap_t *handle, int max_packets, pcap_handler callback,
 		}
 		packets_to_read = handlep->packets_left;
 
-		if (!PACKET_COUNT_IS_UNLIMITED(max_packets) &&
-		    packets_to_read > (max_packets - pkts)) {
+		if (packets_to_read > (max_packets - pkts)) {
 			/*
-			 * We've been given a maximum number of packets
-			 * to process, and there are more packets in
-			 * this buffer than that.  Only process enough
+			 * There are more packets in the buffer than
+			 * the number of packets we have left to
+			 * process to get up to the maximum number
+			 * of packets to process.  Only process enough
 			 * of them to get us up to that maximum.
 			 */
 			packets_to_read = max_packets - pkts;

pcap-netfilter-linux.c

@@ -136,6 +136,13 @@ netfilter_read_linux(pcap_t *handle, int max_packets, pcap_handler callback, u_c
 		bp = (unsigned char *)handle->buffer;
 	} else
 		bp = handle->bp;
+
+	/*
+	 * Loop through each message.
+	 *
+	 * This assumes that a single buffer of message will have
+	 * <= INT_MAX packets, so the message count doesn't overflow.
+	 */
 	ep = bp + len;
 	while (bp < ep) {
 		const struct nlmsghdr *nlh = (const struct nlmsghdr *) bp;

pcap-nit.c

@@ -125,6 +125,9 @@ pcap_read_nit(pcap_t *p, int cnt, pcap_handler callback, u_char *user)
 	 * Loop through each packet.  The increment expression
 	 * rounds up to the next int boundary past the end of
 	 * the previous packet.
+	 *
+	 * This assumes that a single buffer of packets will have
+	 * <= INT_MAX packets, so the packet count doesn't overflow.
 	 */
 	n = 0;
 	ep = bp + cc;

pcap-npf.c

@@ -36,6 +36,7 @@
 #endif
 
 #include <errno.h>
+#include <limits.h> /* for INT_MAX */
 #define PCAP_DONT_INCLUDE_PCAP_BPF_H
 #include <Packet32.h>
 #include <pcap-int.h>
@@ -633,6 +634,9 @@ pcap_read_npf(pcap_t *p, int cnt, pcap_handler callback, u_char *user)
 
 	/*
 	 * Loop through each packet.
+	 *
+	 * This assumes that a single buffer of packets will have
+	 * <= INT_MAX packets, so the packet count doesn't overflow.
 	 */
 #define bhp ((struct bpf_hdr *)bp)
 	n = 0;
@@ -792,6 +796,21 @@ pcap_read_win32_dag(pcap_t *p, int cnt, pcap_handler callback, u_char *user)
 
 	endofbuf = (char*)header + cc;
 
+	/*
+	 * This can conceivably process more than INT_MAX packets,
+	 * which would overflow the packet count, causing it either
+	 * to look like a negative number, and thus cause us to
+	 * return a value that looks like an error, or overflow
+	 * back into positive territory, and thus cause us to
+	 * return a too-low count.
+	 *
+	 * Therefore, if the packet count is unlimited, we clip
+	 * it at INT_MAX; this routine is not expected to
+	 * process packets indefinitely, so that's not an issue.
+	 */
+	if (PACKET_COUNT_IS_UNLIMITED(cnt))
+		cnt = INT_MAX;
+
 	/*
 	 * Cycle through the packets
 	 */