log/batch: refuse planted symlinks at --log-file and --write/read-batch

- log: open the --log-file / daemon log file through
   safe_open_no_attacker_symlinks() so a planted symlink can't redirect the
   privileged log write;
 - batch: open the --write-batch/--read-batch files the same way (+ an
   S_ISREG check), and restore O_BINARY / an O_CLOEXEC fallback on the batch
   opens.

Tests: log-file-symlink, batch-file-symlink.
(cherry picked from commit 2b2435f6b8fea3eb282b1e6ad405f01d0648708b)
This commit is contained in:
Andrew Tridgell committed 2026-06-15 15:26:15 +10:00
1 parent 16bf029373
commit 62bf55fcb7
3 files changed
+35 -4

No files matched your search

+21 -3
View File
@@ -224,27 +224,45 @@ static void write_filter_rules(int fd)
/* This sets batch_fd and (for --write-batch) batch_sh_fd. */
void open_batch_files(void)
{
/* --write-batch/--read-batch are operator-supplied; a planted symlink
* could truncate+overwrite an arbitrary file (write side) or stream
* attacker bytes into the protocol parser (read side). Refuse symlinks
* not owned by uid 0 or our euid anywhere in the path. */
if (write_batch) {
char filename[MAXPATHLEN];
stringjoin(filename, sizeof filename, batch_name, ".sh", NULL);
batch_sh_fd = do_open(filename, O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR | S_IXUSR);
batch_sh_fd = safe_open_no_attacker_symlinks(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, S_IRUSR | S_IWUSR | S_IXUSR);
if (batch_sh_fd < 0) {
rsyserr(FERROR, errno, "Batch file %s open error", full_fname(filename));
exit_cleanup(RERR_FILESELECT);
}
batch_fd = do_open(batch_name, O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR);
/* O_BINARY: the batch stream is binary protocol data; without it
* Cygwin et al apply CRLF translation and corrupt it. Unlike
* do_open(), safe_open_no_attacker_symlinks passes flags verbatim. */
batch_fd = safe_open_no_attacker_symlinks(batch_name, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, S_IRUSR | S_IWUSR);
} else if (strcmp(batch_name, "-") == 0)
batch_fd = STDIN_FILENO;
else
batch_fd = do_open(batch_name, O_RDONLY, S_IRUSR | S_IWUSR);
batch_fd = safe_open_no_attacker_symlinks(batch_name, O_RDONLY | O_BINARY, S_IRUSR | S_IWUSR);
if (batch_fd < 0) {
rsyserr(FERROR, errno, "Batch file %s open error", full_fname(batch_name));
exit_cleanup(RERR_FILEIO);
}
/* --read-batch: the file's bytes drive the protocol parser, so refuse
* non-regular files (FIFO, device, socket) at the batch path. */
if (!write_batch && batch_fd != STDIN_FILENO) {
STRUCT_STAT st;
if (do_fstat(batch_fd, &st) == 0 && !S_ISREG(st.st_mode)) {
rprintf(FERROR, "Batch file %s is not a regular file\n",
full_fname(batch_name));
exit_cleanup(RERR_FILEIO);
}
}
}
/* This routine tries to write out an equivalent --read-batch command
+9 -1
View File
@@ -153,7 +153,15 @@ static void syslog_init()
static void logfile_open(void)
{
mode_t old_umask = umask(022 | orig_umask);
logfile_fp = fopen(logfile_name, "a");
/* --log-file/`log file =` are operator-supplied paths that may transit
* attacker-writable dirs; a planted symlink could redirect root's log
* into e.g. /root/.ssh/authorized_keys. Refuse symlinks not owned by
* uid 0 or our euid. */
int fd = safe_open_no_attacker_symlinks(logfile_name,
O_WRONLY | O_APPEND | O_CREAT, 0644);
logfile_fp = fd >= 0 ? fdopen(fd, "a") : NULL;
if (!logfile_fp && fd >= 0)
close(fd);
umask(old_umask);
if (!logfile_fp) {
int fopen_errno = errno;
+5
View File
@@ -300,6 +300,11 @@ int make_path(char *fname, int flags)
int safe_open_no_attacker_symlinks(const char *path, int flags, mode_t mode)
{
#if defined AT_FDCWD && defined O_NOFOLLOW
/* O_CLOEXEC predates some still-supported targets; mirror rand_bytes()'s
* fallback in syscall.c so a build without it still compiles. */
#ifndef O_CLOEXEC
#define O_CLOEXEC 0
#endif
if (!path || !*path) {
errno = EINVAL;
return -1;