]> The Tcpdump Group git mirrors - libpcap/commitdiff
Always check for EOF after reading a character from /etc/ethers.
authorGuy Harris <[email protected]>
Thu, 31 Aug 2017 20:22:43 +0000 (13:22 -0700)
committerGuy Harris <[email protected]>
Thu, 31 Aug 2017 20:22:43 +0000 (13:22 -0700)
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.)

etherent.c

index 441920d60b71643ccd1b1caaaea2c0b0b91e8389..0f0ef0aeec410c8190ccb4ee3133faf854189c26 100644 (file)
@@ -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);
+       }
 }