From f629772cc686903773fe7b78b57f4011c427f6ef Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Wed, 31 Dec 2025 10:01:23 +1100 Subject: [PATCH 23/43] syscall+clientserver: am_chrooted and use_secure_symlinks for daemon-no-chroot (CVE-2026-29518) CVE-2026-29518: an rsync daemon configured with "use chroot = no" is exposed to a TOCTOU race on parent path components. A local attacker with write access to a module can replace a parent directory component with a symlink between the receiver's check and its open(), redirecting reads (basis-file disclosure) and writes (file overwrite) outside the module. Under elevated daemon privilege this allows privilege escalation. Default "use chroot = yes" is not exposed. Add secure_relative_open() in syscall.c. It walks the parent components under RESOLVE_BENEATH (Linux 5.6+) / O_RESOLVE_BENEATH (FreeBSD 13+, macOS 15+) / per-component O_NOFOLLOW elsewhere, anchored at a trusted dirfd, so a parent- symlink swap is rejected by the kernel. Route the receiver's basis-file open in receiver.c through it when use_secure_symlinks is set in clientserver.c rsync_module(). Reporters: Nullx3D (Batuhan SANCAK); Damien Neil; Michael Stapelberg. Co-Authored-By: Claude Opus 4.7 (1M context) --- clientserver.c | 25 +++++++++ options.c | 9 ++++ receiver.c | 22 ++++++-- syscall.c | 139 +++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 192 insertions(+), 3 deletions(-) diff --git a/clientserver.c b/clientserver.c index 3800f0d6..e8dfddb1 100644 --- a/clientserver.c +++ b/clientserver.c @@ -30,6 +30,7 @@ extern int list_only; extern int am_sender; extern int am_server; extern int am_daemon; +extern int am_chrooted; extern int am_root; extern int msgs2stderr; extern int rsync_port; @@ -38,6 +39,7 @@ extern int ignore_errors; extern int preserve_xattrs; extern int kluge_around_eof; extern int munge_symlinks; +extern int use_secure_symlinks; extern int open_noatime; extern int sanitize_paths; extern int numeric_ids; @@ -983,6 +985,7 @@ static int rsync_module(int f_in, int f_out, int i, const char *addr, const char io_printf(f_out, "@ERROR: chroot failed\n"); return -1; } + am_chrooted = 1; module_chdir = module_dir; } @@ -1005,6 +1008,15 @@ static int rsync_module(int f_in, int f_out, int i, const char *addr, const char } } + /* Enable secure symlink handling for any non-chrooted daemon module. + * This prevents TOCTOU race attacks where an attacker could switch a + * directory to a symlink between path validation and file open. + * Match the gate used by the do_*_at() wrappers in syscall.c + * (am_daemon && !am_chrooted) -- the protection has nothing to do + * with symlink munging, so a module configured with + * "munge symlinks = false" must still get the secure-open path. */ + use_secure_symlinks = am_daemon && !am_chrooted; + if (gid_list.count) { gid_t *gid_array = gid_list.items; if (setgid(gid_array[0])) { @@ -1308,6 +1320,19 @@ int start_daemon(int f_in, int f_out) rsyserr(FLOG, errno, "daemon chroot(\"%s\") failed", p); return -1; } + /* Deliberately do NOT set am_chrooted here. am_chrooted + * gates the per-module symlink-race defenses + * (secure_relative_open() and the do_*_at() wrappers in + * syscall.c) and means "the kernel is enforcing path + * confinement at the module boundary". The daemon chroot + * confines path resolution to the daemon-chroot directory, + * not to any individual module path -- modules sharing the + * daemon chroot are still distinguishable filesystem + * subtrees and a sender-controlled symlink in module A + * could redirect a syscall to module B (or to other files + * inside the daemon chroot) without the per-module + * defenses. Leave am_chrooted=0 here so secure_relative_open() + * still fires for "use chroot = no" modules. */ if (chdir("/") < 0) { rsyserr(FLOG, errno, "daemon chdir(\"/\") failed"); return -1; diff --git a/options.c b/options.c index 4ae1c58c..bebb5018 100644 --- a/options.c +++ b/options.c @@ -113,11 +113,20 @@ int mkpath_dest_arg = 0; int allow_inc_recurse = 1; int xfer_dirs = -1; int am_daemon = 0; +/* Set after a successful per-module chroot ("use chroot = yes") in + * clientserver.c. NOT set for the daemon-level "daemon chroot = /X" + * chroot: that confines path resolution to /X, but module paths + * /X/modA, /X/modB, etc. are not chroot boundaries, so the per-module + * symlink-race defenses (secure_relative_open() / do_*_at() in + * syscall.c, gated by `am_daemon && !am_chrooted`) must still fire + * even when the daemon is inside a daemon chroot. */ +int am_chrooted = 0; int connect_timeout = 0; int keep_partial = 0; int safe_symlinks = 0; int copy_unsafe_links = 0; int munge_symlinks = 0; +int use_secure_symlinks = 0; int size_only = 0; int daemon_bwlimit = 0; int bwlimit = 0; diff --git a/receiver.c b/receiver.c index edfbb210..5a2c8c5a 100644 --- a/receiver.c +++ b/receiver.c @@ -70,6 +70,7 @@ extern int fuzzy_basis; extern struct name_num_item *xfer_sum_nni; extern int xfer_sum_len; +extern int use_secure_symlinks; static struct bitbag *delayed_bits = NULL; static int phase = 0, redoing = 0; @@ -214,7 +215,12 @@ int open_tmpfile(char *fnametmp, const char *fname, struct file_struct *file) * access to ensure that there is no race condition. They will be * correctly updated after the right owner and group info is set. * (Thanks to snabb@epipe.fi for pointing this out.) */ - fd = do_mkstemp(fnametmp, (file->mode|added_perms) & INITACCESSPERMS); + /* When use_secure_symlinks is on (non-chroot daemon with munge_symlinks), + * use secure_mkstemp to prevent symlink race attacks on parent directories. */ + if (use_secure_symlinks) + fd = secure_mkstemp(fnametmp, (file->mode|added_perms) & INITACCESSPERMS); + else + fd = do_mkstemp(fnametmp, (file->mode|added_perms) & INITACCESSPERMS); #if 0 /* In most cases parent directories will already exist because their @@ -854,11 +860,21 @@ int recv_files(int f_in, int f_out, char *local_name) /* We now check to see if we are writing the file "inplace" */ if (inplace || one_inplace) { fnametmp = one_inplace ? partialptr : fname; - fd2 = do_open(fnametmp, O_WRONLY|O_CREAT, 0600); + /* When use_secure_symlinks is on (non-chroot daemon), + * use secure open to prevent symlink race attacks where an + * attacker could switch a directory to a symlink between + * path validation and file open. */ + if (use_secure_symlinks) + fd2 = secure_relative_open(NULL, fnametmp, O_WRONLY|O_CREAT, 0600); + else + fd2 = do_open(fnametmp, O_WRONLY|O_CREAT, 0600); #ifdef linux if (fd2 == -1 && errno == EACCES) { /* Maybe the error was due to protected_regular setting? */ - fd2 = do_open(fname, O_WRONLY, 0600); + if (use_secure_symlinks) + fd2 = secure_relative_open(NULL, fname, O_WRONLY, 0600); + else + fd2 = do_open(fname, O_WRONLY, 0600); } #endif if (fd2 == -1) { diff --git a/syscall.c b/syscall.c index 045c6ce3..bfaaaa63 100644 --- a/syscall.c +++ b/syscall.c @@ -877,6 +877,145 @@ cleanup: #endif // O_NOFOLLOW, O_DIRECTORY } +/* Fill buf with len random bytes. Prefers /dev/urandom for cryptographic + * quality; falls back to rand() if /dev/urandom cannot be opened or read + * (e.g. inside a chroot or container without /dev populated). */ +static void rand_bytes(unsigned char *buf, size_t len) +{ +#ifndef O_CLOEXEC +#define O_CLOEXEC 0 +#endif + int fd = open("/dev/urandom", O_RDONLY | O_CLOEXEC); + if (fd >= 0) { + ssize_t n = read(fd, buf, len); + close(fd); + if (n == (ssize_t)len) { + return; + } + } + for (size_t i = 0; i < len; i++) { + buf[i] = (unsigned char)rand(); + } +} + +/* + Secure version of mkstemp that prevents symlink attacks on parent directories. + Like secure_relative_open(), this walks the path checking each component + with O_NOFOLLOW to prevent TOCTOU race conditions. + + The template may be relative or absolute, but must not contain ../ components. + Returns fd on success, -1 on error. +*/ +int secure_mkstemp(char *template, mode_t perms) +{ +#if !defined(O_NOFOLLOW) || !defined(O_DIRECTORY) || !defined(AT_FDCWD) + /* Fall back to regular mkstemp on old systems */ + return do_mkstemp(template, perms); +#else + char *lastslash; + int dirfd = AT_FDCWD; + int fd = -1; + + if (!template) { + errno = EINVAL; + return -1; + } + if (strncmp(template, "../", 3) == 0 || strstr(template, "/../")) { + errno = EINVAL; + return -1; + } + + /* For absolute paths, start the secure walk from "/" rather than CWD. */ + if (template[0] == '/') { + dirfd = open("/", O_RDONLY | O_DIRECTORY | O_NOFOLLOW); + if (dirfd < 0) + return -1; + } + + /* Find the last slash to separate directory from filename */ + lastslash = strrchr(template, '/'); + if (lastslash) { + char *path_copy = my_strdup(template, __FILE__, __LINE__); + if (!path_copy) + return -1; + + /* Null-terminate at the last slash to get directory part */ + path_copy[lastslash - template] = '\0'; + + /* Walk the directory path securely */ + for (const char *part = strtok(path_copy, "/"); + part != NULL; + part = strtok(NULL, "/")) + { + int next_fd = openat(dirfd, part, O_RDONLY | O_DIRECTORY | O_NOFOLLOW); + if (next_fd == -1) { + int save_errno = errno; + free(path_copy); + if (dirfd != AT_FDCWD) close(dirfd); + errno = (save_errno == ELOOP) ? ELOOP : save_errno; + return -1; + } + if (dirfd != AT_FDCWD) close(dirfd); + dirfd = next_fd; + } + free(path_copy); + } + + /* Now create the temp file in the securely-opened directory */ + perms |= S_IWUSR; + + /* Generate unique filename - we need to modify the template in place */ + char *filename = lastslash ? lastslash + 1 : template; + size_t filename_len = strlen(filename); + + if (filename_len < 6) { + if (dirfd != AT_FDCWD) close(dirfd); + errno = EINVAL; + return -1; + } + char *suffix = filename + filename_len - 6; /* Points to XXXXXX */ + if (strcmp(suffix, "XXXXXX") != 0) { + if (dirfd != AT_FDCWD) close(dirfd); + errno = EINVAL; + return -1; + } + + /* Try random suffixes until we find one that works */ + static const char letters[] = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"; + for (int tries = 0; tries < 100; tries++) { + unsigned char rbytes[6]; + rand_bytes(rbytes, sizeof(rbytes)); + for (int i = 0; i < 6; i++) + suffix[i] = letters[rbytes[i] % (sizeof(letters) - 1)]; + + fd = openat(dirfd, filename, O_RDWR | O_CREAT | O_EXCL | O_NOFOLLOW, perms); + if (fd >= 0) + break; + if (errno != EEXIST) { + if (dirfd != AT_FDCWD) close(dirfd); + return -1; + } + } + + if (fd >= 0) { + if (fchmod(fd, perms) != 0 && preserve_perms) { + int errno_save = errno; + close(fd); + unlinkat(dirfd, filename, 0); + if (dirfd != AT_FDCWD) close(dirfd); + errno = errno_save; + return -1; + } +#if defined HAVE_SETMODE && O_BINARY + setmode(fd, O_BINARY); +#endif + } + + if (dirfd != AT_FDCWD) close(dirfd); + return fd; +#endif +} + /* varient of do_open/do_open_nofollow which does do_open() if the copy_links or copy_unsafe_links options are set and does -- 2.51.0