diff --git a/Makefile.in b/Makefile.in index c2fe775..699d995 100644 --- a/Makefile.in +++ b/Makefile.in @@ -57,13 +57,13 @@ TLS_OBJ = tls.o syscall.o util2.o t_stub.o lib/compat.o lib/snprintf.o lib/perms # Programs we must have to run the test cases CHECK_PROGS = rsync$(EXEEXT) tls$(EXEEXT) getgroups$(EXEEXT) getfsdev$(EXEEXT) \ - testrun$(EXEEXT) trimslash$(EXEEXT) t_unsafe$(EXEEXT) wildtest$(EXEEXT) \ - simdtest$(EXEEXT) + testrun$(EXEEXT) trimslash$(EXEEXT) t_unsafe$(EXEEXT) t_chmod_secure$(EXEEXT) \ + t_secure_relpath$(EXEEXT) wildtest$(EXEEXT) simdtest$(EXEEXT) CHECK_SYMLINKS = testsuite/chown-fake.test testsuite/devices-fake.test testsuite/xattrs-hlink.test # Objects for CHECK_PROGS to clean -CHECK_OBJS=tls.o testrun.o getgroups.o getfsdev.o t_stub.o t_unsafe.o trimslash.o wildtest.o +CHECK_OBJS=tls.o testrun.o getgroups.o getfsdev.o t_stub.o t_unsafe.o t_chmod_secure.o t_secure_relpath.o trimslash.o wildtest.o # note that the -I. is needed to handle config.h when using VPATH .c.o: @@ -179,6 +179,14 @@ T_UNSAFE_OBJ = t_unsafe.o syscall.o util1.o util2.o t_stub.o lib/compat.o lib/sn t_unsafe$(EXEEXT): $(T_UNSAFE_OBJ) $(CC) $(CFLAGS) $(LDFLAGS) -o $@ $(T_UNSAFE_OBJ) $(LIBS) +T_CHMOD_SECURE_OBJ = t_chmod_secure.o syscall.o util1.o util2.o t_stub.o lib/compat.o lib/snprintf.o lib/wildmatch.o lib/permstring.o +t_chmod_secure$(EXEEXT): $(T_CHMOD_SECURE_OBJ) + $(CC) $(CFLAGS) $(LDFLAGS) -o $@ $(T_CHMOD_SECURE_OBJ) $(LIBS) + +T_SECURE_RELPATH_OBJ = t_secure_relpath.o syscall.o util1.o util2.o t_stub.o lib/compat.o lib/snprintf.o lib/wildmatch.o lib/permstring.o +t_secure_relpath$(EXEEXT): $(T_SECURE_RELPATH_OBJ) + $(CC) $(CFLAGS) $(LDFLAGS) -o $@ $(T_SECURE_RELPATH_OBJ) $(LIBS) + .PHONY: conf conf: configure.sh config.h.in diff --git a/NEWS.md b/NEWS.md index 3c22ac5..da09538 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,192 @@ +# NEWS for rsync 3.4.3 (20 May 2026) + +## Changes in this version: + +### SECURITY FIXES: + +Six CVEs are fixed in this release. All six are assigned by +VulnCheck as CNA. Affected versions are 3.4.2 and earlier in every +case. Three of the six (CVE-2026-29518, CVE-2026-43617, +CVE-2026-43619) require non-default daemon configuration to reach: +the first and third need `use chroot = no` for a module, the second +needs `daemon chroot = ...` set in rsyncd.conf. Two (CVE-2026-43618, +CVE-2026-43620) are reachable from a normal pull or a normal +authenticated daemon connection. The sixth (CVE-2026-45232) is +reachable only when `RSYNC_PROXY` is set and the proxy (or a MITM) +returns a pathological response. Many thanks to the external +researchers who reported these issues. + +- CVE-2026-29518 (CVSS v4.0 7.3, HIGH): TOCTOU symlink race condition + allowing local privilege escalation in daemon mode without chroot. + An rsync daemon configured with "use chroot = no" was exposed to a + time-of-check / time-of-use race on parent path components: a local + attacker with write access to a module could 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. Default "use chroot = yes" is + not exposed. `secure_relative_open()` (added in 3.4.0 for + CVE-2024-12086) was previously unused in the daemon-no-chroot + case; the fix enables it there and reroutes the sender's + read-path opens through it. Reported by Nullx3D (Batuhan Sancak), + Damien Neil and Michael Stapelberg. + +- CVE-2026-43617 (CVSS v3.1 4.8, MEDIUM): Hostname/ACL bypass on an + rsync daemon configured with `daemon chroot = /X` in rsyncd.conf + when the chroot tree lacks DNS resolution support. The + reverse-DNS lookup of the connecting client was performed *after* + the daemon chroot had been entered; if /X did not contain the + libc resolver fixtures (`/etc/resolv.conf`, `/etc/nsswitch.conf`, + `/etc/hosts`, NSS service modules) the lookup failed and the + connecting hostname was set to "UNKNOWN", causing hostname-based + deny rules to silently fail open. IP-based ACLs are unaffected. + The per-module `use chroot` setting is unrelated to this issue. + The fix performs the lookup before entering the daemon chroot. + Reported by MegaManSec. + +- CVE-2026-43618 (CVSS v3.1 8.1, HIGH): Integer overflow in the + compressed-token decoder enabling remote memory disclosure to an + authenticated daemon peer. The receiver accumulated a 32-bit + signed counter without overflow checking; a malicious sender could + trigger an overflow that, with careful manipulation, leaked process + memory contents to the attacker -- environment variables, + passwords, heap and library pointers -- significantly weakening + ASLR. The fix bounds the counter and adds wire-input validation in + several adjacent places (defence-in-depth). Workaround for older + releases: `refuse options = compress` in rsyncd.conf. Reported by + Omar Elsayed. + +- CVE-2026-43619 (CVSS v3.1 6.3, MEDIUM): Symlink races on path-based + system calls in "use chroot = no" daemon mode (generalisation of + CVE-2026-29518). Earlier fixes for symlink races on the receiver's + open() call missed the same race class on every other path-based + system call: chmod, lchown, utimes, rename, unlink, mkdir, symlink, + mknod, link, rmdir and lstat. The fix routes each affected + path-based syscall through a parent dirfd opened under + RESOLVE_BENEATH-equivalent kernel-enforced confinement (openat2 on + Linux 5.6+, O_RESOLVE_BENEATH on FreeBSD 13+ and macOS 15+, + per-component O_NOFOLLOW walk elsewhere). Default "use chroot = + yes" is not exposed. Reported by Andrew Tridgell as a follow-on + audit of CVE-2026-29518. + +- CVE-2026-43620 (CVSS v3.1 6.5, MEDIUM): Out-of-bounds read in the + receiver's recv_files() enabling remote denial-of-service of any + client pulling from a malicious server (incomplete fix of commit + 797e17f). The earlier parent_ndx<0 guard added to send_files() was + not applied to the visually-identical block in recv_files(). A + malicious rsync server can drive any connecting client into a + deterministic SIGSEGV by setting CF_INC_RECURSE in the + compatibility flags and sending a crafted file list and transfer + record. inc_recurse is the protocol-30+ default, so no special + options are required on the victim. Workaround for older + releases: `--no-inc-recursive` on the client. Reported by Pratham + Gupta. + +- CVE-2026-45232 (CVSS v3.1 3.1, LOW): Off-by-one out-of-bounds stack + write in the rsync client's HTTP CONNECT proxy handler + (`establish_proxy_connection()` in `socket.c`). After issuing the + CONNECT request, rsync read the proxy's first response line one + byte at a time into a 1024-byte stack buffer with the bound + `cp < &buffer[sizeof buffer - 1]`. If the proxy (or a MITM in + front of it) returned 1023+ bytes on that first line without a + newline terminator, `cp` exited the loop pointing at a buffer slot + the loop never wrote, leaving `*cp` holding stale stack data from + the earlier `snprintf()` of the outgoing CONNECT request. The + post-loop logic then wrote a single `\0` one byte past the end of + the buffer on the stack. Reach is client-side only, and only when + `RSYNC_PROXY` is set so rsync tunnels an `rsync://` connection + through an HTTP CONNECT proxy. The written byte is always `\0` + and the offset is fixed by the buffer size, not attacker-chosen, + so this is not an arbitrary-write primitive: practical impact is + corruption of one adjacent stack byte and possible later + misbehaviour or crash. The fix detects the "buffer filled without + finding `\n`" case explicitly by position and refuses the response + with "proxy response line too long". Reported by Aisle Research + via Michal Ruprich (rsync-3.4.1-2.el10 QE). + +In addition to the six CVE fixes, this release adds defence-in-depth +hardening on several adjacent paths: bounded wire-supplied counts and +lengths in flist/io/acls/xattrs, a guard against length underflow in +cumulative `snprintf()` callers, a parent block-index bounds check on +the receiver, a NULL check in `read_delay_line()`, a lower ceiling on +`MAX_WIRE_DEL_STAT` to avoid signed-int overflow in the +`read_del_stats()` accumulator, rejection of hyphen-prefixed +remote-shell hostnames (defence-in-depth against argv-injection in +tooling that forwards untrusted input into the hostspec position; +reported by Aisle Research via Michal Ruprich), and a NULL-check on +`localtime_r()` in `timestring()` to keep a malicious server from +crashing the client by advertising a file with an out-of-range +modtime. + +### BUG FIXES: + +- Fixed a regression introduced by the 3.4.0 secure_relative_open() + CVE fix where legitimate directory symlinks on the receiver side + (e.g. when using `-K` / `--copy-dirlinks`) caused "failed + verification -- update discarded" errors on delta transfers. The + old code rejected every symlink in the path with a per-component + `O_NOFOLLOW` walk; the receiver now uses kernel-enforced "stay + below dirfd" path resolution where available. Fixes #715. + +### PORTABILITY / BUILD: + +- secure_relative_open() now uses `openat2(RESOLVE_BENEATH | + RESOLVE_NO_MAGICLINKS)` on Linux 5.6+, and `openat()` with + `O_RESOLVE_BENEATH` on FreeBSD 13+ and macOS 15+ (Sequoia) / + iOS 18+. The kernel rejects ".." escapes, absolute symlinks, and + symlinks whose target lies outside the starting directory, while + still following symlinks that resolve within it -- the same + trade-off that fixes the issue #715 regression without weakening + the original CVE protection. Other platforms (Solaris, OpenBSD, + NetBSD, Cygwin) retain the previous per-component `O_NOFOLLOW` + walk; on those platforms the issue #715 regression remains + visible. + +- testsuite/xattrs: ignore `SUNWattr_*` in the Solaris `xls` + helper. + +### DEVELOPER RELATED: + +- Added testsuite/symlink-dirlink-basis.test (taken from PR #864 + by Samuel Henrique) covering the issue #715 regression and + several edge cases (`--backup`, `--inplace`, `--partial-dir` + with protocol < 29, top-level files). The test skips on + platforms without a RESOLVE_BENEATH equivalent. + +- Added regression tests for the new security fixes: + `chmod-symlink-race.test`, `chdir-symlink-race.test`, + `bare-do-open-symlink-race.test`, `alt-dest-symlink-race.test`, + `copy-dest-source-symlink.test`, `sender-flist-symlink-leak.test`, + `secure-relpath-validation.test`, `daemon-chroot-acl.test` and + `daemon-refuse-compress.test`. The symlink-race tests skip on + Cygwin, Solaris, OpenBSD and NetBSD (no RESOLVE_BENEATH + equivalent on those platforms). + +- runtests.py now errors early with a clear message when any of + the test helper programs (`tls`, `trimslash`, `t_unsafe`, + `t_chmod_secure`, `t_secure_relpath`, `wildtest`, `getgroups`, + `getfsdev`) are missing, instead of letting many tests fail with + confusing "not found" errors. + +- Added OpenBSD and NetBSD CI jobs that run `make check` on those + platforms. + +- Added Ubuntu 22.04 and AlmaLinux 8 CI workflows so future + backports to the two mainstream LTS families build and test on + the same CI surface as trunk. + +- testsuite/protected-regular.test now runs unprivileged via + `unshare` with user-namespace UID mapping, falling back to skip + if `unshare`/`uidmap` is not available; previously it required + real root. + +- Added `symlink-dirlink-basis` to the Cygwin CI's expected-skipped + list. + +- Removed the old release system (replaced by the new release + script in 3.4.2). + +------------------------------------------------------------------------------ + # NEWS for rsync 3.4.2 (28 Apr 2026) ## Changes in this version: @@ -4980,6 +5169,7 @@ to develop and test fixes. | RELEASE DATE | VER. | DATE OF COMMIT\* | PROTOCOL | |--------------|--------|------------------|-------------| +| 20 May 2026 | 3.4.3 | | 32 | | 28 Apr 2026 | 3.4.2 | | 32 | | 16 Jan 2025 | 3.4.1 | | 32 | | 15 Jan 2025 | 3.4.0 | 15 Jan 2025 | 32 | diff --git a/aclocal.m4 b/aclocal.m4 index 0432168..8eb22a4 100644 --- a/aclocal.m4 +++ b/aclocal.m4 @@ -1,6 +1,6 @@ -# generated automatically by aclocal 1.16.5 -*- Autoconf -*- +# generated automatically by aclocal 1.17 -*- Autoconf -*- -# Copyright (C) 1996-2021 Free Software Foundation, Inc. +# Copyright (C) 1996-2024 Free Software Foundation, Inc. # This file is free software; the Free Software Foundation # gives unlimited permission to copy and/or distribute it, diff --git a/acls.c b/acls.c index 4d67ff4..c60a708 100644 --- a/acls.c +++ b/acls.c @@ -697,7 +697,7 @@ static uint32 recv_acl_access(int f, uchar *name_follows_ptr) static uchar recv_ida_entries(int f, ida_entries *ent) { uchar computed_mask_bits = 0; - int i, count = read_varint(f); + int i, count = read_varint_bounded(f, 0, MAX_WIRE_ACL_COUNT, "ACL count"); ent->idas = count ? new_array(id_access, count) : NULL; ent->count = count; diff --git a/backup.c b/backup.c index 686cb29..ae8cb49 100644 --- a/backup.c +++ b/backup.c @@ -39,7 +39,7 @@ static int validate_backup_dir(void) { STRUCT_STAT st; - if (do_lstat(backup_dir_buf, &st) < 0) { + if (do_lstat_at(backup_dir_buf, &st) < 0) { if (errno == ENOENT) return 0; rsyserr(FERROR, errno, "backup lstat %s failed", backup_dir_buf); @@ -98,7 +98,7 @@ static BOOL copy_valid_path(const char *fname) for ( ; b; name = b + 1, b = strchr(name, '/')) { *b = '\0'; - while (do_mkdir(backup_dir_buf, ACCESSPERMS) < 0) { + while (do_mkdir_at(backup_dir_buf, ACCESSPERMS) < 0) { if (errno == EEXIST) { val = validate_backup_dir(); if (val > 0) @@ -197,7 +197,7 @@ static inline int link_or_rename(const char *from, const char *to, if (IS_SPECIAL(stp->st_mode) || IS_DEVICE(stp->st_mode)) return 0; /* Use copy code. */ #endif - if (do_link(from, to) == 0) { + if (do_link_at(from, to) == 0) { if (DEBUG_GTE(BACKUP, 1)) rprintf(FINFO, "make_backup: HLINK %s successful.\n", from); return 2; @@ -207,7 +207,7 @@ static inline int link_or_rename(const char *from, const char *to, return 0; } #endif - if (do_rename(from, to) == 0) { + if (do_rename_at(from, to) == 0) { if (stp->st_nlink > 1 && !S_ISDIR(stp->st_mode)) { /* If someone has hard-linked the file into the backup * dir, rename() might return success but do nothing! */ @@ -246,7 +246,7 @@ int make_backup(const char *fname, BOOL prefer_rename) goto success; if (errno == EEXIST || errno == EISDIR) { STRUCT_STAT bakst; - if (do_lstat(buf, &bakst) == 0) { + if (do_lstat_at(buf, &bakst) == 0) { int flags = get_del_for_flag(bakst.st_mode) | DEL_FOR_BACKUP | DEL_RECURSE; if (delete_item(buf, bakst.st_mode, flags) != 0) return 0; @@ -277,7 +277,7 @@ int make_backup(const char *fname, BOOL prefer_rename) /* Check to see if this is a device file, or link */ if ((am_root && preserve_devices && IS_DEVICE(file->mode)) || (preserve_specials && IS_SPECIAL(file->mode))) { - if (do_mknod(buf, file->mode, sx.st.st_rdev) < 0) + if (do_mknod_at(buf, file->mode, sx.st.st_rdev) < 0) rsyserr(FERROR, errno, "mknod %s failed", full_fname(buf)); else if (DEBUG_GTE(BACKUP, 1)) rprintf(FINFO, "make_backup: DEVICE %s successful.\n", fname); @@ -294,7 +294,7 @@ int make_backup(const char *fname, BOOL prefer_rename) } ret = 2; } else { - if (do_symlink(sl, buf) < 0) + if (do_symlink_at(sl, buf) < 0) rsyserr(FERROR, errno, "link %s -> \"%s\"", full_fname(buf), sl); else if (DEBUG_GTE(BACKUP, 1)) rprintf(FINFO, "make_backup: SYMLINK %s successful.\n", fname); diff --git a/cleanup.c b/cleanup.c index 40d26ba..0493fbb 100644 --- a/cleanup.c +++ b/cleanup.c @@ -198,7 +198,7 @@ NORETURN void _exit_cleanup(int code, const char *file, int line) switch_step++; if (cleanup_fname) - do_unlink(cleanup_fname); + do_unlink_at(cleanup_fname); if (exit_code) kill_all(SIGUSR1); if (cleanup_pid && cleanup_pid == getpid()) { diff --git a/clientserver.c b/clientserver.c index 3800f0d..14daba3 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])) { @@ -1300,6 +1312,28 @@ int start_daemon(int f_in, int f_out) if (lp_proxy_protocol() && !read_proxy_protocol_header(f_in)) return -1; + /* Do reverse DNS lookup before chroot/setuid. The result is cached, + * so the later client_name() call will use this cached value. This + * ensures hostname-based ACLs work even when DNS is unavailable + * after chroot. + * + * "reverse lookup" can be set globally OR per-module, so we also + * scan each module: a deployment with "reverse lookup = no" in the + * global section but "reverse lookup = yes" in a specific module + * still triggers a post-chroot lookup at access-check time + * (rsync_module() in this file), which would also fail in the + * chroot and turn hostname-based deny rules into silent bypasses. */ + { + int need_reverse = lp_reverse_lookup(-1); + int j, num_modules = lp_num_modules(); + for (j = 0; !need_reverse && j < num_modules; j++) { + if (lp_reverse_lookup(j)) + need_reverse = 1; + } + if (need_reverse) + (void)client_name(client_addr(f_in)); + } + p = lp_daemon_chroot(); if (*p) { log_init(0); /* Make use we've initialized syslog before chrooting. */ @@ -1308,6 +1342,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/debian/changelog b/debian/changelog index 52b9936..41b95ad 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,16 @@ +rsync (3.4.3+ds1-2) unstable; urgency=high + + * d/t/upstream-tests: Build t_chmod_secure and t_secure_relpath + + -- Samuel Henrique Tue, 19 May 2026 18:23:33 -0700 + +rsync (3.4.3+ds1-1) unstable; urgency=high + + * New upstream version 3.4.3+ds1 + * d/p/syscall_use_openat2...: Drop merged patch + + -- Samuel Henrique Tue, 19 May 2026 17:47:34 -0700 + rsync (3.4.2+ds1-2) unstable; urgency=medium * d/p/syscall_use_openat2...: New patch to fix symlink handling on the diff --git a/debian/patches/series b/debian/patches/series index 6902014..e5a8535 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -2,4 +2,3 @@ disable_reconfigure_req.diff skip_devices_test_non_linux.patch env_shebang.patch fix_rrsync_man_generation.patch -syscall_use_openat2_RESOLVE_BENEATH_on_Linux_for_secure_relative_open.patch diff --git a/debian/patches/syscall_use_openat2_RESOLVE_BENEATH_on_Linux_for_secure_relative_open.patch b/debian/patches/syscall_use_openat2_RESOLVE_BENEATH_on_Linux_for_secure_relative_open.patch deleted file mode 100644 index e514ba3..0000000 --- a/debian/patches/syscall_use_openat2_RESOLVE_BENEATH_on_Linux_for_secure_relative_open.patch +++ /dev/null @@ -1,386 +0,0 @@ -From 4fa7156ccdb2ad34b034d18fe2fd6cd79adef8a1 Mon Sep 17 00:00:00 2001 -From: Andrew Tridgell -Date: Thu, 30 Apr 2026 08:39:22 +1000 -Subject: [PATCH] syscall: use openat2(RESOLVE_BENEATH) on Linux for - secure_relative_open - -The CVE fix in commit c35e283 made secure_relative_open() walk every -component of relpath with O_NOFOLLOW. That blocks every symlink in the -path, which is stricter than the threat model required: legitimate -directory symlinks within the destination tree (e.g. when using -K / ---copy-dirlinks) are also rejected, breaking delta transfers with -"failed verification -- update discarded". See issue #715. - -On Linux 5.6+, openat2(RESOLVE_BENEATH | RESOLVE_NO_MAGICLINKS) gives -us exactly what we want: the kernel rejects any resolution that would -escape the starting directory (via "..", absolute paths, or symlinks -pointing outside dirfd) while still following symlinks that resolve -within it. /proc magic-links are blocked too. - -Use openat2 first; fall back to the existing per-component O_NOFOLLOW -walk on ENOSYS (kernel < 5.6). The lexical "../" checks at the head -of the function are kept as defense in depth. The Linux gate is -plain #ifdef __linux__: the runtime ENOSYS fallback covers the only -case that actually matters (header present + old kernel), and any -Linux build environment without linux/openat2.h will fail with a -clear "no such file" error rather than silently disabling the -protection. - -Verified manually that openat2(RESOLVE_BENEATH) blocks all four -escape patterns (absolute symlink, ../ symlink, lexical .., absolute -path) while allowing direct and within-tree symlinks. The new -testsuite/symlink-dirlink-basis.test (taken from PR #864 by Samuel -Henrique) exercises the issue #715 regression and passes; full -make check passes 47/47. - -Test: testsuite/symlink-dirlink-basis.test (8 scenarios) -Fixes: https://github.com/RsyncProject/rsync/issues/715 - -Co-Authored-By: Claude Opus 4.7 (1M context) ---- - syscall.c | 62 ++++++- - testsuite/symlink-dirlink-basis.test | 247 +++++++++++++++++++++++++++ - 2 files changed, 304 insertions(+), 5 deletions(-) - create mode 100755 testsuite/symlink-dirlink-basis.test - -diff --git a/syscall.c b/syscall.c -index ec0e0708a..66c6d29c7 100644 ---- a/syscall.c -+++ b/syscall.c -@@ -33,6 +33,11 @@ - #include - #endif - -+#ifdef __linux__ -+#include -+#include -+#endif -+ - #include "ifuncs.h" - - extern int dry_run; -@@ -720,12 +725,49 @@ int do_open_nofollow(const char *pathname, int flags) - /* - open a file relative to a base directory. The basedir can be NULL, - in which case the current working directory is used. The relpath -- must be a relative path, and the relpath must not contain any -- elements in the path which follow symlinks (ie. like O_NOFOLLOW, but -- applies to all path components, not just the last component) -- -- The relpath must also not contain any ../ elements in the path -+ must be a relative path. The kernel must guarantee that resolution -+ cannot escape basedir (or the cwd, when basedir is NULL): no ".." -+ jumps above the start, no symlinks pointing outside, no absolute -+ paths, no /proc magic-link tricks. -+ -+ Symlinks *within* basedir are followed normally — earlier rsync -+ versions rejected every symlink with O_NOFOLLOW on each component, -+ which broke legitimate directory symlinks on the receiver side -+ (https://github.com/RsyncProject/rsync/issues/715). The escape -+ prevention is handled by the kernel via openat2(RESOLVE_BENEATH) -+ on Linux 5.6+; older systems fall back to the per-component -+ O_NOFOLLOW walk below. -+ -+ The relpath must also not contain any ../ elements in the path. - */ -+ -+#ifdef __linux__ -+static int secure_relative_open_linux(const char *basedir, const char *relpath, int flags, mode_t mode) -+{ -+ struct open_how how; -+ int dirfd, retfd; -+ -+ memset(&how, 0, sizeof how); -+ how.flags = flags; -+ how.mode = mode; -+ how.resolve = RESOLVE_BENEATH | RESOLVE_NO_MAGICLINKS; -+ -+ if (basedir == NULL) { -+ dirfd = AT_FDCWD; -+ } else { -+ dirfd = openat(AT_FDCWD, basedir, O_RDONLY | O_DIRECTORY); -+ if (dirfd == -1) -+ return -1; -+ } -+ -+ retfd = syscall(SYS_openat2, dirfd, relpath, &how, sizeof how); -+ -+ if (dirfd != AT_FDCWD) -+ close(dirfd); -+ return retfd; -+} -+#endif -+ - int secure_relative_open(const char *basedir, const char *relpath, int flags, mode_t mode) - { - if (!relpath || relpath[0] == '/') { -@@ -739,6 +781,16 @@ int secure_relative_open(const char *basedir, const char *relpath, int flags, mo - return -1; - } - -+#ifdef __linux__ -+ { -+ int fd = secure_relative_open_linux(basedir, relpath, flags, mode); -+ /* ENOSYS = kernel < 5.6 doesn't have the syscall even though -+ * glibc/kernel-headers do; fall through to the portable path. */ -+ if (fd != -1 || errno != ENOSYS) -+ return fd; -+ } -+#endif -+ - #if !defined(O_NOFOLLOW) || !defined(O_DIRECTORY) || !defined(AT_FDCWD) - // really old system, all we can do is live with the risks - if (!basedir) { -diff --git a/testsuite/symlink-dirlink-basis.test b/testsuite/symlink-dirlink-basis.test -new file mode 100755 -index 000000000..9065dd814 ---- /dev/null -+++ b/testsuite/symlink-dirlink-basis.test -@@ -0,0 +1,247 @@ -+#!/bin/sh -+ -+# Test that updating a file through a directory symlink works when using -+# -K (--copy-dirlinks). This is a regression test for: -+# https://github.com/RsyncProject/rsync/issues/715 -+# -+# The CVE fix in commit c35e283 introduced secure_relative_open() which -+# uses O_NOFOLLOW on all path components, breaking legitimate directory -+# symlinks on the receiver side. The fix splits the path into basedir -+# (dirname, symlinks followed) and basename (O_NOFOLLOW) so that -+# directory symlinks are traversed while the final file component is -+# still protected. -+# -+# The regression only manifests when delta matching is triggered (i.e., -+# the sender finds matching blocks in the old file). Small files with -+# completely different content are transferred in full and don't trigger -+# the bug. We use a large file with a small modification to ensure -+# delta transfer is used. -+# -+# In addition to the original regression, this test covers edge cases -+# in the fix itself: -+# - --backup with directory symlinks (finish_transfer pointer identity) -+# - --partial-dir with protocol < 29 (fnamecmp != partialptr guard) -+# - --inplace with directory symlinks (updating_basis_or_equiv check) -+# - Files without a dirname (top-level files, no split needed) -+ -+. "$suitedir/rsync.fns" -+ -+RSYNC_RSH="$scratchdir/src/support/lsh.sh" -+export RSYNC_RSH -+ -+# $HOME is set to $scratchdir by rsync.fns -+# localhost: destination will cd to $HOME (i.e., $scratchdir) -+ -+# Helper: create a large file suitable for delta transfers. -+# ~32KB is large enough for rsync's block matching to find matches. -+make_testfile() { -+ dd if=/dev/urandom of="$1" bs=1024 count=32 2>/dev/null \ -+ || test_fail "failed to create test file $1" -+} -+ -+# Set up source tree -+srcbase="$tmpdir/src" -+ -+###################################################################### -+# Test 1: Basic directory symlink update (the original issue #715) -+###################################################################### -+ -+mkdir -p "$HOME/real-dir" -+ln -s real-dir "$HOME/dir" -+ -+mkdir -p "$srcbase/dir" -+make_testfile "$srcbase/dir/file" -+ -+# First transfer (initial): should create the file through the symlink -+(cd "$srcbase" && $RSYNC -KRlptv --rsync-path="$RSYNC" dir/file localhost:) \ -+ || test_fail "test 1: initial transfer failed" -+ -+if [ ! -f "$HOME/real-dir/file" ]; then -+ test_fail "test 1: initial transfer did not create file through symlink" -+fi -+ -+diff "$srcbase/dir/file" "$HOME/real-dir/file" >/dev/null \ -+ || test_fail "test 1: initial transfer content mismatch" -+ -+# Small modification to trigger delta transfer -+echo "appended update" >> "$srcbase/dir/file" -+sleep 1 -+touch "$srcbase/dir/file" -+ -+# Second transfer (update): was failing with "failed verification" -+(cd "$srcbase" && $RSYNC -KRlptv --rsync-path="$RSYNC" dir/file localhost:) \ -+ || test_fail "test 1: update through directory symlink failed" -+ -+diff "$srcbase/dir/file" "$HOME/real-dir/file" >/dev/null \ -+ || test_fail "test 1: update transfer content mismatch" -+ -+###################################################################### -+# Test 2: Compression (-z) as in the original reproducer -+###################################################################### -+ -+echo "another line" >> "$srcbase/dir/file" -+sleep 1 -+touch "$srcbase/dir/file" -+ -+(cd "$srcbase" && $RSYNC -KRlptzv --rsync-path="$RSYNC" dir/file localhost:) \ -+ || test_fail "test 2: compressed update through directory symlink failed" -+ -+diff "$srcbase/dir/file" "$HOME/real-dir/file" >/dev/null \ -+ || test_fail "test 2: compressed update content mismatch" -+ -+###################################################################### -+# Test 3: Nested directory symlinks (nested/sub/data.txt where -+# "nested" is a symlink to "nested_real") -+###################################################################### -+ -+mkdir -p "$HOME/nested_real/sub" -+ln -s nested_real "$HOME/nested" -+ -+mkdir -p "$srcbase/nested/sub" -+make_testfile "$srcbase/nested/sub/data.txt" -+ -+(cd "$srcbase" && $RSYNC -KRlptv --rsync-path="$RSYNC" nested/sub/data.txt localhost:) \ -+ || test_fail "test 3: initial nested transfer failed" -+ -+echo "appended nested" >> "$srcbase/nested/sub/data.txt" -+sleep 1 -+touch "$srcbase/nested/sub/data.txt" -+ -+(cd "$srcbase" && $RSYNC -KRlptv --rsync-path="$RSYNC" nested/sub/data.txt localhost:) \ -+ || test_fail "test 3: update through nested directory symlink failed" -+ -+diff "$srcbase/nested/sub/data.txt" "$HOME/nested_real/sub/data.txt" >/dev/null \ -+ || test_fail "test 3: nested update content mismatch" -+ -+###################################################################### -+# Test 4: --backup with directory symlinks -+# -+# Exercises the finish_transfer() "fnamecmp == fname" pointer -+# comparison that determines whether to update fnamecmp to the -+# backup name. If broken, --backup would reference a renamed file -+# for xattr handling. -+###################################################################### -+ -+# Reset destination -+rm -f "$HOME/real-dir/file" "$HOME/real-dir/file~" -+ -+make_testfile "$srcbase/dir/file" -+ -+(cd "$srcbase" && $RSYNC -KRlptv --rsync-path="$RSYNC" dir/file localhost:) \ -+ || test_fail "test 4: initial transfer for backup test failed" -+ -+echo "backup update" >> "$srcbase/dir/file" -+sleep 1 -+touch "$srcbase/dir/file" -+ -+(cd "$srcbase" && $RSYNC -KRlptv --backup --rsync-path="$RSYNC" dir/file localhost:) \ -+ || test_fail "test 4: update with --backup through directory symlink failed" -+ -+diff "$srcbase/dir/file" "$HOME/real-dir/file" >/dev/null \ -+ || test_fail "test 4: backup update content mismatch" -+ -+if [ ! -f "$HOME/real-dir/file~" ]; then -+ test_fail "test 4: backup file was not created" -+fi -+ -+###################################################################### -+# Test 5: --inplace with directory symlinks -+# -+# Exercises the updating_basis_or_equiv check which uses -+# "fnamecmp == fname". With --inplace, rsync writes directly to -+# the destination file instead of a temp file. -+###################################################################### -+ -+rm -f "$HOME/real-dir/file" "$HOME/real-dir/file~" -+ -+make_testfile "$srcbase/dir/file" -+ -+(cd "$srcbase" && $RSYNC -KRlptv --inplace --rsync-path="$RSYNC" dir/file localhost:) \ -+ || test_fail "test 5: initial inplace transfer failed" -+ -+echo "inplace update" >> "$srcbase/dir/file" -+sleep 1 -+touch "$srcbase/dir/file" -+ -+(cd "$srcbase" && $RSYNC -KRlptv --inplace --rsync-path="$RSYNC" dir/file localhost:) \ -+ || test_fail "test 5: inplace update through directory symlink failed" -+ -+diff "$srcbase/dir/file" "$HOME/real-dir/file" >/dev/null \ -+ || test_fail "test 5: inplace update content mismatch" -+ -+###################################################################### -+# Test 6: Top-level file (no dirname, no split needed) -+# -+# Ensures the dirname/basename split is not attempted for files -+# at the top level (file->dirname is NULL). -+###################################################################### -+ -+make_testfile "$srcbase/topfile" -+mkdir -p "$HOME" -+ -+(cd "$srcbase" && $RSYNC -Rlptv --rsync-path="$RSYNC" topfile localhost:) \ -+ || test_fail "test 6: initial top-level transfer failed" -+ -+echo "toplevel update" >> "$srcbase/topfile" -+sleep 1 -+touch "$srcbase/topfile" -+ -+(cd "$srcbase" && $RSYNC -Rlptv --rsync-path="$RSYNC" topfile localhost:) \ -+ || test_fail "test 6: top-level update failed" -+ -+diff "$srcbase/topfile" "$HOME/topfile" >/dev/null \ -+ || test_fail "test 6: top-level update content mismatch" -+ -+###################################################################### -+# Test 7: --partial-dir with protocol < 29 -+# -+# For protocol < 29, fnamecmp_type stays FNAMECMP_FNAME even when -+# fnamecmp is set to partialptr. The dirname/basename split must -+# NOT trigger in this case (guarded by "fnamecmp == fname"). -+###################################################################### -+ -+rm -f "$HOME/real-dir/file" -+make_testfile "$srcbase/dir/file" -+ -+(cd "$srcbase" && $RSYNC -KRlptv --protocol=28 --partial-dir=.rsync-partial \ -+ --rsync-path="$RSYNC" dir/file localhost:) \ -+ || test_fail "test 7: initial proto28 partial-dir transfer failed" -+ -+echo "partial-dir update" >> "$srcbase/dir/file" -+sleep 1 -+touch "$srcbase/dir/file" -+ -+(cd "$srcbase" && $RSYNC -KRlptv --protocol=28 --partial-dir=.rsync-partial \ -+ --rsync-path="$RSYNC" dir/file localhost:) \ -+ || test_fail "test 7: proto28 partial-dir update through dirlink failed" -+ -+diff "$srcbase/dir/file" "$HOME/real-dir/file" >/dev/null \ -+ || test_fail "test 7: proto28 partial-dir update content mismatch" -+ -+###################################################################### -+# Test 8: Protocol < 29 basic directory symlink update -+# -+# Exercises the protocol < 29 code path and its fallback logic -+# (clearing basedir on retry). -+###################################################################### -+ -+rm -f "$HOME/real-dir/file" -+make_testfile "$srcbase/dir/file" -+ -+(cd "$srcbase" && $RSYNC -KRlptv --protocol=28 \ -+ --rsync-path="$RSYNC" dir/file localhost:) \ -+ || test_fail "test 8: initial proto28 transfer failed" -+ -+echo "proto28 update" >> "$srcbase/dir/file" -+sleep 1 -+touch "$srcbase/dir/file" -+ -+(cd "$srcbase" && $RSYNC -KRlptv --protocol=28 \ -+ --rsync-path="$RSYNC" dir/file localhost:) \ -+ || test_fail "test 8: proto28 update through directory symlink failed" -+ -+diff "$srcbase/dir/file" "$HOME/real-dir/file" >/dev/null \ -+ || test_fail "test 8: proto28 update content mismatch" -+ -+# The script would have aborted on error, so getting here means we've won. -+exit 0 diff --git a/debian/tests/upstream-tests b/debian/tests/upstream-tests index 9fc184e..6234237 100644 --- a/debian/tests/upstream-tests +++ b/debian/tests/upstream-tests @@ -5,7 +5,7 @@ echo "debian/rules override_dh_auto_configure " debian/rules override_dh_auto_configure # Supress gcc warnings (autopkg treats them as failures) -make tls getgroups getfsdev trimslash t_unsafe wildtest testrun 2>/dev/null +make tls getgroups getfsdev t_chmod_secure t_secure_relpath trimslash t_unsafe wildtest testrun 2>/dev/null # Run tests rsync_bin="/usr/bin/rsync" ./runtests.py diff --git a/delete.c b/delete.c index 89c1f8d..4a52122 100644 --- a/delete.c +++ b/delete.c @@ -98,7 +98,7 @@ static enum delret delete_dir_contents(char *fname, uint16 flags) strlcpy(p, fp->basename, remainder); if (!(fp->mode & S_IWUSR) && !am_root && fp->flags & FLAG_OWNED_BY_US) - do_chmod(fname, fp->mode | S_IWUSR); + do_chmod_at(fname, fp->mode | S_IWUSR); /* Save stack by recursing to ourself directly. */ if (S_ISDIR(fp->mode)) { if (delete_dir_contents(fname, flags | DEL_RECURSE) != DR_SUCCESS) @@ -139,7 +139,7 @@ enum delret delete_item(char *fbuf, uint16 mode, uint16 flags) } if (flags & DEL_NO_UID_WRITE) - do_chmod(fbuf, mode | S_IWUSR); + do_chmod_at(fbuf, mode | S_IWUSR); if (S_ISDIR(mode) && !(flags & DEL_DIR_IS_EMPTY)) { /* This only happens on the first call to delete_item() since @@ -160,7 +160,7 @@ enum delret delete_item(char *fbuf, uint16 mode, uint16 flags) if (S_ISDIR(mode)) { what = "rmdir"; - ok = do_rmdir(fbuf) == 0; + ok = do_rmdir_at(fbuf) == 0; } else { if (make_backups > 0 && !(flags & DEL_FOR_BACKUP) && (backup_dir || !is_backup_file(fbuf))) { what = "make_backup"; diff --git a/flist.c b/flist.c index 7bf3030..2ec07f5 100644 --- a/flist.c +++ b/flist.c @@ -840,9 +840,9 @@ static struct file_struct *recv_file_entry(int f, struct file_list *flist, int x } if (xflags & XMIT_MOD_NSEC) #ifndef CAN_SET_NSEC - (void)read_varint(f); + (void)read_varint_bounded(f, 0, MAX_WIRE_NSEC, "modtime_nsec"); #else - modtime_nsec = read_varint(f); + modtime_nsec = read_varint_bounded(f, 0, MAX_WIRE_NSEC, "modtime_nsec"); else modtime_nsec = 0; #endif @@ -861,8 +861,19 @@ static struct file_struct *recv_file_entry(int f, struct file_list *flist, int x #endif } #endif - if (!(xflags & XMIT_SAME_MODE)) + if (!(xflags & XMIT_SAME_MODE)) { mode = from_wire_mode(read_int(f)); + /* Reject modes whose type bits are not one of the standard + * file types; otherwise garbage mode values propagate through + * the file-type checks below unpredictably. */ + if (!S_ISREG(mode) && !S_ISDIR(mode) && !S_ISLNK(mode) + && !S_ISCHR(mode) && !S_ISBLK(mode) + && !S_ISFIFO(mode) && !S_ISSOCK(mode)) { + rprintf(FERROR, "invalid file mode 0%o for %s [%s]\n", + (unsigned)mode, lastname, who_am_i()); + exit_cleanup(RERR_PROTOCOL); + } + } if (atimes_ndx && !S_ISDIR(mode) && !(xflags & XMIT_SAME_ATIME)) { atime = read_varlong(f, 4); #if SIZEOF_TIME_T < SIZEOF_INT64 diff --git a/generator.c b/generator.c index b56fa56..4d4ae72 100644 --- a/generator.c +++ b/generator.c @@ -229,11 +229,13 @@ static int read_delay_line(char *buf, int *flags_p) *flags_p = 0; if (sscanf(bp, "%x ", &mode) != 1) { - invalid_data: - rprintf(FERROR, "ERROR: invalid data in delete-delay file.\n"); - return -1; + goto invalid_data; + } + past_space = strchr(bp, ' '); + if (!past_space) { + goto invalid_data; } - past_space = strchr(bp, ' ') + 1; + past_space++; len = j - read_pos - (past_space - bp) + 1; /* count the '\0' */ read_pos = j + 1; @@ -247,6 +249,10 @@ static int read_delay_line(char *buf, int *flags_p) memcpy(buf, past_space, len); return mode; + +invalid_data: + rprintf(FERROR, "ERROR: invalid data in delete-delay file.\n"); + return -1; } static void do_delayed_deletions(char *delbuf) @@ -984,7 +990,7 @@ static int try_dests_reg(struct file_struct *file, char *fname, int ndx, if (find_exact_for_existing) { if (alt_dest_type == LINK_DEST && real_st.st_dev == sxp->st.st_dev && real_st.st_ino == sxp->st.st_ino) return -1; - if (do_unlink(fname) < 0 && errno != ENOENT) + if (do_unlink_at(fname) < 0 && errno != ENOENT) goto got_nothing_for_ya; } #ifdef SUPPORT_HARD_LINKS @@ -1112,7 +1118,7 @@ static int try_dests_non(struct file_struct *file, char *fname, int ndx, && !IS_SPECIAL(file->mode) && !IS_DEVICE(file->mode) #endif && !S_ISDIR(file->mode)) { - if (do_link(cmpbuf, fname) < 0) { + if (do_link_at(cmpbuf, fname) < 0) { rsyserr(FERROR_XFER, errno, "failed to hard-link %s with %s", cmpbuf, fname); @@ -1315,7 +1321,7 @@ static void recv_generator(char *fname, struct file_struct *file, int ndx, } } if (relative_paths && !implied_dirs && file->mode != 0 - && do_stat(dn, &sx.st) < 0) { + && do_stat_at(dn, &sx.st) < 0) { if (dry_run) goto parent_is_dry_missing; if (make_path(fname, MKP_DROP_NAME | MKP_SKIP_SLASH) < 0) { @@ -1427,7 +1433,7 @@ static void recv_generator(char *fname, struct file_struct *file, int ndx, && (stype == FT_DIR || delete_item(fname, sx.st.st_mode, del_opts | DEL_FOR_DIR) != 0)) goto cleanup; /* Any errors get reported later. */ - if (do_mkdir(fname, (file->mode|added_perms) & 0700) == 0) + if (do_mkdir_at(fname, (file->mode|added_perms) & 0700) == 0) file->flags |= FLAG_DIR_CREATED; goto cleanup; } @@ -1469,10 +1475,10 @@ static void recv_generator(char *fname, struct file_struct *file, int ndx, itemize(fnamecmp, file, ndx, statret, &sx, statret ? ITEM_LOCAL_CHANGE : 0, 0, NULL); } - if (real_ret != 0 && do_mkdir(fname,file->mode|added_perms) < 0 && errno != EEXIST) { + if (real_ret != 0 && do_mkdir_at(fname,file->mode|added_perms) < 0 && errno != EEXIST) { if (!relative_paths || errno != ENOENT || make_path(fname, MKP_DROP_NAME | MKP_SKIP_SLASH) < 0 - || (do_mkdir(fname, file->mode|added_perms) < 0 && errno != EEXIST)) { + || (do_mkdir_at(fname, file->mode|added_perms) < 0 && errno != EEXIST)) { rsyserr(FERROR_XFER, errno, "recv_generator: mkdir %s failed", full_fname(fname)); @@ -1499,7 +1505,7 @@ static void recv_generator(char *fname, struct file_struct *file, int ndx, #ifdef HAVE_CHMOD if (!am_root && (file->mode & S_IRWXU) != S_IRWXU && dir_tweaking) { mode_t mode = file->mode | S_IRWXU; - if (do_chmod(fname, mode) < 0) { + if (do_chmod_at(fname, mode) < 0) { rsyserr(FERROR_XFER, errno, "failed to modify permissions on %s", full_fname(fname)); @@ -1808,7 +1814,7 @@ static void recv_generator(char *fname, struct file_struct *file, int ndx, ; else if (quick_check_ok(FT_REG, fnamecmp, file, &sx.st)) { if (partialptr) { - do_unlink(partialptr); + do_unlink_at(partialptr); handle_partial_dir(partialptr, PDIR_DELETE); } set_file_attrs(fname, file, &sx, NULL, maybe_ATTRS_REPORT | maybe_ATTRS_ACCURATE_TIME); @@ -1896,7 +1902,7 @@ static void recv_generator(char *fname, struct file_struct *file, int ndx, back_file = NULL; goto cleanup; } - if ((f_copy = do_open(backupptr, O_WRONLY | O_CREAT | O_TRUNC | O_EXCL, 0600)) < 0) { + if ((f_copy = do_open_at(backupptr, O_WRONLY | O_CREAT | O_TRUNC | O_EXCL, 0600)) < 0) { rsyserr(FERROR_XFER, errno, "open %s", full_fname(backupptr)); unmake_file(back_file); back_file = NULL; @@ -2016,7 +2022,7 @@ int atomic_create(struct file_struct *file, char *fname, const char *slnk, const if (slnk) { #ifdef SUPPORT_LINKS - if (do_symlink(slnk, create_name) < 0) { + if (do_symlink_at(slnk, create_name) < 0) { rsyserr(FERROR_XFER, errno, "symlink %s -> \"%s\" failed", full_fname(create_name), slnk); return 0; @@ -2032,7 +2038,7 @@ int atomic_create(struct file_struct *file, char *fname, const char *slnk, const return 0; #endif } else { - if (do_mknod(create_name, file->mode, rdev) < 0) { + if (do_mknod_at(create_name, file->mode, rdev) < 0) { rsyserr(FERROR_XFER, errno, "mknod %s failed", full_fname(create_name)); return 0; @@ -2040,14 +2046,14 @@ int atomic_create(struct file_struct *file, char *fname, const char *slnk, const } if (!skip_atomic) { - if (do_rename(tmpname, fname) < 0) { + if (do_rename_at(tmpname, fname) < 0) { char *full_tmpname = strdup(full_fname(tmpname)); if (full_tmpname == NULL) out_of_memory("atomic_create"); rsyserr(FERROR_XFER, errno, "rename %s -> \"%s\" failed", full_tmpname, full_fname(fname)); free(full_tmpname); - do_unlink(tmpname); + do_unlink_at(tmpname); return 0; } } @@ -2111,7 +2117,7 @@ static void touch_up_dirs(struct file_list *flist, int ndx) continue; fname = f_name(file, NULL); if (fix_dir_perms) - do_chmod(fname, file->mode); + do_chmod_at(fname, file->mode); if (need_retouch_dir_times) { STRUCT_STAT st; if (link_stat(fname, &st, 0) == 0 && mtime_differs(&st, file)) { @@ -2146,6 +2152,8 @@ void check_for_finished_files(int itemizing, enum logcode code, int check_redo) if (send_failed) ndx = get_hlink_num(); flist = flist_for_ndx(ndx, "check_for_finished_files.1"); + if (ndx < flist->ndx_start) + exit_cleanup(RERR_PROTOCOL); file = flist->files[ndx - flist->ndx_start]; assert(file->flags & FLAG_HLINKED); if (send_failed) @@ -2174,6 +2182,8 @@ void check_for_finished_files(int itemizing, enum logcode code, int check_redo) flist = cur_flist; cur_flist = flist_for_ndx(ndx, "check_for_finished_files.2"); + if (ndx < cur_flist->ndx_start) + exit_cleanup(RERR_PROTOCOL); file = cur_flist->files[ndx - cur_flist->ndx_start]; if (solo_file) diff --git a/hlink.c b/hlink.c index 2c14407..eb36730 100644 --- a/hlink.c +++ b/hlink.c @@ -454,7 +454,7 @@ int hard_link_check(struct file_struct *file, int ndx, char *fname, int hard_link_one(struct file_struct *file, const char *fname, const char *oldname, int terse) { - if (do_link(oldname, fname) < 0) { + if (do_link_at(oldname, fname) < 0) { enum logcode code; if (terse) { if (!INFO_GTE(NAME, 1)) diff --git a/io.c b/io.c index ec34c4b..08e7e0a 100644 --- a/io.c +++ b/io.c @@ -1090,6 +1090,9 @@ static void got_flist_entry_status(enum festatus status, int ndx) { struct file_list *flist = flist_for_ndx(ndx, "got_flist_entry_status"); + if (ndx < flist->ndx_start) + exit_cleanup(RERR_PROTOCOL); + if (remove_source_files) { active_filecnt--; active_bytecnt -= F_LENGTH(flist->files[ndx - flist->ndx_start]); @@ -1865,6 +1868,45 @@ int64 read_varlong(int f, uchar min_bytes) return u.x; } +/* Read an int32 and verify lo <= v <= hi. On out-of-range, abort with a + * protocol error naming "what". The bound is co-located with the read so it + * cannot be forgotten by a downstream user. */ +int32 read_int_bounded(int f, int32 lo, int32 hi, const char *what) +{ + int32 v = read_int(f); + if (v < lo || v > hi) { + rprintf(FERROR, "wire value %s out of range: %ld not in [%ld,%ld] [%s]\n", + what, (long)v, (long)lo, (long)hi, who_am_i()); + exit_cleanup(RERR_PROTOCOL); + } + return v; +} + +/* As read_int_bounded but for varint-encoded values. */ +int32 read_varint_bounded(int f, int32 lo, int32 hi, const char *what) +{ + int32 v = read_varint(f); + if (v < lo || v > hi) { + rprintf(FERROR, "wire value %s out of range: %ld not in [%ld,%ld] [%s]\n", + what, (long)v, (long)lo, (long)hi, who_am_i()); + exit_cleanup(RERR_PROTOCOL); + } + return v; +} + +/* Read a varint that will be used as a size_t. Rejects negative values + * (which would wrap to ~SIZE_MAX) and values exceeding the supplied max. */ +size_t read_varint_size(int f, size_t max, const char *what) +{ + int32 v = read_varint(f); + if (v < 0 || (size_t)v > max) { + rprintf(FERROR, "wire size %s out of range: %ld > %lu [%s]\n", + what, (long)v, (unsigned long)max, who_am_i()); + exit_cleanup(RERR_PROTOCOL); + } + return (size_t)v; +} + int64 read_longint(int f) { #if SIZEOF_INT64 >= 8 @@ -1971,6 +2013,21 @@ void read_sum_head(int f, struct sum_struct *sum) (long)sum->count, who_am_i()); exit_cleanup(RERR_PROTOCOL); } + /* Guard against integer overflow in downstream allocations sized by + * count*element_size. my_alloc uses divide-not-multiply so it is + * already wraparound-safe, but checking here gives a clearer error + * and also covers the (size_t)count * xfer_sum_len arithmetic that + * is performed *before* reaching my_alloc. */ + if (xfer_sum_len > 0 && (size_t)sum->count > SIZE_MAX / (size_t)xfer_sum_len) { + rprintf(FERROR, "Invalid checksum count %ld (too large) [%s]\n", + (long)sum->count, who_am_i()); + exit_cleanup(RERR_PROTOCOL); + } + if ((size_t)sum->count > SIZE_MAX / sizeof(struct sum_buf)) { + rprintf(FERROR, "Invalid checksum count %ld (sum_buf overflow) [%s]\n", + (long)sum->count, who_am_i()); + exit_cleanup(RERR_PROTOCOL); + } sum->blength = read_int(f); if (sum->blength < 0 || sum->blength > max_blength) { rprintf(FERROR, "Invalid block length %ld [%s]\n", diff --git a/log.c b/log.c index e4ba1cc..b948f16 100644 --- a/log.c +++ b/log.c @@ -456,11 +456,17 @@ void rsyserr(enum logcode code, int errcode, const char *format, ...) char buf[BIGPATHBUFLEN]; size_t len; + /* snprintf returns the would-have-been length on truncation, so + * each cumulative call must be guarded; if not, sizeof buf - len + * can underflow when promoted to size_t and the next call writes + * past the buffer. */ len = snprintf(buf, sizeof buf, RSYNC_NAME ": [%s] ", who_am_i()); - va_start(ap, format); - len += vsnprintf(buf + len, sizeof buf - len, format, ap); - va_end(ap); + if (len < sizeof buf) { + va_start(ap, format); + len += vsnprintf(buf + len, sizeof buf - len, format, ap); + va_end(ap); + } if (len < sizeof buf) { len += snprintf(buf + len, sizeof buf - len, diff --git a/main.c b/main.c index ccad28a..78f0b83 100644 --- a/main.c +++ b/main.c @@ -239,11 +239,11 @@ void write_del_stats(int f) void read_del_stats(int f) { - stats.deleted_files = read_varint(f); - stats.deleted_files += stats.deleted_dirs = read_varint(f); - stats.deleted_files += stats.deleted_symlinks = read_varint(f); - stats.deleted_files += stats.deleted_devices = read_varint(f); - stats.deleted_files += stats.deleted_specials = read_varint(f); + stats.deleted_files = read_varint_bounded(f, 0, MAX_WIRE_DEL_STAT, "deleted_files"); + stats.deleted_files += stats.deleted_dirs = read_varint_bounded(f, 0, MAX_WIRE_DEL_STAT, "deleted_dirs"); + stats.deleted_files += stats.deleted_symlinks = read_varint_bounded(f, 0, MAX_WIRE_DEL_STAT, "deleted_symlinks"); + stats.deleted_files += stats.deleted_devices = read_varint_bounded(f, 0, MAX_WIRE_DEL_STAT, "deleted_devices"); + stats.deleted_files += stats.deleted_specials = read_varint_bounded(f, 0, MAX_WIRE_DEL_STAT, "deleted_specials"); } static void become_copy_as_user() @@ -394,9 +394,18 @@ static void output_itemized_counts(const char *prefix, int *counts) counts[0] -= counts[1] + counts[2] + counts[3] + counts[4]; for (j = 0; j < 5; j++) { if (counts[j]) { + /* snprintf can return more than its size arg + * on truncation; keep len <= sizeof buf - 2 so + * the closing ')' and trailing NUL always + * have room and the next iteration's + * sizeof buf - len - 2 cannot underflow. */ + if (len >= (int)sizeof buf - 2) + break; len += snprintf(buf+len, sizeof buf - len - 2, "%s%s: %s", pre, labels[j], comma_num(counts[j])); + if (len > (int)sizeof buf - 2) + len = (int)sizeof buf - 2; pre = ", "; } } @@ -1559,6 +1568,10 @@ static int start_client(int argc, char *argv[]) shell_user = shell_machine; shell_machine = p+1; } + if (*shell_machine == '-') { + rprintf(FERROR, "Invalid remote host: hostnames may not start with '-'.\n"); + exit_cleanup(RERR_SYNTAX); + } } if (DEBUG_GTE(CMD, 2)) { diff --git a/options.c b/options.c index 58ed035..3c2d235 100644 --- a/options.c +++ b/options.c @@ -114,11 +114,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/packaging/branch-from-patch b/packaging/branch-from-patch deleted file mode 100755 index 40e5653..0000000 --- a/packaging/branch-from-patch +++ /dev/null @@ -1,174 +0,0 @@ -#!/usr/bin/env -S python3 -B - -# This script turns one or more diff files in the patches dir (which is -# expected to be a checkout of the rsync-patches git repo) into a branch -# in the main rsync git checkout. This allows the applied patch to be -# merged with the latest rsync changes and tested. To update the diff -# with the resulting changes, see the patch-update script. - -import os, sys, re, argparse, glob - -sys.path = ['packaging'] + sys.path - -from pkglib import * - -def main(): - global created, info, local_branch - - cur_branch, args.base_branch = check_git_state(args.base_branch, not args.skip_check, args.patches_dir) - - local_branch = get_patch_branches(args.base_branch) - - if args.delete_local_branches: - for name in sorted(local_branch): - branch = f"patch/{args.base_branch}/{name}" - cmd_chk(['git', 'branch', '-D', branch]) - local_branch = set() - - if args.add_missing: - for fn in sorted(glob.glob(f"{args.patches_dir}/*.diff")): - name = re.sub(r'\.diff$', '', re.sub(r'.+/', '', fn)) - if name not in local_branch and fn not in args.patch_files: - args.patch_files.append(fn) - - if not args.patch_files: - return - - for fn in args.patch_files: - if not fn.endswith('.diff'): - die(f"Filename is not a .diff file: {fn}") - if not os.path.isfile(fn): - die(f"File not found: {fn}") - - scanned = set() - info = { } - - patch_list = [ ] - for fn in args.patch_files: - m = re.match(r'^(?P.*?)(?P[^/]+)\.diff$', fn) - patch = argparse.Namespace(**m.groupdict()) - if patch.name in scanned: - continue - patch.fn = fn - - lines = [ ] - commit_hash = None - with open(patch.fn, 'r', encoding='utf-8') as fh: - for line in fh: - m = re.match(r'^based-on: (\S+)', line) - if m: - commit_hash = m[1] - break - if (re.match(r'^index .*\.\..* \d', line) - or re.match(r'^diff --git ', line) - or re.match(r'^--- (old|a)/', line)): - break - lines.append(re.sub(r'\s*\Z', "\n", line, 1)) - info_txt = ''.join(lines).strip() + "\n" - lines = None - - parent = args.base_branch - patches = re.findall(r'patch -p1 <%s/(\S+)\.diff' % args.patches_dir, info_txt) - if patches: - last = patches.pop() - if last != patch.name: - warn(f"No identity patch line in {patch.fn}") - patches.append(last) - if patches: - parent = patches.pop() - if parent not in scanned: - diff_fn = patch.dir + parent + '.diff' - if not os.path.isfile(diff_fn): - die(f"Failed to find parent of {patch.fn}: {parent}") - # Add parent to args.patch_files so that we will look for the - # parent's parent. Any duplicates will be ignored. - args.patch_files.append(diff_fn) - else: - warn(f"No patch lines found in {patch.fn}") - - info[patch.name] = [ parent, info_txt, commit_hash ] - - patch_list.append(patch) - - created = set() - for patch in patch_list: - create_branch(patch) - - cmd_chk(['git', 'checkout', args.base_branch]) - - -def create_branch(patch): - if patch.name in created: - return - created.add(patch.name) - - parent, info_txt, commit_hash = info[patch.name] - parent = argparse.Namespace(dir=patch.dir, name=parent, fn=patch.dir + parent + '.diff') - - if parent.name == args.base_branch: - parent_branch = commit_hash if commit_hash else args.base_branch - else: - create_branch(parent) - parent_branch = '/'.join(['patch', args.base_branch, parent.name]) - - branch = '/'.join(['patch', args.base_branch, patch.name]) - print("\n" + '=' * 64) - print(f"Processing {branch} ({parent_branch})") - - if patch.name in local_branch: - cmd_chk(['git', 'branch', '-D', branch]) - - cmd_chk(['git', 'checkout', '-b', branch, parent_branch]) - - info_fn = 'PATCH.' + patch.name - with open(info_fn, 'w', encoding='utf-8') as fh: - fh.write(info_txt) - cmd_chk(['git', 'add', info_fn]) - - with open(patch.fn, 'r', encoding='utf-8') as fh: - patch_txt = fh.read() - - cmd_run('patch -p1'.split(), input=patch_txt) - - for fn in glob.glob('*.orig') + glob.glob('*/*.orig'): - os.unlink(fn) - - pos = 0 - new_file_re = re.compile(r'\nnew file mode (?P\d+)\s+--- /dev/null\s+\+\+\+ b/(?P.+)') - while True: - m = new_file_re.search(patch_txt, pos) - if not m: - break - os.chmod(m['fn'], int(m['mode'], 8)) - cmd_chk(['git', 'add', m['fn']]) - pos = m.end() - - while True: - cmd_chk('git status'.split()) - ans = input('Press Enter to commit, Ctrl-C to abort, or type a wild-name to add a new file: ') - if ans == '': - break - cmd_chk("git add " + ans, shell=True) - - while True: - s = cmd_run(['git', 'commit', '-a', '-m', f"Creating branch from {patch.name}.diff."]) - if not s.returncode: - break - s = cmd_run([os.environ.get('SHELL', '/bin/sh')]) - if s.returncode: - die('Aborting due to shell error code') - - -if __name__ == '__main__': - parser = argparse.ArgumentParser(description="Create a git patch branch from an rsync patch file.", add_help=False) - parser.add_argument('--branch', '-b', dest='base_branch', metavar='BASE_BRANCH', default='master', help="The branch the patch is based on. Default: master.") - parser.add_argument('--add-missing', '-a', action='store_true', help="Add a branch for every patches/*.diff that doesn't have a branch.") - parser.add_argument('--skip-check', action='store_true', help="Skip the check that ensures starting with a clean branch.") - parser.add_argument('--delete', dest='delete_local_branches', action='store_true', help="Delete all the local patch/BASE/* branches, not just the ones that are being recreated.") - parser.add_argument('--patches-dir', '-p', metavar='DIR', default='patches', help="Override the location of the rsync-patches dir. Default: patches.") - parser.add_argument('patch_files', metavar='patches/DIFF_FILE', nargs='*', help="Specify what patch diff files to process. Default: all of them.") - parser.add_argument("--help", "-h", action="help", help="Output this help message and exit.") - args = parser.parse_args() - main() - -# vim: sw=4 et ft=python diff --git a/packaging/lsb/rsync.spec b/packaging/lsb/rsync.spec index 29d1ab5..388d434 100644 --- a/packaging/lsb/rsync.spec +++ b/packaging/lsb/rsync.spec @@ -1,6 +1,6 @@ Summary: A fast, versatile, remote (and local) file-copying tool Name: rsync -Version: 3.4.2 +Version: 3.4.3 %define fullversion %{version} Release: 1 %define srcdir src @@ -79,5 +79,5 @@ rm -rf $RPM_BUILD_ROOT %dir /etc/rsync-ssl/certs %changelog -* Tue Apr 28 2026 Rsync Project -Released 3.4.2. +* Wed May 20 2026 Rsync Project +Released 3.4.3. diff --git a/packaging/patch-update b/packaging/patch-update deleted file mode 100755 index fd56a9d..0000000 --- a/packaging/patch-update +++ /dev/null @@ -1,244 +0,0 @@ -#!/usr/bin/env -S python3 -B - -# This script is used to turn one or more of the "patch/BASE/*" branches -# into one or more diffs in the "patches" directory. Pass the option -# --gen if you want generated files in the diffs. Pass the name of -# one or more diffs if you want to just update a subset of all the -# diffs. - -import os, sys, re, argparse, time, shutil - -sys.path = ['packaging'] + sys.path - -from pkglib import * - -MAKE_GEN_CMDS = [ - './prepare-source'.split(), - 'cd build && if test -f config.status ; then ./config.status ; else ../configure ; fi', - 'make -C build gen'.split(), - ] -TMP_DIR = "patches.gen" - -os.environ['GIT_MERGE_AUTOEDIT'] = 'no' - -def main(): - global master_commit, parent_patch, description, completed, last_touch - - if not os.path.isdir(args.patches_dir): - die(f'No "{args.patches_dir}" directory was found.') - if not os.path.isdir('.git'): - die('No ".git" directory present in the current dir.') - - starting_branch, args.base_branch = check_git_state(args.base_branch, not args.skip_check, args.patches_dir) - - master_commit = latest_git_hash(args.base_branch) - - if cmd_txt_chk(['packaging/prep-auto-dir']).out == '': - die('You must setup an auto-build-save dir to use this script.') - - if args.gen: - if os.path.lexists(TMP_DIR): - die(f'"{TMP_DIR}" must not exist in the current directory.') - gen_files = get_gen_files() - os.mkdir(TMP_DIR, 0o700) - for cmd in MAKE_GEN_CMDS: - cmd_chk(cmd) - cmd_chk(['rsync', '-a', *gen_files, f'{TMP_DIR}/master/']) - - last_touch = int(time.time()) - - # Start by finding all patches so that we can load all possible parents. - patches = sorted(list(get_patch_branches(args.base_branch))) - - parent_patch = { } - description = { } - - for patch in patches: - branch = f"patch/{args.base_branch}/{patch}" - desc = '' - proc = cmd_pipe(['git', 'diff', '-U1000', f"{args.base_branch}...{branch}", '--', f"PATCH.{patch}"]) - in_diff = False - for line in proc.stdout: - if in_diff: - if not re.match(r'^[ +]', line): - continue - line = line[1:] - m = re.search(r'patch -p1 = int(time.time()): - time.sleep(1) - cmd_chk(['git', 'checkout', starting_branch]) - cmd_chk(['packaging/prep-auto-dir'], discard='output') - - -def update_patch(patch): - global last_touch - - completed.add(patch) # Mark it as completed early to short-circuit any (bogus) dependency loops. - - parent = parent_patch.get(patch, None) - if parent: - if parent not in completed: - if not update_patch(parent): - return 0 - based_on = parent = f"patch/{args.base_branch}/{parent}" - else: - parent = args.base_branch - based_on = master_commit - - print(f"======== {patch} ========") - - while args.gen and last_touch >= int(time.time()): - time.sleep(1) - - branch = f"patch/{args.base_branch}/{patch}" - s = cmd_run(['git', 'checkout', branch]) - if s.returncode != 0: - return 0 - - s = cmd_run(['git', 'merge', based_on]) - ok = s.returncode == 0 - skip_shell = False - if not ok or args.cmd or args.make or args.shell: - cmd_chk(['packaging/prep-auto-dir'], discard='output') - if not ok: - print(f'"git merge {based_on}" incomplete -- please fix.') - if not run_a_shell(parent, patch): - return 0 - if not args.make and not args.cmd: - skip_shell = True - if args.make: - if cmd_run(['packaging/smart-make']).returncode != 0: - if not run_a_shell(parent, patch): - return 0 - if not args.cmd: - skip_shell = True - if args.cmd: - if cmd_run(args.cmd).returncode != 0: - if not run_a_shell(parent, patch): - return 0 - skip_shell = True - if args.shell and not skip_shell: - if not run_a_shell(parent, patch): - return 0 - - with open(f"{args.patches_dir}/{patch}.diff", 'w', encoding='utf-8') as fh: - fh.write(description[patch]) - fh.write(f"\nbased-on: {based_on}\n") - - if args.gen: - gen_files = get_gen_files() - for cmd in MAKE_GEN_CMDS: - cmd_chk(cmd) - cmd_chk(['rsync', '-a', *gen_files, f"{TMP_DIR}/{patch}/"]) - else: - gen_files = [ ] - last_touch = int(time.time()) - - proc = cmd_pipe(['git', 'diff', based_on]) - skipping = False - for line in proc.stdout: - if skipping: - if not re.match(r'^diff --git a/', line): - continue - skipping = False - elif re.match(r'^diff --git a/PATCH', line): - skipping = True - continue - if not re.match(r'^index ', line): - fh.write(line) - proc.communicate() - - if args.gen: - e_tmp_dir = re.escape(TMP_DIR) - diff_re = re.compile(r'^(diff -Nurp) %s/[^/]+/(.*?) %s/[^/]+/(.*)' % (e_tmp_dir, e_tmp_dir)) - minus_re = re.compile(r'^\-\-\- %s/[^/]+/([^\t]+)\t.*' % e_tmp_dir) - plus_re = re.compile(r'^\+\+\+ %s/[^/]+/([^\t]+)\t.*' % e_tmp_dir) - - if parent == args.base_branch: - parent_dir = 'master' - else: - m = re.search(r'([^/]+)$', parent) - parent_dir = m[1] - - proc = cmd_pipe(['diff', '-Nurp', f"{TMP_DIR}/{parent_dir}", f"{TMP_DIR}/{patch}"]) - for line in proc.stdout: - line = diff_re.sub(r'\1 a/\2 b/\3', line) - line = minus_re.sub(r'--- a/\1', line) - line = plus_re.sub(r'+++ b/\1', line) - fh.write(line) - proc.communicate() - - return 1 - - -def run_a_shell(parent, patch): - m = re.search(r'([^/]+)$', parent) - parent_dir = m[1] - os.environ['PS1'] = f"[{parent_dir}] {patch}: " - - while True: - s = cmd_run([os.environ.get('SHELL', '/bin/sh')]) - if s.returncode != 0: - ans = input("Abort? [n/y] ") - if re.match(r'^y', ans, flags=re.I): - return False - continue - cur_branch, is_clean, status_txt = check_git_status(0) - if is_clean: - break - print(status_txt, end='') - - cmd_run('rm -f build/*.o build/*/*.o') - - return True - - -if __name__ == '__main__': - parser = argparse.ArgumentParser(description="Turn a git branch back into a diff files in the patches dir.", add_help=False) - parser.add_argument('--branch', '-b', dest='base_branch', metavar='BASE_BRANCH', default='master', help="The branch the patch is based on. Default: master.") - parser.add_argument('--skip-check', action='store_true', help="Skip the check that ensures starting with a clean branch.") - parser.add_argument('--make', '-m', action='store_true', help="Run the smart-make script in every patch branch.") - parser.add_argument('--cmd', '-c', help="Run a command in every patch branch.") - parser.add_argument('--shell', '-s', action='store_true', help="Launch a shell for every patch/BASE/* branch updated, not just when a conflict occurs.") - parser.add_argument('--gen', metavar='DIR', nargs='?', const='', help='Include generated files. Optional DIR value overrides the default of using the "patches" dir.') - parser.add_argument('--patches-dir', '-p', metavar='DIR', default='patches', help="Override the location of the rsync-patches dir. Default: patches.") - parser.add_argument('patch_files', metavar='patches/DIFF_FILE', nargs='*', help="Specify what patch diff files to process. Default: all of them.") - parser.add_argument("--help", "-h", action="help", help="Output this help message and exit.") - args = parser.parse_args() - if args.gen == '': - args.gen = args.patches_dir - elif args.gen is not None: - args.patches_dir = args.gen - main() - -# vim: sw=4 et ft=python diff --git a/packaging/release-rsync b/packaging/release-rsync deleted file mode 100755 index 3abecc2..0000000 --- a/packaging/release-rsync +++ /dev/null @@ -1,385 +0,0 @@ -#!/usr/bin/env -S python3 -B - -# This script expects the directory ~/samba-rsync-ftp to exist and to be a -# copy of the /home/ftp/pub/rsync dir on samba.org. When the script is done, -# the git repository in the current directory will be updated, and the local -# ~/samba-rsync-ftp dir will be ready to be rsynced to samba.org. See the -# script samba-rsync for an easy way to initialize the local ftp copy and to -# thereafter update the remote files from your local copy. - -# This script also expects to be able to gpg sign the resulting tar files -# using your default gpg key. Make sure that the html download.html file -# has a link to the relevant keys that are authorized to sign the tar files -# and also make sure that the following commands work as expected: -# -# touch TeMp -# gpg --sign TeMp -# gpg --verify TeMp.gpg -# gpg --sign TeMp -# rm TeMp* -# -# The second time you sign the file it should NOT prompt you for your password -# (unless the timeout period has passed). It will prompt about overriding the -# existing TeMp.gpg file, though. - -import os, sys, re, argparse, glob, shutil, signal -from datetime import datetime -from getpass import getpass - -sys.path = ['packaging'] + sys.path - -from pkglib import * - -os.environ['LESS'] = 'mqeiXR'; # Make sure that -F is turned off and -R is turned on. -dest = os.environ['HOME'] + '/samba-rsync-ftp' -ORIGINAL_PATH = os.environ['PATH'] - -def main(): - if not os.path.isfile('packaging/release-rsync'): - die('You must run this script from the top of your rsync checkout.') - - now = datetime.now().astimezone() # Requires python 3.6 or later - cl_today = now.strftime('* %a %b %d %Y') - year = now.strftime('%Y') - ztoday = now.strftime('%d %b %Y') - today = ztoday.lstrip('0') - - # The MAINTAINER_TZ_OFFSET is a float number of hours vs UTC. It can start with '-' but not '+'. - tz_now = now.strftime('%z') - tz_num = tz_now[0:1].replace('+', '') + str(float(tz_now[1:3]) + float(tz_now[3:]) / 60) - - curdir = os.getcwd() - - signal.signal(signal.SIGINT, signal_handler) - - if cmd_txt_chk(['packaging/prep-auto-dir']).out == '': - die('You must setup an auto-build-save dir to use this script.'); - - auto_dir, gen_files = get_gen_files(True) - gen_pathnames = [ os.path.join(auto_dir, fn) for fn in gen_files ] - - dash_line = '=' * 74 - - print(f"""\ -{dash_line} -== This will release a new version of rsync onto an unsuspecting world. == -{dash_line} -""") - - with open('build/rsync.1') as fh: - for line in fh: - if line.startswith(r'.\" prefix='): - doc_prefix = line.split('=')[1].strip() - if doc_prefix != '/usr': - warn(f"*** The documentation was built with prefix {doc_prefix} instead of /usr ***") - die("*** Read the md2man script for a way to override this. ***") - break - if line.startswith('.P'): - die("Failed to find the prefix comment at the start of the rsync.1 manpage.") - - if not os.path.isdir(dest): - die(dest, "dest does not exist") - if not os.path.isdir('.git'): - die("There is no .git dir in the current directory.") - if os.path.lexists('a'): - die('"a" must not exist in the current directory.') - if os.path.lexists('b'): - die('"b" must not exist in the current directory.') - - curversion = get_rsync_version() - - # All version values are strings! - lastversion, last_protocol_version, pdate = get_NEWS_version_info() - protocol_version, subprotocol_version = get_protocol_versions() - - version = curversion - m = re.search(r'pre(\d+)', version) - if m: - version = re.sub(r'pre\d+', 'pre' + str(int(m[1]) + 1), version) - else: - version = version.replace('dev', 'pre1') - - ans = input(f"Please enter the version number of this release: [{version}] ") - if ans == '.': - version = re.sub(r'pre\d+', '', version) - elif ans != '': - version = ans - if not re.match(r'^[\d.]+(pre\d+)?$', version): - die(f'Invalid version: "{version}"') - - v_ver = 'v' + version - rsync_ver = 'rsync-' + version - - if os.path.lexists(rsync_ver): - die(f'"{rsync_ver}" must not exist in the current directory.') - - out = cmd_txt_chk(['git', 'tag', '-l', v_ver]).out - if out != '': - print(f"Tag {v_ver} already exists.") - ans = input("\nDelete tag or quit? [Q/del] ") - if not re.match(r'^del', ans, flags=re.I): - die("Aborted") - cmd_chk(['git', 'tag', '-d', v_ver]) - - version = re.sub(r'[-.]*pre[-.]*', 'pre', version) - if 'pre' in version and not curversion.endswith('dev'): - lastversion = curversion - - ans = input(f"Enter the previous version to produce a patch against: [{lastversion}] ") - if ans != '': - lastversion = ans - lastversion = re.sub(r'[-.]*pre[-.]*', 'pre', lastversion) - - rsync_lastver = 'rsync-' + lastversion - if os.path.lexists(rsync_lastver): - die(f'"{rsync_lastver}" must not exist in the current directory.') - - m = re.search(r'(pre\d+)', version) - pre = m[1] if m else '' - - release = '0.1' if pre else '1' - ans = input(f"Please enter the RPM release number of this release: [{release}] ") - if ans != '': - release = ans - if pre: - release += '.' + pre - - finalversion = re.sub(r'pre\d+', '', version) - proto_changed = protocol_version != last_protocol_version - if proto_changed: - if finalversion in pdate: - proto_change_date = pdate[finalversion] - else: - while True: - ans = input("On what date did the protocol change to {protocol_version} get checked in? (dd Mmm yyyy) ") - if re.match(r'^\d\d \w\w\w \d\d\d\d$', ans): - break - proto_change_date = ans - else: - proto_change_date = ' ' * 11 - - if 'pre' in lastversion: - if not pre: - die("You should not diff a release version against a pre-release version.") - srcdir = srcdiffdir = lastsrcdir = 'src-previews' - skipping = ' ** SKIPPING **' - elif pre: - srcdir = srcdiffdir = 'src-previews' - lastsrcdir = 'src' - skipping = ' ** SKIPPING **' - else: - srcdir = lastsrcdir = 'src' - srcdiffdir = 'src-diffs' - skipping = '' - - print(f""" -{dash_line} -version is "{version}" -lastversion is "{lastversion}" -dest is "{dest}" -curdir is "{curdir}" -srcdir is "{srcdir}" -srcdiffdir is "{srcdiffdir}" -lastsrcdir is "{lastsrcdir}" -release is "{release}" - -About to: - - tweak SUBPROTOCOL_VERSION in rsync.h, if needed - - tweak the version in version.h and the spec files - - tweak NEWS.md to ensure header values are correct - - generate configure.sh, config.h.in, and proto.h - - page through the differences -""") - ans = input(" ") - - specvars = { - 'Version:': finalversion, - 'Release:': release, - '%define fullversion': f'%{{version}}{pre}', - 'Released': version + '.', - '%define srcdir': srcdir, - } - - tweak_files = 'version.h rsync.h'.split() - tweak_files += glob.glob('packaging/*.spec') - tweak_files += glob.glob('packaging/*/*.spec') - - for fn in tweak_files: - with open(fn, 'r', encoding='utf-8') as fh: - old_txt = txt = fh.read() - if fn == 'version.h': - x_re = re.compile(r'^(#define RSYNC_VERSION).*', re.M) - msg = f"Unable to update RSYNC_VERSION in {fn}" - txt = replace_or_die(x_re, r'\1 "%s"' % version, txt, msg) - x_re = re.compile(r'^(#define MAINTAINER_TZ_OFFSET).*', re.M) - msg = f"Unable to update MAINTAINER_TZ_OFFSET in {fn}" - txt = replace_or_die(x_re, r'\1 ' + tz_num, txt, msg) - elif '.spec' in fn: - for var, val in specvars.items(): - x_re = re.compile(r'^%s .*' % re.escape(var), re.M) - txt = replace_or_die(x_re, var + ' ' + val, txt, f"Unable to update {var} in {fn}") - x_re = re.compile(r'^\* \w\w\w \w\w\w \d\d \d\d\d\d (.*)', re.M) - txt = replace_or_die(x_re, r'%s \1' % cl_today, txt, f"Unable to update ChangeLog header in {fn}") - elif fn == 'rsync.h': - x_re = re.compile(r'(#define\s+SUBPROTOCOL_VERSION)\s+(\d+)') - repl = lambda m: m[1] + ' ' + ('0' if not pre or not proto_changed else '1' if m[2] == '0' else m[2]) - txt = replace_or_die(x_re, repl, txt, f"Unable to find SUBPROTOCOL_VERSION define in {fn}") - elif fn == 'NEWS.md': - efv = re.escape(finalversion) - x_re = re.compile(r'^# NEWS for rsync %s \(UNRELEASED\)\s+## Changes in this version:\n' % efv - + r'(\n### PROTOCOL NUMBER:\s+- The protocol number was changed to \d+\.\n)?') - rel_day = 'UNRELEASED' if pre else today - repl = (f'# NEWS for rsync {finalversion} ({rel_day})\n\n' - + '## Changes in this version:\n') - if proto_changed: - repl += f'\n### PROTOCOL NUMBER:\n\n - The protocol number was changed to {protocol_version}.\n' - good_top = re.sub(r'\(.*?\)', '(UNRELEASED)', repl, 1) - msg = f"The top lines of {fn} are not in the right format. It should be:\n" + good_top - txt = replace_or_die(x_re, repl, txt, msg) - x_re = re.compile(r'^(\| )(\S{2} \S{3} \d{4})(\s+\|\s+%s\s+\| ).{11}(\s+\| )\S{2}(\s+\|+)$' % efv, re.M) - repl = lambda m: m[1] + (m[2] if pre else ztoday) + m[3] + proto_change_date + m[4] + protocol_version + m[5] - txt = replace_or_die(x_re, repl, txt, f'Unable to find "| ?? ??? {year} | {finalversion} | ... |" line in {fn}') - else: - die(f"Unrecognized file in tweak_files: {fn}") - - if txt != old_txt: - print(f"Updating {fn}") - with open(fn, 'w', encoding='utf-8') as fh: - fh.write(txt) - - cmd_chk(['packaging/year-tweak']) - - print(dash_line) - cmd_run("git diff".split()) - - srctar_name = f"{rsync_ver}.tar.gz" - diff_name = f"{rsync_lastver}-{version}.diffs.gz" - srctar_file = os.path.join(dest, srcdir, srctar_name) - pattar_file = os.path.join(dest, srcdir, pattar_name) - diff_file = os.path.join(dest, srcdiffdir, diff_name) - lasttar_file = os.path.join(dest, lastsrcdir, rsync_lastver + '.tar.gz') - - print(f"""\ -{dash_line} - -About to: - - git commit all changes - - run a full build, ensuring that the manpages & configure.sh are up-to-date - - merge the {args.master_branch} branch into the patch/{args.master_branch}/* branches -""") - ans = input(" ") - - s = cmd_run(['git', 'commit', '-a', '-m', f'Preparing for release of {version} [buildall]']) - if s.returncode: - die('Aborting') - - cmd_chk('touch configure.ac && packaging/smart-make && make gen') - - print('Creating any missing patch branches.') - s = cmd_run(f'packaging/branch-from-patch --branch={args.master_branch} --add-missing') - if s.returncode: - die('Aborting') - - if re.match(r'^y', ans, re.I): - print(f'\nRunning smart-make on all "patch/{args.master_branch}/*" branches ...') - cmd_run(f"packaging/patch-update --branch={args.master_branch} --skip-check --make") - - print(f"""\ -{dash_line} - -About to: - - create signed tag for this release: {v_ver} - - create release diffs, "{diff_name}" - - create release tar, "{srctar_name}" - - update top-level README.md, NEWS.md, TODO, and ChangeLog - - update top-level rsync*.html manpages - - gpg-sign the release files - - update hard-linked top-level release files{skipping} -""") - ans = input(" ") - - # TODO: is there a better way to ensure that our passphrase is in the agent? - cmd_run("touch TeMp; gpg --sign TeMp; rm TeMp*") - - out = cmd_txt(f"git tag -s -m 'Version {version}.' {v_ver}", capture='combined').out - print(out, end='') - if 'bad passphrase' in out or 'failed' in out: - die('Aborting') - - os.environ['PATH'] = ORIGINAL_PATH - - # Extract the generated files from the old tar. - tweaked_gen_files = [ os.path.join(rsync_lastver, fn) for fn in gen_files ] - cmd_run(['tar', 'xzf', lasttar_file, *tweaked_gen_files]) - os.rename(rsync_lastver, 'a') - - print(f"Creating {diff_file} ...") - cmd_chk(['rsync', '-a', *gen_pathnames, 'b/']) - - sed_script = r's:^((---|\+\+\+) [ab]/[^\t]+)\t.*:\1:' # CAUTION: must not contain any single quotes! - cmd_chk(f"(git diff v{lastversion} {v_ver} -- ':!.github'; diff -upN a b | sed -r '{sed_script}') | gzip -9 >{diff_file}") - shutil.rmtree('a') - os.rename('b', rsync_ver) - - print(f"Creating {srctar_file} ...") - cmd_chk(f"git archive --format=tar --prefix={rsync_ver}/ {v_ver} | tar xf -") - cmd_chk(f"support/git-set-file-times --quiet --prefix={rsync_ver}/") - cmd_chk(['fakeroot', 'tar', 'czf', srctar_file, '--exclude=.github', rsync_ver]) - shutil.rmtree(rsync_ver) - - print(f"Updating the other files in {dest} ...") - md_files = 'README.md NEWS.md INSTALL.md'.split() - html_files = [ fn for fn in gen_pathnames if fn.endswith('.html') ] - cmd_chk(['rsync', '-a', *md_files, *html_files, dest]) - cmd_chk(["./md-convert", "--dest", dest, *md_files]) - - cmd_chk(f"git log --name-status | gzip -9 >{dest}/ChangeLog.gz") - - for fn in (srctar_file, pattar_file, diff_file): - asc_fn = fn + '.asc' - if os.path.lexists(asc_fn): - os.unlink(asc_fn) - res = cmd_run(['gpg', '--batch', '-ba', fn]) - if res.returncode != 0 and res.returncode != 2: - die("gpg signing failed") - - if not pre: - for find in f'{dest}/rsync-*.gz {dest}/rsync-*.asc {dest}/src-previews/rsync-*diffs.gz*'.split(): - for fn in glob.glob(find): - os.unlink(fn) - top_link = [ - srctar_file, f"{srctar_file}.asc", - pattar_file, f"{pattar_file}.asc", - diff_file, f"{diff_file}.asc", - ] - for fn in top_link: - os.link(fn, re.sub(r'/src(-\w+)?/', '/', fn)) - - print(f"""\ -{dash_line} - -Local changes are done. When you're satisfied, push the git repository -and rsync the release files. Remember to announce the release on *BOTH* -rsync-announce@lists.samba.org and rsync@lists.samba.org (and the web)! -""") - - -def replace_or_die(regex, repl, txt, die_msg): - m = regex.search(txt) - if not m: - die(die_msg) - return regex.sub(repl, txt, 1) - - -def signal_handler(sig, frame): - die("\nAborting due to SIGINT.") - - -if __name__ == '__main__': - parser = argparse.ArgumentParser(description="Prepare a new release of rsync in the git repo & ftp dir.", add_help=False) - parser.add_argument('--branch', '-b', dest='master_branch', default='master', help="The branch to release. Default: master.") - parser.add_argument("--help", "-h", action="help", help="Output this help message and exit.") - args = parser.parse_args() - main() - -# vim: sw=4 et ft=python diff --git a/receiver.c b/receiver.c index edfbb21..f49931b 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 @@ -312,7 +318,12 @@ static int receive_data(int f_in, char *fname_r, int fd_r, OFF_T size_r, } } - while ((i = recv_token(f_in, &data)) != 0) { + while (1) { + data = NULL; + i = recv_token(f_in, &data); + if (i == 0) + break; + if (INFO_GTE(PROGRESS, 1)) show_progress(offset, total_size); @@ -320,6 +331,10 @@ static int receive_data(int f_in, char *fname_r, int fd_r, OFF_T size_r, maybe_send_keepalive(time(NULL), MSK_ALLOW_FLUSH | MSK_ACTIVE_RECEIVER); if (i > 0) { + if (!data) { + rprintf(FERROR, "Invalid literal token with no data [%s]\n", who_am_i()); + exit_cleanup(RERR_PROTOCOL); + } if (DEBUG_GTE(DELTASUM, 3)) { rprintf(FINFO,"data recv %d at %s\n", i, big_num(offset)); @@ -337,6 +352,11 @@ static int receive_data(int f_in, char *fname_r, int fd_r, OFF_T size_r, } i = -(i+1); + if (i < 0 || i >= sum.count) { + rprintf(FERROR, "Invalid block index %d (count=%ld) [%s]\n", + i, (long)sum.count, who_am_i()); + exit_cleanup(RERR_PROTOCOL); + } offset2 = i * (OFF_T)sum.blength; len = sum.blength; if (i == (int)sum.count-1 && sum.remainder != 0) @@ -436,7 +456,7 @@ static void handle_delayed_updates(char *local_name) } /* We don't use robust_rename() here because the * partial-dir must be on the same drive. */ - if (do_rename(partialptr, fname) < 0) { + if (do_rename_at(partialptr, fname) < 0) { rsyserr(FERROR_XFER, errno, "rename failed for %s (from %s)", full_fname(fname), partialptr); @@ -452,7 +472,10 @@ static void handle_delayed_updates(char *local_name) static void no_batched_update(int ndx, BOOL is_redo) { struct file_list *flist = flist_for_ndx(ndx, "no_batched_update"); - struct file_struct *file = flist->files[ndx - flist->ndx_start]; + struct file_struct *file; + if (ndx < flist->ndx_start) + exit_cleanup(RERR_PROTOCOL); + file = flist->files[ndx - flist->ndx_start]; rprintf(FERROR_XFER, "(No batched update for%s \"%s\")\n", is_redo ? " resend of" : "", f_name(file, NULL)); @@ -589,6 +612,8 @@ int recv_files(int f_in, int f_out, char *local_name) if (ndx - cur_flist->ndx_start >= 0) file = cur_flist->files[ndx - cur_flist->ndx_start]; + else if (cur_flist->parent_ndx < 0) + exit_cleanup(RERR_PROTOCOL); else file = dir_flist->files[cur_flist->parent_ndx]; fname = local_name ? local_name : f_name(file, fbuf); @@ -854,11 +879,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) { @@ -910,7 +945,7 @@ int recv_files(int f_in, int f_out, char *local_name) recv_ok = -1; else if (fnamecmp == partialptr) { if (!one_inplace) - do_unlink(partialptr); + do_unlink_at(partialptr); handle_partial_dir(partialptr, PDIR_DELETE); } } else if (keep_partial && partialptr && (!one_inplace || delay_updates)) { @@ -919,7 +954,7 @@ int recv_files(int f_in, int f_out, char *local_name) "Unable to create partial-dir for %s -- discarding %s.\n", local_name ? local_name : f_name(file, NULL), recv_ok ? "completed file" : "partial file"); - do_unlink(fnametmp); + do_unlink_at(fnametmp); recv_ok = -1; } else if (!finish_transfer(partialptr, fnametmp, fnamecmp, NULL, file, recv_ok, !partial_dir)) @@ -930,7 +965,7 @@ int recv_files(int f_in, int f_out, char *local_name) } else partialptr = NULL; } else if (!one_inplace) - do_unlink(fnametmp); + do_unlink_at(fnametmp); cleanup_disable(); diff --git a/rrsync.1 b/rrsync.1 index dbabb51..cdaf4e2 100644 --- a/rrsync.1 +++ b/rrsync.1 @@ -1,4 +1,4 @@ -.TH "rrsync" "1" "28 Apr 2026" "rrsync from rsync 3.4.2" "User Commands" +.TH "rrsync" "1" "8 May 2026" "rrsync from rsync 3.4.3" "User Commands" .\" prefix=/usr .P .SH "NAME" @@ -155,7 +155,7 @@ command="rrsync -ro results" ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAIEAmk... .P .SH "VERSION" .P -This manpage is current for version 3.4.2 of rsync. +This manpage is current for version 3.4.3 of rsync. .P .SH "CREDITS" .P diff --git a/rrsync.1.html b/rrsync.1.html index b52652d..1c62400 100644 --- a/rrsync.1.html +++ b/rrsync.1.html @@ -156,7 +156,7 @@

FILES

SEE ALSO

rsync(1), rsyncd.conf(5)

VERSION

-

This manpage is current for version 3.4.2 of rsync.

+

This manpage is current for version 3.4.3 of rsync.

CREDITS

rsync is distributed under the GNU General Public License. See the file COPYING for details.

@@ -165,5 +165,5 @@

CREDITS

AUTHOR

The original rrsync perl script was written by Joe Smith. Many people have later contributed to it. The python version was created by Wayne Davison.

-

28 Apr 2026

+

8 May 2026

diff --git a/rsync-ssl.1 b/rsync-ssl.1 index eedac46..4bd97ce 100644 --- a/rsync-ssl.1 +++ b/rsync-ssl.1 @@ -1,4 +1,4 @@ -.TH "rsync-ssl" "1" "28 Apr 2026" "rsync-ssl from rsync 3.4.2" "User Commands" +.TH "rsync-ssl" "1" "8 May 2026" "rsync-ssl from rsync 3.4.3" "User Commands" .\" prefix=/usr .P .SH "NAME" @@ -130,7 +130,7 @@ Please report bugs! See the web site at .P .SH "VERSION" .P -This manpage is current for version 3.4.2 of rsync. +This manpage is current for version 3.4.3 of rsync. .P .SH "CREDITS" .P diff --git a/rsync-ssl.1.html b/rsync-ssl.1.html index 236d564..5d625f9 100644 --- a/rsync-ssl.1.html +++ b/rsync-ssl.1.html @@ -140,7 +140,7 @@

CAVEATS

BUGS

Please report bugs! See the web site at https://rsync.samba.org/.

VERSION

-

This manpage is current for version 3.4.2 of rsync.

+

This manpage is current for version 3.4.3 of rsync.

CREDITS

Rsync is distributed under the GNU General Public License. See the file COPYING for details.

@@ -150,5 +150,5 @@

AUTHOR

This manpage was written by Wayne Davison.

Mailing lists for support and development are available at https://lists.samba.org/.

-

28 Apr 2026

+

8 May 2026

diff --git a/rsync.1 b/rsync.1 index 43bc38f..247ab6d 100644 --- a/rsync.1 +++ b/rsync.1 @@ -1,4 +1,4 @@ -.TH "rsync" "1" "28 Apr 2026" "rsync 3.4.2" "User Commands" +.TH "rsync" "1" "8 May 2026" "rsync 3.4.3" "User Commands" .\" prefix=/usr .P .SH "NAME" @@ -5029,7 +5029,7 @@ Please report bugs! See the web site at .P .SH "VERSION" .P -This manpage is current for version 3.4.2 of rsync. +This manpage is current for version 3.4.3 of rsync. .P .SH "INTERNAL OPTIONS" .P diff --git a/rsync.1.html b/rsync.1.html index c4096e7..ded224a 100644 --- a/rsync.1.html +++ b/rsync.1.html @@ -4494,7 +4494,7 @@

BUGS

Please report bugs! See the web site at https://rsync.samba.org/.

VERSION

-

This manpage is current for version 3.4.2 of rsync.

+

This manpage is current for version 3.4.3 of rsync.

INTERNAL OPTIONS

The options --server and --sender are used internally by rsync, and should never be typed by a user under normal circumstances. Some awareness of these @@ -4524,5 +4524,5 @@

AUTHOR

people from around the world have helped to maintain and improve it.

Mailing lists for support and development are available at https://lists.samba.org/.

-

28 Apr 2026

+

8 May 2026

diff --git a/rsync.c b/rsync.c index b130aba..1d2ae82 100644 --- a/rsync.c +++ b/rsync.c @@ -547,7 +547,7 @@ int set_file_attrs(const char *fname, struct file_struct *file, stat_x *sxp, if (am_root >= 0) { uid_t uid = change_uid ? (uid_t)F_OWNER(file) : sxp->st.st_uid; gid_t gid = change_gid ? (gid_t)F_GROUP(file) : sxp->st.st_gid; - if (do_lchown(fname, uid, gid) != 0) { + if (do_lchown_at(fname, uid, gid) != 0) { /* We shouldn't have attempted to change uid * or gid unless have the privilege. */ rsyserr(FERROR_XFER, errno, "%s %s failed", @@ -657,7 +657,7 @@ int set_file_attrs(const char *fname, struct file_struct *file, stat_x *sxp, #ifdef HAVE_CHMOD if (!BITS_EQUAL(sxp->st.st_mode, new_mode, CHMOD_BITS)) { - int ret = am_root < 0 ? 0 : do_chmod(fname, new_mode); + int ret = am_root < 0 ? 0 : do_chmod_at(fname, new_mode); if (ret < 0) { rsyserr(FERROR_XFER, errno, "failed to set permissions on %s", @@ -758,7 +758,7 @@ int finish_transfer(const char *fname, const char *fnametmp, full_fname(fnametmp), fname); if (!partialptr || (ret == -2 && temp_copy_name) || robust_rename(fnametmp, partialptr, NULL, file->mode) < 0) - do_unlink(fnametmp); + do_unlink_at(fnametmp); return 0; } if (ret == 0) { @@ -774,7 +774,7 @@ int finish_transfer(const char *fname, const char *fnametmp, ok_to_set_time ? ATTRS_ACCURATE_TIME : ATTRS_SKIP_MTIME | ATTRS_SKIP_ATIME | ATTRS_SKIP_CRTIME); if (temp_copy_name) { - if (do_rename(fnametmp, fname) < 0) { + if (do_rename_at(fnametmp, fname) < 0) { rsyserr(FERROR_XFER, errno, "rename %s -> \"%s\"", full_fname(fnametmp), fname); return 0; diff --git a/rsync.h b/rsync.h index 479ac48..cdc2d2c 100644 --- a/rsync.h +++ b/rsync.h @@ -163,6 +163,29 @@ /* For compatibility with older rsyncs */ #define OLD_MAX_BLOCK_SIZE ((int32)1 << 29) +/* Policy ceilings on attacker-controlled wire values. Picked well above any + * legitimate filesystem / protocol traffic but well below sizes that could + * cause integer overflow or DoS-grade allocations. See input_checking.txt. + * + * Note on MAX_WIRE_XATTR_DATALEN: xattr datum size is bounded only by the + * wire-format maximum (signed int32 varint, ~2GB). macOS resource forks + * are transferred as the com.apple.ResourceFork xattr and can legitimately + * be many GB; --max-alloc (default 1GB, configurable) is the real + * allocation cap. read_varint_size() still rejects negative values so a + * hostile peer cannot wrap to ~SIZE_MAX. */ +#define MAX_WIRE_XATTR_COUNT 65536 +#define MAX_WIRE_XATTR_NAMELEN 4096 +#define MAX_WIRE_XATTR_DATALEN ((int32)0x7fffffff) +#define MAX_WIRE_ACL_COUNT 65536 +#define MAX_WIRE_NSEC 999999999 +/* MAX_WIRE_DEL_STAT is the per-category cap for read_del_stats() in main.c, + * which accumulates 5 wire-supplied counts into the int32 stats.deleted_files + * accumulator. Capped at 2^28 so 5 * 2^28 = 1.34 GB stays under INT32_MAX + * (2.15 GB) with margin -- a higher cap (e.g. 2^30) would let a hostile peer + * supplying 3+ max-sized counts overflow the accumulator, which is signed-int + * UB. 2^28 is still well above any plausible real transfer's deletion count. */ +#define MAX_WIRE_DEL_STAT ((int32)1 << 28) + #define ROUND_UP_1024(siz) ((siz) & (1024-1) ? ((siz) | (1024-1)) + 1 : (siz)) #define IOERR_GENERAL (1<<0) /* For backward compatibility, this must == 1 */ diff --git a/rsyncd.conf.5 b/rsyncd.conf.5 index ddd6b63..b4a8cbe 100644 --- a/rsyncd.conf.5 +++ b/rsyncd.conf.5 @@ -1,4 +1,4 @@ -.TH "rsyncd.conf" "5" "28 Apr 2026" "rsyncd.conf from rsync 3.4.2" "User Commands" +.TH "rsyncd.conf" "5" "8 May 2026" "rsyncd.conf from rsync 3.4.3" "User Commands" .\" prefix=/usr .P .SH "NAME" @@ -1305,7 +1305,7 @@ Please report bugs! The rsync bug tracking system is online at .P .SH "VERSION" .P -This manpage is current for version 3.4.2 of rsync. +This manpage is current for version 3.4.3 of rsync. .P .SH "CREDITS" .P diff --git a/rsyncd.conf.5.html b/rsyncd.conf.5.html index e70650e..3e54401 100644 --- a/rsyncd.conf.5.html +++ b/rsyncd.conf.5.html @@ -1199,7 +1199,7 @@

BUGS

Please report bugs! The rsync bug tracking system is online at https://rsync.samba.org/.

VERSION

-

This manpage is current for version 3.4.2 of rsync.

+

This manpage is current for version 3.4.3 of rsync.

CREDITS

Rsync is distributed under the GNU General Public License. See the file COPYING for details.

@@ -1213,5 +1213,5 @@

AUTHOR

people from around the world have helped to maintain and improve it.

Mailing lists for support and development are available at https://lists.samba.org/.

-

28 Apr 2026

+

8 May 2026

diff --git a/runtests.py b/runtests.py index 54ca0a0..08f67bb 100755 --- a/runtests.py +++ b/runtests.py @@ -297,6 +297,23 @@ def main(): sys.stderr.write(f"srcdir {srcdir} is not a directory\n") sys.exit(2) + # Helper programs the test scripts invoke directly. Missing any of these + # would cause many tests to fail with confusing "not found" errors, so + # check up front and point the user at the make target that builds them. + required_helpers = ['tls', 'trimslash', 't_unsafe', 't_chmod_secure', + 't_secure_relpath', + 'wildtest', 'getgroups', 'getfsdev'] + missing = [h for h in required_helpers + if not os.path.isfile(os.path.join(tooldir, h))] + if missing: + sys.stderr.write( + f"runtests.py: missing test helper program(s) in {tooldir}: " + f"{', '.join(missing)}\n" + f"Build them with: make {' '.join(missing)}\n" + f"or run the full test target: make check\n" + ) + sys.exit(2) + testuser = get_testuser() # Print header diff --git a/sender.c b/sender.c index b1588b7..033f87e 100644 --- a/sender.c +++ b/sender.c @@ -48,6 +48,8 @@ extern int make_backups; extern int inplace; extern int inplace_partial; extern int batch_fd; +extern int use_secure_symlinks; +extern char *module_dir; extern int write_batch; extern int file_old_total; extern BOOL want_progress_now; @@ -138,6 +140,8 @@ void successful_send(int ndx) return; flist = flist_for_ndx(ndx, "successful_send"); + if (ndx < flist->ndx_start) + exit_cleanup(RERR_PROTOCOL); file = flist->files[ndx - flist->ndx_start]; if (!change_pathname(file, NULL, 0)) return; @@ -352,7 +356,25 @@ void send_files(int f_in, int f_out) exit_cleanup(RERR_PROTOCOL); } - fd = do_open_checklinks(fname); + if (use_secure_symlinks) { + /* Open from module root to prevent TOCTOU race where + * change_pathname's chdir follows a directory symlink. + * Reconstruct the full path relative to module_dir + * from F_PATHNAME (path) and f_name (fname). */ + char secure_path[MAXPATHLEN]; + int slen = snprintf(secure_path, sizeof secure_path, "%s%s%s", path, slash, fname); + if (slen >= (int)sizeof secure_path) { + io_error |= IOERR_GENERAL; + rprintf(FERROR_XFER, "path too long: %s%s%s\n", path, slash, fname); + free_sums(s); + if (protocol_version >= 30) + send_msg_int(MSG_NO_SEND, ndx); + continue; + } + fd = secure_relative_open(module_dir, secure_path, O_RDONLY, 0); + } else { + fd = do_open_checklinks(fname); + } if (fd == -1) { if (errno == ENOENT) { enum logcode c = am_daemon && protocol_version < 28 ? FERROR : FWARNING; diff --git a/socket.c b/socket.c index c2075ad..6a8f6f4 100644 --- a/socket.c +++ b/socket.c @@ -47,21 +47,23 @@ static struct sigaction sigact; static int sock_exec(const char *prog); +#define PROXY_BUF_SIZE 1024 + /* Establish a proxy connection on an open socket to a web proxy by using the * CONNECT method. If proxy_user and proxy_pass are not NULL, they are used to * authenticate to the proxy using the "Basic" proxy-authorization protocol. */ static int establish_proxy_connection(int fd, char *host, int port, char *proxy_user, char *proxy_pass) { - char *cp, buffer[1024]; - char *authhdr, authbuf[1024]; + char *cp, buffer[PROXY_BUF_SIZE + 1]; + char *authhdr, authbuf[PROXY_BUF_SIZE + 1]; int len; if (proxy_user && proxy_pass) { - stringjoin(buffer, sizeof buffer, + stringjoin(buffer, PROXY_BUF_SIZE, proxy_user, ":", proxy_pass, NULL); len = strlen(buffer); - if ((len*8 + 5) / 6 >= (int)sizeof authbuf - 3) { + if ((len*8 + 5) / 6 >= PROXY_BUF_SIZE - 3) { rprintf(FERROR, "authentication information is too long\n"); return -1; @@ -74,14 +76,14 @@ static int establish_proxy_connection(int fd, char *host, int port, char *proxy_ authhdr = ""; } - len = snprintf(buffer, sizeof buffer, "CONNECT %s:%d HTTP/1.0%s%s\r\n\r\n", host, port, authhdr, authbuf); - assert(len > 0 && len < (int)sizeof buffer); + len = snprintf(buffer, PROXY_BUF_SIZE, "CONNECT %s:%d HTTP/1.0%s%s\r\n\r\n", host, port, authhdr, authbuf); + assert(len > 0 && len < PROXY_BUF_SIZE); if (write(fd, buffer, len) != len) { rsyserr(FERROR, errno, "failed to write to proxy"); return -1; } - for (cp = buffer; cp < &buffer[sizeof buffer - 1]; cp++) { + for (cp = buffer; cp < &buffer[PROXY_BUF_SIZE - 1]; cp++) { if (read(fd, cp, 1) != 1) { rsyserr(FERROR, errno, "failed to read from proxy"); return -1; @@ -90,11 +92,13 @@ static int establish_proxy_connection(int fd, char *host, int port, char *proxy_ break; } - if (*cp != '\n') - cp++; - *cp-- = '\0'; - if (*cp == '\r') - *cp = '\0'; + if (cp == &buffer[PROXY_BUF_SIZE - 1]) { + rprintf(FERROR, "proxy response line too long\n"); + return -1; + } + *cp = '\0'; + if (cp > buffer && cp[-1] == '\r') + cp[-1] = '\0'; if (strncmp(buffer, "HTTP/", 5) != 0) { rprintf(FERROR, "bad response from proxy -- %s\n", buffer); @@ -110,7 +114,7 @@ static int establish_proxy_connection(int fd, char *host, int port, char *proxy_ } /* throw away the rest of the HTTP header */ while (1) { - for (cp = buffer; cp < &buffer[sizeof buffer - 1]; cp++) { + for (cp = buffer; cp < &buffer[PROXY_BUF_SIZE]; cp++) { if (read(fd, cp, 1) != 1) { rsyserr(FERROR, errno, "failed to read from proxy"); diff --git a/syscall.c b/syscall.c index ec0e070..e317bcc 100644 --- a/syscall.c +++ b/syscall.c @@ -33,6 +33,11 @@ #include #endif +#ifdef __linux__ +#include +#include +#endif + #include "ifuncs.h" extern int dry_run; @@ -88,6 +93,63 @@ int do_unlink(const char *path) return unlink(path); } +/* + Symlink-race-safe variant of do_unlink() for receiver-side use. See + the comment on do_chmod_at() for the threat model. unlink() resolves + parent components, so a parent-symlink swap can delete an outside + file under the daemon's authority. Defence: open the parent of path + under secure_relative_open() and use unlinkat() (flags=0) against + that dirfd. + + Falls through to do_unlink() for the same dry-run / non-daemon / + chrooted / no-parent / absolute-path cases as the other wrappers. +*/ +int do_unlink_at(const char *path) +{ +#ifdef AT_FDCWD + extern int am_daemon, am_chrooted; + char dirpath[MAXPATHLEN]; + const char *bname; + const char *slash; + int dfd, ret, e; + size_t dlen; + + if (dry_run) return 0; + RETURN_ERROR_IF_RO_OR_LO; + + if (!am_daemon || am_chrooted) + return unlink(path); + + if (!path || !*path || *path == '/') + return unlink(path); + + slash = strrchr(path, '/'); + if (!slash) + return unlink(path); + + dlen = slash - path; + if (dlen >= sizeof dirpath) { + errno = ENAMETOOLONG; + return -1; + } + memcpy(dirpath, path, dlen); + dirpath[dlen] = '\0'; + bname = slash + 1; + + dfd = secure_relative_open(NULL, dirpath, O_RDONLY | O_DIRECTORY, 0); + if (dfd < 0) + return -1; + + ret = unlinkat(dfd, bname, 0); + e = errno; + close(dfd); + errno = e; + return ret; +#else + return do_unlink(path); +#endif +} + #ifdef SUPPORT_LINKS int do_symlink(const char *lnk, const char *path) { @@ -112,6 +174,93 @@ int do_symlink(const char *lnk, const char *path) return symlink(lnk, path); } +/* + Symlink-race-safe variant of do_symlink() for receiver-side use. See + the comment on do_chmod_at() for the threat model. Only the parent + directory of `path` needs protection -- symlinkat() does not resolve + the final component (it creates it). Defence: open parent of `path` + under secure_relative_open() and call symlinkat() against that + dirfd. The link target string `lnk` is stored verbatim and not + resolved at creation time, so it doesn't need scrutiny here. + + Falls through to do_symlink() for the --fake-super (am_root < 0) + path -- that code path opens `path` with do_open() which has its + own (separate) symlink-race exposure tracked elsewhere. +*/ +int do_symlink_at(const char *lnk, const char *path) +{ +#ifdef AT_FDCWD + extern int am_daemon, am_chrooted; + char dirpath[MAXPATHLEN]; + const char *bname; + const char *slash; + int dfd, ret, e; + size_t dlen; + + if (dry_run) return 0; + RETURN_ERROR_IF_RO_OR_LO; + + if (!am_daemon || am_chrooted) + return do_symlink(lnk, path); + + if (!path || !*path || *path == '/') + return do_symlink(lnk, path); + + slash = strrchr(path, '/'); + if (!slash) + return do_symlink(lnk, path); + + dlen = slash - path; + if (dlen >= sizeof dirpath) { + errno = ENAMETOOLONG; + return -1; + } + memcpy(dirpath, path, dlen); + dirpath[dlen] = '\0'; + bname = slash + 1; + + dfd = secure_relative_open(NULL, dirpath, O_RDONLY | O_DIRECTORY, 0); + if (dfd < 0) + return -1; + +#if defined NO_SYMLINK_XATTRS || defined NO_SYMLINK_USER_XATTRS + /* For --fake-super, do_symlink writes the link target into a + * regular file rather than creating a real symlink. Do that + * here against the secure dirfd, with O_NOFOLLOW so a pre- + * planted symlink at the basename can't redirect the file + * creation. (Previously the fake-super branch fell through to + * the bare-path do_symlink at the top of the function.) */ + if (am_root < 0) { + int len = strlen(lnk); + int fd = openat(dfd, bname, + O_WRONLY | O_CREAT | O_TRUNC | O_NOFOLLOW, + S_IWUSR | S_IRUSR); + if (fd < 0) { + e = errno; + close(dfd); + errno = e; + return -1; + } + ret = (write(fd, lnk, len) == len) ? 0 : -1; + if (close(fd) < 0) + ret = -1; + e = errno; + close(dfd); + errno = e; + return ret; + } +#endif + + ret = symlinkat(lnk, dfd, bname); + e = errno; + close(dfd); + errno = e; + return ret; +#else + return do_symlink(lnk, path); +#endif +} + #if defined NO_SYMLINK_XATTRS || defined NO_SYMLINK_USER_XATTRS ssize_t do_readlink(const char *path, char *buf, size_t bufsiz) { @@ -148,6 +297,106 @@ int do_link(const char *old_path, const char *new_path) return link(old_path, new_path); #endif } + +/* + Symlink-race-safe variant of do_link() for receiver-side use. See + the comment on do_chmod_at() for the threat model. link() resolves + parent components of *both* old_path and new_path, so a parent- + symlink swap on either side can plant the new hard link outside + the module, or hard-link an outside file into the module (read + disclosure). + + Defence: open each parent under secure_relative_open() and use + linkat() between the two dirfds, reusing one when the parents + match. flags=0 matches the existing do_link() (don't follow a + symbolic-link old_path). Only available on systems with linkat(); + pre-AT_FDCWD systems fall through to do_link(). +*/ +int do_link_at(const char *old_path, const char *new_path) +{ +#if defined AT_FDCWD && defined HAVE_LINKAT + extern int am_daemon, am_chrooted; + char old_dirpath[MAXPATHLEN], new_dirpath[MAXPATHLEN]; + const char *old_bname, *new_bname; + const char *old_slash, *new_slash; + int old_dfd = AT_FDCWD, new_dfd = AT_FDCWD; + BOOL old_owns = False, new_owns = False; + int ret, e; + size_t old_dlen = 0, new_dlen = 0; + + if (dry_run) return 0; + RETURN_ERROR_IF_RO_OR_LO; + + if (!am_daemon || am_chrooted) + return do_link(old_path, new_path); + + if (!old_path || !*old_path || *old_path == '/' + || !new_path || !*new_path || *new_path == '/') + return do_link(old_path, new_path); + + old_slash = strrchr(old_path, '/'); + new_slash = strrchr(new_path, '/'); + + /* Resolve each path's parent dir independently. A path without a + * slash lives in CWD (AT_FDCWD), no parent open required. A path + * with a slash needs secure_relative_open to confine its parent + * resolution -- otherwise a parent symlink (e.g. --link-dest=cd + * where cd -> /outside) lets the kernel-level linkat(AT_FDCWD, + * "cd/target.txt", ...) escape the module. */ + if (old_slash) { + old_dlen = old_slash - old_path; + if (old_dlen >= sizeof old_dirpath) { errno = ENAMETOOLONG; return -1; } + memcpy(old_dirpath, old_path, old_dlen); + old_dirpath[old_dlen] = '\0'; + old_bname = old_slash + 1; + old_dfd = secure_relative_open(NULL, old_dirpath, O_RDONLY | O_DIRECTORY, 0); + if (old_dfd < 0) + return -1; + old_owns = True; + } else { + old_bname = old_path; + } + + if (new_slash) { + new_dlen = new_slash - new_path; + if (new_dlen >= sizeof new_dirpath) { + e = ENAMETOOLONG; + if (old_owns) close(old_dfd); + errno = e; + return -1; + } + memcpy(new_dirpath, new_path, new_dlen); + new_dirpath[new_dlen] = '\0'; + new_bname = new_slash + 1; + if (old_owns && old_dlen == new_dlen + && memcmp(old_dirpath, new_dirpath, old_dlen) == 0) { + new_dfd = old_dfd; + } else { + new_dfd = secure_relative_open(NULL, new_dirpath, O_RDONLY | O_DIRECTORY, 0); + if (new_dfd < 0) { + e = errno; + if (old_owns) close(old_dfd); + errno = e; + return -1; + } + new_owns = True; + } + } else { + new_bname = new_path; + } + + ret = linkat(old_dfd, old_bname, new_dfd, new_bname, 0); + e = errno; + if (new_owns) + close(new_dfd); + if (old_owns) + close(old_dfd); + errno = e; + return ret; +#else + return do_link(old_path, new_path); +#endif +} #endif int do_lchown(const char *path, uid_t owner, gid_t group) @@ -160,6 +409,66 @@ int do_lchown(const char *path, uid_t owner, gid_t group) return lchown(path, owner, group); } +/* + Symlink-race-safe variant of do_lchown() for receiver-side use. See the + comment on do_chmod_at() for the threat model and design rationale. + + Resolves the parent directory under secure_relative_open() and invokes + fchownat(..., AT_SYMLINK_NOFOLLOW) against that dirfd, so that an + attacker who substitutes a symlink into one of the parent components + cannot redirect the chown outside the receiver's confinement. The + AT_SYMLINK_NOFOLLOW flag matches lchown()'s "do not follow a final- + component symlink" semantics. + + Falls through to do_lchown() in the dry-run / non-daemon / chrooted / + absolute-path / no-parent cases, identical to do_chmod_at(). +*/ +int do_lchown_at(const char *fname, uid_t owner, gid_t group) +{ +#ifdef AT_FDCWD + extern int am_daemon, am_chrooted; + char dirpath[MAXPATHLEN]; + const char *bname; + const char *slash; + int dfd, ret, e; + size_t dlen; + + if (dry_run) return 0; + RETURN_ERROR_IF_RO_OR_LO; + + if (!am_daemon || am_chrooted) + return do_lchown(fname, owner, group); + + if (!fname || !*fname || *fname == '/') + return do_lchown(fname, owner, group); + + slash = strrchr(fname, '/'); + if (!slash) + return do_lchown(fname, owner, group); + + dlen = slash - fname; + if (dlen >= sizeof dirpath) { + errno = ENAMETOOLONG; + return -1; + } + memcpy(dirpath, fname, dlen); + dirpath[dlen] = '\0'; + bname = slash + 1; + + dfd = secure_relative_open(NULL, dirpath, O_RDONLY | O_DIRECTORY, 0); + if (dfd < 0) + return -1; + + ret = fchownat(dfd, bname, owner, group, AT_SYMLINK_NOFOLLOW); + e = errno; + close(dfd); + errno = e; + return ret; +#else + return do_lchown(fname, owner, group); +#endif +} + int do_mknod(const char *pathname, mode_t mode, dev_t dev) { if (dry_run) return 0; @@ -210,6 +519,99 @@ int do_mknod(const char *pathname, mode_t mode, dev_t dev) #endif } +/* + Symlink-race-safe variant of do_mknod() for receiver-side use. See + the comment on do_chmod_at() for the threat model. Defence: open + the parent of pathname under secure_relative_open() and use + mknodat() against that dirfd. mknodat() covers both regular-file + (S_IFREG with dev=0) and FIFO (S_IFIFO) and device-node creation. + + Fake-super (am_root < 0) is handled inline against the secure + parent dirfd: it creates a regular empty file (the same file-as- + metadata-placeholder pattern do_mknod uses) via openat() with + O_NOFOLLOW. Sockets fall through to do_mknod() because their + bind(2) takes a path argument with no portable bindat() variant; + this is documented as a residual. +*/ +int do_mknod_at(const char *pathname, mode_t mode, dev_t dev) +{ +#ifdef AT_FDCWD + extern int am_daemon, am_chrooted; + char dirpath[MAXPATHLEN]; + const char *bname; + const char *slash; + int dfd, ret, e; + size_t dlen; + + if (dry_run) return 0; + RETURN_ERROR_IF_RO_OR_LO; + + if (!am_daemon || am_chrooted) + return do_mknod(pathname, mode, dev); + +#if !defined MKNOD_CREATES_SOCKETS && defined HAVE_SYS_UN_H + if (S_ISSOCK(mode)) + return do_mknod(pathname, mode, dev); +#endif + + if (!pathname || !*pathname || *pathname == '/') + return do_mknod(pathname, mode, dev); + + slash = strrchr(pathname, '/'); + if (!slash) + return do_mknod(pathname, mode, dev); + + dlen = slash - pathname; + if (dlen >= sizeof dirpath) { + errno = ENAMETOOLONG; + return -1; + } + memcpy(dirpath, pathname, dlen); + dirpath[dlen] = '\0'; + bname = slash + 1; + + dfd = secure_relative_open(NULL, dirpath, O_RDONLY | O_DIRECTORY, 0); + if (dfd < 0) + return -1; + + if (am_root < 0) { + /* For --fake-super, do_mknod creates a regular empty + * file as a placeholder for the special-file metadata + * (which is stored in xattrs elsewhere). Do that against + * the secure dirfd, with O_NOFOLLOW so a pre-planted + * symlink at the basename can't redirect the file + * creation. */ + int fd = openat(dfd, bname, + O_WRONLY | O_CREAT | O_TRUNC | O_NOFOLLOW, + S_IWUSR | S_IRUSR); + if (fd < 0) { + e = errno; + close(dfd); + errno = e; + return -1; + } + ret = (close(fd) < 0) ? -1 : 0; + e = errno; + close(dfd); + errno = e; + return ret; + } + +#if !defined MKNOD_CREATES_FIFOS && defined HAVE_MKFIFO + if (S_ISFIFO(mode)) + ret = mkfifoat(dfd, bname, mode); + else +#endif + ret = mknodat(dfd, bname, mode, dev); + e = errno; + close(dfd); + errno = e; + return ret; +#else + return do_mknod(pathname, mode, dev); +#endif +} + int do_rmdir(const char *pathname) { if (dry_run) return 0; @@ -217,6 +619,57 @@ int do_rmdir(const char *pathname) return rmdir(pathname); } +/* + Symlink-race-safe variant of do_rmdir(). See do_unlink_at() above; + same shape but with AT_REMOVEDIR set to require the target be a + directory. +*/ +int do_rmdir_at(const char *pathname) +{ +#ifdef AT_FDCWD + extern int am_daemon, am_chrooted; + char dirpath[MAXPATHLEN]; + const char *bname; + const char *slash; + int dfd, ret, e; + size_t dlen; + + if (dry_run) return 0; + RETURN_ERROR_IF_RO_OR_LO; + + if (!am_daemon || am_chrooted) + return rmdir(pathname); + + if (!pathname || !*pathname || *pathname == '/') + return rmdir(pathname); + + slash = strrchr(pathname, '/'); + if (!slash) + return rmdir(pathname); + + dlen = slash - pathname; + if (dlen >= sizeof dirpath) { + errno = ENAMETOOLONG; + return -1; + } + memcpy(dirpath, pathname, dlen); + dirpath[dlen] = '\0'; + bname = slash + 1; + + dfd = secure_relative_open(NULL, dirpath, O_RDONLY | O_DIRECTORY, 0); + if (dfd < 0) + return -1; + + ret = unlinkat(dfd, bname, AT_REMOVEDIR); + e = errno; + close(dfd); + errno = e; + return ret; +#else + return do_rmdir(pathname); +#endif +} + int do_open(const char *pathname, int flags, mode_t mode) { if (flags != O_RDONLY) { @@ -232,6 +685,76 @@ int do_open(const char *pathname, int flags, mode_t mode) return open(pathname, flags | O_BINARY, mode); } +/* + Symlink-race-safe variant of do_open() for receiver-side use. See + the comment on do_chmod_at() for the threat model. open() resolves + parent components, so a parent-symlink swap can redirect the open + to a file outside the module. This wrapper is defence-in-depth for + bare-path do_open() sites that callers know are otherwise + protected by secure parent-syscalls (e.g. generator.c's in-place + backup creation, where robust_unlink() rejects the symlinked + parent before this open is reached): if any of those upstream + protections is later removed or regresses, the open here still + refuses to escape the module. + + Defence: open the parent of pathname under secure_relative_open() + and call openat() against the resulting dirfd with O_NOFOLLOW + (so the basename itself isn't followed if it happens to be a + pre-planted symlink, which is what we want for O_CREAT|O_EXCL). +*/ +int do_open_at(const char *pathname, int flags, mode_t mode) +{ +#ifdef AT_FDCWD + extern int am_daemon, am_chrooted; + char dirpath[MAXPATHLEN]; + const char *bname; + const char *slash; + int dfd, ret, e; + size_t dlen; + + if (flags != O_RDONLY) { + RETURN_ERROR_IF(dry_run, 0); + RETURN_ERROR_IF_RO_OR_LO; + } + + if (!am_daemon || am_chrooted) + return do_open(pathname, flags, mode); + + if (!pathname || !*pathname || *pathname == '/') + return do_open(pathname, flags, mode); + + slash = strrchr(pathname, '/'); + if (!slash) + return do_open(pathname, flags, mode); + + dlen = slash - pathname; + if (dlen >= sizeof dirpath) { + errno = ENAMETOOLONG; + return -1; + } + memcpy(dirpath, pathname, dlen); + dirpath[dlen] = '\0'; + bname = slash + 1; + + dfd = secure_relative_open(NULL, dirpath, O_RDONLY | O_DIRECTORY, 0); + if (dfd < 0) + return -1; + +#ifdef O_NOATIME + if (open_noatime) + flags |= O_NOATIME; +#endif + + ret = openat(dfd, bname, flags | O_NOFOLLOW | O_BINARY, mode); + e = errno; + close(dfd); + errno = e; + return ret; +#else + return do_open(pathname, flags, mode); +#endif +} + #ifdef HAVE_CHMOD int do_chmod(const char *path, mode_t mode) { @@ -276,6 +799,86 @@ int do_chmod(const char *path, mode_t mode) return code; return 0; } + +/* + Symlink-race-safe variant of do_chmod() for receiver-side use. + + Threat model: on a daemon running with "use chroot = no" (the prerequisite + for CVE-2026-29518), a local attacker can race a symlink swap of one of + the parent directory components of a path the receiver is about to chmod. + Because chmod() resolves symlinks at every component, the swap redirects + the chmod outside the receiver's confinement. + + Defence: open the *parent* directory of fname under secure_relative_open() + (which uses openat2(RESOLVE_BENEATH) on Linux 5.6+, openat() with + O_RESOLVE_BENEATH on FreeBSD 13+ and macOS 15+ (Sequoia), or a per-component + O_NOFOLLOW walk elsewhere) and do fchmodat() against that dirfd. A symlink + substituted into one of the parent components is then either followed + within the tree (legitimate dir-symlinks still work) or rejected by the + kernel (escape attempts fail). + + Final-component handling matches do_chmod(): fchmodat() with flag 0 + follows a symlink at the final component, which is the same behaviour as + chmod() and matches every current call site (the file being chmod'd is + one the receiver itself just created or transferred). For the rare case + where the caller wants to chmod a symlink-as-an-object (S_ISLNK in the + mode bits), we fall through to do_chmod() which has portability code for + that case. + + Falls back to do_chmod() for absolute paths and for paths with no parent + component, where there is nothing to protect against. +*/ +int do_chmod_at(const char *fname, mode_t mode) +{ +#ifdef AT_FDCWD + extern int am_daemon, am_chrooted; + char dirpath[MAXPATHLEN]; + const char *bname; + const char *slash; + int dfd, ret, e; + size_t dlen; + + if (dry_run) return 0; + RETURN_ERROR_IF_RO_OR_LO; + + /* Only the daemon-without-chroot case is exposed to the symlink- + * race attack: a chroot already confines the receiver, and a + * non-daemon rsync runs with the user's own authority so a + * symlink they planted can only redirect to files they could + * already access. Everywhere else, fall through to plain + * do_chmod() to avoid the dirfd-open overhead on every call. */ + if (!am_daemon || am_chrooted) + return do_chmod(fname, mode); + + if (!fname || !*fname || *fname == '/' || S_ISLNK(mode)) + return do_chmod(fname, mode); + + slash = strrchr(fname, '/'); + if (!slash) + return do_chmod(fname, mode); + + dlen = slash - fname; + if (dlen >= sizeof dirpath) { + errno = ENAMETOOLONG; + return -1; + } + memcpy(dirpath, fname, dlen); + dirpath[dlen] = '\0'; + bname = slash + 1; + + dfd = secure_relative_open(NULL, dirpath, O_RDONLY | O_DIRECTORY, 0); + if (dfd < 0) + return -1; + + ret = fchmodat(dfd, bname, mode, 0); + e = errno; + close(dfd); + errno = e; + return ret; +#else + return do_chmod(fname, mode); +#endif +} #endif int do_rename(const char *old_path, const char *new_path) @@ -285,6 +888,89 @@ int do_rename(const char *old_path, const char *new_path) return rename(old_path, new_path); } +/* + Symlink-race-safe variant of do_rename() for receiver-side use. See + the comment on do_chmod_at() for the threat model and design rationale. + + rename() is the central tmp -> final operation in rsync; if either the + source or the destination has an attacker-substituted symlink in one + of its parent components, the rename can publish or vanish files + outside the module. Defence: open the parent of *each* path under + secure_relative_open() and use renameat() against the resulting + dirfds. When old_path and new_path share the same parent (the common + case -- tmp file living next to its final name), we reuse the same + dirfd for both sides. + + Falls through to do_rename() in dry-run, non-daemon, chrooted, no- + parent and absolute-path cases, identical to the other do_*_at() + wrappers. +*/ +int do_rename_at(const char *old_path, const char *new_path) +{ +#ifdef AT_FDCWD + extern int am_daemon, am_chrooted; + char old_dirpath[MAXPATHLEN], new_dirpath[MAXPATHLEN]; + const char *old_bname, *new_bname; + const char *old_slash, *new_slash; + int old_dfd = -1, new_dfd = -1, ret = -1, e; + size_t old_dlen, new_dlen; + + if (dry_run) return 0; + RETURN_ERROR_IF_RO_OR_LO; + + if (!am_daemon || am_chrooted) + return do_rename(old_path, new_path); + + if (!old_path || !*old_path || *old_path == '/' + || !new_path || !*new_path || *new_path == '/') + return do_rename(old_path, new_path); + + old_slash = strrchr(old_path, '/'); + new_slash = strrchr(new_path, '/'); + if (!old_slash || !new_slash) + return do_rename(old_path, new_path); + + old_dlen = old_slash - old_path; + new_dlen = new_slash - new_path; + if (old_dlen >= sizeof old_dirpath || new_dlen >= sizeof new_dirpath) { + errno = ENAMETOOLONG; + return -1; + } + memcpy(old_dirpath, old_path, old_dlen); + old_dirpath[old_dlen] = '\0'; + memcpy(new_dirpath, new_path, new_dlen); + new_dirpath[new_dlen] = '\0'; + old_bname = old_slash + 1; + new_bname = new_slash + 1; + + old_dfd = secure_relative_open(NULL, old_dirpath, O_RDONLY | O_DIRECTORY, 0); + if (old_dfd < 0) + return -1; + + if (old_dlen == new_dlen && memcmp(old_dirpath, new_dirpath, old_dlen) == 0) { + new_dfd = old_dfd; + } else { + new_dfd = secure_relative_open(NULL, new_dirpath, O_RDONLY | O_DIRECTORY, 0); + if (new_dfd < 0) { + e = errno; + close(old_dfd); + errno = e; + return -1; + } + } + + ret = renameat(old_dfd, old_bname, new_dfd, new_bname); + e = errno; + if (new_dfd != old_dfd) + close(new_dfd); + close(old_dfd); + errno = e; + return ret; +#else + return do_rename(old_path, new_path); +#endif +} + #ifdef HAVE_FTRUNCATE int do_ftruncate(int fd, OFF_T size) { @@ -327,6 +1013,66 @@ int do_mkdir(char *path, mode_t mode) return mkdir(path, mode); } +/* + Symlink-race-safe variant of do_mkdir() for receiver-side use. See + the comment on do_chmod_at() for the threat model and design rationale. + + mkdir() resolves parent symlinks at every component, so a parent- + component swap can place an attacker-named directory outside the + module. Defence: open the parent of fname under secure_relative_open() + and call mkdirat() against that dirfd. + + Mutates path in place to trim trailing slashes (matches do_mkdir()). + Falls through to do_mkdir() in dry-run, non-daemon, chrooted, no- + parent and absolute-path cases. +*/ +int do_mkdir_at(char *path, mode_t mode) +{ +#ifdef AT_FDCWD + extern int am_daemon, am_chrooted; + char dirpath[MAXPATHLEN]; + const char *bname; + const char *slash; + int dfd, ret, e; + size_t dlen; + + if (dry_run) return 0; + RETURN_ERROR_IF_RO_OR_LO; + trim_trailing_slashes(path); + + if (!am_daemon || am_chrooted) + return mkdir(path, mode); + + if (!path || !*path || *path == '/') + return mkdir(path, mode); + + slash = strrchr(path, '/'); + if (!slash) + return mkdir(path, mode); + + dlen = slash - path; + if (dlen >= sizeof dirpath) { + errno = ENAMETOOLONG; + return -1; + } + memcpy(dirpath, path, dlen); + dirpath[dlen] = '\0'; + bname = slash + 1; + + dfd = secure_relative_open(NULL, dirpath, O_RDONLY | O_DIRECTORY, 0); + if (dfd < 0) + return -1; + + ret = mkdirat(dfd, bname, mode); + e = errno; + close(dfd); + errno = e; + return ret; +#else + return do_mkdir(path, mode); +#endif +} + /* like mkstemp but forces permissions */ int do_mkstemp(char *template, mode_t perms) { @@ -380,6 +1126,76 @@ int do_lstat(const char *path, STRUCT_STAT *st) #endif } +/* + Symlink-race-safe variants of do_stat() / do_lstat() for receiver- + side use. See the comment on do_chmod_at() for the threat model. + stat() and lstat() resolve parent components, so a parent-symlink + swap can make the receiver's stat see attributes of a victim file + outside the module -- which then drives later behaviour (e.g. + "this isn't a directory, delete it" -> attacker-controlled unlink + on something outside the module). + + Defence: open the parent under secure_relative_open() and use + fstatat() with AT_SYMLINK_NOFOLLOW (lstat) or 0 (stat) against + that dirfd. Same fall-through gating as the other wrappers. +*/ +static int do_xstat_at(const char *path, STRUCT_STAT *st, int at_flags, int (*fallback)(const char *, STRUCT_STAT *)) +{ +#ifdef AT_FDCWD + extern int am_daemon, am_chrooted; + char dirpath[MAXPATHLEN]; + const char *bname; + const char *slash; + int dfd, ret, e; + size_t dlen; + + if (!am_daemon || am_chrooted) + return fallback(path, st); + + if (!path || !*path || *path == '/') + return fallback(path, st); + + slash = strrchr(path, '/'); + if (!slash) + return fallback(path, st); + + dlen = slash - path; + if (dlen >= sizeof dirpath) { + errno = ENAMETOOLONG; + return -1; + } + memcpy(dirpath, path, dlen); + dirpath[dlen] = '\0'; + bname = slash + 1; + + dfd = secure_relative_open(NULL, dirpath, O_RDONLY | O_DIRECTORY, 0); + if (dfd < 0) + return -1; + + ret = fstatat(dfd, bname, st, at_flags); + e = errno; + close(dfd); + errno = e; + return ret; +#else + return fallback(path, st); +#endif +} + +int do_stat_at(const char *path, STRUCT_STAT *st) +{ + return do_xstat_at(path, st, 0, do_stat); +} + +int do_lstat_at(const char *path, STRUCT_STAT *st) +{ +#ifdef SUPPORT_LINKS + return do_xstat_at(path, st, AT_SYMLINK_NOFOLLOW, do_lstat); +#else + return do_xstat_at(path, st, 0, do_stat); +#endif +} + int do_fstat(int fd, STRUCT_STAT *st) { #ifdef USE_STAT64_FUNCS @@ -401,12 +1217,26 @@ OFF_T do_lseek(int fd, OFF_T offset, int whence) #ifdef HAVE_SETATTRLIST int do_setattrlist_times(const char *path, STRUCT_STAT *stp) { + extern int am_daemon, am_chrooted; struct attrlist attrList; struct timespec ts[2]; if (dry_run) return 0; RETURN_ERROR_IF_RO_OR_LO; + /* setattrlist() takes a raw path and follows parent symlinks + * (FSOPT_NOFOLLOW only blocks the final component). On a + * daemon-no-chroot deployment, return ENOSYS so set_times()' + * tier walk falls through to do_utimensat_at(), which routes + * the timestamp update through a secure parent dirfd. The + * macOS-specific attribute set this function would have used + * (ATTR_CMN_MODTIME / ATTR_CMN_ACCTIME) is the same set + * utimensat() handles, so no functionality is lost. */ + if (am_daemon && !am_chrooted) { + errno = ENOSYS; + return -1; + } + /* Yes, this is in the opposite order of utime and similar. */ ts[0].tv_sec = stp->st_mtime; ts[0].tv_nsec = stp->ST_MTIME_NSEC; @@ -423,12 +1253,25 @@ int do_setattrlist_times(const char *path, STRUCT_STAT *stp) #ifdef SUPPORT_CRTIMES int do_setattrlist_crtime(const char *path, time_t crtime) { + extern int am_daemon, am_chrooted; struct attrlist attrList; struct timespec ts; if (dry_run) return 0; RETURN_ERROR_IF_RO_OR_LO; + /* Same path-follows-parent-symlinks concern as + * do_setattrlist_times. There is no portable at-aware variant + * of setattrlist that targets ATTR_CMN_CRTIME, so on a + * daemon-no-chroot deployment we return -1 and accept that + * crtime preservation is silently dropped for that file (the + * caller treats this as "crtime not updated"). The transfer + * itself continues normally. */ + if (am_daemon && !am_chrooted) { + errno = ENOSYS; + return -1; + } + ts.tv_sec = crtime; ts.tv_nsec = 0; @@ -444,10 +1287,19 @@ int do_setattrlist_crtime(const char *path, time_t crtime) time_t get_create_time(const char *path, STRUCT_STAT *stp) { #ifdef HAVE_GETATTRLIST + extern int am_daemon, am_chrooted; static struct create_time attrBuf; struct attrlist attrList; (void)stp; + /* getattrlist() is also path-based and follows parent + * symlinks. In daemon-no-chroot, refuse rather than read the + * crtime of a file the parent-symlink chain might point at + * outside the module. The caller's "no crtime available" + * path returns 0; the file gets a fresh crtime instead of + * preserving the source's. */ + if (am_daemon && !am_chrooted) + return 0; memset(&attrList, 0, sizeof attrList); attrList.bitmapcount = ATTR_BIT_MAP_COUNT; attrList.commonattr = ATTR_CMN_CRTIME; @@ -513,6 +1365,81 @@ int do_utimensat(const char *path, STRUCT_STAT *stp) #endif return utimensat(AT_FDCWD, path, t, AT_SYMLINK_NOFOLLOW); } + +/* + Symlink-race-safe variant of do_utimensat() for receiver-side use. + See the comment on do_chmod_at() for the threat model. utimes() + resolves parent components and follows a final-component symlink; + lutimes() doesn't follow the final component but still resolves + parents. Either way, a parent-symlink swap can redirect the + timestamp update outside the module. Defence: open the parent of + path under secure_relative_open() and call utimensat() with + AT_SYMLINK_NOFOLLOW against that dirfd. + + Falls through to do_utimensat() in the same dry-run / non-daemon / + chrooted / no-parent / absolute-path cases as the other wrappers. + Returns -1 with errno=ENOSYS on systems without utimensat() + (caller is expected to fall back to the legacy tier walk). +*/ +int do_utimensat_at(const char *path, STRUCT_STAT *stp) +{ +#ifdef AT_FDCWD + extern int am_daemon, am_chrooted; + struct timespec t[2]; + char dirpath[MAXPATHLEN]; + const char *bname; + const char *slash; + int dfd, ret, e; + size_t dlen; + + if (dry_run) return 0; + RETURN_ERROR_IF_RO_OR_LO; + + if (!am_daemon || am_chrooted) + return do_utimensat(path, stp); + + if (!path || !*path || *path == '/') + return do_utimensat(path, stp); + + slash = strrchr(path, '/'); + if (!slash) + return do_utimensat(path, stp); + + dlen = slash - path; + if (dlen >= sizeof dirpath) { + errno = ENAMETOOLONG; + return -1; + } + memcpy(dirpath, path, dlen); + dirpath[dlen] = '\0'; + bname = slash + 1; + + t[0].tv_sec = stp->st_atime; +#ifdef ST_ATIME_NSEC + t[0].tv_nsec = stp->ST_ATIME_NSEC; +#else + t[0].tv_nsec = 0; +#endif + t[1].tv_sec = stp->st_mtime; +#ifdef ST_MTIME_NSEC + t[1].tv_nsec = stp->ST_MTIME_NSEC; +#else + t[1].tv_nsec = 0; +#endif + + dfd = secure_relative_open(NULL, dirpath, O_RDONLY | O_DIRECTORY, 0); + if (dfd < 0) + return -1; + + ret = utimensat(dfd, bname, t, AT_SYMLINK_NOFOLLOW); + e = errno; + close(dfd); + errno = e; + return ret; +#else + return do_utimensat(path, stp); +#endif +} #endif #ifdef HAVE_LUTIMES @@ -720,12 +1647,125 @@ int do_open_nofollow(const char *pathname, int flags) /* open a file relative to a base directory. The basedir can be NULL, in which case the current working directory is used. The relpath - must be a relative path, and the relpath must not contain any - elements in the path which follow symlinks (ie. like O_NOFOLLOW, but - applies to all path components, not just the last component) - - The relpath must also not contain any ../ elements in the path + must be a relative path. The kernel must guarantee that resolution + cannot escape basedir (or the cwd, when basedir is NULL): no ".." + jumps above the start, no symlinks pointing outside, no absolute + paths, no /proc magic-link tricks. + + Symlinks *within* basedir are followed normally — earlier rsync + versions rejected every symlink with O_NOFOLLOW on each component, + which broke legitimate directory symlinks on the receiver side + (https://github.com/RsyncProject/rsync/issues/715). The escape + prevention is handled by: + Linux 5.6+: openat2(RESOLVE_BENEATH) + FreeBSD 13+: openat() with O_RESOLVE_BENEATH + macOS 15+ / iOS 18+: openat() with O_RESOLVE_BENEATH (same + flag name, picked up by the same #ifdef; + flag value differs from FreeBSD) + Other systems fall back to the per-component O_NOFOLLOW walk below. + + The relpath must also not contain any ../ elements in the path. */ + +/* Returns 1 if path has any "/"-separated component that is exactly + * "..", 0 otherwise. Used by secure_relative_open's front-door + * validation to reject inputs that the per-component walk fallback + * would otherwise resolve through ".." -- e.g. bare "..", "foo/..", + * "subdir/.." -- which RESOLVE_BENEATH-equivalent kernels reject in + * the kernel but the per-component fallback (NetBSD/OpenBSD/Solaris/ + * Cygwin/pre-5.6 Linux) does not. */ +static int path_has_dotdot_component(const char *path) +{ + const char *p = path; + + while (*p) { + const char *q; + if (*p == '/') { p++; continue; } + q = p; + while (*q && *q != '/') + q++; + if (q - p == 2 && p[0] == '.' && p[1] == '.') + return 1; + p = q; + } + return 0; +} + +#ifdef __linux__ +static int secure_relative_open_linux(const char *basedir, const char *relpath, int flags, mode_t mode) +{ + struct open_how how; + int dirfd, retfd; + + memset(&how, 0, sizeof how); + how.flags = flags; + how.mode = mode; + how.resolve = RESOLVE_BENEATH | RESOLVE_NO_MAGICLINKS; + + if (basedir == NULL) { + dirfd = AT_FDCWD; + } else if (basedir[0] == '/') { + /* Absolute basedir: operator-trusted (module_dir and the + * like). Plain openat. */ + dirfd = openat(AT_FDCWD, basedir, O_RDONLY | O_DIRECTORY); + if (dirfd == -1) + return -1; + } else { + /* Relative basedir: may be wire-influenced via + * --link-dest / --copy-dest / --compare-dest. Resolve it + * under the same RESOLVE_BENEATH guarantee as relpath, so + * a parent symlink on basedir cannot redirect the dirfd + * outside the CWD anchor. */ + struct open_how bhow; + memset(&bhow, 0, sizeof bhow); + bhow.flags = O_RDONLY | O_DIRECTORY; + bhow.resolve = RESOLVE_BENEATH | RESOLVE_NO_MAGICLINKS; + dirfd = syscall(SYS_openat2, AT_FDCWD, basedir, &bhow, sizeof bhow); + if (dirfd == -1) + return -1; + } + + retfd = syscall(SYS_openat2, dirfd, relpath, &how, sizeof how); + + if (dirfd != AT_FDCWD) + close(dirfd); + return retfd; +} +#endif + +#ifdef O_RESOLVE_BENEATH +/* FreeBSD 13+ and macOS 15+ (Sequoia) / iOS 18+: O_RESOLVE_BENEATH is + * an openat() flag with the same "must not escape dirfd" semantics as + * Linux's RESOLVE_BENEATH. The kernel rejects ".." escapes, absolute + * symlinks, and symlinks whose target lies outside dirfd. (FreeBSD and + * Apple use different flag bit values, but the same symbolic name.) */ +static int secure_relative_open_resolve_beneath(const char *basedir, const char *relpath, int flags, mode_t mode) +{ + int dirfd, retfd; + + if (basedir == NULL) { + dirfd = AT_FDCWD; + } else if (basedir[0] == '/') { + /* Absolute basedir: operator-trusted, plain openat. */ + dirfd = openat(AT_FDCWD, basedir, O_RDONLY | O_DIRECTORY); + if (dirfd == -1) + return -1; + } else { + /* Relative basedir: confine its resolution beneath CWD + * (see secure_relative_open_linux for the rationale). */ + dirfd = openat(AT_FDCWD, basedir, O_RDONLY | O_DIRECTORY | O_RESOLVE_BENEATH); + if (dirfd == -1) + return -1; + } + + retfd = openat(dirfd, relpath, flags | O_RESOLVE_BENEATH, mode); + + if (dirfd != AT_FDCWD) + close(dirfd); + return retfd; +} +#endif + int secure_relative_open(const char *basedir, const char *relpath, int flags, mode_t mode) { if (!relpath || relpath[0] == '/') { @@ -733,11 +1773,37 @@ int secure_relative_open(const char *basedir, const char *relpath, int flags, mo errno = EINVAL; return -1; } - if (strncmp(relpath, "../", 3) == 0 || strstr(relpath, "/../")) { - // no ../ elements allowed in the relpath + /* Reject any path with a literal ".." component (bare "..", + * "../foo", "foo/..", "foo/../bar", "subdir/.."). The previous + * substring-based check caught only "../" prefix and "/../" + * substring; bare ".." and trailing "/.." escape on the per- + * component walk fallback used by NetBSD/OpenBSD/Solaris/Cygwin + * and pre-5.6 Linux. RESOLVE_BENEATH on Linux/FreeBSD/macOS + * catches some of these in-kernel with EXDEV, but the front + * door must reject them consistently with EINVAL across all + * platforms so callers can rely on the validation. */ + if (path_has_dotdot_component(relpath)) { errno = EINVAL; return -1; } + if (basedir && basedir[0] != '/' && path_has_dotdot_component(basedir)) { + errno = EINVAL; + return -1; + } + +#ifdef __linux__ + { + int fd = secure_relative_open_linux(basedir, relpath, flags, mode); + /* ENOSYS = kernel < 5.6 doesn't have the syscall even though + * glibc/kernel-headers do; fall through to the portable path. */ + if (fd != -1 || errno != ENOSYS) + return fd; + } +#endif + +#ifdef O_RESOLVE_BENEATH + return secure_relative_open_resolve_beneath(basedir, relpath, flags, mode); +#endif #if !defined(O_NOFOLLOW) || !defined(O_DIRECTORY) || !defined(AT_FDCWD) // really old system, all we can do is live with the risks @@ -750,15 +1816,47 @@ int secure_relative_open(const char *basedir, const char *relpath, int flags, mo #else int dirfd = AT_FDCWD; if (basedir != NULL) { - dirfd = openat(AT_FDCWD, basedir, O_RDONLY | O_DIRECTORY); - if (dirfd == -1) { - return -1; + if (basedir[0] == '/') { + /* Absolute basedir: operator-trusted, plain openat. */ + dirfd = openat(AT_FDCWD, basedir, O_RDONLY | O_DIRECTORY); + if (dirfd == -1) { + return -1; + } + } else { + /* Relative basedir: walk it component-by-component + * with O_NOFOLLOW. This is the per-component + * RESOLVE_BENEATH equivalent for platforms without + * kernel-supported confinement, and matches the + * relpath walk below. Symlinks in basedir are + * rejected outright on this fallback path; the + * Linux openat2 / O_RESOLVE_BENEATH paths above + * still allow within-tree symlinks. */ + char *bcopy = my_strdup(basedir, __FILE__, __LINE__); + if (!bcopy) + return -1; + for (const char *part = strtok(bcopy, "/"); + 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; + if (dirfd != AT_FDCWD) close(dirfd); + free(bcopy); + errno = save_errno; + return -1; + } + if (dirfd != AT_FDCWD) close(dirfd); + dirfd = next_fd; + } + free(bcopy); } } int retfd = -1; char *path_copy = my_strdup(relpath, __FILE__, __LINE__); if (!path_copy) { + if (dirfd != AT_FDCWD) close(dirfd); return -1; } @@ -784,8 +1882,15 @@ int secure_relative_open(const char *basedir, const char *relpath, int flags, mo dirfd = next_fd; } - // the path must be a directory - errno = EINVAL; + /* All components walked as directories. If the caller asked for + * O_DIRECTORY, return the dirfd we built up; otherwise the path + * resolved to a directory but the caller wanted a regular file. */ + if ((flags & O_DIRECTORY) && dirfd != AT_FDCWD) { + retfd = dirfd; + dirfd = AT_FDCWD; + goto cleanup; + } + errno = EISDIR; cleanup: free(path_copy); @@ -796,6 +1901,145 @@ int secure_relative_open(const char *basedir, const char *relpath, int flags, mo #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 diff --git a/t_chmod_secure.c b/t_chmod_secure.c new file mode 100644 index 0000000..114dfb2 --- /dev/null +++ b/t_chmod_secure.c @@ -0,0 +1,117 @@ +/* + * Test harness for do_chmod_at(). Confirms the symlink-TOCTOU + * primitive used by CVE-2026-29518 (and its incomplete-fix follow-up + * for chmod) is closed by do_chmod_at(): a parent directory component + * being a symlink that escapes the receiver's confinement must be + * rejected, while a parent symlink that resolves *within* the tree + * must still work (so legitimate dir-symlinks are not regressed). + * + * Not linked into rsync itself. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include "rsync.h" + +#include + +int dry_run = 0; +int am_root = 0; +int am_sender = 0; +int read_only = 0; +int list_only = 0; +int copy_links = 0; +int copy_unsafe_links = 0; +extern int am_daemon, am_chrooted; + +short info_levels[COUNT_INFO], debug_levels[COUNT_DEBUG]; + +static int errs = 0; + +static void check(const char *label, int actual_rc, int expect_ok, + const char *path, mode_t expected_mode) +{ + struct stat st; + int got_ok = (actual_rc == 0); + if (got_ok != expect_ok) { + fprintf(stderr, "FAIL [%s]: rc=%d errno=%d (%s), expected %s\n", + label, actual_rc, errno, strerror(errno), + expect_ok ? "success" : "rejection"); + errs++; + return; + } + if (path && stat(path, &st) < 0) { + fprintf(stderr, "FAIL [%s]: stat(%s) failed: %s\n", + label, path, strerror(errno)); + errs++; + return; + } + if (path && (st.st_mode & 07777) != expected_mode) { + fprintf(stderr, + "FAIL [%s]: %s mode is 0%o, expected 0%o\n", + label, path, st.st_mode & 07777, expected_mode); + errs++; + return; + } + fprintf(stderr, "OK [%s]\n", label); +} + +int main(int argc, char **argv) +{ + if (argc != 2) { + fprintf(stderr, "usage: %s \n", argv[0]); + return 2; + } + if (chdir(argv[1]) < 0) { + perror("chdir"); + return 2; + } + + /* Simulate the daemon-without-chroot deployment that do_chmod_at() + * defends. With am_daemon=0 or am_chrooted=1 the wrapper falls + * through to plain do_chmod() and the symlink-race test would be + * meaningless. */ + am_daemon = 1; + am_chrooted = 0; + + /* Test layout (all inside the directory we just chdir'd to): + * + * ./realdir/sentinel -- regular target file + * ./inside_link -> realdir -- legitimate dir-symlink within the tree + * ./escape_link -> ../trap -- attacker swap, target outside tree + * ../trap/sentinel -- the file the attacker wants to alter + * + * The shell wrapper that calls this helper has set both sentinel + * files to mode 0600 so we have a clean baseline to compare. + */ + + /* Scenario A: legitimate parent dir-symlink, chmod must succeed. */ + int rc = do_chmod_at("inside_link/sentinel", 0640); + check("A: legit dir-symlink within tree", + rc, 1, "realdir/sentinel", 0640); + + /* Scenario B: parent symlink escapes the tree -- chmod must be + * rejected and the outside file's mode must be unchanged. */ + rc = do_chmod_at("escape_link/sentinel", 0666); + check("B: parent symlink escapes tree (the attack)", + rc, 0, "../trap/sentinel", 0600); + + /* Scenario C: plain relative path with no symlink components, + * regression check that the safe wrapper doesn't break the + * normal case. */ + rc = do_chmod_at("realdir/sentinel", 0644); + check("C: plain relative path (regression check)", + rc, 1, "realdir/sentinel", 0644); + + /* Scenario D: top-level file, no parent directory component. + * Falls back to do_chmod(); should succeed. */ + rc = do_chmod_at("topfile", 0640); + check("D: top-level file, no parent component", + rc, 1, "topfile", 0640); + + if (errs) + fprintf(stderr, "%d failure(s)\n", errs); + return errs ? 1 : 0; +} diff --git a/t_secure_relpath.c b/t_secure_relpath.c new file mode 100644 index 0000000..a0fdf0d --- /dev/null +++ b/t_secure_relpath.c @@ -0,0 +1,151 @@ +/* + * Test harness for secure_relative_open()'s front-door input + * validation. Codex audit Finding 5 noted that the existing check + * + * if (strncmp(relpath, "../", 3) == 0 || strstr(relpath, "/../")) + * + * catches "../foo" and "foo/../bar" but misses bare ".." (an actual + * one-level escape on platforms that fall back to the per-component + * walk), as well as "a/..", "foo/..", and any other form that + * decomposes to a ".." component when split on "/". The kernel- + * enforced RESOLVE_BENEATH (Linux 5.6+) and O_RESOLVE_BENEATH + * (FreeBSD 13+, macOS 15+) reject these in-kernel; the per- + * component fallback used on NetBSD, OpenBSD, Solaris, Cygwin and + * pre-5.6 Linux does not, so the validation must happen at the + * front door. + * + * This helper invokes secure_relative_open() with each suspect + * input and checks both the failure (rc < 0) and the errno + * (EINVAL means "rejected at the front door"). Pre-fix, the kernel + * may reject with a different errno (EXDEV from RESOLVE_BENEATH); + * post-fix, the front-door check catches every variant up front + * with a consistent EINVAL across platforms. + * + * Not linked into rsync itself. + */ + +#include "rsync.h" + +#include + +int dry_run = 0; +int am_root = 0; +int am_sender = 0; +int read_only = 0; +int list_only = 0; +int copy_links = 0; +int copy_unsafe_links = 0; +extern int am_daemon, am_chrooted; + +short info_levels[COUNT_INFO], debug_levels[COUNT_DEBUG]; + +static int errs = 0; + +static void check_relpath(const char *relpath) +{ + int fd; + int saved_errno; + + errno = 0; + fd = secure_relative_open(NULL, relpath, O_RDONLY | O_DIRECTORY, 0); + saved_errno = errno; + + if (fd >= 0) { + fprintf(stderr, + "FAIL [relpath=%-12s]: returned valid fd %d (escape) -- expected -1 EINVAL\n", + relpath, fd); + close(fd); + errs++; + return; + } + + if (saved_errno != EINVAL) { + fprintf(stderr, + "FAIL [relpath=%-12s]: rejected but errno=%d (%s), expected EINVAL\n", + relpath, saved_errno, strerror(saved_errno)); + errs++; + return; + } + + fprintf(stderr, "OK [relpath=%-12s]: rejected with EINVAL\n", relpath); +} + +static void check_basedir(const char *basedir) +{ + int fd; + int saved_errno; + + errno = 0; + fd = secure_relative_open(basedir, "ok", O_RDONLY | O_DIRECTORY, 0); + saved_errno = errno; + + if (fd >= 0) { + fprintf(stderr, + "FAIL [basedir=%-12s]: returned valid fd %d -- expected -1 EINVAL\n", + basedir, fd); + close(fd); + errs++; + return; + } + + if (saved_errno != EINVAL) { + fprintf(stderr, + "FAIL [basedir=%-12s]: rejected but errno=%d (%s), expected EINVAL\n", + basedir, saved_errno, strerror(saved_errno)); + errs++; + return; + } + + fprintf(stderr, "OK [basedir=%-12s]: rejected with EINVAL\n", basedir); +} + +int main(int argc, char **argv) +{ + if (argc != 2) { + fprintf(stderr, "usage: %s \n", argv[0]); + return 2; + } + if (chdir(argv[1]) < 0) { + perror("chdir"); + return 2; + } + + /* secure_relative_open's daemon-only confinement protections only + * fire when am_daemon && !am_chrooted (the threat model is the + * daemon-no-chroot deployment), but the front-door input + * validation runs unconditionally. We set am_daemon anyway so the + * helper exercises the same code shape the receiver does. */ + am_daemon = 1; + am_chrooted = 0; + + mkdir("subdir", 0755); + + /* Each of these relpaths must be rejected with EINVAL at the + * secure_relative_open() front door. ".." is the actual one-level + * escape; the others ("subdir/..", "subdir/../subdir") resolve + * back to the start dir on systems that allow them, but we still + * reject them as defence-in-depth: a path containing a ".." token + * is suspicious and the caller should normalise before passing + * it in. The "../foo" / "foo/../bar" / "/foo" / "/" cases are + * regression checks for the existing checks. */ + check_relpath(".."); + check_relpath("../foo"); + check_relpath("subdir/.."); + check_relpath("subdir/../subdir"); + check_relpath("foo/../bar"); + check_relpath("/foo"); + check_relpath("/"); + + /* Same checks against basedir (which the codex Finding 2 fix + * routes through the same RESOLVE_BENEATH-equivalent). Absolute + * basedirs are operator-trusted and intentionally not validated + * here. */ + check_basedir(".."); + check_basedir("../subdir"); + check_basedir("subdir/.."); + check_basedir("foo/../bar"); + + if (errs) + fprintf(stderr, "\n%d failure(s)\n", errs); + return errs ? 1 : 0; +} diff --git a/t_stub.c b/t_stub.c index eee9272..63bc144 100644 --- a/t_stub.c +++ b/t_stub.c @@ -23,6 +23,8 @@ int do_fsync = 0; int inplace = 0; +int am_daemon = 0; +int am_chrooted = 0; int modify_window = 0; int preallocate_files = 0; int protect_args = 0; diff --git a/testsuite/alt-dest-symlink-race.test b/testsuite/alt-dest-symlink-race.test new file mode 100755 index 0000000..fd36c6e --- /dev/null +++ b/testsuite/alt-dest-symlink-race.test @@ -0,0 +1,113 @@ +#!/bin/sh + +# Copyright (C) 2026 by Andrew Tridgell + +# This program is distributable under the terms of the GNU GPL (see +# COPYING). + +# Regression test for the basedir-confinement gap in +# secure_relative_open(). The function opens basedir with a plain +# openat(AT_FDCWD, basedir, O_RDONLY | O_DIRECTORY), without +# RESOLVE_BENEATH or a per-component O_NOFOLLOW walk, so a parent +# symlink ON basedir is followed unrestrictedly. RESOLVE_BENEATH is +# then applied only to relpath, anchored at the wrong directory. +# +# The receiver's basis-file lookup at receiver.c passes +# basis_dir[fnamecmp_type] (from --copy-dest / --link-dest / +# --compare-dest -- all sender-controllable in daemon mode) as +# basedir. A daemon-module attacker with write access can plant a +# symlink at module/cd -> /outside, then run --link-dest=cd to +# make the daemon's basis-file lookup resolve into /outside, +# leaking the contents of daemon-readable files via the rsync +# delta-rolling read-disclosure primitive. +# +# We detect the escape by leveraging --link-dest: when basis +# matches source exactly (content + mtime + mode), --link-dest +# hard-links the destination to the basis file. With the bug, the +# destination ends up as a hard link to the outside-the-module +# file (same inode). With the fix, no basis is found and the +# destination is a fresh copy (different inode). +# +# The vulnerable code path is the same on every platform +# (including the per-component fallback on systems without +# RESOLVE_BENEATH), so this test is not platform-gated. + +. "$suitedir/rsync.fns" + +mod="$scratchdir/module" +outside="$scratchdir/outside" +src="$scratchdir/src" +conf="$scratchdir/test-rsyncd.conf" + +rm -rf "$mod" "$outside" "$src" +mkdir -p "$mod" "$outside" "$src" + +# Portable inode-number helper (GNU coreutils stat -c, BSD stat -f). +file_inode() { + stat -c %i "$1" 2>/dev/null || stat -f %i "$1" +} + +# Outside-the-module file an attacker would like the daemon to +# treat as a basis. +echo "OUTSIDE_SECRET_DATA" > "$outside/target.txt" +chmod 0644 "$outside/target.txt" + +# The symlink trap planted in the module by the local attacker. +ln -s "$outside" "$mod/cd" + +# Source file matches outside/target.txt exactly (content + mtime +# + mode) so --link-dest will hard-link the destination to the +# basis file iff the daemon's basedir lookup reaches outside/. +echo "OUTSIDE_SECRET_DATA" > "$src/target.txt" +touch -r "$outside/target.txt" "$src/target.txt" +chmod 0644 "$src/target.txt" + +# When running as root the daemon would drop to "nobody" by +# default, which can't write into the test scratch dir. Force the +# daemon to keep our uid/gid in that case so the basis-link +# transfer can actually create the destination file. (Non-root +# can't specify uid/gid in rsyncd.conf -- comment them out then.) +my_uid=`get_testuid` +root_uid=`get_rootuid` +root_gid=`get_rootgid` +uid_setting="uid = $root_uid" +gid_setting="gid = $root_gid" +if test x"$my_uid" != x"$root_uid"; then + uid_setting="#$uid_setting" + gid_setting="#$gid_setting" +fi + +cat > "$conf" </dev/null 2>&1 || true + +if [ ! -f "$mod/target.txt" ]; then + test_fail "destination file was not created -- daemon transfer failed before the test could observe the basedir behaviour" +fi + +outside_inode=$(file_inode "$outside/target.txt") +dst_inode=$(file_inode "$mod/target.txt") + +if [ "$outside_inode" = "$dst_inode" ]; then + test_fail "basedir-escape: --link-dest hard-linked module/target.txt to outside/target.txt (inode $outside_inode); daemon's basis-file lookup followed the parent symlink on the basedir" +fi + +exit 0 diff --git a/testsuite/bare-do-open-symlink-race.test b/testsuite/bare-do-open-symlink-race.test new file mode 100755 index 0000000..e295223 --- /dev/null +++ b/testsuite/bare-do-open-symlink-race.test @@ -0,0 +1,206 @@ +#!/bin/sh + +# Copyright (C) 2026 by Andrew Tridgell + +# This program is distributable under the terms of the GNU GPL (see +# COPYING). + +# Regression test for codex audit Findings 3b and 3c: +# +# 3b: generator.c:1905 -- the in-place backup creation opens +# backupptr via bare do_open(O_WRONLY|O_CREAT|O_TRUNC|O_EXCL). +# With --backup-dir set to an attacker-planted parent symlink, +# the backup file is written outside the module under the +# daemon's authority. +# +# 3c-symlink: syscall.c:207 -- do_symlink_at falls through to bare +# do_symlink for am_root < 0 (fake-super), which then opens +# the destination path with bare open() (final-component +# fake-super file). A parent symlink on the destination path +# redirects the file creation outside the module. +# +# 3c-mknod: syscall.c:506 -- do_mknod_at falls through to bare +# do_mknod for am_root < 0, same path-based open(). For +# FIFOs/sockets/devices the bare path is also used. +# +# Each scenario plants a "secret" file outside the module at a +# location the symlink trap points to. The check is that the +# outside file's content and mode are unchanged after the attack +# attempt. + +. "$suitedir/rsync.fns" + +# All three scenarios depend on receiver-side daemon code paths +# that are only secured on platforms with a working +# secure_relative_open. The chdir/chmod tests already skip the +# same set; mirror that. +case "$(uname -s)" in + SunOS|OpenBSD|NetBSD|CYGWIN*) + test_skipped "secure_relative_open relies on RESOLVE_BENEATH-equivalent kernel support not available on $(uname -s)" + ;; +esac + +mod="$scratchdir/module" +outside="$scratchdir/outside" +src="$scratchdir/src" +conf="$scratchdir/test-rsyncd.conf" + +# Portable inode-and-mode helpers. +file_mode() { + stat -c %a "$1" 2>/dev/null || stat -f %Lp "$1" +} + +setup() { + rm -rf "$mod" "$outside" "$src" + mkdir -p "$mod" "$outside" "$src" + + echo "OUTSIDE_PROTECTED_DATA" > "$outside/target.txt" + chmod 0644 "$outside/target.txt" + outside_pristine="$scratchdir/outside-pristine.txt" + cp -p "$outside/target.txt" "$outside_pristine" + + ln -s "$outside" "$mod/cd" +} + +verify_outside_unchanged() { + label="$1" + mode=$(file_mode "$outside/target.txt") + case "$mode" in + 644|0644) ;; + *) test_fail "$label: outside/target.txt mode changed from 644 to $mode" ;; + esac + if ! cmp -s "$outside/target.txt" "$outside_pristine"; then + test_fail "$label: outside/target.txt content changed -- daemon followed the cd symlink" + fi +} + +verify_outside_unchanged_or_absent() { + label="$1" + target="$2" # specific file under outside/ to check absence of + if [ -e "$outside/$target" ]; then + test_fail "$label: outside/$target was created -- daemon followed the cd symlink" + fi +} + +# When running as root the daemon would drop to "nobody" by default +# and fail to write into the test scratch dir. Force it to keep our +# uid/gid in that case so the receiver actually runs the code paths +# we want to test. +my_uid=`get_testuid` +root_uid=`get_rootuid` +root_gid=`get_rootgid` +uid_setting="uid = $root_uid" +gid_setting="gid = $root_gid" +if test x"$my_uid" != x"$root_uid"; then + uid_setting="#$uid_setting" + gid_setting="#$gid_setting" +fi + + +############################################################ +# Scenario 3b: --inplace --backup --backup-dir=cd +# +# Pre-create module/target.txt so the receiver enters the in-place +# update path; a backup of the existing content must be made +# before the update. With --backup-dir=cd, backupptr resolves to +# "cd/target.txt"; with the bug, robust_unlink and the bare +# do_open at generator.c:1905 both follow the cd symlink, the +# unlink deletes outside/target.txt and the create writes the +# pre-existing module/target.txt content there. +############################################################ + +setup +echo "EXISTING_MODULE_DATA" > "$mod/target.txt" +chmod 0666 "$mod/target.txt" +echo "NEW_DATA_FROM_SENDER" > "$src/target.txt" +chmod 0644 "$src/target.txt" + +cat > "$conf" </dev/null 2>&1 || true + +verify_outside_unchanged "3b inplace+backup-dir=cd" + + +############################################################ +# Scenario 3c-symlink: fake-super symlink push to a path with a +# symlinked parent +# +# With "fake super = yes" set on the module, the receiver +# represents symlinks as fake-super files (regular files with the +# link target written to them). The path-based open() in +# do_symlink's fake-super branch follows parent symlinks. We push +# a single symlink to the destination path "cd/sym" so the +# receiver's create-file call lands at "cd/sym" relative to the +# module root, where cd is the symlink trap. +############################################################ + +setup + +mkdir -p "$src/cd" +ln -s /etc/passwd "$src/cd/sym" + +cat > "$conf" </dev/null 2>&1 || true + +verify_outside_unchanged_or_absent "3c-symlink fake-super symlink push" "sym" + + +############################################################ +# Scenario 3c-mknod: fake-super FIFO push to a path with a +# symlinked parent +# +# Similar to 3c-symlink but for special files. mkfifo works +# without root; we push a FIFO and verify the receiver doesn't +# create a fake-super file at outside/fifo. +############################################################ + +setup + +mkdir -p "$src/cd" +mkfifo "$src/cd/fifo" 2>/dev/null +if [ ! -p "$src/cd/fifo" ]; then + test_skipped "mkfifo unavailable; cannot exercise 3c-mknod" +fi + +cat > "$conf" </dev/null 2>&1 || true + +verify_outside_unchanged_or_absent "3c-mknod fake-super FIFO push" "fifo" + +exit 0 diff --git a/testsuite/chdir-symlink-race.test b/testsuite/chdir-symlink-race.test new file mode 100755 index 0000000..f5d4cb3 --- /dev/null +++ b/testsuite/chdir-symlink-race.test @@ -0,0 +1,135 @@ +#!/bin/sh + +# Copyright (C) 2026 by Andrew Tridgell + +# This program is distributable under the terms of the GNU GPL (see +# COPYING). + +# Regression test for the symlink-TOCTOU class of bug at the receiver's +# chdir(). After the CVE-2026-29518 fix to secure_relative_open(), an +# attack remained where the receiver's chdir() into a destination +# subdirectory followed an attacker-planted symlink, escaping the +# module. Every subsequent path-relative syscall (open, chmod, lchown, +# utimes, etc.) inherited the escape -- secure_relative_open's +# RESOLVE_BENEATH anchor itself was outside the module by then, so it +# stopped protecting against anything. +# +# This test runs an actual rsync daemon (via RSYNC_CONNECT_PROG to +# avoid the network) configured with "use chroot = no", plants a +# symlink at module/subdir -> ../outside, and runs four flavours of +# rsync transfer that previously all reached files in ../outside: +# +# 1. single-file dest = subdir/target.txt (the original poc_chmod) +# 2. -r src/subdir/ to upload/subdir/ (the chdir-escape case) +# 3. -r src/subdir/ to upload/subdir/ (no --size-only: forces basis read+write) +# 4. -r src/ to upload/ (was already protected by the +# original CVE-2026-29518 fix; +# regression-checked here) +# +# All four must leave the outside-the-module sentinel file's mode AND +# content unchanged. + +. "$suitedir/rsync.fns" + +case "$(uname -s)" in + SunOS|OpenBSD|NetBSD|CYGWIN*) + test_skipped "secure chdir relies on RESOLVE_BENEATH-equivalent kernel support not available on $(uname -s)" + ;; +esac + +mod="$scratchdir/module" +outside="$scratchdir/outside" +src="$scratchdir/src" +conf="$scratchdir/test-rsyncd.conf" + +rm -rf "$mod" "$outside" "$src" +mkdir -p "$mod" "$outside" "$src" "$src/subdir" + +# Portable octal-mode helper -- macOS and FreeBSD's stat use -f, GNU +# coreutils stat uses -c. +file_mode() { + stat -c %a "$1" 2>/dev/null || stat -f %Lp "$1" +} + +# The "secret" file outside the module the attacker is trying to alter. +# Save a pristine copy alongside it so we can compare with cmp(1) rather +# than depending on sha1sum/shasum/sha1, which differ across platforms. +echo "OUTSIDE_SECRET_DATA" > "$outside/target.txt" +chmod 0600 "$outside/target.txt" +outside_pristine="$scratchdir/outside-pristine.txt" +cp -p "$outside/target.txt" "$outside_pristine" + +# Symlink trap planted in the module by the local attacker. +ln -s "$outside" "$mod/subdir" + +# Source files the sender will push: same size as the outside target, +# different content, mode 0666 (the perms the attacker tries to push). +SIZE=$(stat -c %s "$outside/target.txt" 2>/dev/null \ + || stat -f %z "$outside/target.txt") +head -c "$SIZE" /dev/urandom > "$src/target.txt" +head -c "$SIZE" /dev/urandom > "$src/subdir/target.txt" +chmod 0666 "$src/target.txt" "$src/subdir/target.txt" + +cat > "$conf" < "$outside/target.txt" +} + +verify_unchanged() { + label="$1" + mode=$(file_mode "$outside/target.txt") + case "$mode" in + 600|0600) ;; + *) test_fail "$label: outside file mode changed from 600 to $mode (chmod escape)" ;; + esac + if ! cmp -s "$outside/target.txt" "$outside_pristine"; then + test_fail "$label: outside file content changed (write escape)" + fi +} + +run_attack() { + label="$1"; shift + reset_outside + RSYNC_CONNECT_PROG="$RSYNC --config=$conf --daemon" \ + $RSYNC "$@" >/dev/null 2>&1 || true + verify_unchanged "$label" +} + +# 1. The original poc_chmod scenario: single file, dest path with +# the symlinked subdir as a path component. With --size-only the +# receiver normally skips the basis open and goes straight to chmod +# -- only the chdir-escape blocks the chmod from reaching outside. +run_attack "single-file --size-only" \ + -tp --size-only \ + "$src/target.txt" rsync://localhost/upload/subdir/target.txt + +# 2. -r push into the symlinked subdir: receiver chdir's into "subdir", +# follows the symlink, ends up in outside. +run_attack "-r --size-only into subdir/" \ + -rtp --size-only \ + "$src/subdir/" rsync://localhost/upload/subdir/ + +# 3. Same but no --size-only -- forces the basis-file open and a real +# rename, so this exercises the read-disclosure and write-escape +# paths together. +run_attack "-r without --size-only into subdir/" \ + -rtp \ + "$src/subdir/" rsync://localhost/upload/subdir/ + +# 4. -r src/ to upload/ -- this case was already covered by the +# original CVE-2026-29518 fix because the receiver stays at module +# root and operates on slashed paths. Regression check. +run_attack "-r --size-only into upload/ root" \ + -rtp --size-only \ + "$src/" rsync://localhost/upload/ + +exit 0 diff --git a/testsuite/chmod-symlink-race.test b/testsuite/chmod-symlink-race.test new file mode 100755 index 0000000..48bbfbb --- /dev/null +++ b/testsuite/chmod-symlink-race.test @@ -0,0 +1,68 @@ +#!/bin/sh + +# Copyright (C) 2026 by Andrew Tridgell + +# This program is distributable under the terms of the GNU GPL (see +# COPYING). + +# Regression test for the symlink-TOCTOU class of bug applied to +# chmod() on the receiver side. The CVE-2026-29518 fix used +# secure_relative_open() for the basis-file open, but every other +# path-based syscall the receiver runs on sender-controllable paths +# is vulnerable to the same primitive: a local attacker swaps a +# symlink into one of the parent directory components between the +# receiver's check and its act, and the syscall escapes the module. +# +# This test exercises the new do_chmod_at() wrapper via the +# t_chmod_secure helper. The helper sets up two scenarios: +# - a parent dir-symlink that resolves WITHIN the module tree +# (legitimate -K-style use, must continue to work) +# - a parent dir-symlink that escapes the module tree (the +# attack, must be rejected) +# plus two regression scenarios (plain relative path, top-level +# file) that just confirm the safe wrapper doesn't break the +# normal case. +# +# The kernel-enforced "stay below dirfd" path resolution is +# only available on Linux 5.6+, FreeBSD 13+, and macOS 15+. +# Skip on platforms that fall back to per-component O_NOFOLLOW +# (Solaris, OpenBSD, NetBSD, Cygwin); the per-component fallback +# would also reject the attack but the legitimate dir-symlink +# scenario would fail there. + +. "$suitedir/rsync.fns" + +case "$(uname -s)" in + SunOS|OpenBSD|NetBSD|CYGWIN*) + test_skipped "do_chmod_at relies on RESOLVE_BENEATH-equivalent kernel support not available on $(uname -s)" + ;; +esac + +mod="$scratchdir/module" +trap_outside="$scratchdir/trap" +rm -rf "$mod" "$trap_outside" +mkdir -p "$mod/realdir" "$trap_outside" + +# Set up the four file-system objects the helper expects: +echo bystander > "$mod/realdir/sentinel" +chmod 0600 "$mod/realdir/sentinel" +echo target > "$trap_outside/sentinel" +chmod 0600 "$trap_outside/sentinel" +ln -s realdir "$mod/inside_link" +ln -s ../trap "$mod/escape_link" +echo top > "$mod/topfile" +chmod 0600 "$mod/topfile" + +"$TOOLDIR/t_chmod_secure" "$mod" || \ + test_fail "t_chmod_secure reported failures (see stderr above)" + +# Sanity-check from the shell side too: the outside file's mode must +# still be 0600 -- the helper checked this, but a second look from +# the shell guards against a helper-internal stat() bug. +mode=$(stat -c '%a' "$trap_outside/sentinel" 2>/dev/null \ + || stat -f '%Lp' "$trap_outside/sentinel" 2>/dev/null) +if [ "$mode" != "600" ]; then + test_fail "outside sentinel mode changed from 600 to $mode -- chmod escaped the module" +fi + +exit 0 diff --git a/testsuite/copy-dest-source-symlink.test b/testsuite/copy-dest-source-symlink.test new file mode 100755 index 0000000..f91ee98 --- /dev/null +++ b/testsuite/copy-dest-source-symlink.test @@ -0,0 +1,98 @@ +#!/bin/sh + +# Copyright (C) 2026 by Andrew Tridgell + +# This program is distributable under the terms of the GNU GPL (see +# COPYING). + +# Regression test for codex audit Finding 3a: copy_file()'s source +# open in copy_altdest_file() is via do_open_nofollow(), which only +# refuses a final-component symlink. Parent components are still +# resolved with normal symlink-following. A daemon module attacker +# who plants a parent symlink at module/cd -> /outside, then runs +# --copy-dest=cd against a source file matching the size+mtime of +# /outside/target.txt, drives the receiver to: +# +# 1. Find a match-level >= 2 basis at "cd/target.txt" +# 2. Call copy_altdest_file -> copy_file(src="cd/target.txt", ...) +# 3. do_open_nofollow follows the "cd" parent symlink and reads +# the contents of /outside/target.txt under the daemon's +# authority +# 4. Copy that content into the module destination +# +# Result: outside/target.txt content lands at module/target.txt, +# accessible to the attacker on a subsequent pull. +# +# We detect by content: src/target.txt and outside/target.txt have +# identical metadata (size + mtime + mode) but different content. +# After the transfer, module/target.txt should match src (no +# basedir escape) -- if it matches outside, the bug copied across +# the symlink boundary. + +. "$suitedir/rsync.fns" + +mod="$scratchdir/module" +outside="$scratchdir/outside" +src="$scratchdir/src" +conf="$scratchdir/test-rsyncd.conf" + +rm -rf "$mod" "$outside" "$src" +mkdir -p "$mod" "$outside" "$src" + +# Outside-the-module file the daemon should not read on the +# attacker's behalf. +echo "OUTSIDE_LEAKED_DATA!" > "$outside/target.txt" +chmod 0644 "$outside/target.txt" + +# The symlink trap. +ln -s "$outside" "$mod/cd" + +# Source: same size, same mtime, same mode as outside -- so the +# generator's link_stat + quick_check_ok finds a match-level >= 2 +# basis and calls copy_altdest_file. +echo "ATTACKER_KNOWN_DATA!" > "$src/target.txt" +touch -r "$outside/target.txt" "$src/target.txt" +chmod 0644 "$src/target.txt" + +# When running as root the daemon would drop to "nobody" by +# default and fail to mkstemp in the scratch dir; force it to +# keep our uid/gid in that case. +my_uid=`get_testuid` +root_uid=`get_rootuid` +root_gid=`get_rootgid` +uid_setting="uid = $root_uid" +gid_setting="gid = $root_gid" +if test x"$my_uid" != x"$root_uid"; then + uid_setting="#$uid_setting" + gid_setting="#$gid_setting" +fi + +cat > "$conf" </dev/null 2>&1 || true + +if [ ! -f "$mod/target.txt" ]; then + test_fail "destination file was not created -- daemon transfer failed before the test could observe the basedir behaviour" +fi + +if cmp -s "$mod/target.txt" "$outside/target.txt"; then + test_fail "basedir-escape via copy_file source: module/target.txt now contains the contents of outside/target.txt -- daemon read /outside via the cd symlink and copied it into the module" +fi + +if ! cmp -s "$mod/target.txt" "$src/target.txt"; then + test_fail "destination doesn't match source content (and isn't outside content either): unexpected state" +fi + +exit 0 diff --git a/testsuite/daemon-chroot-acl.test b/testsuite/daemon-chroot-acl.test new file mode 100644 index 0000000..9d1c1b6 --- /dev/null +++ b/testsuite/daemon-chroot-acl.test @@ -0,0 +1,111 @@ +#!/bin/sh + +# Copyright (C) 2026 by Andrew Tridgell + +# This program is distributable under the terms of the GNU GPL (see +# COPYING). + +# Regression test for GHSA-rjfm-3w2m-jf4f: a hostname-based "hosts deny" +# rule must still match when the daemon performs a 'daemon chroot' and +# the chroot does not contain the NSS files glibc needs for reverse DNS. +# +# Pre-fix, reverse DNS happened *after* the daemon chroot. With an empty +# chroot the NSS lookup failed, client_name() returned "UNKNOWN", and a +# deny rule referring to the connecting hostname silently failed to +# match. +# +# Two scenarios are exercised so we can distinguish the case the fix +# definitely covers from the per-module path that may still be +# vulnerable: +# A. global "reverse lookup = yes" (covered by b6abdb4c) +# B. only module "reverse lookup = yes" (gap to verify) + +. "$suitedir/rsync.fns" + +case `uname -s` in +Linux*) ;; +*) test_skipped "test is Linux-specific (uses chroot+unshare)" ;; +esac + +# We need CAP_SYS_CHROOT. Re-exec under a user namespace if not root. +if ! chroot / /bin/true 2>/dev/null; then + if [ -z "$RSYNC_UNSHARED" ] && unshare --user --map-root-user true 2>/dev/null; then + echo "Re-running under unshare --user --map-root-user..." + RSYNC_UNSHARED=1 exec unshare --user --map-root-user "$SHELL_PATH" $RUNSHFLAGS "$0" + fi + test_skipped "need CAP_SYS_CHROOT (root or unshare --user --map-root-user)" +fi + +# We need 127.0.0.1 to reverse-resolve to a real hostname while NSS is +# still working (i.e. before the daemon's chroot). The daemon will +# look that name up itself as part of its hostname-based ACL check; +# we then deny that name and assert the connection is rejected. +client_hostname=`getent hosts 127.0.0.1 2>/dev/null | awk 'NR==1 {print $2}'` +if [ -z "$client_hostname" ] || [ "$client_hostname" = "127.0.0.1" ]; then + test_skipped "no reverse DNS for 127.0.0.1" +fi + +chrootdir="$scratchdir/chroot" +rm -rf "$chrootdir" +mkdir -p "$chrootdir/modroot" +echo "from chroot" > "$chrootdir/modroot/file1" + +conf="$scratchdir/test-rsyncd.conf" +logfile="$scratchdir/rsyncd.log" + +write_conf() { + cat >"$conf" <"$out" 2>&1 + rc=$? + + echo "----- $label (rsync exit $rc):" + cat "$out" + echo "----- daemon log:" + [ -f "$logfile" ] && cat "$logfile" + echo "-----" + + grep -q '@ERROR.*access denied' "$out" +} + +# Scenario A: global reverse lookup. Covered by b6abdb4c. +write_conf yes yes +if ! run_check "Scenario A (global reverse lookup = yes)"; then + test_fail "Scenario A: hostname deny rule was bypassed" +fi + +# Scenario B: only the per-module reverse-lookup setting is enabled. +# The b6abdb4c fix only pre-warms client_name()'s cache when the +# global setting is on, so the post-chroot lookup in this path may +# still produce "UNKNOWN" and bypass the deny rule. +write_conf no yes +if ! run_check "Scenario B (per-module reverse lookup only)"; then + test_fail "Scenario B: hostname deny rule was bypassed (per-module reverse lookup with daemon chroot still has the bypass)" +fi + +exit 0 diff --git a/testsuite/daemon-refuse-compress.test b/testsuite/daemon-refuse-compress.test new file mode 100644 index 0000000..a24e50d --- /dev/null +++ b/testsuite/daemon-refuse-compress.test @@ -0,0 +1,51 @@ +#!/bin/sh + +# Copyright (C) 2026 by Andrew Tridgell + +# This program is distributable under the terms of the GNU GPL (see +# COPYING). + +# Test that a daemon module configured with "refuse options = compress" +# rejects clients that ask for compression and still serves the same +# transfer when the client does not. + +. "$suitedir/rsync.fns" + +build_rsyncd_conf + +# Append a module that refuses --compress (-z). +cat >>"$conf" </dev/null 2>"$errlog"; then + cat "$errlog" >&2 + test_fail "compressed transfer was not refused" +fi + +grep -- '--compress' "$errlog" >/dev/null || { + cat "$errlog" >&2 + test_fail "expected refuse error mentioning --compress" +} + +# The same transfer without -z must succeed. +rm -rf "$todir" +mkdir "$todir" +checkit "$RSYNC -av localhost::no-compress/ '$todir/'" "$chkdir" "$todir" + +# The script would have aborted on error, so getting here means we've won. +exit 0 diff --git a/testsuite/protected-regular.test b/testsuite/protected-regular.test index 40416b0..d276961 100644 --- a/testsuite/protected-regular.test +++ b/testsuite/protected-regular.test @@ -10,22 +10,28 @@ . "$suitedir/rsync.fns" test -f /proc/sys/fs/protected_regular || test_skipped "Can't find protected_regular setting (only available on Linux)" -pr_lvl=`cat /proc/sys/fs/protected_regular 2>/dev/null` || test_skipped "Can't check if fs.protected_regular is enabled (probably need root)" +pr_lvl=`cat /proc/sys/fs/protected_regular 2>/dev/null` || test_skipped "Can't check if fs.protected_regular is enabled" test "$pr_lvl" != 0 || test_skipped "fs.protected_regular is not enabled" workdir="$tmpdir/files" -mkdir "$workdir" +mkdir -p "$workdir" chmod 1777 "$workdir" echo "Source" > "$workdir/src" echo "" > "$workdir/dst" -chown 5001 "$workdir/dst" || test_skipped "Can't chown (probably need root)" -# Output is only shown in case of an error +if ! chown 5001 "$workdir/dst" 2>/dev/null; then + # Not root - try re-running under unshare with UID mapping + if [ -z "$RSYNC_UNSHARED" ] && unshare --user --map-root-user --map-users 5001:100000:1 true 2>/dev/null; then + echo "Re-running under unshare with UID mapping..." + RSYNC_UNSHARED=1 exec unshare --user --map-root-user --map-users 5001:100000:1 "$SHELL_PATH" $RUNSHFLAGS "$0" + fi + test_skipped "Can't chown (need root or unshare with uidmap)" +fi + echo "Contents of $workdir:" ls -al "$workdir" $RSYNC --inplace "$workdir/src" "$workdir/dst" || test_fail -# The script would have aborted on error, so getting here means we've won. exit 0 diff --git a/testsuite/proxy-response-line-too-long.test b/testsuite/proxy-response-line-too-long.test new file mode 100755 index 0000000..7f55c43 --- /dev/null +++ b/testsuite/proxy-response-line-too-long.test @@ -0,0 +1,128 @@ +#!/bin/sh + +# Copyright (C) 2026 by Andrew Tridgell + +# This program is distributable under the terms of the GNU GPL (see +# COPYING). + +# Regression test for the off-by-one stack OOB write in +# establish_proxy_connection() in socket.c when a malicious or +# man-in-the-middle HTTP proxy returns a first response line of +# 1023+ bytes without a '\n' terminator. +# +# Pre-fix: the read loop walked buffer[0..sizeof-2] one byte at a +# time, then post-loop logic did "if (*cp != '\n') cp++; *cp-- = +# '\0';". If no newline arrived before the loop filled the buffer, +# cp was left at &buffer[sizeof-1] (never written by the loop), +# *cp held stale stack bytes, and cp++ pushed cp one past the array. +# The null-termination then wrote one byte out of bounds on the +# stack. AddressSanitizer reports stack-buffer-overflow at the +# null-termination site. +# +# Post-fix: the bound-exhaustion case is detected by position and +# rejected with an "proxy response line too long" message, so no +# OOB write occurs and rsync exits with a non-signal status. + +. "$suitedir/rsync.fns" + +command -v python3 >/dev/null 2>&1 || test_skipped "python3 not available" + +workdir="$scratchdir/workdir" +mkdir -p "$workdir" +cd "$workdir" + +port_file="$workdir/port" +proxy_log="$workdir/proxy.log" + +# A minimal TCP listener: binds to an ephemeral port on 127.0.0.1, +# writes the chosen port to $port_file *before* accept() so the test +# can synchronise without a sleep, accepts one connection, reads +# until end-of-headers or 64 KiB, sends exactly 1023 bytes of 'X' +# with no '\n', then closes. +python3 - "$port_file" >"$proxy_log" 2>&1 <<'PYEOF' & +import socket, sys, os +port_file = sys.argv[1] +s = socket.socket(socket.AF_INET, socket.SOCK_STREAM) +s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) +s.bind(("127.0.0.1", 0)) +port = s.getsockname()[1] +tmp = port_file + ".tmp" +with open(tmp, "w") as fp: + fp.write("%d\n" % port) +os.rename(tmp, port_file) # atomic visibility to the shell side +s.listen(1) +conn, _ = s.accept() +conn.settimeout(5) +try: + data = b"" + while b"\r\n\r\n" not in data and len(data) < 65536: + chunk = conn.recv(8192) + if not chunk: + break + data += chunk +except socket.timeout: + pass +conn.sendall(b"X" * 1023) # exactly the buffer-1 trigger size +try: + conn.shutdown(socket.SHUT_RDWR) +except OSError: + pass +conn.close() +s.close() +PYEOF +proxy_pid=$! + +# Wait up to ~10s for the listener to publish its port. +i=0 +while [ ! -s "$port_file" ] && [ $i -lt 10 ]; do + sleep 1 + i=$((i + 1)) +done + +if [ ! -s "$port_file" ]; then + kill "$proxy_pid" 2>/dev/null + cat "$proxy_log" >&2 2>/dev/null + test_fail "proxy listener never published a port" +fi + +port=`cat "$port_file"` +case "$port" in + *[!0-9]*|"") kill "$proxy_pid" 2>/dev/null; test_fail "bogus port from listener: '$port'" ;; +esac + +# Run rsync through the malicious proxy. Any rsync:// URL works: +# the proxy intercepts the CONNECT and never forwards anywhere. +rsync_err="$workdir/rsync.err" + +# rsync MUST exit non-zero here (the proxy is misbehaving). +# Use `|| status=$?` so we capture the real exit code under `sh -e`; +# `if ! cmd; then status=$?` would only ever see 0 because the `!` +# is the last command before `$?`. +status=0 +RSYNC_PROXY="127.0.0.1:$port" \ + $RSYNC rsync://example.invalid:873/whatever/ "$workdir/out/" \ + >/dev/null 2>"$rsync_err" || status=$? + +# Reap the listener. +wait "$proxy_pid" 2>/dev/null || true + +# 1. rsync must not have crashed (SIGSEGV/SIGABRT report >= 128). +if [ "$status" -ge 128 ]; then + cat "$rsync_err" >&2 + test_fail "rsync killed by signal (status=$status) -- possible stack OOB regression" +fi + +# 2. rsync must have actually exited non-zero (i.e. saw the bad proxy). +if [ "$status" -eq 0 ]; then + cat "$rsync_err" >&2 + test_fail "rsync returned success despite malformed proxy response" +fi + +# 3. The new error message must appear. +if ! grep -q "proxy response line too long" "$rsync_err"; then + cat "$rsync_err" >&2 + test_fail "expected 'proxy response line too long' in rsync stderr" +fi + +echo "OK: over-long proxy response line rejected cleanly without crashing" +exit 0 diff --git a/testsuite/secure-relpath-validation.test b/testsuite/secure-relpath-validation.test new file mode 100755 index 0000000..5b77f7c --- /dev/null +++ b/testsuite/secure-relpath-validation.test @@ -0,0 +1,34 @@ +#!/bin/sh + +# Copyright (C) 2026 by Andrew Tridgell + +# This program is distributable under the terms of the GNU GPL (see +# COPYING). + +# Regression test for codex audit Finding 5: secure_relative_open()'s +# front-door input check rejects "../foo" and "foo/../bar" but +# misses bare "..", "subdir/..", and other variants whose "/"-split +# components contain a literal "..". The kernel-enforced +# RESOLVE_BENEATH (Linux 5.6+) and O_RESOLVE_BENEATH +# (FreeBSD 13+, macOS 15+) reject these in-kernel; the per-component +# walk fallback used on NetBSD, OpenBSD, Solaris, Cygwin and pre-5.6 +# Linux does not -- so the validation must happen at the front door. +# +# This test invokes the t_secure_relpath helper, which calls +# secure_relative_open() with each suspect input and verifies the +# return value is -1 with errno == EINVAL. EINVAL is the marker +# that the front-door rejected the input, not the kernel; pre-fix +# the kernel returns -1 with EXDEV (or, on the per-component +# fallback, may return a valid fd at all -- "escape"). + +. "$suitedir/rsync.fns" + +testdir="$scratchdir/relpath-test" +rm -rf "$testdir" +mkdir -p "$testdir" + +if ! "$TOOLDIR/t_secure_relpath" "$testdir"; then + test_fail "t_secure_relpath rejected one or more inputs incorrectly (see stderr above for the specific case)" +fi + +exit 0 diff --git a/testsuite/sender-flist-symlink-leak.test b/testsuite/sender-flist-symlink-leak.test new file mode 100755 index 0000000..011d93d --- /dev/null +++ b/testsuite/sender-flist-symlink-leak.test @@ -0,0 +1,90 @@ +#!/bin/sh + +# Copyright (C) 2026 by Andrew Tridgell + +# This program is distributable under the terms of the GNU GPL (see +# COPYING). + +# Regression test for codex re-check finding: the sender-side file- +# list generator can still follow an attacker-planted symlink out of +# the module via change_pathname() -> change_dir(...,CD_SKIP_CHDIR) +# followed by change_dir(...,CD_NORMAL). The CD_SKIP_CHDIR sets +# skipped_chdir=1, and the next CD_NORMAL call's secure-branch in +# util1.c is gated on `!skipped_chdir`, so the secure path is +# bypassed and a raw chdir(curr_dir) follows attacker-controlled +# symlinks during flist generation. +# +# Reach: rsync daemon module with `use chroot = no`. A local +# attacker plants module/cd -> /outside. A client (innocent or +# malicious) pulls rsync:////cd/. The daemon, as +# sender, enumerates files in /outside and ships their metadata +# (names, sizes, modes, mtimes) to the client. The actual content +# transfer fails later at the secure_relative_open step with EXDEV, +# but by then the metadata has already leaked. +# +# We detect by running a dry-run pull of the symlinked subdir and +# checking whether the client's --list-only output mentions any +# file from /outside. With the bug, /outside/secret.txt appears in +# the list with its size; with the fix, the daemon's chdir into +# the symlinked subdir is rejected and no /outside file is listed. + +. "$suitedir/rsync.fns" + +case "$(uname -s)" in + SunOS|OpenBSD|NetBSD|CYGWIN*) + test_skipped "secure change_dir relies on RESOLVE_BENEATH-equivalent kernel support not available on $(uname -s)" + ;; +esac + +mod="$scratchdir/module" +outside="$scratchdir/outside" +listfile="$scratchdir/listed.txt" +conf="$scratchdir/test-rsyncd.conf" + +rm -rf "$mod" "$outside" +mkdir -p "$mod" "$outside" + +# Outside-the-module file the daemon should NOT enumerate to clients. +# A distinctive name + non-trivial size makes the leak easy to spot. +echo "OUTSIDE_PROTECTED_FILE_USED_AS_LEAK_DETECTOR" > "$outside/leak_marker.txt" +chmod 0644 "$outside/leak_marker.txt" + +# The symlink trap planted by the local attacker. +ln -s "$outside" "$mod/cd" + +my_uid=`get_testuid` +root_uid=`get_rootuid` +root_gid=`get_rootgid` +uid_setting="uid = $root_uid" +gid_setting="gid = $root_gid" +if test x"$my_uid" != x"$root_uid"; then + uid_setting="#$uid_setting" + gid_setting="#$gid_setting" +fi + +cat > "$conf" < "$listfile" 2>&1 || true + +if grep -q "leak_marker\.txt" "$listfile"; then + echo "----- leaked listing follows" >&2 + sed 's/^/ /' "$listfile" >&2 + echo "----- leaked listing ends" >&2 + test_fail "sender flist leak: outside/leak_marker.txt was enumerated to the client (daemon's chdir followed the cd symlink during flist generation)" +fi + +exit 0 diff --git a/testsuite/symlink-dirlink-basis.test b/testsuite/symlink-dirlink-basis.test new file mode 100755 index 0000000..a14eb5c --- /dev/null +++ b/testsuite/symlink-dirlink-basis.test @@ -0,0 +1,259 @@ +#!/bin/sh + +# Test that updating a file through a directory symlink works when using +# -K (--copy-dirlinks). This is a regression test for: +# https://github.com/RsyncProject/rsync/issues/715 +# +# The CVE fix in commit c35e283 introduced secure_relative_open() which +# uses O_NOFOLLOW on all path components, breaking legitimate directory +# symlinks on the receiver side. The fix splits the path into basedir +# (dirname, symlinks followed) and basename (O_NOFOLLOW) so that +# directory symlinks are traversed while the final file component is +# still protected. +# +# The regression only manifests when delta matching is triggered (i.e., +# the sender finds matching blocks in the old file). Small files with +# completely different content are transferred in full and don't trigger +# the bug. We use a large file with a small modification to ensure +# delta transfer is used. +# +# In addition to the original regression, this test covers edge cases +# in the fix itself: +# - --backup with directory symlinks (finish_transfer pointer identity) +# - --partial-dir with protocol < 29 (fnamecmp != partialptr guard) +# - --inplace with directory symlinks (updating_basis_or_equiv check) +# - Files without a dirname (top-level files, no split needed) + +. "$suitedir/rsync.fns" + +# secure_relative_open() uses kernel-enforced "stay below dirfd" via +# openat2(RESOLVE_BENEATH) on Linux 5.6+ and openat(O_RESOLVE_BENEATH) +# on FreeBSD 13+. Other platforms fall back to a per-component +# O_NOFOLLOW walk that rejects every symlink including legitimate +# directory symlinks -- the very case this test exercises. Skip on +# those rather than report a known failure. +case "$(uname -s)" in + SunOS|OpenBSD|NetBSD|CYGWIN*) + test_skipped "secure_relative_open lacks RESOLVE_BENEATH equivalent on $(uname -s); issue #715 still affects this platform" + ;; +esac + +RSYNC_RSH="$scratchdir/src/support/lsh.sh" +export RSYNC_RSH + +# $HOME is set to $scratchdir by rsync.fns +# localhost: destination will cd to $HOME (i.e., $scratchdir) + +# Helper: create a large file suitable for delta transfers. +# ~32KB is large enough for rsync's block matching to find matches. +make_testfile() { + dd if=/dev/urandom of="$1" bs=1024 count=32 2>/dev/null \ + || test_fail "failed to create test file $1" +} + +# Set up source tree +srcbase="$tmpdir/src" + +###################################################################### +# Test 1: Basic directory symlink update (the original issue #715) +###################################################################### + +mkdir -p "$HOME/real-dir" +ln -s real-dir "$HOME/dir" + +mkdir -p "$srcbase/dir" +make_testfile "$srcbase/dir/file" + +# First transfer (initial): should create the file through the symlink +(cd "$srcbase" && $RSYNC -KRlptv --rsync-path="$RSYNC" dir/file localhost:) \ + || test_fail "test 1: initial transfer failed" + +if [ ! -f "$HOME/real-dir/file" ]; then + test_fail "test 1: initial transfer did not create file through symlink" +fi + +diff "$srcbase/dir/file" "$HOME/real-dir/file" >/dev/null \ + || test_fail "test 1: initial transfer content mismatch" + +# Small modification to trigger delta transfer +echo "appended update" >> "$srcbase/dir/file" +sleep 1 +touch "$srcbase/dir/file" + +# Second transfer (update): was failing with "failed verification" +(cd "$srcbase" && $RSYNC -KRlptv --rsync-path="$RSYNC" dir/file localhost:) \ + || test_fail "test 1: update through directory symlink failed" + +diff "$srcbase/dir/file" "$HOME/real-dir/file" >/dev/null \ + || test_fail "test 1: update transfer content mismatch" + +###################################################################### +# Test 2: Compression (-z) as in the original reproducer +###################################################################### + +echo "another line" >> "$srcbase/dir/file" +sleep 1 +touch "$srcbase/dir/file" + +(cd "$srcbase" && $RSYNC -KRlptzv --rsync-path="$RSYNC" dir/file localhost:) \ + || test_fail "test 2: compressed update through directory symlink failed" + +diff "$srcbase/dir/file" "$HOME/real-dir/file" >/dev/null \ + || test_fail "test 2: compressed update content mismatch" + +###################################################################### +# Test 3: Nested directory symlinks (nested/sub/data.txt where +# "nested" is a symlink to "nested_real") +###################################################################### + +mkdir -p "$HOME/nested_real/sub" +ln -s nested_real "$HOME/nested" + +mkdir -p "$srcbase/nested/sub" +make_testfile "$srcbase/nested/sub/data.txt" + +(cd "$srcbase" && $RSYNC -KRlptv --rsync-path="$RSYNC" nested/sub/data.txt localhost:) \ + || test_fail "test 3: initial nested transfer failed" + +echo "appended nested" >> "$srcbase/nested/sub/data.txt" +sleep 1 +touch "$srcbase/nested/sub/data.txt" + +(cd "$srcbase" && $RSYNC -KRlptv --rsync-path="$RSYNC" nested/sub/data.txt localhost:) \ + || test_fail "test 3: update through nested directory symlink failed" + +diff "$srcbase/nested/sub/data.txt" "$HOME/nested_real/sub/data.txt" >/dev/null \ + || test_fail "test 3: nested update content mismatch" + +###################################################################### +# Test 4: --backup with directory symlinks +# +# Exercises the finish_transfer() "fnamecmp == fname" pointer +# comparison that determines whether to update fnamecmp to the +# backup name. If broken, --backup would reference a renamed file +# for xattr handling. +###################################################################### + +# Reset destination +rm -f "$HOME/real-dir/file" "$HOME/real-dir/file~" + +make_testfile "$srcbase/dir/file" + +(cd "$srcbase" && $RSYNC -KRlptv --rsync-path="$RSYNC" dir/file localhost:) \ + || test_fail "test 4: initial transfer for backup test failed" + +echo "backup update" >> "$srcbase/dir/file" +sleep 1 +touch "$srcbase/dir/file" + +(cd "$srcbase" && $RSYNC -KRlptv --backup --rsync-path="$RSYNC" dir/file localhost:) \ + || test_fail "test 4: update with --backup through directory symlink failed" + +diff "$srcbase/dir/file" "$HOME/real-dir/file" >/dev/null \ + || test_fail "test 4: backup update content mismatch" + +if [ ! -f "$HOME/real-dir/file~" ]; then + test_fail "test 4: backup file was not created" +fi + +###################################################################### +# Test 5: --inplace with directory symlinks +# +# Exercises the updating_basis_or_equiv check which uses +# "fnamecmp == fname". With --inplace, rsync writes directly to +# the destination file instead of a temp file. +###################################################################### + +rm -f "$HOME/real-dir/file" "$HOME/real-dir/file~" + +make_testfile "$srcbase/dir/file" + +(cd "$srcbase" && $RSYNC -KRlptv --inplace --rsync-path="$RSYNC" dir/file localhost:) \ + || test_fail "test 5: initial inplace transfer failed" + +echo "inplace update" >> "$srcbase/dir/file" +sleep 1 +touch "$srcbase/dir/file" + +(cd "$srcbase" && $RSYNC -KRlptv --inplace --rsync-path="$RSYNC" dir/file localhost:) \ + || test_fail "test 5: inplace update through directory symlink failed" + +diff "$srcbase/dir/file" "$HOME/real-dir/file" >/dev/null \ + || test_fail "test 5: inplace update content mismatch" + +###################################################################### +# Test 6: Top-level file (no dirname, no split needed) +# +# Ensures the dirname/basename split is not attempted for files +# at the top level (file->dirname is NULL). +###################################################################### + +make_testfile "$srcbase/topfile" +mkdir -p "$HOME" + +(cd "$srcbase" && $RSYNC -Rlptv --rsync-path="$RSYNC" topfile localhost:) \ + || test_fail "test 6: initial top-level transfer failed" + +echo "toplevel update" >> "$srcbase/topfile" +sleep 1 +touch "$srcbase/topfile" + +(cd "$srcbase" && $RSYNC -Rlptv --rsync-path="$RSYNC" topfile localhost:) \ + || test_fail "test 6: top-level update failed" + +diff "$srcbase/topfile" "$HOME/topfile" >/dev/null \ + || test_fail "test 6: top-level update content mismatch" + +###################################################################### +# Test 7: --partial-dir with protocol < 29 +# +# For protocol < 29, fnamecmp_type stays FNAMECMP_FNAME even when +# fnamecmp is set to partialptr. The dirname/basename split must +# NOT trigger in this case (guarded by "fnamecmp == fname"). +###################################################################### + +rm -f "$HOME/real-dir/file" +make_testfile "$srcbase/dir/file" + +(cd "$srcbase" && $RSYNC -KRlptv --protocol=28 --partial-dir=.rsync-partial \ + --rsync-path="$RSYNC" dir/file localhost:) \ + || test_fail "test 7: initial proto28 partial-dir transfer failed" + +echo "partial-dir update" >> "$srcbase/dir/file" +sleep 1 +touch "$srcbase/dir/file" + +(cd "$srcbase" && $RSYNC -KRlptv --protocol=28 --partial-dir=.rsync-partial \ + --rsync-path="$RSYNC" dir/file localhost:) \ + || test_fail "test 7: proto28 partial-dir update through dirlink failed" + +diff "$srcbase/dir/file" "$HOME/real-dir/file" >/dev/null \ + || test_fail "test 7: proto28 partial-dir update content mismatch" + +###################################################################### +# Test 8: Protocol < 29 basic directory symlink update +# +# Exercises the protocol < 29 code path and its fallback logic +# (clearing basedir on retry). +###################################################################### + +rm -f "$HOME/real-dir/file" +make_testfile "$srcbase/dir/file" + +(cd "$srcbase" && $RSYNC -KRlptv --protocol=28 \ + --rsync-path="$RSYNC" dir/file localhost:) \ + || test_fail "test 8: initial proto28 transfer failed" + +echo "proto28 update" >> "$srcbase/dir/file" +sleep 1 +touch "$srcbase/dir/file" + +(cd "$srcbase" && $RSYNC -KRlptv --protocol=28 \ + --rsync-path="$RSYNC" dir/file localhost:) \ + || test_fail "test 8: proto28 update through directory symlink failed" + +diff "$srcbase/dir/file" "$HOME/real-dir/file" >/dev/null \ + || test_fail "test 8: proto28 update content mismatch" + +# The script would have aborted on error, so getting here means we've won. +exit 0 diff --git a/testsuite/xattrs.test b/testsuite/xattrs.test index d94d5f9..c0f3784 100644 --- a/testsuite/xattrs.test +++ b/testsuite/xattrs.test @@ -38,7 +38,10 @@ EOF xls() { for fn in "${@}"; do runat "$fn" "$SHELL_PATH" < CHUNK_SIZE) { + rprintf(FERROR, "invalid uncompressed token length %ld [%s]\n", + (long)i, who_am_i()); + exit_cleanup(RERR_PROTOCOL); + } residue = i; } @@ -494,9 +502,52 @@ static char *cbuf; static char *dbuf; /* for decoding runs of tokens */ +#define MAX_TOKEN_INDEX ((int32)0x7ffffffe) + static int32 rx_token; static int32 rx_run; +static NORETURN void invalid_compressed_token(void) +{ + rprintf(FERROR, "invalid token number in compressed stream\n"); + exit_cleanup(RERR_PROTOCOL); +} + +static int32 recv_compressed_token_num(int f, int32 flag) +{ + if (flag & TOKEN_REL) { + int32 incr = flag & 0x3f; + if (rx_token > MAX_TOKEN_INDEX - incr) + invalid_compressed_token(); + rx_token += incr; + flag >>= 6; + } else { + rx_token = read_int(f); + if (rx_token < 0 || rx_token > MAX_TOKEN_INDEX) + invalid_compressed_token(); + } + + if (flag & 1) { + rx_run = read_byte(f); + rx_run += read_byte(f) << 8; + if (rx_run <= 0 || rx_token > MAX_TOKEN_INDEX - rx_run) + invalid_compressed_token(); + recv_state = r_running; + } + + return -1 - rx_token; +} + +static int32 recv_compressed_token_run(void) +{ + if (rx_run <= 0 || rx_token >= MAX_TOKEN_INDEX) + invalid_compressed_token(); + ++rx_token; + if (--rx_run == 0) + recv_state = r_idle; + return -1 - rx_token; +} + /* Receive a deflated token and inflate it */ static int32 recv_deflated_token(int f, char **data) { @@ -587,22 +638,7 @@ static int32 recv_deflated_token(int f, char **data) } /* here we have a token of some kind */ - if (flag & TOKEN_REL) { - rx_token += flag & 0x3f; - flag >>= 6; - } else { - rx_token = read_int(f); - if (rx_token < 0) { - rprintf(FERROR, "invalid token number in compressed stream\n"); - exit_cleanup(RERR_PROTOCOL); - } - } - if (flag & 1) { - rx_run = read_byte(f); - rx_run += read_byte(f) << 8; - recv_state = r_running; - } - return -1 - rx_token; + return recv_compressed_token_num(f, flag); case r_inflating: rx_strm.next_out = (Bytef *)dbuf; @@ -622,10 +658,7 @@ static int32 recv_deflated_token(int f, char **data) break; case r_running: - ++rx_token; - if (--rx_run == 0) - recv_state = r_idle; - return -1 - rx_token; + return recv_compressed_token_run(); } } } @@ -836,22 +869,7 @@ static int32 recv_zstd_token(int f, char **data) return 0; } /* here we have a token of some kind */ - if (flag & TOKEN_REL) { - rx_token += flag & 0x3f; - flag >>= 6; - } else { - rx_token = read_int(f); - if (rx_token < 0) { - rprintf(FERROR, "invalid token number in compressed stream\n"); - exit_cleanup(RERR_PROTOCOL); - } - } - if (flag & 1) { - rx_run = read_byte(f); - rx_run += read_byte(f) << 8; - recv_state = r_running; - } - return -1 - rx_token; + return recv_compressed_token_num(f, flag); case r_inflated: /* zstd doesn't get into this state */ break; @@ -882,10 +900,7 @@ static int32 recv_zstd_token(int f, char **data) break; case r_running: - ++rx_token; - if (--rx_run == 0) - recv_state = r_idle; - return -1 - rx_token; + return recv_compressed_token_run(); } } } @@ -1005,22 +1020,7 @@ static int32 recv_compressed_token(int f, char **data) } /* here we have a token of some kind */ - if (flag & TOKEN_REL) { - rx_token += flag & 0x3f; - flag >>= 6; - } else { - rx_token = read_int(f); - if (rx_token < 0) { - rprintf(FERROR, "invalid token number in compressed stream\n"); - exit_cleanup(RERR_PROTOCOL); - } - } - if (flag & 1) { - rx_run = read_byte(f); - rx_run += read_byte(f) << 8; - recv_state = r_running; - } - return -1 - rx_token; + return recv_compressed_token_num(f, flag); case r_inflating: avail_out = LZ4_decompress_safe(next_in, dbuf, avail_in, size); @@ -1036,10 +1036,7 @@ static int32 recv_compressed_token(int f, char **data) break; case r_running: - ++rx_token; - if (--rx_run == 0) - recv_state = r_idle; - return -1 - rx_token; + return recv_compressed_token_run(); } } } diff --git a/util1.c b/util1.c index 25ac7c9..36c1b68 100644 --- a/util1.c +++ b/util1.c @@ -141,7 +141,7 @@ int set_times(const char *fname, STRUCT_STAT *stp) #ifdef HAVE_UTIMENSAT #include "case_N.h" - if (do_utimensat(fname, stp) == 0) + if (do_utimensat_at(fname, stp) == 0) break; if (errno != ENOSYS) return -1; @@ -336,7 +336,13 @@ static int unlink_and_reopen(const char *dest, mode_t mode) mode |= S_IWUSR; #endif mode &= INITACCESSPERMS; - if ((ofd = do_open(dest, O_WRONLY | O_CREAT | O_TRUNC | O_EXCL, mode)) < 0) { + /* Use do_open_at so the create/truncate goes through a secure + * parent dirfd in the daemon-no-chroot deployment. Otherwise + * an attacker could swap a parent component with a symlink in + * the window between robust_unlink (which uses do_unlink_at, + * already secure) and the create here, and redirect the new + * file outside the module. */ + if ((ofd = do_open_at(dest, O_WRONLY | O_CREAT | O_TRUNC | O_EXCL, mode)) < 0) { int save_errno = errno; rsyserr(FERROR_XFER, save_errno, "open %s", full_fname(dest)); errno = save_errno; @@ -360,12 +366,23 @@ static int unlink_and_reopen(const char *dest, mode_t mode) * --copy-dest options. */ int copy_file(const char *source, const char *dest, int tmpfilefd, mode_t mode) { + extern int am_daemon, am_chrooted; int ifd, ofd; char buf[1024 * 8]; int len; /* Number of bytes read into `buf'. */ OFF_T prealloc_len = 0, offset = 0; - if ((ifd = do_open_nofollow(source, O_RDONLY)) < 0) { + /* On a daemon without chroot, route the source open through + * secure_relative_open so a parent-symlink on the source path + * (e.g. --copy-dest=cd where cd is a symlink to an outside + * directory) cannot redirect the read to a file the daemon can + * see but the attacker should not. Plain do_open_nofollow only + * refuses a final-component symlink; parents are still followed. */ + if (am_daemon && !am_chrooted && source && *source && source[0] != '/') + ifd = secure_relative_open(NULL, source, O_RDONLY | O_NOFOLLOW, 0); + else + ifd = do_open_nofollow(source, O_RDONLY); + if (ifd < 0) { int save_errno = errno; rsyserr(FERROR_XFER, errno, "open %s", full_fname(source)); errno = save_errno; @@ -479,13 +496,13 @@ int copy_file(const char *source, const char *dest, int tmpfilefd, mode_t mode) int robust_unlink(const char *fname) { #ifndef ETXTBSY - return do_unlink(fname); + return do_unlink_at(fname); #else static int counter = 1; int rc, pos, start; char path[MAXPATHLEN]; - rc = do_unlink(fname); + rc = do_unlink_at(fname); if (rc == 0 || errno != ETXTBSY) return rc; @@ -515,7 +532,7 @@ int robust_unlink(const char *fname) } /* maybe we should return rename()'s exit status? Nah. */ - if (do_rename(fname, path) != 0) { + if (do_rename_at(fname, path) != 0) { errno = ETXTBSY; return -1; } @@ -538,7 +555,7 @@ int robust_rename(const char *from, const char *to, const char *partialptr, return 0; while (tries--) { - if (do_rename(from, to) == 0) + if (do_rename_at(from, to) == 0) return 0; switch (errno) { @@ -559,7 +576,7 @@ int robust_rename(const char *from, const char *to, const char *partialptr, } if (copy_file(from, to, -1, mode) != 0) return -2; - do_unlink(from); + do_unlink_at(from); return 1; default: return -1; @@ -1116,6 +1133,7 @@ char *sanitize_path(char *dest, const char *p, const char *rootdir, int depth, i * Also cleans the path using the clean_fname() function. */ int change_dir(const char *dir, int set_path_only) { + extern int am_daemon, am_chrooted; static int initialised, skipped_chdir; unsigned int len; @@ -1154,10 +1172,57 @@ int change_dir(const char *dir, int set_path_only) curr_dir[curr_dir_len++] = '/'; memcpy(curr_dir + curr_dir_len, dir, len + 1); - if (!set_path_only && chdir(curr_dir)) { - curr_dir_len = save_dir_len; - curr_dir[curr_dir_len] = '\0'; - return 0; + if (!set_path_only) { + int chdir_failed; + /* In the daemon-without-chroot deployment we must not + * follow a symlink in any component of the chdir + * target -- otherwise CWD escapes the module and + * every subsequent path-relative syscall (open, + * chmod, lchown, ...) inherits the escape, which + * defeats secure_relative_open's RESOLVE_BENEATH + * anchor and re-opens the CVE-2026-29518 class of + * symlink TOCTOU attacks. Use the secure resolver + * to get a confined dirfd, then fchdir() to it. + * + * If skipped_chdir is set, a previous CD_SKIP_CHDIR + * call buffered an absolute prefix in curr_dir + * (e.g. change_pathname's CD_SKIP_CHDIR to orig_dir) + * without syncing the kernel's CWD. Resolve `dir` + * relative to that prefix as basedir so the secure + * branch still anchors at the operator-trusted + * directory rather than wherever the kernel CWD + * happens to be. */ + if (am_daemon && !am_chrooted) { + const char *basedir = NULL; + char prefix[MAXPATHLEN]; + int dfd; + if (skipped_chdir) { + if (save_dir_len >= sizeof prefix) { + errno = ENAMETOOLONG; + chdir_failed = 1; + goto chdir_cleanup; + } + memcpy(prefix, curr_dir, save_dir_len); + prefix[save_dir_len] = '\0'; + basedir = prefix; + } + dfd = secure_relative_open(basedir, dir, + O_RDONLY | O_DIRECTORY, 0); + if (dfd < 0) { + chdir_failed = 1; + } else { + chdir_failed = fchdir(dfd) != 0; + close(dfd); + } + } else { + chdir_failed = chdir(curr_dir) != 0; + } + chdir_cleanup: + if (chdir_failed) { + curr_dir_len = save_dir_len; + curr_dir[curr_dir_len] = '\0'; + return 0; + } } skipped_chdir = set_path_only; } @@ -1285,20 +1350,20 @@ int handle_partial_dir(const char *fname, int create) dir = partial_fname; if (create) { STRUCT_STAT st; - int statret = do_lstat(dir, &st); + int statret = do_lstat_at(dir, &st); if (statret == 0 && !S_ISDIR(st.st_mode)) { - if (do_unlink(dir) < 0) { + if (do_unlink_at(dir) < 0) { *fn = '/'; return 0; } statret = -1; } - if (statret < 0 && do_mkdir(dir, 0700) < 0) { + if (statret < 0 && do_mkdir_at(dir, 0700) < 0) { *fn = '/'; return 0; } } else - do_rmdir(dir); + do_rmdir_at(dir); *fn = '/'; return 1; @@ -1394,7 +1459,12 @@ char *timestring(time_t t) static char buffers[4][20]; /* We support 4 simultaneous timestring results. */ char *TimeBuf = buffers[ndx = (ndx + 1) % 4]; struct tm tmp, *tm = localtime_r(&t, &tmp); - int len = snprintf(TimeBuf, sizeof buffers[0], "%4d/%02d/%02d %02d:%02d:%02d", + int len; + if (!tm) { + strlcpy(TimeBuf, "(time out of range)", sizeof buffers[0]); + return TimeBuf; + } + len = snprintf(TimeBuf, sizeof buffers[0], "%4d/%02d/%02d %02d:%02d:%02d", (int)tm->tm_year + 1900, (int)tm->tm_mon + 1, (int)tm->tm_mday, (int)tm->tm_hour, (int)tm->tm_min, (int)tm->tm_sec); assert(len > 0); /* Silence gcc warning */ diff --git a/version.h b/version.h index dcea322..0b69641 100644 --- a/version.h +++ b/version.h @@ -1,2 +1,2 @@ -#define RSYNC_VERSION "3.4.2" +#define RSYNC_VERSION "3.4.3" #define MAINTAINER_TZ_OFFSET 10.0 diff --git a/xattrs.c b/xattrs.c index 65166ee..99795f2 100644 --- a/xattrs.c +++ b/xattrs.c @@ -697,6 +697,13 @@ int recv_xattr_request(struct file_struct *file, int f_in) rxa = lst->items; num = 0; while ((rel_pos = read_varint(f_in)) != 0) { + /* Detect signed overflow before the accumulating add. A hostile + * peer could otherwise wrap 'num' to land on an arbitrary value. */ + if ((rel_pos > 0 && num > INT_MAX - rel_pos) + || (rel_pos < 0 && num < INT_MIN - rel_pos)) { + rprintf(FERROR, "xattr rel_pos accumulation overflow [%s]\n", who_am_i()); + exit_cleanup(RERR_PROTOCOL); + } num += rel_pos; if (am_sender) { /* The sender-related num values are only in order on the sender. @@ -742,7 +749,7 @@ int recv_xattr_request(struct file_struct *file, int f_in) } old_datum = rxa->datum; - rxa->datum_len = read_varint(f_in); + rxa->datum_len = read_varint_size(f_in, MAX_WIRE_XATTR_DATALEN, "xattr datum_len"); if (SIZE_MAX - rxa->name_len < rxa->datum_len) overflow_exit("recv_xattr_request"); @@ -783,7 +790,8 @@ void receive_xattr(int f, struct file_struct *file) return; } - if ((count = read_varint(f)) != 0) { + count = read_varint_bounded(f, 0, MAX_WIRE_XATTR_COUNT, "xattr count"); + if (count != 0) { (void)EXPAND_ITEM_LIST(&temp_xattr, rsync_xa, count); temp_xattr.count = 0; } @@ -791,8 +799,8 @@ void receive_xattr(int f, struct file_struct *file) for (num = 1; num <= count; num++) { char *ptr, *name; rsync_xa *rxa; - size_t name_len = read_varint(f); - size_t datum_len = read_varint(f); + size_t name_len = read_varint_size(f, MAX_WIRE_XATTR_NAMELEN, "xattr name_len"); + size_t datum_len = read_varint_size(f, MAX_WIRE_XATTR_DATALEN, "xattr datum_len"); size_t dget_len = datum_len > MAX_FULL_DATUM ? 1 + (size_t)xattr_sum_len : datum_len; size_t extra_len = MIGHT_NEED_RPRE ? RPRE_LEN : 0; if (SIZE_MAX - dget_len < extra_len || SIZE_MAX - dget_len - extra_len < name_len) @@ -1086,7 +1094,7 @@ int set_xattr(const char *fname, const struct file_struct *file, const char *fna && !S_ISLNK(sxp->st.st_mode) #endif && access(fname, W_OK) < 0 - && do_chmod(fname, (sxp->st.st_mode & CHMOD_BITS) | S_IWUSR) == 0) + && do_chmod_at(fname, (sxp->st.st_mode & CHMOD_BITS) | S_IWUSR) == 0) added_write_perm = 1; ndx = F_XATTR(file); @@ -1094,7 +1102,7 @@ int set_xattr(const char *fname, const struct file_struct *file, const char *fna lst = &glst->xa_items; int return_value = rsync_xal_set(fname, lst, fnamecmp, sxp); if (added_write_perm) /* remove the temporary write permission */ - do_chmod(fname, sxp->st.st_mode); + do_chmod_at(fname, sxp->st.st_mode); return return_value; } @@ -1211,7 +1219,7 @@ int set_stat_xattr(const char *fname, struct file_struct *file, mode_t new_mode) mode = (fst.st_mode & _S_IFMT) | (fmode & ACCESSPERMS) | (S_ISDIR(fst.st_mode) ? 0700 : 0600); if (fst.st_mode != mode) - do_chmod(fname, mode); + do_chmod_at(fname, mode); if (!IS_DEVICE(fst.st_mode)) fst.st_rdev = 0; /* just in case */ @@ -1249,7 +1257,12 @@ int set_stat_xattr(const char *fname, struct file_struct *file, mode_t new_mode) int x_stat(const char *fname, STRUCT_STAT *fst, STRUCT_STAT *xst) { - int ret = do_stat(fname, fst); + /* Use the *_at variants so that on a daemon-no-chroot deployment + * the metadata read goes through a secure parent dirfd instead + * of bare path resolution. The *_at wrappers fall through to + * plain do_stat outside the daemon-no-chroot context, so this + * change is transparent for non-daemon use. */ + int ret = do_stat_at(fname, fst); if ((ret < 0 || get_stat_xattr(fname, -1, fst, xst) < 0) && xst) xst->st_mode = 0; return ret; @@ -1257,7 +1270,7 @@ int x_stat(const char *fname, STRUCT_STAT *fst, STRUCT_STAT *xst) int x_lstat(const char *fname, STRUCT_STAT *fst, STRUCT_STAT *xst) { - int ret = do_lstat(fname, fst); + int ret = do_lstat_at(fname, fst); if ((ret < 0 || get_stat_xattr(fname, -1, fst, xst) < 0) && xst) xst->st_mode = 0; return ret;