diff --git a/batch.c b/batch.c index accc4c6e..a0845f67 100644 --- a/batch.c +++ b/batch.c @@ -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 diff --git a/log.c b/log.c index b948f16a..7d17d8fa 100644 --- a/log.c +++ b/log.c @@ -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; diff --git a/util1.c b/util1.c index 6c0fbf5f..fdd5f0cb 100644 --- a/util1.c +++ b/util1.c @@ -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;