From: Guy Harris <guy@alum.mit.edu>
Date: Thu, 31 Aug 2017 20:22:43 +0000 (-0700)
Subject: Always check for EOF after reading a character from /etc/ethers.
X-Git-Tag: libpcap-1.9-bp~785
X-Git-Url: https://git.tcpdump.org/libpcap/commitdiff_plain/4aeb99eac7e8b192fd398050faa167a05ad2fbf1

Always check for EOF after reading a character from /etc/ethers.

We can get an EOF at any time, either due to a file that doesn't end
cleanly with an NL or due to an I/O error.  In pcap_next_etherent(),
every time we call a routine that returns a character from the file,
check for EOF and return NULL.

(According to the C90 standard (and presumably all subsequent standards),
1) you can safely pass EOF to all the isXXX() macros and 2) EOF is not a
space character, so we don't need to check for EOF there.)

This should address the concerns of Coverity CID 1416964; hopefully
Coverity realizes that getc() returns either a nonnegative value or EOF,
and therefore that checking for EOF is sufficient to ensure that the
return value is nonnegative.  (Given how much it complains about
strcmp() with a string constant, because some tests can be evaluated at
compile time and therefore some code happens to be unreachable in that
particular call - code that will be optimized out by any competent
compiler - I'm not going to *assume* Coverity has a clue there.)
---

diff --git a/etherent.c b/etherent.c
index 441920d6..0f0ef0ae 100644
--- a/etherent.c
+++ b/etherent.c
@@ -101,16 +101,20 @@ pcap_next_etherent(FILE *fp)
 	static struct pcap_etherent e;
 
 	memset((char *)&e, 0, sizeof(e));
-	do {
+	for (;;) {
 		/* Find addr */
 		c = skip_space(fp);
+		if (c == EOF)
+			return (NULL);
 		if (c == '\n')
 			continue;
 
 		/* If this is a comment, or first thing on line
-		   cannot be etehrnet address, skip the line. */
+		   cannot be Ethernet address, skip the line. */
 		if (!isxdigit(c)) {
 			c = skip_line(fp);
+			if (c == EOF)
+				return (NULL);
 			continue;
 		}
 
@@ -118,25 +122,33 @@ pcap_next_etherent(FILE *fp)
 		for (i = 0; i < 6; i += 1) {
 			d = xdtoi(c);
 			c = getc(fp);
+			if (c == EOF)
+				return (NULL);
 			if (isxdigit(c)) {
 				d <<= 4;
 				d |= xdtoi(c);
 				c = getc(fp);
+				if (c == EOF)
+					return (NULL);
 			}
 			e.addr[i] = d;
 			if (c != ':')
 				break;
 			c = getc(fp);
+			if (c == EOF)
+				return (NULL);
 		}
-		if (c == EOF)
-			break;
 
 		/* Must be whitespace */
 		if (!isspace(c)) {
 			c = skip_line(fp);
+			if (c == EOF)
+				return (NULL);
 			continue;
 		}
 		c = skip_space(fp);
+		if (c == EOF)
+			return (NULL);
 
 		/* hit end of line... */
 		if (c == '\n')
@@ -144,6 +156,8 @@ pcap_next_etherent(FILE *fp)
 
 		if (c == '#') {
 			c = skip_line(fp);
+			if (c == EOF)
+				return (NULL);
 			continue;
 		}
 
@@ -154,7 +168,9 @@ pcap_next_etherent(FILE *fp)
 		do {
 			*bp++ = c;
 			c = getc(fp);
-		} while (!isspace(c) && c != EOF && --d > 0);
+			if (c == EOF)
+				return (NULL);
+		} while (!isspace(c) && --d > 0);
 		*bp = '\0';
 
 		/* Eat trailing junk */
@@ -162,8 +178,5 @@ pcap_next_etherent(FILE *fp)
 			(void)skip_line(fp);
 
 		return &e;
-
-	} while (c != EOF);
-
-	return (NULL);
+	}
 }