Mitigate file check before open

This commit should mitigate some race conditions where files are changed after their status is checked. Ticket: None Changelog: None
cfengine · Sep 2, 2019 · 9901c48 · 9901c48
1 parent 8e38a98
commit 9901c48
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 29 deletions.
diff --git a/cf-execd/cf-execd-runner.c b/cf-execd/cf-execd-runner.c
@@ -337,21 +337,21 @@ void LocalExec(const ExecConfig *config)
 
     free(line);
     cf_pclose(pp);
-    Log(LOG_LEVEL_DEBUG, "Closing fp");
-    fclose(fp);
-
     Log(LOG_LEVEL_VERBOSE,
         complete ? "Command is complete" : "Terminated command");
 
     if (count)
     {
+        Log(LOG_LEVEL_DEBUG, "Closing fp");
+        fclose(fp);
         Log(LOG_LEVEL_VERBOSE, "Mailing result");
         MailResult(config, filename);
     }
     else
     {
         Log(LOG_LEVEL_VERBOSE, "No output");
         unlink(filename);
+        fclose(fp);
     }
 }
 
@@ -583,19 +583,29 @@ static void MailResult(const ExecConfig *config, const char *file)
     time_t now = time(NULL);
 #endif
 
+    FILE *fp = safe_fopen(file, "r");
+    if (fp == NULL)
+    {
+        Log(LOG_LEVEL_ERR, "Mail report: couldn't open file '%s'. (fopen: %s)", file, GetErrorStr());
+        return;
+    }
+
     Log(LOG_LEVEL_VERBOSE, "Mail report: sending result...");
 
     {
+        int fd = fileno(fp);
         struct stat statbuf;
-        if (stat(file, &statbuf) == -1)
+        if (fstat(fd, &statbuf) == -1)
         {
             Log(LOG_LEVEL_ERR, "Mail report: failed to stat file '%s' [errno: %d]", file, errno);
+            fclose(fp);
             return;
         }
 
         if (statbuf.st_size == 0)
         {
             unlink(file);
+            fclose(fp);
             Log(LOG_LEVEL_DEBUG, "Mail report: nothing to report in file '%s'", file);
             return;
         }
@@ -609,6 +619,7 @@ static void MailResult(const ExecConfig *config, const char *file)
         if (CompareResultEqualOrFiltered(config, file, prev_file))
         {
             Log(LOG_LEVEL_VERBOSE, "Mail report: previous output is the same as current so do not mail it");
+            fclose(fp);
             return;
         }
     }
@@ -617,24 +628,19 @@ static void MailResult(const ExecConfig *config, const char *file)
     {
         /* Syslog should have done this */
         Log(LOG_LEVEL_VERBOSE, "Mail report: empty mail server or address - skipping");
+        fclose(fp);
         return;
     }
 
     if (config->mail_max_lines == 0)
     {
         Log(LOG_LEVEL_DEBUG, "Mail report: not mailing because EmailMaxLines was zero");
+        fclose(fp);
         return;
     }
 
     Log(LOG_LEVEL_DEBUG, "Mail report: mailing results of '%s' to '%s'", file, config->mail_to_address);
 
-    FILE *fp = fopen(file, "r");
-    if (fp == NULL)
-    {
-        Log(LOG_LEVEL_ERR, "Mail report: couldn't open file '%s'. (fopen: %s)", file, GetErrorStr());
-        return;
-    }
-
     int sd = ConnectToSmtpSocket(config);
     if (sd < 0)
     {

diff --git a/libenv/sysinfo.c b/libenv/sysinfo.c
@@ -922,16 +922,26 @@ static void Get3Environment(EvalContext *ctx)
     snprintf(env, CF_BUFSIZE, "%s/%s", GetStateDir(), CF_ENV_FILE);
     MapName(env);
 
-    if (stat(env, &statbuf) == -1)
+    FILE *fp = safe_fopen(env, "r");
+    if (fp == NULL)
     {
         Log(LOG_LEVEL_VERBOSE, "Unable to detect environment from cf-monitord");
         return;
     }
 
+    int fd = fileno(fp);
+    if (fstat(fd, &statbuf) == -1)
+    {
+        Log(LOG_LEVEL_VERBOSE, "Unable to detect environment from cf-monitord");
+        fclose(fp);
+        return;
+    }
+
     if (statbuf.st_mtime < (now - 60 * 60))
     {
         Log(LOG_LEVEL_VERBOSE, "Environment data are too old - discarding");
         unlink(env);
+        fclose(fp);
         return;
     }
 
@@ -945,13 +955,6 @@ static void Get3Environment(EvalContext *ctx)
 
     Log(LOG_LEVEL_VERBOSE, "Loading environment...");
 
-    FILE *fp = fopen(env, "r");
-    if (fp == NULL)
-    {
-        Log(LOG_LEVEL_VERBOSE, "Unable to detect environment from cf-monitord");
-        return;
-    }
-
     for(;;)
     {
         name[0] = '\0';

diff --git a/libpromises/monitoring_read.c b/libpromises/monitoring_read.c
@@ -135,31 +135,28 @@ MonitoringSlot *Nova_MakeSlot(const char *name, const char *description,
 
 void Nova_LoadSlots(void)
 {
-    FILE *f;
     char filename[CF_BUFSIZE];
     int i;
 
     snprintf(filename, CF_BUFSIZE - 1, "%s%cts_key", GetStateDir(), FILE_SEPARATOR);
 
-    struct stat sb;
-
-    if (stat(filename, &sb) != 0)
+    FILE *f = safe_fopen(filename, "r");
+    if (f == NULL)
     {
         return;
     }
 
-    if(sb.st_mtime <= slots_load_time)
+    int fd = fileno(f);
+    struct stat sb;
+    if ((fstat(fd, &sb) != 0) ||
+        (sb.st_mtime <= slots_load_time))
     {
+        fclose(f);
         return;
     }
 
     slots_load_time = sb.st_mtime;
 
-    if ((f = fopen(filename, "r")) == NULL)
-    {
-        return;
-    }
-
     for (i = 0; i < CF_OBSERVABLES; ++i)
     {
         if (i < ob_spare)