use openat2 RESOLVE_BENEATH for secure_relative_open when available

.github/workflows/cygwin-build.yml

@@ -39,7 +39,7 @@ jobs:
     - name: info
       run: bash -c '/usr/local/bin/rsync --version'
     - name: check
-      run: bash -c 'RSYNC_EXPECT_SKIPPED=acls-default,acls,chown,devices,dir-sgid,open-noatime,protected-regular,simd-checksum make check'
+      run: bash -c 'RSYNC_EXPECT_SKIPPED=acls-default,acls,chown,devices,dir-sgid,open-noatime,protected-regular,simd-checksum,symlink-dirlink-basis make check'
     - name: ssl file list
       run: bash -c 'PATH="/usr/local/bin:$PATH" rsync-ssl --no-motd download.samba.org::rsyncftp/ || true'
     - name: save artifact

syscall.c

@@ -33,6 +33,11 @@
 #include <sys/syscall.h>
 #endif
 
+#ifdef __linux__
+#include <sys/syscall.h>
+#include <linux/openat2.h>
+#endif
+
 #include "ifuncs.h"
 
 extern int dry_run;
@@ -720,12 +725,79 @@ 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.
 */
+
+#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
+
+#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 {
+		dirfd = openat(AT_FDCWD, basedir, O_RDONLY | O_DIRECTORY);
+		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] == '/') {
@@ -739,6 +811,20 @@ 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
+
+#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
 	if (!basedir) {

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