Squelch a Clang static analyzer complaint, and fix a real-but-unlikely issue.

Avoid a not-a-real-problem warning from the Clang static analyzer.

Fix an real-but-unlikely (requires a file with a huge number of IDBs)
issue that I *thought* was the source of the static analyzer's complaint
but that, in fact, it apparently hadn't noticed, namely the possibility
of a multiplication overflowing.

Fix a typo while we're at it.

1 files changed, 59 insertions, 2 deletions

diff --git a/sf-pcap-ng.c b/sf-pcap-ng.c
index 6579f30a..1acb06c7 100644
--- a/sf-pcap-ng.c
+++ b/sf-pcap-ng.c
@@ -222,7 +222,7 @@ struct pcap_ng_if {
 struct pcap_ng_sf {
 	u_int user_tsresol;		/* time stamp resolution requested by the user */
 	bpf_u_int32 ifcount;		/* number of interfaces seen in this capture */
-	bpf_u_int32 ifaces_size;	/* size of arrary below */
+	bpf_u_int32 ifaces_size;	/* size of array below */
 	struct pcap_ng_if *ifaces;	/* array of interface information */
 };
 
@@ -546,9 +546,22 @@ add_interface(pcap_t *p, struct block_cursor *cursor, char *errbuf)
 		/*
 		 * We need to grow the array.
 		 */
-		if (ps->ifaces == NULL) {
+		if (ps->ifaces_size == 0) {
 			/*
 			 * It's currently empty.
+			 *
+			 * (The Clang static analyzer doesn't do enought,
+			 * err, umm, dataflow *analysis* to realize that
+			 * ps->ifaces_size == 0 if ps->ifaces == NULL,
+			 * and so complains about a possible zero argument
+			 * to realloc(), so we check for the former
+			 * condition to shut it up.
+			 *
+			 * However, it doesn't complain that one of the
+			 * multiplications below could overflow, which is
+			 * a real, albeit extremely unlikely, problem (you'd
+			 * need a pcap-ng file with tens of millions of
+			 * interfaces).)
 			 */
 			ps->ifaces_size = 1;
 			ps->ifaces = malloc(sizeof (struct pcap_ng_if));
@@ -556,8 +569,52 @@ add_interface(pcap_t *p, struct block_cursor *cursor, char *errbuf)
 			/*
 			 * It's not currently empty; double its size.
 			 * (Perhaps overkill once we have a lot of interfaces.)
+			 *
+			 * Check for overflow if we double it.
+			 */
+			if (ps->ifaces_size * 2 < ps->ifaces_size) {
+				/*
+				 * The maximum number of interfaces before
+				 * ps->ifaces_size overflows is the largest
+				 * possible 32-bit power of 2, as we do
+				 * size doubling.
+				 */
+				snprintf(errbuf, PCAP_ERRBUF_SIZE,
+				    "more than %u interfaces in the file",
+				    0x80000000U);
+				return (0);
+			}
+
+			/*
+			 * ps->ifaces_size * 2 doesn't overflow, so it's
+			 * safe to multiply.
 			 */
 			ps->ifaces_size *= 2;
+
+			/*
+			 * Now make sure that's not so big that it overflows
+			 * if we multiply by sizeof (struct pcap_ng_if).
+			 *
+			 * That can happen on 32-bit platforms, with a 32-bit
+			 * size_t; it shouldn't happen on 64-bit platforms,
+			 * with a 64-bit size_t, as ps->ifaces_size is
+			 * 32 bits.
+			 */
+			if (ps->ifaces_size * sizeof (struct pcap_ng_if) < ps->ifaces_size) {
+				/*
+				 * As this fails only with 32-bit size_t,
+				 * the multiplication was 32x32->32, and
+				 * the largest 32-bit value that can safely
+				 * be multiplied by sizeof (struct pcap_ng_if)
+				 * without overflow is the largest 32-bit
+				 * (unsigned) value divided by
+				 * sizeof (struct pcap_ng_if).
+				 */
+				snprintf(errbuf, PCAP_ERRBUF_SIZE,
+				    "more than %u interfaces in the file",
+				    0xFFFFFFFFU / ((u_int)sizeof (struct pcap_ng_if)));
+				return (0);
+			}
 			ps->ifaces = realloc(ps->ifaces, ps->ifaces_size * sizeof (struct pcap_ng_if));
 		}
 		if (ps->ifaces == NULL) {