Make sure the *adjusted* snapshot length is the same for all IDBs.

If the snapshot length is 0, or doesn't fit in an int, we adjust it.  If
we see an IDB after the first IDB, perform the same adjustment on *its*
snapshot length before comparing it with the adjusted snapshot length
for the first IDB.

(Yes, this means that if the IDBs have different snapshot lengths, but
they both adjust to the same value, they will be treated as equal.
That's not a bug, that's a feature - the only reason we insist that they
be the same is so that the value that pcap_snapshot() returns, which,
for pcapng files, is the adjusted snapshot length for the first IDB,
applies to all IDBs, so the IDBs all just have to have the same adjusted
snapshot length.)

This should fix GitHub issue #832.

diff --git a/pcap-int.h b/pcap-int.h
index c46c5a519d9b05032c7aea1ae5e53bda31f33383..9a46019cb2f142dcb34da644f1650c448f0be786 100644 (file)
--- a/pcap-int.h
+++ b/pcap-int.h
@@ -474,11 +474,15 @@ int       add_addr_to_if(pcap_if_list_t *, const char *, bpf_u_int32,
  * "pcap_open_offline_common()" allocates and fills in a pcap_t, for use
  * by pcap_open_offline routines.
  *
+ * "pcap_adjust_snapshot()" adjusts the snapshot to be non-zero and
+ * fit within an int.
+ *
  * "sf_cleanup()" closes the file handle associated with a pcap_t, if
  * appropriate, and frees all data common to all modules for handling
  * savefile types.
  */
 pcap_t *pcap_open_offline_common(char *ebuf, size_t size);
+bpf_u_int32 pcap_adjust_snapshot(bpf_u_int32 linktype, bpf_u_int32 snaplen);
 void   sf_cleanup(pcap_t *p);
 
 /*

diff --git a/savefile.c b/savefile.c
index d22e48c5952bb5f4130e49bd5d0e4c48635d3233..a3db8ff4c1cfdab86063178f96a769aed7fb15e2 100644 (file)
--- a/savefile.c
+++ b/savefile.c
@@ -43,6 +43,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <limits.h> /* for INT_MAX */
 
 #include "pcap-int.h"
 
@@ -52,6 +53,7 @@
 
 #include "sf-pcap.h"
 #include "sf-pcapng.h"
+#include "pcap-common.h"
 
 #ifdef _WIN32
 /*
@@ -332,6 +334,33 @@ pcap_t* pcap_hopen_offline(intptr_t osfd, char *errbuf)
 }
 #endif
 
+/*
+ * Given a link-layer header type and snapshot length, return a
+ * snapshot length to use when reading the file; it's guaranteed
+ * to be > 0 and <= INT_MAX.
+ *
+ * XXX - the only reason why we limit it to <= INT_MAX is so that
+ * it fits in p->snapshot, and the only reason that p->snapshot is
+ * signed is that pcap_snapshot() returns an int, not an unsigned int.
+ */
+bpf_u_int32
+pcap_adjust_snapshot(bpf_u_int32 linktype, bpf_u_int32 snaplen)
+{
+       if (snaplen == 0 || snaplen > INT_MAX) {
+               /*
+                * Bogus snapshot length; use the maximum for this
+                * link-layer type as a fallback.
+                *
+                * XXX - we don't clamp snapshot lengths that are
+                * <= INT_MAX but > max_snaplen_for_dlt(linktype),
+                * so a capture file could cause us to allocate
+                * a Really Big Buffer.
+                */
+               snaplen = max_snaplen_for_dlt(linktype);
+       }
+       return snaplen;
+}
+
 static pcap_t *(*check_headers[])(const uint8_t *, FILE *, u_int, char *, int *) = {
        pcap_check_header,
        pcap_ng_check_header

diff --git a/sf-pcap.c b/sf-pcap.c
index 9fab5997d0f42502a5c5fee36a02ec04308e7c16..c48faf5da5041f7476508de483af57c38936b2ee 100644 (file)
--- a/sf-pcap.c
+++ b/sf-pcap.c
@@ -249,20 +249,9 @@ pcap_check_header(const uint8_t *magic, FILE *fp, u_int precision, char *errbuf,
        p->swapped = swapped;
        p->version_major = hdr.version_major;
        p->version_minor = hdr.version_minor;
-       if (hdr.snaplen == 0 || hdr.snaplen > INT_MAX) {
-               /*
-                * Bogus snapshot length; use the maximum for this
-                * link-layer type as a fallback.
-                *
-                * XXX - the only reason why snapshot is signed is
-                * that pcap_snapshot() returns an int, not an
-                * unsigned int.
-                */
-               p->snapshot = max_snaplen_for_dlt(hdr.linktype);
-       } else
-               p->snapshot = hdr.snaplen;
        p->linktype = linktype_to_dlt(LT_LINKTYPE(hdr.linktype));
        p->linktype_ext = LT_LINKTYPE_EXT(hdr.linktype);
+       p->snapshot = pcap_adjust_snapshot(p->linktype, hdr.snaplen);
 
        p->next_packet_op = pcap_next_packet;
 

diff --git a/sf-pcapng.c b/sf-pcapng.c
index 2881da349b7f631666e39a7c7792b61f82e8d3ff..997ad90419f1d4c5057e3db92cd5ad17747424de 100644 (file)
--- a/sf-pcapng.c
+++ b/sf-pcapng.c
@@ -32,7 +32,6 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
-#include <limits.h> /* for INT_MAX */
 
 #include "pcap-int.h"
 
@@ -1048,19 +1047,8 @@ pcap_ng_check_header(const uint8_t *magic, FILE *fp, u_int precision,
        }
 
 done:
-       if (idbp->snaplen == 0 || idbp->snaplen > INT_MAX) {
-               /*
-                * Bogus snapshot length; use the maximum for this
-                * link-layer type as a fallback.
-                *
-                * XXX - the only reason why snapshot is signed is
-                * that pcap_snapshot() returns an int, not an
-                * unsigned int.
-                */
-               p->snapshot = max_snaplen_for_dlt(idbp->linktype);
-       } else
-               p->snapshot = idbp->snaplen;
        p->linktype = linktype_to_dlt(idbp->linktype);
+       p->snapshot = pcap_adjust_snapshot(p->linktype, idbp->snaplen);
        p->linktype_ext = 0;
 
        /*
@@ -1255,7 +1243,13 @@ pcap_ng_next_packet(pcap_t *p, struct pcap_pkthdr *hdr, u_char **data)
                                    idbp->linktype);
                                return (-1);
                        }
-                       if ((bpf_u_int32)p->snapshot != idbp->snaplen) {
+
+                       /*
+                        * Check against the *adjusted* value of this IDB's
+                        * snapshot length.
+                        */
+                       if ((bpf_u_int32)p->snapshot !=
+                           pcap_adjust_snapshot(p->linktype, idbp->snaplen)) {
                                pcap_snprintf(p->errbuf, PCAP_ERRBUF_SIZE,
                                    "an interface has a snapshot length %u different from the type of the first interface",
                                    idbp->snaplen);