charon: Avoid potential TOCTOU issues when accessing/writing PID file
The previous code could potentially truncate and change ownership of a file that's a symlink to an unintended target file (requires the attacker to be able to create the symlink in the directory the PID file is located, which generally requires root privileges).
This commit is contained in:
+35
-17
@@ -186,15 +186,22 @@ static void segv_handler(int signal)
|
||||
static bool check_pidfile()
|
||||
{
|
||||
struct stat stb;
|
||||
int fd, flags = 0;
|
||||
|
||||
if (stat(PID_FILE, &stb) == 0)
|
||||
#ifndef WIN32
|
||||
flags |= O_NOFOLLOW;
|
||||
#endif
|
||||
fd = open(PID_FILE, O_RDONLY | flags);
|
||||
if (fd != -1)
|
||||
{
|
||||
pidfile = fopen(PID_FILE, "r");
|
||||
if (pidfile)
|
||||
if (fstat(fd, &stb) == 0 && S_ISREG(stb.st_mode))
|
||||
{
|
||||
char buf[64];
|
||||
pid_t pid = 0;
|
||||
|
||||
pidfile = fdopen(fd, "r");
|
||||
if (pidfile)
|
||||
{
|
||||
memset(buf, 0, sizeof(buf));
|
||||
if (fread(buf, 1, sizeof(buf), pidfile))
|
||||
{
|
||||
@@ -203,26 +210,34 @@ static bool check_pidfile()
|
||||
}
|
||||
fclose(pidfile);
|
||||
pidfile = NULL;
|
||||
}
|
||||
else
|
||||
{
|
||||
close(fd);
|
||||
}
|
||||
if (pid && pid != getpid() && kill(pid, 0) == 0)
|
||||
{
|
||||
DBG1(DBG_DMN, "charon already running ('"PID_FILE"' exists)");
|
||||
return TRUE;
|
||||
}
|
||||
}
|
||||
else
|
||||
{
|
||||
close(fd);
|
||||
}
|
||||
}
|
||||
|
||||
if (fd != -1 || errno != ENOENT)
|
||||
{
|
||||
DBG1(DBG_DMN, "removing pidfile '"PID_FILE"', process not running");
|
||||
unlink(PID_FILE);
|
||||
}
|
||||
|
||||
/* create new pidfile */
|
||||
pidfile = fopen(PID_FILE, "w");
|
||||
if (pidfile)
|
||||
{
|
||||
int fd;
|
||||
|
||||
fd = fileno(pidfile);
|
||||
/* create new pidfile securely without following symlinks */
|
||||
fd = open(PID_FILE, O_CREAT | O_EXCL | O_WRONLY, 0644);
|
||||
if (fd == -1)
|
||||
{
|
||||
DBG1(DBG_DMN, "unable to determine fd for '"PID_FILE"'");
|
||||
DBG1(DBG_DMN, "unable to create pidfile '"PID_FILE"'");
|
||||
return TRUE;
|
||||
}
|
||||
if (fcntl(fd, F_SETFD, FD_CLOEXEC) == -1)
|
||||
@@ -248,15 +263,18 @@ static bool check_pidfile()
|
||||
ignore_result(fchown(fd, -1,
|
||||
lib->caps->get_gid(lib->caps)));
|
||||
}
|
||||
pidfile = fdopen(fd, "w");
|
||||
if (!pidfile)
|
||||
{
|
||||
DBG1(DBG_DMN, "unable to open pidfile '"PID_FILE"': %s",
|
||||
strerror(errno));
|
||||
close(fd);
|
||||
unlink(PID_FILE);
|
||||
return TRUE;
|
||||
}
|
||||
fprintf(pidfile, "%d\n", getpid());
|
||||
fflush(pidfile);
|
||||
return FALSE;
|
||||
}
|
||||
else
|
||||
{
|
||||
DBG1(DBG_DMN, "unable to create pidfile '"PID_FILE"'");
|
||||
return TRUE;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user