mirror of
https://github.com/RsyncProject/rsync.git
synced 2026-11-05 11:33:23 -05:00
backup: confine cross-tree operator-path metadata via a pinned fd
A symlink race on a non-daemon --backup-dir let a local attacker redirect rsync's backup writes and chmods outside the backup tree when rsync runs as root. make_backup() sets operator_path_resolve, but set_file_attrs() had no held parent dir fd for an absolute backup path (held_dfd_for() returns -1), so its chmod/chown fell through to the path-based wrappers and, for an absolute path, to raw chmod()/lchown(). copy_valid_path() mirrors the source dir's attrs onto each backup subdir; when an attacker flips a backup component to a symlink in that window the raw lchown retags the planted symlink as root-owned -- laundering it into a "trusted" (uid 0) symlink that the owner-walk then follows, so the backup rename/chmod escapes the tree. Pin the leaf inode of a cross-tree operator path with an O_NOFOLLOW open via the operator owner-walk resolver and drive fchmod/fchown off that fd; a raced symlink leaf makes the open fail and the op is refused, never redirected. Gate on the INTENDED type (new_mode), not the attacker-controlled on-disk type. As root any open failure is the race (a real owned leaf never fails); a non-root operator, which cannot launder a uid-0 symlink, falls back to the legacy path op on a benign EACCES. --insecure-links opts back out. Adds do_fchmod()/do_fchown() fd wrappers. Residual cross-tree metadata sinks (times, ACLs, xattrs, and --temp-dir's finish_transfer set_file_attrs) are not covered here and are tracked for a follow-up.
This commit is contained in:
1 parent
4e9285b084
commit
ea9e707b46
2 files changed
+63
-2
No files matched your search
@@ -37,6 +37,7 @@ extern int omit_dir_times;
|
||||
extern int omit_link_times;
|
||||
extern int am_root;
|
||||
extern int am_server;
|
||||
extern int operator_path_resolve;
|
||||
extern int am_daemon;
|
||||
extern int am_sender;
|
||||
extern int am_receiver;
|
||||
@@ -496,6 +497,9 @@ int set_file_attrs(const char *fname, struct file_struct *file, stat_x *sxp,
|
||||
int inherit;
|
||||
int dfd = -1; /* held dir fd for the entry's own dir, or -1 */
|
||||
const char *leaf = NULL; /* leaf of fname relative to dfd */
|
||||
int op_leaf_fd = -1; /* O_NOFOLLOW fd pinning a cross-tree operator leaf */
|
||||
int op_pin = 0; /* drive chmod/chown off op_leaf_fd for a cross-tree leaf */
|
||||
int op_refuse = 0; /* pin open hit the symlink-race signal: refuse, don't redirect */
|
||||
#if defined SUPPORT_XATTRS || defined SUPPORT_ACLS
|
||||
int held_fd = -1; /* held O_NOFOLLOW fd for fd-based xattr/ACL ops, or -1 */
|
||||
#endif
|
||||
@@ -549,6 +553,38 @@ int set_file_attrs(const char *fname, struct file_struct *file, stat_x *sxp,
|
||||
held_fd = openat(dfd, leaf, O_RDONLY | O_NOFOLLOW | O_NONBLOCK | O_NOCTTY | O_CLOEXEC);
|
||||
#endif
|
||||
|
||||
/* A cross-tree operator path (an absolute --backup-dir/--temp-dir/--*-dest
|
||||
* leaf) has no held parent dfd, so the chmod/chown below would re-resolve the
|
||||
* full path and could follow a parent component an attacker flips to a
|
||||
* symlink mid-operation -- and an lchown through such a component even retags
|
||||
* the planted symlink as ours (trust laundering that then defeats the
|
||||
* owner-walk). Pin the leaf inode itself with an O_NOFOLLOW open via the
|
||||
* operator owner-walk resolver and drive fchmod/fchown off that fd. A raced
|
||||
* symlink leaf makes the open fail, leaving op_leaf_fd == -1: the metadata op
|
||||
* is then refused, never redirected. --insecure-links opts back out (the
|
||||
* resolver in do_open_at honours it), and a genuine symlink leaf (a symlink
|
||||
* backup) keeps the existing l-variant path. */
|
||||
/* Gate on the INTENDED type (new_mode), not the on-disk type (sxp->st): the
|
||||
* attacker controls the latter via the flip, and a dir component that has
|
||||
* just been flipped to a symlink must still take the pinned path so the
|
||||
* O_NOFOLLOW open refuses it -- otherwise the lchown would launder it. */
|
||||
op_pin = operator_path_resolve && dfd < 0 && !symlink_optout_allowed()
|
||||
&& (S_ISREG(new_mode) || S_ISDIR(new_mode) || S_ISFIFO(new_mode));
|
||||
if (op_pin && am_root >= 0) {
|
||||
op_leaf_fd = do_open_at(fname, O_RDONLY | O_NONBLOCK | O_NOCTTY | O_CLOEXEC, 0);
|
||||
/* When running as root (the uid-0 trust-laundering case) an O_RDONLY open
|
||||
* of a real owned reg/dir/fifo leaf never fails for permission reasons, so
|
||||
* ANY failure here means the leaf is being raced (a symlink refused by
|
||||
* O_NOFOLLOW / owner-walk -> ELOOP, or vanished mid-flip -> ENOENT):
|
||||
* refuse, never redirect via a re-resolvable path. A non-root operator
|
||||
* cannot launder a uid-0 symlink, but can still hit a legitimate EACCES on
|
||||
* an owned-but-unreadable leaf; there we only treat the explicit
|
||||
* symlink-race signal (ELOOP) as a refusal and otherwise fall back to the
|
||||
* legacy path op rather than spuriously failing a real file. */
|
||||
if (op_leaf_fd < 0 && (am_root > 0 || errno == ELOOP))
|
||||
op_refuse = 1;
|
||||
}
|
||||
|
||||
if (inherit && S_ISDIR(new_mode) && sxp->st.st_mode & S_ISGID) {
|
||||
/* We just created this directory and its setgid
|
||||
* bit is on, so make sure it stays on. */
|
||||
@@ -587,8 +623,10 @@ 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 ((dfd >= 0 ? do_lchown_atfd(dfd, leaf, uid, gid)
|
||||
: do_lchown_at(fname, uid, gid)) != 0) {
|
||||
if ((op_leaf_fd >= 0 ? do_fchown(op_leaf_fd, uid, gid)
|
||||
: op_refuse ? (errno = ELOOP, -1)
|
||||
: dfd >= 0 ? do_lchown_atfd(dfd, leaf, uid, gid)
|
||||
: 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",
|
||||
@@ -705,6 +743,8 @@ 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
|
||||
: op_leaf_fd >= 0 ? do_fchmod(op_leaf_fd, new_mode)
|
||||
: op_refuse ? (errno = ELOOP, -1)
|
||||
: dfd >= 0 && !S_ISLNK(new_mode) ? do_chmod_atfd(dfd, leaf, new_mode)
|
||||
: do_chmod_at(fname, new_mode);
|
||||
if (ret < 0) {
|
||||
@@ -729,6 +769,8 @@ int set_file_attrs(const char *fname, struct file_struct *file, stat_x *sxp,
|
||||
if (held_fd >= 0)
|
||||
close(held_fd);
|
||||
#endif
|
||||
if (op_leaf_fd >= 0)
|
||||
close(op_leaf_fd);
|
||||
if (sxp == &sx2)
|
||||
free_stat_x(&sx2);
|
||||
return updated;
|
||||
|
||||
@@ -3413,6 +3413,25 @@ int do_lchown_atfd(int dfd, const char *name, uid_t owner, gid_t group)
|
||||
#endif
|
||||
}
|
||||
|
||||
/* Mode/owner on an already-open fd (no path, no symlink to follow): the
|
||||
* race-free way to set metadata on a cross-tree operator-path leaf that was
|
||||
* pinned with O_NOFOLLOW. See set_file_attrs(). */
|
||||
int do_fchown(int fd, uid_t owner, gid_t group)
|
||||
{
|
||||
if (dry_run) return 0;
|
||||
RETURN_ERROR_IF_RO_OR_LO;
|
||||
return fchown(fd, owner, group);
|
||||
}
|
||||
|
||||
#ifdef HAVE_CHMOD
|
||||
int do_fchmod(int fd, mode_t mode)
|
||||
{
|
||||
if (dry_run) return 0;
|
||||
RETURN_ERROR_IF_RO_OR_LO;
|
||||
return fchmod(fd, mode);
|
||||
}
|
||||
#endif
|
||||
|
||||
#ifdef HAVE_UTIMENSAT
|
||||
int do_utimensat_atfd(int dfd, const char *name, STRUCT_STAT *stp)
|
||||
{
|
||||
|
||||
Reference in new issue
Block a user