From 0bfcd3b0f2ddc01a4b664e48752bffa0ddb2869f Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Sat, 25 Jul 2026 09:29:33 +1000 Subject: [PATCH] syscall: honor operator_path_resolve in do_chmod_at/do_lchown_at Every other mutating do_*_at() wrapper (unlink, symlink, link, mknod, rmdir, open, mkdir, rename) resolves an operator-supplied path through owner_walk_parent() when operator_path_resolve is set. do_chmod_at() and do_lchown_at() did not look at the flag at all, and both hand an absolute name straight to the unconfined full-path do_chmod()/do_lchown(). set_file_attrs() is called with operator_path_resolve = 1 precisely so that "a flipped temp-dir parent then can't redirect the chmod/chown" (rsync.c). With no held dirfd -- an absolute --temp-dir or --partial-dir -- both fell back to these two wrappers, so that promise did not hold. Give them the same ownership-walk branch the others use. S_ISLNK(mode) still takes do_chmod()'s lchmod()/setattrlist() path. The missing branch was spotted by Omar Elsayed in review on the partial-dir EACCES recovery PR, together with the fix approach. Suggested-by: Omar Elsayed --- syscall.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/syscall.c b/syscall.c index 44016dfa..5e00dfa7 100644 --- a/syscall.c +++ b/syscall.c @@ -1009,6 +1009,25 @@ int do_lchown_at(const char *fname, uid_t owner, gid_t group) if (dry_run) return 0; RETURN_ERROR_IF_RO_OR_LO; +#if defined O_NOFOLLOW && defined O_DIRECTORY + /* Operator-supplied path: resolve the parent via the ownership walk, as + * the other do_*_at() wrappers do. Without this the caller's + * operator_path_resolve has no effect here, and an absolute name would + * fall straight through to the unconfined full-path do_lchown(). */ + if (operator_path_resolve && fname && *fname) { + if (symlink_optout_allowed()) + return do_lchown(fname, owner, group); + dfd = owner_walk_parent(fname, &bname); + if (dfd < 0) + return -1; + ret = fchownat(dfd, bname, owner, group, AT_SYMLINK_NOFOLLOW); + e = errno; + close(dfd); + errno = e; + return ret; + } +#endif + if (!secure_relpath_active()) return do_lchown(fname, owner, group); @@ -1579,6 +1598,26 @@ int do_chmod_at(const char *fname, mode_t mode) if (dry_run) return 0; RETURN_ERROR_IF_RO_OR_LO; +#if defined O_NOFOLLOW && defined O_DIRECTORY + /* Operator-supplied path: resolve the parent via the ownership walk, as + * the other do_*_at() wrappers do. Without this the caller's + * operator_path_resolve has no effect here, and an absolute name would + * fall straight through to the unconfined full-path do_chmod(). + * S_ISLNK(mode) still needs do_chmod()'s lchmod()/setattrlist() handling. */ + if (operator_path_resolve && fname && *fname && !S_ISLNK(mode)) { + if (symlink_optout_allowed()) + return do_chmod(fname, mode); + dfd = owner_walk_parent(fname, &bname); + if (dfd < 0) + return -1; + ret = do_fchmodat_nofollow(dfd, bname, mode); + e = errno; + close(dfd); + errno = e; + return ret; + } +#endif + /* 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