lei.chen/myxce
1
0
Fork 1
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(worker): xcecd's fixed /tmp files never follow planted links (bug-1869)

Browse source 
xcecd runs as root and kept files at fixed names in world-writable /tmp,
opened by path:

- main.c created <logdir or /tmp>/xcecd_worker.log and then chmod(path,
  0666)'d it: a symlink planted there made root chmod 0666 any file
  (e.g. /etc/shadow).
- process_worker_manager.c appended xcecd_worker's stdout/stderr to
  /tmp/xcecd_worker.log through any planted symlink/hardlink.
- xcecd_prepare_restart() created /tmp/.xcecd.restarting through a
  dangling symlink (an empty root-owned file anywhere), and
  xcecd_check_restart_type() trusted access() on it, so any user could make
  an unplanned restart look planned.

xcecd_tmpfile.h:
- xcecd_open_owned_file(): reuses an existing file only if it is our own
  regular single-link file (so a legitimate log keeps being appended to);
  otherwise unlinks whatever is there and creates a fresh file with
  O_EXCL|O_NOFOLLOW. O_NONBLOCK so a planted FIFO cannot hang startup.
- xcecd_owned_file_exists(): lstat-based; only our own regular file counts
  as the restart flag. Anything else at the name is removed and ignored.
- The worker log's 0666 mode is now set with fchmod() on the fd.

New test_xcecd_tmpfile.c (7 tests; with the old behaviour the symlink,
dangling-symlink and hardlink cases FAIL and the FIFO case hangs);
worker suite 12/12. Sequential HA soak (5 rounds, xcecd_virtual) PASS.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Rongsong Shen 2026-09-26 08:13:24 +00:00
parent 5f53211f9d
commit 5df33e69d9
Signed by untrusted user: rshen
GPG key ID: 80533536B0EB651C
6 changed files with 308 additions and 11 deletions
Download patch file Download diff file Expand all files Collapse all files

69
src/worker/include/xcecd_tmpfile.h Normal file
View file

@ -0,0 +1,69 @@
#pragma once
/*
* xcecd_tmpfile.h — root-owned files at fixed names in shared directories.
*
* xcecd runs as root and keeps a few files at fixed, well-known names in
* world-writable /tmp (the xcecd_worker log, the planned-restart flag).
* Opening, creating or chmod'ing such a name by path would follow a symlink
* (or write through a hardlink) a local user planted there first, letting
* them append to, create or chmod 0666 any file on the host (bug-1869).
* These helpers only ever act on a regular, single-link file owned by us.
*/
#include <errno.h>
#include <fcntl.h>
#include <stdbool.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <unistd.h>
/* Open `path` for writing (`extra_flags` e.g. O_APPEND; O_TRUNC is not
* meaningful here), creating it with `mode` if absent. An existing file is
* reused only if it is our own regular, single-link file; anything else at
* the name -- a symlink (dangling or not), a hardlink to some other file, a
* FIFO, another user's file -- is removed (root may unlink in a sticky dir)
* and replaced with a fresh file created O_EXCL|O_NOFOLLOW, which never
* follows a link and fails if something reappears in between. Opening is
* O_NONBLOCK so a planted FIFO cannot hang the caller. Change the mode with
* fchmod() on the returned fd, never chmod() on the path. Returns an fd or
* -errno. */
static inline int xcecd_open_owned_file(const char* path, int extra_flags,
mode_t mode)
{
const int base = O_WRONLY | O_NOFOLLOW | O_NONBLOCK | O_CLOEXEC
| extra_flags;
struct stat st;
int attempt;
int fd;
for (attempt = 0; attempt < 3; attempt++) {
fd = open(path, base | O_CREAT, mode);
if (fd >= 0) {
if (fstat(fd, &st) == 0 && S_ISREG(st.st_mode)
&& st.st_nlink == 1 && st.st_uid == geteuid())
return fd;
close(fd);
} else if (errno != ELOOP && errno != ENXIO && errno != EISDIR) {
/* ELOOP: symlink; ENXIO: FIFO without a reader; EISDIR: dir */
return -errno;
}
if (unlink(path) < 0 && errno != ENOENT)
return -errno;
fd = open(path, base | O_CREAT | O_EXCL, mode);
if (fd >= 0)
return fd;
if (errno != EEXIST)
return -errno;
}
return -EEXIST;
}
/* True iff `path` is our own regular file (lstat, so a symlink planted at
* the name never counts). */
static inline bool xcecd_owned_file_exists(const char* path)
{
struct stat st;
return lstat(path, &st) == 0 && S_ISREG(st.st_mode)
&& st.st_uid == geteuid();
}

18
src/worker/src/main.c
View file

@ -1,5 +1,6 @@
#include "sqlite3.h"
#include "xcecd.h"
#include "xcecd_tmpfile.h"
#include "xce/cluster.h"
#include <errno.h>
#include <getopt.h>
@ -591,15 +592,18 @@ int main(int argc, char* argv[])
}
#ifndef VIRTUAL_CLUSTER
/* create worker log file */
int worker_log_fd = open(worker_log, O_WRONLY | O_CREAT | O_APPEND, 0644);
if (worker_log_fd != -1) {
/* create worker log file -- bug-1869: never by path (a symlink planted
* at <logdir or /tmp>/xcecd_worker.log would get chmod 0666'd) */
int worker_log_fd = xcecd_open_owned_file(worker_log, O_APPEND, 0644);
if (worker_log_fd >= 0) {
XCECD_LOG(XCE_LOG_INFO, "Worker log file created: %s", worker_log);
/* close it and wait real worker to use */
fchmod(worker_log_fd, 0666);
close(worker_log_fd);
} else {
XCECD_LOG(XCE_LOG_ERROR, "Failed to create worker log %s: %s",
worker_log, strerror(-worker_log_fd));
}
/* close it and wait real worker to use */
close(worker_log_fd);
chmod(worker_log, 0666);
#endif
if (xcecd_config_init(&config) != 0) {

6
src/worker/src/process_worker_manager.c
View file

@ -6,6 +6,7 @@
* by default.
*/
#include "process_worker_manager.h"
#include "xcecd_tmpfile.h"
#include "xcelog.h"
#include <errno.h>
#include <stdbool.h>
@ -64,7 +65,10 @@ int process_manager_start_worker(process_manager_t* manager)
proc->command = strdup("xcecd_worker");
proc->log_file = strdup("/tmp/xcecd_worker.log");
proc->output_fd = open(proc->log_file, O_WRONLY | O_CREAT | O_APPEND, 0644);
/* bug-1869: fixed name in /tmp, opened as root */
proc->output_fd = xcecd_open_owned_file(proc->log_file, O_APPEND, 0644);
if (proc->output_fd < 0)
proc->output_fd = -1;
if (proc->output_fd == -1) {
XCECD_LOG(XCE_LOG_ERROR, "Failed to open log file: %s",
strerror(errno));

19
src/worker/src/xcecd.c
View file

@ -1,6 +1,7 @@
#include "xcecd.h"
#include "xcecd_cgroups.h"
#include "xcecd_utils.h"
#include "xcecd_tmpfile.h"
#include <errno.h>
#include <fcntl.h>
#include <libgen.h>
@ -612,8 +613,9 @@ void xcecd_cleanup(void)
void xcecd_prepare_restart()
{
int fd = open(RESTART_FLAG_FILE, O_CREAT | O_WRONLY, 0644);
if (fd != -1) {
/* bug-1869: fixed name in /tmp, created as root */
int fd = xcecd_open_owned_file(RESTART_FLAG_FILE, 0, 0644);
if (fd >= 0) {
close(fd);
} else {
XCECD_LOG(XCE_LOG_ERROR, "Failed to create restart flag file.");
@ -622,11 +624,22 @@ void xcecd_prepare_restart()
bool xcecd_check_restart_type(void)
{
if (access(RESTART_FLAG_FILE, F_OK) == 0) {
struct stat st;
/* bug-1869: only our own regular file is the flag -- anyone can create
* /tmp/.xcecd.restarting and would otherwise make an unplanned restart
* look planned. Something else at the name is removed and ignored. */
if (xcecd_owned_file_exists(RESTART_FLAG_FILE)) {
XCECD_LOG(XCE_LOG_DEBUG, "XCECD daemon is performing planned restart.");
unlink(RESTART_FLAG_FILE);
return true;
} else {
if (lstat(RESTART_FLAG_FILE, &st) == 0) {
XCECD_LOG(XCE_LOG_WARN,
"ignoring and removing %s: not a root-owned regular "
"file", RESTART_FLAG_FILE);
unlink(RESTART_FLAG_FILE);
}
if (access(xcecd_ctx.db_path, F_OK) == 0) {
XCECD_LOG(XCE_LOG_DEBUG,
"XCECD daemon is performing unplanned restart.");

13
src/worker/testsuite/meson.build
View file

@ -193,3 +193,16 @@ test('worker job_output_path test', job_output_path_test,
suite: 'worker',
timeout: 60,
env: ['ASAN_OPTIONS=detect_leaks=0'])
# bug-1869: xcecd's own fixed-name /tmp files (xcecd_worker.log, the
# restart flag) must not follow links planted at those names.
xcecd_tmpfile_test = executable('xcecd_tmpfile_test',
'test_xcecd_tmpfile.c',
include_directories: [worker_test_incdir],
dependencies: worker_test_deps,
install: false)
test('worker xcecd_tmpfile test', xcecd_tmpfile_test,
suite: 'worker',
timeout: 60,
env: ['ASAN_OPTIONS=detect_leaks=0'])

194
src/worker/testsuite/test_xcecd_tmpfile.c Normal file
View file

@ -0,0 +1,194 @@
/*
* test_xcecd_tmpfile.c — bug-1869: xcecd (root) keeps files at fixed names
* in world-writable /tmp. Opening them must never follow a symlink or write
* through a hardlink planted at the name, must not block on a FIFO, and must
* keep appending to its own legitimate file; the restart flag must only
* count when it is our own regular file.
*/
#include <errno.h>
#include <fcntl.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/stat.h>
#include <unistd.h>
#include <unity/unity.h>
#include "xcecd_tmpfile.h"
static char g_dir[64];
static char g_path[128];
static char g_victim[128];
void setUp(void)
{
int fd;
strcpy(g_dir, "/tmp/xce_tmpfile_XXXXXX");
TEST_ASSERT_NOT_NULL(mkdtemp(g_dir));
snprintf(g_path, sizeof(g_path), "%s/fixed.log", g_dir);
snprintf(g_victim, sizeof(g_victim), "%s/victim", g_dir);
fd = open(g_victim, O_WRONLY | O_CREAT | O_EXCL, 0600);
TEST_ASSERT_TRUE(fd >= 0);
TEST_ASSERT_EQUAL_INT(8, (int)write(fd, "PRECIOUS", 8));
close(fd);
}
void tearDown(void)
{
unlink(g_path);
unlink(g_victim);
rmdir(g_dir);
}
static void assert_victim_untouched(void)
{
char buf[32] = { 0 };
struct stat st;
int fd = open(g_victim, O_RDONLY);
TEST_ASSERT_TRUE(fd >= 0);
TEST_ASSERT_EQUAL_INT(8, (int)read(fd, buf, sizeof(buf) - 1));
close(fd);
TEST_ASSERT_EQUAL_STRING_MESSAGE("PRECIOUS", buf,
"a planted link's target must not be written");
TEST_ASSERT_EQUAL_INT(0, stat(g_victim, &st));
TEST_ASSERT_EQUAL_INT_MESSAGE(0600, (int)(st.st_mode & 0777),
"a planted link's target must not be chmod'ed");
}
/* the name must now hold our own fresh regular file */
static void assert_own_regular_file(int fd)
{
struct stat st;
TEST_ASSERT_EQUAL_INT(0, lstat(g_path, &st));
TEST_ASSERT_TRUE(S_ISREG(st.st_mode));
TEST_ASSERT_EQUAL_INT(1, (int)st.st_nlink);
TEST_ASSERT_EQUAL_INT((int)geteuid(), (int)st.st_uid);
TEST_ASSERT_EQUAL_INT(2, (int)write(fd, "ok", 2));
}
static void test_open_creates_file(void)
{
int fd = xcecd_open_owned_file(g_path, O_APPEND, 0644);
TEST_ASSERT_TRUE(fd >= 0);
assert_own_regular_file(fd);
close(fd);
}
static void test_open_appends_to_own_existing_file(void)
{
char buf[32] = { 0 };
int fd = open(g_path, O_WRONLY | O_CREAT, 0644);
TEST_ASSERT_TRUE(fd >= 0);
TEST_ASSERT_EQUAL_INT(4, (int)write(fd, "old\n", 4));
close(fd);
fd = xcecd_open_owned_file(g_path, O_APPEND, 0644);
TEST_ASSERT_TRUE(fd >= 0);
TEST_ASSERT_EQUAL_INT(4, (int)write(fd, "new\n", 4));
close(fd);
fd = open(g_path, O_RDONLY);
TEST_ASSERT_EQUAL_INT(8, (int)read(fd, buf, sizeof(buf) - 1));
close(fd);
TEST_ASSERT_EQUAL_STRING_MESSAGE("old\nnew\n", buf,
"a legitimate existing log must be appended to, not replaced");
}
static void test_open_replaces_planted_symlink(void)
{
int fd;
TEST_ASSERT_EQUAL_INT(0, symlink(g_victim, g_path));
fd = xcecd_open_owned_file(g_path, O_APPEND, 0644);
TEST_ASSERT_TRUE(fd >= 0);
TEST_ASSERT_EQUAL_INT(0, fchmod(fd, 0666));
assert_own_regular_file(fd);
close(fd);
assert_victim_untouched();
}
static void test_open_replaces_planted_dangling_symlink(void)
{
char target[160];
struct stat st;
int fd;
snprintf(target, sizeof(target), "%s/created-by-root", g_dir);
TEST_ASSERT_EQUAL_INT(0, symlink(target, g_path));
fd = xcecd_open_owned_file(g_path, 0, 0644);
TEST_ASSERT_TRUE(fd >= 0);
assert_own_regular_file(fd);
close(fd);
TEST_ASSERT_TRUE_MESSAGE(lstat(target, &st) < 0,
"a dangling symlink's target must not be created");
}
static void test_open_replaces_planted_hardlink(void)
{
int fd;
TEST_ASSERT_EQUAL_INT(0, link(g_victim, g_path));
fd = xcecd_open_owned_file(g_path, O_APPEND, 0644);
TEST_ASSERT_TRUE(fd >= 0);
TEST_ASSERT_EQUAL_INT(0, fchmod(fd, 0666));
assert_own_regular_file(fd);
close(fd);
assert_victim_untouched();
}
static void test_open_does_not_block_on_planted_fifo(void)
{
int fd;
TEST_ASSERT_EQUAL_INT(0, mkfifo(g_path, 0666));
alarm(5); /* a blocking open() would hang the suite; fail loudly */
fd = xcecd_open_owned_file(g_path, O_APPEND, 0644);
alarm(0);
TEST_ASSERT_TRUE(fd >= 0);
assert_own_regular_file(fd);
close(fd);
}
static void test_flag_exists_only_for_own_regular_file(void)
{
int fd;
TEST_ASSERT_FALSE(xcecd_owned_file_exists(g_path));
TEST_ASSERT_EQUAL_INT(0, symlink(g_victim, g_path));
TEST_ASSERT_FALSE_MESSAGE(xcecd_owned_file_exists(g_path),
"a symlink at the flag name is not the flag");
unlink(g_path);
TEST_ASSERT_EQUAL_INT(0, mkfifo(g_path, 0600));
TEST_ASSERT_FALSE_MESSAGE(xcecd_owned_file_exists(g_path),
"a FIFO at the flag name is not the flag");
unlink(g_path);
fd = open(g_path, O_WRONLY | O_CREAT, 0644);
TEST_ASSERT_TRUE(fd >= 0);
close(fd);
TEST_ASSERT_TRUE(xcecd_owned_file_exists(g_path));
}
int main(void)
{
/* unbuffered, so a hang in the FIFO test still shows earlier results */
setvbuf(stdout, NULL, _IONBF, 0);
UNITY_BEGIN();
RUN_TEST(test_open_creates_file);
RUN_TEST(test_open_appends_to_own_existing_file);
RUN_TEST(test_open_replaces_planted_symlink);
RUN_TEST(test_open_replaces_planted_dangling_symlink);
RUN_TEST(test_open_replaces_planted_hardlink);
RUN_TEST(test_open_does_not_block_on_planted_fifo);
RUN_TEST(test_flag_exists_only_for_own_regular_file);
return UNITY_END();
}