sgwdynspeed: fix resource leak and file stat/use race
authorFerry Huberts <ferry.huberts@pelagic.nl>
Mon, 22 Oct 2012 12:46:14 +0000 (14:46 +0200)
committerFerry Huberts <ferry.huberts@pelagic.nl>
Mon, 22 Oct 2012 13:02:51 +0000 (15:02 +0200)
Coverity:
CID 739680 (#1 of 1): Resource leak (RESOURCE_LEAK)
At (32): Variable "fd" going out of scope leaks the storage it points to.

CID 739697 (#1 of 1): Time of check time of use (TOCTOU)
At (1): Calling function "stat(char const * restrict,
                               struct stat * restrict)"
        to perform check on "fileName".
At (4): Calling function "fopen(char const * restrict,
                                char const * restrict)"
        that uses "fileName" after a check function. This can cause a
        time-of-check, time-of-use race condition.

Signed-off-by: Ferry Huberts <ferry.huberts@pelagic.nl>
lib/sgwdynspeed/src/speedFile.c

index 2c7a98c..0b89118 100644 (file)
@@ -9,6 +9,8 @@
 
 /* System includes */
 #include <stddef.h>
+#include <unistd.h>
+#include <fcntl.h>
 #include <stdlib.h>
 #include <regex.h>
 #include <errno.h>
@@ -157,8 +159,9 @@ static char line[LINE_LENGTH];
  * @param fileName the filename
  */
 void readSpeedFile(char * fileName) {
+       int fd;
        struct stat statBuf;
-       FILE * fd = NULL;
+       FILE * fp = NULL;
        unsigned int lineNumber = 0;
        char * name = NULL;
        char * value = NULL;
@@ -167,7 +170,13 @@ void readSpeedFile(char * fileName) {
        bool uplinkSet = false;
        bool downlinkSet = false;
 
-       if (stat(fileName, &statBuf)) {
+       fd = open(fileName, O_RDONLY);
+       if (fd < 0) {
+               /* could not access the file */
+               goto out;
+       }
+
+       if (fstat(fd, &statBuf)) {
                /* could not access the file */
                goto out;
        }
@@ -177,14 +186,14 @@ void readSpeedFile(char * fileName) {
                goto out;
        }
 
-       fd = fopen(fileName, "r");
-       if (!fd) {
+       fp = fdopen(fd, "r");
+       if (!fp) {
                goto out;
        }
 
        memcpy(&cachedStat.timeStamp, &statBuf.st_mtime, sizeof(cachedStat.timeStamp));
 
-       while (fgets(line, LINE_LENGTH, fd)) {
+       while (fgets(line, LINE_LENGTH, fp)) {
                regmatch_t pmatch[regexNameValuematchCount];
 
                lineNumber++;
@@ -222,7 +231,8 @@ void readSpeedFile(char * fileName) {
                }
        }
 
-       fclose(fd);
+       fclose(fp);
+       fp = NULL;
 
        if (uplinkSet) {
                olsr_cnf->smart_gw_uplink = uplink;
@@ -234,5 +244,11 @@ void readSpeedFile(char * fileName) {
          refresh_smartgw_netmask();
        }
 
-       out: return;
+       out: if (fp) {
+               fclose(fp);
+       }
+       if (fd >= 0) {
+               close(fd);
+       }
+       return;
 }