diff --git a/backup.c b/backup.c index ddf5b902..e7d555c5 100644 --- a/backup.c +++ b/backup.c @@ -289,7 +289,7 @@ static int make_backup_inner(const char *fname, BOOL prefer_rename) * unsafe symlink. */ if (preserve_links && S_ISLNK(sx.st.st_mode) && safe_symlinks) { char lnkbuf[MAXPATHLEN]; - int llen = do_readlink(fname, lnkbuf, MAXPATHLEN - 1); + int llen = vfs_readlink(fname, lnkbuf, MAXPATHLEN - 1); /* A failed readlink means we can't verify the target, so fail * closed: skip the backup rather than let the hard-link fast path * preserve a possibly-unsafe symlink unchecked. */ @@ -375,9 +375,7 @@ static int make_backup_inner(const char *fname, BOOL prefer_rename) } ret = 2; } else { - /* symlink has no ownership-walk branch (see vfs/symlink.c), so - * flags=0 reproduces the old vfs_symlink_at behavior here. */ - if (vfs_symlink(sl, VFS_AT_FDCWD, buf, 0) < 0) + if (vfs_symlink(sl, VFS_AT_FDCWD, buf, VFS_OPERATOR_PATH) < 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/delete.c b/delete.c index 97b4d3d9..19ba0a3e 100644 --- a/delete.c +++ b/delete.c @@ -228,7 +228,9 @@ enum delret delete_item(char *fbuf, uint16 mode, uint16 flags) const char *leaf; int dfd = del_held_dfd(fbuf, &leaf); what = "rmdir"; - ok = (dfd >= 0 ? vfs_unlink(dfd, leaf, VFS_REMOVEDIR) : vfs_unlink(VFS_AT_FDCWD, fbuf, VFS_REMOVEDIR)) == 0; + ok = (dfd >= 0 ? vfs_unlink(dfd, leaf, VFS_REMOVEDIR) + : vfs_unlink(VFS_AT_FDCWD, fbuf, + VFS_REMOVEDIR | ((flags & DEL_FOR_BACKUP) ? VFS_OPERATOR_PATH : 0))) == 0; } else { if (make_backups > 0 && !(flags & DEL_FOR_BACKUP) && (backup_dir || !is_backup_file(fbuf))) { what = "make_backup"; diff --git a/util1.c b/util1.c index 7faa0300..515a7a2f 100644 --- a/util1.c +++ b/util1.c @@ -1095,7 +1095,7 @@ int handle_partial_dir(const char *fname, int create) return 0; } } else - vfs_unlink(VFS_AT_FDCWD, dir, VFS_REMOVEDIR); + vfs_unlink(VFS_AT_FDCWD, dir, VFS_REMOVEDIR | VFS_OPERATOR_PATH); *fn = '/'; return 1; diff --git a/vfs/robust.c b/vfs/robust.c index f0af5652..c1244717 100644 --- a/vfs/robust.c +++ b/vfs/robust.c @@ -125,9 +125,18 @@ int robust_rename(const char *from, const char *to, const char *partialptr, return -2; to = partialptr; } - if (copy_file(from, to, -1, mode, 0) != 0) + /* Cross-fs fallback: copy then unlink. An absolute --temp-dir + * source / --partial-dir dest is an operator path whose parents + * the plain-libc arm would otherwise follow -- confine them + * through the ownership walk (VFS_OPERATOR_PATH) so a raced + * parent symlink can't redirect the dest-write or the + * source-unlink out of the module. copy_file already confines + * the source READ; a relative in-module path stays on the + * secure-relative arm, so only flag an absolute (operator) + * path. */ + if (copy_file(from, to, -1, mode, *to == '/' ? VFS_OPERATOR_PATH : 0) != 0) return -2; - vfs_unlink(VFS_AT_FDCWD, from, 0); + vfs_unlink(VFS_AT_FDCWD, from, *from == '/' ? VFS_OPERATOR_PATH : 0); return 1; default: return -1; diff --git a/vfs/symlink.c b/vfs/symlink.c index d223093e..d48faed1 100644 --- a/vfs/symlink.c +++ b/vfs/symlink.c @@ -63,13 +63,8 @@ static int vfs__symlink_plain(const char *lnk, const char *path) bare-path vfs_symlink() there, whose plain open() followed such a symlink. */ -/* NOTE: unlike vfs_mkdir/vfs_mknod, the symlink secure path has no ownership-walk - * branch -- VFS_OPERATOR_PATH is accepted but resolves the same as the default - * secure receiver walk (the link target is stored verbatim and never resolved at - * creation; only the parent dir is confined). Pre-existing asymmetry. */ static int vfs__symlink_secure(const char *lnk, const char *path, int flags) { - (void)flags; #ifdef AT_FDCWD char dirpath[MAXPATHLEN]; const char *bname; @@ -81,32 +76,48 @@ static int vfs__symlink_secure(const char *lnk, const char *path, int flags) if (dry_run) return 0; RETURN_ERROR_IF_RO_OR_LO; - if (!vfs_relpath_active()) - return vfs__symlink_plain(lnk, path); - - if (!path || !*path || *path == '/') - return vfs__symlink_plain(lnk, path); - - /* A path with a slash needs vfs_resolve_open to confine its parent; - * a top-level path is in CWD (AT_FDCWD), no parent to subvert. The leaf - * is protected below either way (symlinkat() won't follow it; the - * fake-super openat() uses O_NOFOLLOW). */ - slash = strrchr(path, '/'); - if (slash) { - dlen = slash - path; - if (dlen >= sizeof dirpath) { - errno = ENAMETOOLONG; - return -1; - } - memcpy(dirpath, path, dlen); - dirpath[dlen] = '\0'; - bname = slash + 1; - dfd = vfs_resolve_open(NULL, dirpath, O_RDONLY | O_DIRECTORY, 0); +#if defined O_NOFOLLOW && defined O_DIRECTORY + if (flags & VFS_OPERATOR_PATH) { + /* Operator path (e.g. an absolute --backup-dir): confine the + * parent with the ownership walk, then fall through to the shared + * leaf-creation below so fake-super emulation is preserved. */ + if (vfs_symlink_optout_allowed()) + return vfs__symlink_plain(lnk, path); + dfd = vfs_owner_walk_parent(path, &bname, 1); if (dfd < 0) return -1; owns = True; - } else { - bname = path; + } else +#endif + { + (void)flags; + if (!vfs_relpath_active()) + return vfs__symlink_plain(lnk, path); + + if (!path || !*path || *path == '/') + return vfs__symlink_plain(lnk, path); + + /* A path with a slash needs vfs_resolve_open to confine its + * parent; a top-level path is in CWD (AT_FDCWD), no parent to + * subvert. The leaf is protected below either way (symlinkat() + * won't follow it; the fake-super openat() uses O_NOFOLLOW). */ + slash = strrchr(path, '/'); + if (slash) { + dlen = slash - path; + if (dlen >= sizeof dirpath) { + errno = ENAMETOOLONG; + return -1; + } + memcpy(dirpath, path, dlen); + dirpath[dlen] = '\0'; + bname = slash + 1; + dfd = vfs_resolve_open(NULL, dirpath, O_RDONLY | O_DIRECTORY, 0); + if (dfd < 0) + return -1; + owns = True; + } else { + bname = path; + } } #if defined NO_SYMLINK_XATTRS || defined NO_SYMLINK_USER_XATTRS @@ -229,8 +240,8 @@ static int vfs__symlink_atfd(const char *lnk, int dfd, const char *name) /* Unified symlink creation. dirfd == VFS_AT_FDCWD resolves `path`; a real held * dirfd makes `path` a single validated component under it. flags: - * VFS_ALLOW_SYMLINK (trusted, plain symlink), default 0 (secure receiver - * resolve). See the note on vfs__symlink_secure re VFS_OPERATOR_PATH. */ + * VFS_ALLOW_SYMLINK (trusted, plain symlink), VFS_OPERATOR_PATH (operator + * path: ownership walk), default 0 (secure receiver resolve). */ int vfs_symlink(const char *lnk, int dirfd, const char *path, int flags) { if (dirfd != VFS_AT_FDCWD) { diff --git a/vfs/unlink.c b/vfs/unlink.c index 3562a898..e6c73294 100644 --- a/vfs/unlink.c +++ b/vfs/unlink.c @@ -19,8 +19,9 @@ /* Secure receiver-side resolve for an unlink/rmdir. unlink() resolves parent * components, so a parent-symlink swap can delete an outside file under the * daemon's authority -- defence is to resolve the parent securely and unlinkat() - * the leaf. unlink (not rmdir) honours the operator ownership walk; the rmdir - * path has no owner-walk branch (pre-existing -- matched the old vfs_rmdir_at). + * the leaf. Both unlink and rmdir honour the operator ownership walk + * (VFS_OPERATOR_PATH): a foreign-owned parent component is refused while the + * operator's own is followed, absolute and relative alike. * Falls through to a plain unlink()/rmdir() in non-daemon/sender, chrooted, * no-parent and absolute-path cases. */ static int vfs__unlink_secure(const char *path, int flags) @@ -32,13 +33,13 @@ static int vfs__unlink_secure(const char *path, int flags) int dfd, ret, e, atflag = rmdir_op ? AT_REMOVEDIR : 0; size_t dlen; - if (!rmdir_op && (flags & VFS_OPERATOR_PATH)) { + if (flags & VFS_OPERATOR_PATH) { if (vfs_symlink_optout_allowed()) - return unlink(path); + return rmdir_op ? rmdir(path) : unlink(path); dfd = vfs_owner_walk_parent(path, &bname, 1); if (dfd < 0) return -1; - ret = unlinkat(dfd, bname, 0); + ret = unlinkat(dfd, bname, atflag); e = errno; close(dfd); errno = e; @@ -75,7 +76,7 @@ static int vfs__unlink_secure(const char *path, int flags) /* Unified unlink/rmdir. dirfd == VFS_AT_FDCWD resolves `path`; a real held * dirfd makes `path` a single component removed directly under it. flags: * VFS_REMOVEDIR (rmdir/AT_REMOVEDIR instead of unlink), VFS_ALLOW_SYMLINK - * (trusted, plain), VFS_OPERATOR_PATH (operator path; unlink only), default 0 + * (trusted, plain), VFS_OPERATOR_PATH (operator path: ownership walk), default 0 * (secure receiver resolve). */ int vfs_unlink(int dirfd, const char *path, int flags) { diff --git a/vfs/vfs.h b/vfs/vfs.h index 84f4ca98..326f1c49 100644 --- a/vfs/vfs.h +++ b/vfs/vfs.h @@ -27,7 +27,7 @@ * it (from vfs_opendir/vfs_get_dirfd); no resolution. * * The operator-path policy is this explicit per-call flag, never ambient state. - * VFS_REMOVEDIR turns vfs_unlink into rmdir. chmod/lchown/symlink have no + * VFS_REMOVEDIR turns vfs_unlink into rmdir. chmod/lchown have no * ownership-walk branch (VFS_OPERATOR_PATH resolves as the default secure walk). * Two-path ops (vfs_rename_at, vfs_link_at) and open (distinct nofollow/ * checklinks variants) keep explicit forms + a vfs_flags arg; vfs_fstat and the