From 0ea3fa92a8955b065f052fb378aab253622f1a4e Mon Sep 17 00:00:00 2001
From: Alexander Bluhm <alexander.bluhm@gmx.net>
Date: Mon, 18 Mar 2019 14:06:39 +0100
Subject: [PATCH] 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.
---
 src/suricata.c | 25 +++++++++++++++++++++----
 src/suricata.h |  2 +-
 2 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/src/suricata.c b/src/suricata.c
index a700fa10a9..a9c5620585 100644
--- 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 7e11c00855..e62956698b 100644
--- 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;