Avoid use-after-free during pid file cleanup. This replaces my

previous fix with the version commited upstream.
OK jasper@ gonzalo@
This commit is contained in:
bluhm 2019-03-26 13:46:24 +00:00
parent 113c5b8bd2
commit e4f86710dc
4 changed files with 76 additions and 19 deletions

View File

@ -1,9 +1,10 @@
# $OpenBSD: Makefile,v 1.13 2019/03/18 17:54:33 jasper Exp $
# $OpenBSD: Makefile,v 1.14 2019/03/26 13:46:24 bluhm Exp $
COMMENT = high performance network IDS, IPS and security monitoring
SURICATA_V = 4.1.3
SUPDATE_V = 1.0.4
REVISION = 0
DISTNAME = suricata-${SURICATA_V}
CATEGORIES = security

View File

@ -1,31 +1,71 @@
$OpenBSD: patch-src_suricata_c,v 1.1 2019/03/05 12:38:24 bluhm Exp $
$OpenBSD: patch-src_suricata_c,v 1.2 2019/03/26 13:46:24 bluhm Exp $
Fix pid file removal if it is specified in config file.
https://github.com/OISF/suricata/commit/0ea3fa92a8955b065f052fb378aab253622f1a4e
Use setresuid/gid() directly to change user and group. Otherwise
Suricata uses libcap-ng on Linux and runs as root elsewhere.
Index: src/suricata.c
--- src/suricata.c.orig
+++ src/suricata.c
@@ -399,6 +399,8 @@ static void GlobalsDestroy(SCInstance *suri)
MpmHSGlobalCleanup();
#endif
+ /* The pid file name may be in config memory, but is needed later. */
+ suri->pid_filename = strdup(suri->pid_filename);
ConfDeInit();
#ifdef HAVE_LUAJIT
LuajitFreeStatesPool();
@@ -408,6 +410,8 @@ static void GlobalsDestroy(SCInstance *suri)
@@ -408,6 +408,8 @@ static void GlobalsDestroy(SCInstance *suri)
SCThresholdConfGlobalFree();
SCPidfileRemove(suri->pid_filename);
+ free((char *)suri->pid_filename);
+ SCFree(suri->pid_filename);
+ suri->pid_filename = NULL;
}
/** \brief make sure threads can stop the engine by calling this
@@ -3023,6 +3027,7 @@ int main(int argc, char **argv)
@@ -1712,7 +1714,12 @@ static TmEcode ParseCommandLine(int argc, char** argv,
}
#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;
@@ -2174,14 +2181,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;
}
@@ -2192,6 +2208,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"
@@ -3027,6 +3044,7 @@ int main(int argc, char **argv)
#endif
#endif

View File

@ -0,0 +1,16 @@
$OpenBSD: patch-src_suricata_h,v 1.1 2019/03/26 13:46:24 bluhm Exp $
https://github.com/OISF/suricata/commit/0ea3fa92a8955b065f052fb378aab253622f1a4e
Index: src/suricata.h
--- src/suricata.h.orig
+++ 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;

View File

@ -1,4 +1,4 @@
$OpenBSD: patch-suricata_yaml_in,v 1.3 2019/03/05 12:38:24 bluhm Exp $
$OpenBSD: patch-suricata_yaml_in,v 1.4 2019/03/26 13:46:24 bluhm Exp $
Switch user and group to avoid running as root.
To remove pid file its directory must be writable by suricata user.
@ -7,7 +7,7 @@ Remove rules files not present by default.
Index: suricata.yaml.in
--- suricata.yaml.in.orig
+++ suricata.yaml.in
@@ -1039,9 +1039,9 @@ asn1-max-frames: 256
@@ -1042,9 +1042,9 @@ asn1-max-frames: 256
##
# Run suricata as user and group.
@ -20,7 +20,7 @@ Index: suricata.yaml.in
# Some logging module will use that name in event as identifier. The default
# value is the hostname
@@ -1050,7 +1050,7 @@ asn1-max-frames: 256
@@ -1053,7 +1053,7 @@ asn1-max-frames: 256
# Default location of the pid file. The pid file is only used in
# daemon mode (start Suricata with -D). If not running in daemon mode
# the --pidfile command line option must be used to create a pid file.
@ -29,7 +29,7 @@ Index: suricata.yaml.in
# Daemon working directory
# Suricata will change directory to this one if provided
@@ -1845,9 +1845,21 @@ mpipe:
@@ -1848,9 +1848,21 @@ mpipe:
## file configuration".
##