Bug #484 CVE-2018-17582 Check for corrupt PCAP files

* Check for packets that are larger than 262144 bytes * Check for capture lengths that are greater than packet length Example of a corrupt PCAP file ... sudo src/tcpreplay -i ens33 --unique-ip -t --loop 4 get_next_paket_01 safe_pcap_next ERROR: Invalid packet length in send_packets.c:get_next_packet() line 1054: 8388670 is greater than maximum 262144
appneta · Oct 18, 2018 · 68f67b1 · 68f67b1
1 parent a047d2a
commit 68f67b1
Show file tree

Hide file tree

Showing 9 changed files with 74 additions and 10 deletions.
diff --git a/configure.ac b/configure.ac
@@ -4,7 +4,7 @@ dnl $Id$
 AC_PREREQ([2.69])
 
 dnl Set version info here!
-AC_INIT([tcpreplay],[4.3.0-beta1],
+AC_INIT([tcpreplay],[4.3.0-beta2],
     [https://github.com/appneta/tcpreplay/issues],
     [tcpreplay],
     [http://tcpreplay.sourceforge.net/])

diff --git a/docs/CHANGELOG b/docs/CHANGELOG
@@ -1,3 +1,6 @@
+10/18/2018 Version 4.3.0 beta2
+    - CVE-2018-17582 heap-buffer-overflow in get_next_packet (#484)
+
 01/18/2018 Version 4.3.0 beta1
     - Travis CI build fails due to new build images (#432)
     - Unable to build with libpcap 1.8.1 (#430)

diff --git a/src/common/utils.c b/src/common/utils.c
@@ -48,7 +48,8 @@ _our_safe_malloc(size_t len, const char *funcname, const int line, const char *f
     u_char *ptr;
 
     if ((ptr = malloc(len)) == NULL) {
-        fprintf(stderr, "ERROR in %s:%s() line %d: Unable to malloc() %zu bytes", file, funcname, line, len);
+        fprintf(stderr, "ERROR in %s:%s() line %d: Unable to malloc() %zu bytes/n",
+                file, funcname, line, len);
         exit(-1);
     }
 
@@ -119,6 +120,58 @@ _our_safe_free(void *ptr, const char *funcname, const int line, const char *file
     ptr = NULL;
 }
 
+/**
+ * get next packet in pcap file
+ */
+#define MAX_PCAP_PACKET_LEN (262144) /* this matches Wireshark maximum size */
+u_char *_our_safe_pcap_next(pcap_t *pcap,  struct pcap_pkthdr *pkthdr,
+        const char *funcname, const int line, const char *file)
+{
+    u_char *pktdata = (u_char *)pcap_next(pcap, pkthdr);
+
+    if (pktdata) {
+        if (pkthdr->len > MAX_PCAP_PACKET_LEN) {
+            fprintf(stderr, "safe_pcap_next ERROR: Invalid packet length in %s:%s() line %d: %u is greater than maximum %u\n",
+                    file, funcname, line, pkthdr->len, MAX_PCAP_PACKET_LEN);
+            exit(-1);
+        }
+
+        if (pkthdr->len < pkthdr->caplen) {
+            fprintf(stderr, "safe_pcap_next ERROR: Invalid packet length in %s:%s() line %d: packet length %u is less than capture length %u\n",
+                    file, funcname, line, pkthdr->len, pkthdr->caplen);
+            exit(-1);
+        }
+    }
+
+    return pktdata;
+}
+
+/**
+ * get next packet in pcap file (extended)
+ */
+int _our_safe_pcap_next_ex(pcap_t *pcap, struct pcap_pkthdr **pkthdr,
+        const u_char **pktdata, const char *funcname,
+        const int line, const char *file)
+{
+    int res = pcap_next_ex(pcap, pkthdr, pktdata);
+
+    if (*pktdata && *pkthdr) {
+        if ((*pkthdr)->len > MAX_PCAP_PACKET_LEN) {
+            fprintf(stderr, "safe_pcap_next_ex ERROR: Invalid packet length in %s:%s() line %d: %u is greater than maximum %u\n",
+                    file, funcname, line, (*pkthdr)->len, MAX_PCAP_PACKET_LEN);
+            exit(-1);
+        }
+
+        if ((*pkthdr)->len < (*pkthdr)->caplen) {
+            fprintf(stderr, "safe_pcap_next_ex ERROR: Invalid packet length in %s:%s() line %d: packet length %u is less than capture length %u\n",
+                    file, funcname, line, (*pkthdr)->len, (*pkthdr)->caplen);
+            exit(-1);
+        }
+    }
+
+    return res;
+}
+
 /**
  * Print various packet statistics
  */

diff --git a/src/common/utils.h b/src/common/utils.h
@@ -63,6 +63,15 @@ char *_our_safe_strdup(const char *str, const char *, const int, const char *);
 #define safe_free(x) _our_safe_free(x, __FUNCTION__, __LINE__, __FILE__)
 void _our_safe_free(void *ptr, const char *, const int, const char *);
 
+#define safe_pcap_next(x, y) _our_safe_pcap_next(x, y, __FUNCTION__, __LINE__, __FILE__)
+u_char *_our_safe_pcap_next(pcap_t *pcap,  struct pcap_pkthdr *pkthdr,
+        const char *funcname, const int line, const char *file);
+
+#define safe_pcap_next_ex(x, y, z) _our_safe_pcap_next_ex(x, y, z, __FUNCTION__, __LINE__, __FILE__)
+int _our_safe_pcap_next_ex(pcap_t *pcap, struct pcap_pkthdr **pkthdr,
+        const u_char **pktdata, const char *funcname,
+        const int line, const char *file);
+
 #define MAX_ARGS 128
 
 #ifndef HAVE_INET_ATON

diff --git a/src/config.h.in b/src/config.h.in
@@ -594,8 +594,7 @@
    slash. */
 #undef LSTAT_FOLLOWS_SLASHED_SYMLINK
 
-/* Define to the sub-directory in which libtool stores uninstalled libraries.
-   */
+/* Define to the sub-directory where libtool stores uninstalled libraries. */
 #undef LT_OBJDIR
 
 /* Define to 1 if `major', `minor', and `makedev' are declared in <mkdev.h>.

diff --git a/src/send_packets.c b/src/send_packets.c
@@ -1021,7 +1021,7 @@ get_next_packet(tcpreplay_t *ctx, pcap_t *pcap, struct pcap_pkthdr *pkthdr, int
             /*
              * We should read the pcap file, and cache the results
              */
-            pktdata = (u_char *)pcap_next(pcap, pkthdr);
+            pktdata = safe_pcap_next(pcap, pkthdr);
             if (pktdata != NULL) {
                 if (*prev_packet == NULL) {
                     /*
@@ -1051,7 +1051,7 @@ get_next_packet(tcpreplay_t *ctx, pcap_t *pcap, struct pcap_pkthdr *pkthdr, int
         /*
          * Read pcap file as normal
          */
-        pktdata = (u_char *)pcap_next(pcap, pkthdr);
+        pktdata = safe_pcap_next(pcap, pkthdr);
     }
 
     /* this get's casted to a const on the way out */

diff --git a/src/tcpliveplay.c b/src/tcpliveplay.c
@@ -505,7 +505,7 @@ setup_sched(struct tcp_sched* sched){
     }
 
     /*Before sending any packet, setup the schedule with the proper parameters*/
-    while((packet = pcap_next(local_handle,&header))) {
+    while((packet = safe_pcap_next(local_handle,&header))) {
         pkt_counter++; /*increment number of packets seen*/
 
         memcpy(&sched[i].pkthdr, &header, sizeof(struct pcap_pkthdr));
@@ -985,7 +985,7 @@ rewrite(input_addr* new_remoteip, struct mac_addr* new_remotemac, input_addr* my
     }
 
     /*Modify each packet's IP & MAC based on the passed args then do a checksum of each packet*/
-    for (pkt_counter = 0; pcap_next_ex(pcap, &header, &packet) > 0; pkt_counter++){
+    for (pkt_counter = 0; safe_pcap_next_ex(pcap, &header, &packet) > 0; pkt_counter++){
 
         if (!warned && header->len > header->caplen) {
             fprintf(stderr, "warning: packet capture truncated to %d byte packets\n",

diff --git a/src/tcpprep.c b/src/tcpprep.c
@@ -331,7 +331,7 @@ process_raw_packets(pcap_t * pcap)
 
     assert(pcap);
 
-    while ((pktdata = pcap_next(pcap, &pkthdr)) != NULL) {
+    while ((pktdata = safe_pcap_next(pcap, &pkthdr)) != NULL) {
         packetnum++;
 
         dbgx(1, "Packet " COUNTER_SPEC, packetnum);

diff --git a/src/tcprewrite.c b/src/tcprewrite.c
@@ -258,7 +258,7 @@ rewrite_packets(tcpedit_t *tcpedit, pcap_t *pin, pcap_dumper_t *pout)
      * Keep sending while we have packets or until
      * we've sent enough packets
      */
-    while ((pktconst = pcap_next(pin, pkthdr_ptr)) != NULL) {
+    while ((pktconst = safe_pcap_next(pin, pkthdr_ptr)) != NULL) {
         packetnum++;
         dbgx(2, "packet " COUNTER_SPEC " caplen %d", packetnum, pkthdr.caplen);