denozord/strongswan-ext
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

file-logger: Prevent potential race when opening log files and change permission

Browse Source 
This fixes a potential TOCTOU issue with opening log files.  The use of
`chown()` instead of `fchown()` could theoretically allow modifying the
ownership of an unintended file.

The log file now also is not world-readable anymore.

Also, the patch fixes the log groups for the two `(f)chown()` errors.

Fixes: d35d669180 ("Make syslog and file loggers configurable at runtime")
This commit is contained in:
Tobias Brunner
2026-07-24 13:55:04 +02:00
parent 50ac563292
commit ea7a721dd8
1 changed files with 20 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/libcharon/bus/listeners/file_logger.c
+20 -9
View File
@@ -1,5 +1,5 @@
/* /*
* Copyright (C) 2012-2024 Tobias Brunner * Copyright (C) 2012-2026 Tobias Brunner
* Copyright (C) 2006 Martin Willi * Copyright (C) 2006 Martin Willi
* *
* Copyright (C) secunet Security Networks AG * Copyright (C) secunet Security Networks AG
@@ -19,6 +19,7 @@
#include <string.h> #include <string.h>
#include <time.h> #include <time.h>
#include <errno.h> #include <errno.h>
#include <fcntl.h>
#include <unistd.h> #include <unistd.h>
#include <sys/types.h> #include <sys/types.h>
@@ -306,32 +307,42 @@ METHOD(file_logger_t, open_, void,
} }
else else
{ {
file = fopen(this->filename, append ? "a" : "w"); int fd, flags = O_WRONLY | O_CREAT | (append ? O_APPEND : O_TRUNC);
if (file == NULL)
fd = open(this->filename, flags, 0660);
if (fd < 0)
{ {
DBG1(DBG_DMN, "opening file %s for logging failed: %s", DBG1(DBG_DMN, "opening file '%s' for logging failed: %s",
this->filename, strerror(errno)); this->filename, strerror(errno));
return; return;
} }
#ifdef HAVE_CHOWN #ifdef HAVE_CHOWN
if (lib->caps->check(lib->caps, CAP_CHOWN)) if (lib->caps->check(lib->caps, CAP_CHOWN))
{ {
if (chown(this->filename, lib->caps->get_uid(lib->caps), if (fchown(fd, lib->caps->get_uid(lib->caps),
lib->caps->get_gid(lib->caps)) != 0) lib->caps->get_gid(lib->caps)) != 0)
{ {
DBG1(DBG_NET, "changing owner/group for '%s' failed: %s", DBG1(DBG_DMN, "changing owner/group for '%s' failed: %s",
this->filename, strerror(errno)); this->filename, strerror(errno));
} }
} }
else else
{ {
if (chown(this->filename, -1, lib->caps->get_gid(lib->caps)) != 0) if (fchown(fd, -1, lib->caps->get_gid(lib->caps)) != 0)
{ {
DBG1(DBG_NET, "changing group for '%s' failed: %s", DBG1(DBG_DMN, "changing group for '%s' failed: %s",
this->filename, strerror(errno)); this->filename, strerror(errno));
} }
} }
#endif /* HAVE_CHOWN */ #endif /* HAVE_CHOWN */
file = fdopen(fd, append ? "a" : "w");
if (!file)
{
DBG1(DBG_DMN, "opening file %s for logging failed: %s",
this->filename, strerror(errno));
close(fd);
return;
}
#ifdef HAVE_SETLINEBUF #ifdef HAVE_SETLINEBUF
if (flush_line) if (flush_line)
{ {