Avoid use-after-free during pid file cleanup.

In case the pid file is given in the config file, the file name is
stored in volatile memory.  Removal of the pid file happens after
cleanup of config memory.  Create a copy of the name which will be
freed after the pid file has been removed.

diff --git a/src/suricata.c b/src/suricata.c
index a700fa10a9246039f370c3a00984cebf35076442..a9c562058511037f20227764599a3212cfb3a800 100644 (file)
--- a/src/suricata.c
+++ b/src/suricata.c
@@ -407,6 +407,8 @@ static void GlobalsDestroy(SCInstance *suri)
     SCThresholdConfGlobalFree();
 
     SCPidfileRemove(suri->pid_filename);
+    SCFree(suri->pid_filename);
+    suri->pid_filename = NULL;
 }
 
 /** \brief make sure threads can stop the engine by calling this
@@ -1682,7 +1684,12 @@ static TmEcode ParseCommandLine(int argc, char** argv, SCInstance *suri)
             }
 #endif /* OS_WIN32 */
             else if(strcmp((long_opts[option_index]).name, "pidfile") == 0) {
-                suri->pid_filename = optarg;
+                suri->pid_filename = SCStrdup(optarg);
+                if (suri->pid_filename == NULL) {
+                    SCLogError(SC_ERR_MEM_ALLOC, "strdup failed: %s",
+                        strerror(errno));
+                    return TM_ECODE_FAILED;
+                }
             }
             else if(strcmp((long_opts[option_index]).name, "disable-detection") == 0) {
                 g_detect_disabled = suri->disabled_detect = 1;
@@ -2125,14 +2132,23 @@ static int WindowsInitService(int argc, char **argv)
 static int MayDaemonize(SCInstance *suri)
 {
     if (suri->daemon == 1 && suri->pid_filename == NULL) {
-        if (ConfGet("pid-file", &suri->pid_filename) == 1) {
-            SCLogInfo("Use pid file %s from config file.", suri->pid_filename);
+        const char *pid_filename;
+
+        if (ConfGet("pid-file", &pid_filename) == 1) {
+            SCLogInfo("Use pid file %s from config file.", pid_filename);
         } else {
-            suri->pid_filename = DEFAULT_PID_FILENAME;
+            pid_filename = DEFAULT_PID_FILENAME;
+        }
+        /* The pid file name may be in config memory, but is needed later. */
+        suri->pid_filename = SCStrdup(pid_filename);
+        if (suri->pid_filename == NULL) {
+            SCLogError(SC_ERR_MEM_ALLOC, "strdup failed: %s", strerror(errno));
+            return TM_ECODE_FAILED;
         }
     }
 
     if (suri->pid_filename != NULL && SCPidfileTestRunning(suri->pid_filename) != 0) {
+        SCFree(suri->pid_filename);
         suri->pid_filename = NULL;
         return TM_ECODE_FAILED;
     }
@@ -2143,6 +2159,7 @@ static int MayDaemonize(SCInstance *suri)
 
     if (suri->pid_filename != NULL) {
         if (SCPidfileCreate(suri->pid_filename) != 0) {
+            SCFree(suri->pid_filename);
             suri->pid_filename = NULL;
             SCLogError(SC_ERR_PIDFILE_DAEMON,
                     "Unable to create PID file, concurrent run of"

diff --git a/src/suricata.h b/src/suricata.h
index 7e11c008551d4abec84b7d86137c79204134823f..e62956698b048498014c0827cca2031c93ff6b21 100644 (file)
--- a/src/suricata.h
+++ b/src/suricata.h
@@ -137,7 +137,7 @@ typedef struct SCInstance_ {
     char pcap_dev[128];
     char *sig_file;
     int sig_file_exclusive;
-    const char *pid_filename;
+    char *pid_filename;
     char *regex_arg;
 
     char *keyword_info;