diff --git a/backup.c b/backup.c index 4c211a3f..0106bc67 100644 --- a/backup.c +++ b/backup.c @@ -375,7 +375,9 @@ static int make_backup_inner(const char *fname, BOOL prefer_rename) } ret = 2; } else { - if (vfs_symlink_at(sl, buf) < 0) + /* 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) 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/generator.c b/generator.c index 59a977b9..9f005313 100644 --- a/generator.c +++ b/generator.c @@ -1417,9 +1417,9 @@ static int gen_entry_symlink(const char *slnk, const char *path, struct file_str int dfd = vfs_cached_dirfd(path, file); if (dfd >= 0) { const char *slash = strrchr(path, '/'); - return vfs_symlink_atfd(slnk, dfd, slash ? slash + 1 : path); + return vfs_symlink(slnk, dfd, slash ? slash + 1 : path, 0); } - return vfs_symlink_at(slnk, path); + return vfs_symlink(slnk, VFS_AT_FDCWD, path, 0); } /* True when this build compiled no fd-relative primitive able to create this diff --git a/t_symlink_secure.c b/t_symlink_secure.c index 551b70d6..ea348cfe 100644 --- a/t_symlink_secure.c +++ b/t_symlink_secure.c @@ -121,7 +121,7 @@ int main(int argc, char **argv) /* Pre-fix fallback: a no-slash path went to vfs_symlink()/vfs_mknod(), * which open() the basename without O_NOFOLLOW. */ #ifdef TEST_SYMLINK_PLACEHOLDER - vfs_symlink("VULN_SYM_PAYLOAD", "sympath"); + vfs_symlink("VULN_SYM_PAYLOAD", VFS_AT_FDCWD, "sympath", VFS_ALLOW_SYMLINK); check_clobbered("poc vfs_symlink bare", "../outside/secret_sym", "VULN_SYM_PAYLOAD"); #endif @@ -133,12 +133,12 @@ int main(int argc, char **argv) /* Fixed wrappers: a bare-path basename symlink must not be followed; * the victim outside the module stays untouched. */ #ifdef TEST_SYMLINK_PLACEHOLDER - vfs_symlink_at("FIXED_SYM_PAYLOAD", "sympath"); - check_preserved("vfs_symlink_at bare", "../outside/secret_sym", "VICTIM_SYM"); + vfs_symlink("FIXED_SYM_PAYLOAD", VFS_AT_FDCWD, "sympath", 0); + check_preserved("vfs_symlink bare", "../outside/secret_sym", "VICTIM_SYM"); /* Slashed path for parity (already protected before the fix). */ - vfs_symlink_at("FIXED_SYM_PAYLOAD", "sub/sympath2"); - check_preserved("vfs_symlink_at slashed", "../outside/secret_sym2", "VICTIM_SYM2"); + vfs_symlink("FIXED_SYM_PAYLOAD", VFS_AT_FDCWD, "sub/sympath2", 0); + check_preserved("vfs_symlink slashed", "../outside/secret_sym2", "VICTIM_SYM2"); #endif vfs_mknod(VFS_AT_FDCWD, "nodpath", S_IFCHR | 0600, 0, 0); diff --git a/vfs/symlink.c b/vfs/symlink.c index 7c87b42a..75ed7d2d 100644 --- a/vfs/symlink.c +++ b/vfs/symlink.c @@ -19,7 +19,7 @@ #include "vfs/vfs_internal.h" #ifdef SUPPORT_LINKS -int vfs_symlink(const char *lnk, const char *path) +static int vfs__symlink_plain(const char *lnk, const char *path) { if (dry_run) return 0; RETURN_ERROR_IF_RO_OR_LO; @@ -63,8 +63,13 @@ int vfs_symlink(const char *lnk, const char *path) bare-path vfs_symlink() there, whose plain open() followed such a symlink. */ -int vfs_symlink_at(const char *lnk, const char *path) +/* 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; @@ -77,10 +82,10 @@ int vfs_symlink_at(const char *lnk, const char *path) RETURN_ERROR_IF_RO_OR_LO; if (!vfs_relpath_active()) - return vfs_symlink(lnk, path); + return vfs__symlink_plain(lnk, path); if (!path || !*path || *path == '/') - return vfs_symlink(lnk, 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 @@ -191,7 +196,7 @@ ssize_t vfs_readlink_atfd(int dfd, const char *name, char *buf, size_t bufsiz) } #endif -int vfs_symlink_atfd(const char *lnk, int dfd, const char *name) +static int vfs__symlink_atfd(const char *lnk, int dfd, const char *name) { #ifdef AT_FDCWD if (dry_run) return 0; @@ -221,3 +226,23 @@ int vfs_symlink_atfd(const char *lnk, int dfd, const char *name) return -1; #endif } + +/* 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. */ +int vfs_symlink(const char *lnk, int dirfd, const char *path, int flags) +{ + if (dirfd != VFS_AT_FDCWD) { + if (!path || !*path || strchr(path, '/') + || (path[0] == '.' && (path[1] == '\0' + || (path[1] == '.' && path[2] == '\0')))) { + errno = EINVAL; + return -1; + } + return vfs__symlink_atfd(lnk, dirfd, path); + } + if (flags & VFS_ALLOW_SYMLINK) + return vfs__symlink_plain(lnk, path); + return vfs__symlink_secure(lnk, path, flags); +} diff --git a/vfs/vfs.h b/vfs/vfs.h index 9c470fca..44c0bc2e 100644 --- a/vfs/vfs.h +++ b/vfs/vfs.h @@ -145,9 +145,7 @@ int vfs_chmod_atfd(int dfd, const char *name, mode_t mode); /* symlink/readlink (vfs/symlink.c). vfs_readlink is a function only in * fake-super builds; otherwise it is a macro -> readlink() (see rsync.h). */ -int vfs_symlink(const char *lnk, const char *path); -int vfs_symlink_at(const char *lnk, const char *path); -int vfs_symlink_atfd(const char *lnk, int dfd, const char *name); +int vfs_symlink(const char *lnk, int dirfd, const char *path, int flags); ssize_t vfs_readlink_atfd(int dfd, const char *name, char *buf, size_t bufsiz); #if defined NO_SYMLINK_XATTRS || defined NO_SYMLINK_USER_XATTRS ssize_t vfs_readlink(const char *path, char *buf, size_t bufsiz);