Skip to content

Commit 595a443

Browse files
samuelophclaude
samueloph
and
claude
committed
receiver: fix basis file open for directory symlinks (issue #715)
The secure_relative_open() change (c35e283) passes the full destination path as relpath with basedir=NULL for FNAMECMP_FNAME, applying O_NOFOLLOW to every component including intermediate directories. This breaks legitimate directory symlinks on the receiver (e.g. -K), causing "failed verification -- update discarded" on delta transfers when the basis file can't be opened. Fix by splitting file->dirname into basedir and file->basename into fnamecmp for the FNAMECMP_FNAME case. secure_relative_open() already follows symlinks in basedir, so directory symlinks are traversed while O_NOFOLLOW still protects the final component. After reconstruction, restore fnamecmp = fname to preserve pointer identity for downstream checks (finish_transfer's backup handling, updating_basis_or_equiv, directory-is-basis). The split guard includes "fnamecmp == fname" because protocol < 29 may set fnamecmp to partialptr without updating fnamecmp_type. Also clear basedir in the daemon filter fallback and the protocol < 29 retry path to prevent stale values from prior cases. The CVE fix is preserved: FNAMECMP_FUZZY and basis_dir paths still use secure_relative_open() with O_NOFOLLOW on all relpath components. For FNAMECMP_FNAME, the generator and receiver write path already follow directory symlinks in the same way, so no new information channel is created. file->dirname is validated against ".." by clean_fname(). Fixes: #715 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent b905ab2 commit 595a443

2 files changed

Lines changed: 268 additions & 0 deletions

File tree

‎receiver.c‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -753,6 +753,7 @@ int recv_files(int f_in, int f_out, char *local_name)
753753
&& check_filter(&daemon_filter_list, FLOG, fnamecmp, 0) < 0)) {
754754
fnamecmp = fname;
755755
fnamecmp_type = FNAMECMP_FNAME;
756+
basedir = NULL;
756757
}
757758
} else {
758759
/* Reminder: --inplace && --partial-dir are never
@@ -768,13 +769,27 @@ int recv_files(int f_in, int f_out, char *local_name)
768769
fnamecmp = fname;
769770
}
770771

772+
/* For FNAMECMP_FNAME, split into basedir (dirname) + fnamecmp
773+
* (basename) so that secure_relative_open() follows symlinks
774+
* in the directory part. This is needed because the receiver's
775+
* destination may legitimately contain directory symlinks
776+
* (e.g. when using -K/--copy-dirlinks). The basedir is opened
777+
* following symlinks, while the file component uses O_NOFOLLOW.
778+
* See: https://github.com/RsyncProject/rsync/issues/715 */
779+
if (fnamecmp_type == FNAMECMP_FNAME && basedir == NULL
780+
&& fnamecmp == fname && file->dirname && !local_name) {
781+
basedir = file->dirname;
782+
fnamecmp = (char*)file->basename;
783+
}
784+
771785
/* open the file */
772786
fd1 = secure_relative_open(basedir, fnamecmp, O_RDONLY, 0);
773787

774788
if (fd1 == -1 && protocol_version < 29) {
775789
if (fnamecmp != fname) {
776790
fnamecmp = fname;
777791
fnamecmp_type = FNAMECMP_FNAME;
792+
basedir = NULL;
778793
fd1 = do_open_nofollow(fnamecmp, O_RDONLY);
779794
}
780795

@@ -792,6 +807,12 @@ int recv_files(int f_in, int f_out, char *local_name)
792807
// path name as a single string
793808
pathjoin(fnamecmpbuf, sizeof fnamecmpbuf, basedir, fnamecmp);
794809
fnamecmp = fnamecmpbuf;
810+
/* For FNAMECMP_FNAME, the reconstructed path is
811+
* identical to fname. Restore the pointer so that
812+
* downstream pointer comparisons (e.g. in
813+
* finish_transfer) continue to work. */
814+
if (fnamecmp_type == FNAMECMP_FNAME)
815+
fnamecmp = fname;
795816
}
796817

797818
one_inplace = inplace_partial && fnamecmp_type == FNAMECMP_PARTIAL_DIR;

‎testsuite/symlink-dirlink-basis.test‎

Lines changed: 247 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,247 @@
1+
#!/bin/sh
2+
3+
# Test that updating a file through a directory symlink works when using
4+
# -K (--copy-dirlinks). This is a regression test for:
5+
# https://github.com/RsyncProject/rsync/issues/715
6+
#
7+
# The CVE fix in commit c35e283 introduced secure_relative_open() which
8+
# uses O_NOFOLLOW on all path components, breaking legitimate directory
9+
# symlinks on the receiver side. The fix splits the path into basedir
10+
# (dirname, symlinks followed) and basename (O_NOFOLLOW) so that
11+
# directory symlinks are traversed while the final file component is
12+
# still protected.
13+
#
14+
# The regression only manifests when delta matching is triggered (i.e.,
15+
# the sender finds matching blocks in the old file). Small files with
16+
# completely different content are transferred in full and don't trigger
17+
# the bug. We use a large file with a small modification to ensure
18+
# delta transfer is used.
19+
#
20+
# In addition to the original regression, this test covers edge cases
21+
# in the fix itself:
22+
# - --backup with directory symlinks (finish_transfer pointer identity)
23+
# - --partial-dir with protocol < 29 (fnamecmp != partialptr guard)
24+
# - --inplace with directory symlinks (updating_basis_or_equiv check)
25+
# - Files without a dirname (top-level files, no split needed)
26+
27+
. "$suitedir/rsync.fns"
28+
29+
RSYNC_RSH="$scratchdir/src/support/lsh.sh"
30+
export RSYNC_RSH
31+
32+
# $HOME is set to $scratchdir by rsync.fns
33+
# localhost: destination will cd to $HOME (i.e., $scratchdir)
34+
35+
# Helper: create a large file suitable for delta transfers.
36+
# ~32KB is large enough for rsync's block matching to find matches.
37+
make_testfile() {
38+
dd if=/dev/urandom of="$1" bs=1024 count=32 2>/dev/null \
39+
|| test_fail "failed to create test file $1"
40+
}
41+
42+
# Set up source tree
43+
srcbase="$tmpdir/src"
44+
45+
######################################################################
46+
# Test 1: Basic directory symlink update (the original issue #715)
47+
######################################################################
48+
49+
mkdir -p "$HOME/real-dir"
50+
ln -s real-dir "$HOME/dir"
51+
52+
mkdir -p "$srcbase/dir"
53+
make_testfile "$srcbase/dir/file"
54+
55+
# First transfer (initial): should create the file through the symlink
56+
(cd "$srcbase" && $RSYNC -KRlptv --rsync-path="$RSYNC" dir/file localhost:) \
57+
|| test_fail "test 1: initial transfer failed"
58+
59+
if [ ! -f "$HOME/real-dir/file" ]; then
60+
test_fail "test 1: initial transfer did not create file through symlink"
61+
fi
62+
63+
diff "$srcbase/dir/file" "$HOME/real-dir/file" >/dev/null \
64+
|| test_fail "test 1: initial transfer content mismatch"
65+
66+
# Small modification to trigger delta transfer
67+
echo "appended update" >> "$srcbase/dir/file"
68+
sleep 1
69+
touch "$srcbase/dir/file"
70+
71+
# Second transfer (update): was failing with "failed verification"
72+
(cd "$srcbase" && $RSYNC -KRlptv --rsync-path="$RSYNC" dir/file localhost:) \
73+
|| test_fail "test 1: update through directory symlink failed"
74+
75+
diff "$srcbase/dir/file" "$HOME/real-dir/file" >/dev/null \
76+
|| test_fail "test 1: update transfer content mismatch"
77+
78+
######################################################################
79+
# Test 2: Compression (-z) as in the original reproducer
80+
######################################################################
81+
82+
echo "another line" >> "$srcbase/dir/file"
83+
sleep 1
84+
touch "$srcbase/dir/file"
85+
86+
(cd "$srcbase" && $RSYNC -KRlptzv --rsync-path="$RSYNC" dir/file localhost:) \
87+
|| test_fail "test 2: compressed update through directory symlink failed"
88+
89+
diff "$srcbase/dir/file" "$HOME/real-dir/file" >/dev/null \
90+
|| test_fail "test 2: compressed update content mismatch"
91+
92+
######################################################################
93+
# Test 3: Nested directory symlinks (nested/sub/data.txt where
94+
# "nested" is a symlink to "nested_real")
95+
######################################################################
96+
97+
mkdir -p "$HOME/nested_real/sub"
98+
ln -s nested_real "$HOME/nested"
99+
100+
mkdir -p "$srcbase/nested/sub"
101+
make_testfile "$srcbase/nested/sub/data.txt"
102+
103+
(cd "$srcbase" && $RSYNC -KRlptv --rsync-path="$RSYNC" nested/sub/data.txt localhost:) \
104+
|| test_fail "test 3: initial nested transfer failed"
105+
106+
echo "appended nested" >> "$srcbase/nested/sub/data.txt"
107+
sleep 1
108+
touch "$srcbase/nested/sub/data.txt"
109+
110+
(cd "$srcbase" && $RSYNC -KRlptv --rsync-path="$RSYNC" nested/sub/data.txt localhost:) \
111+
|| test_fail "test 3: update through nested directory symlink failed"
112+
113+
diff "$srcbase/nested/sub/data.txt" "$HOME/nested_real/sub/data.txt" >/dev/null \
114+
|| test_fail "test 3: nested update content mismatch"
115+
116+
######################################################################
117+
# Test 4: --backup with directory symlinks
118+
#
119+
# Exercises the finish_transfer() "fnamecmp == fname" pointer
120+
# comparison that determines whether to update fnamecmp to the
121+
# backup name. If broken, --backup would reference a renamed file
122+
# for xattr handling.
123+
######################################################################
124+
125+
# Reset destination
126+
rm -f "$HOME/real-dir/file" "$HOME/real-dir/file~"
127+
128+
make_testfile "$srcbase/dir/file"
129+
130+
(cd "$srcbase" && $RSYNC -KRlptv --rsync-path="$RSYNC" dir/file localhost:) \
131+
|| test_fail "test 4: initial transfer for backup test failed"
132+
133+
echo "backup update" >> "$srcbase/dir/file"
134+
sleep 1
135+
touch "$srcbase/dir/file"
136+
137+
(cd "$srcbase" && $RSYNC -KRlptv --backup --rsync-path="$RSYNC" dir/file localhost:) \
138+
|| test_fail "test 4: update with --backup through directory symlink failed"
139+
140+
diff "$srcbase/dir/file" "$HOME/real-dir/file" >/dev/null \
141+
|| test_fail "test 4: backup update content mismatch"
142+
143+
if [ ! -f "$HOME/real-dir/file~" ]; then
144+
test_fail "test 4: backup file was not created"
145+
fi
146+
147+
######################################################################
148+
# Test 5: --inplace with directory symlinks
149+
#
150+
# Exercises the updating_basis_or_equiv check which uses
151+
# "fnamecmp == fname". With --inplace, rsync writes directly to
152+
# the destination file instead of a temp file.
153+
######################################################################
154+
155+
rm -f "$HOME/real-dir/file" "$HOME/real-dir/file~"
156+
157+
make_testfile "$srcbase/dir/file"
158+
159+
(cd "$srcbase" && $RSYNC -KRlptv --inplace --rsync-path="$RSYNC" dir/file localhost:) \
160+
|| test_fail "test 5: initial inplace transfer failed"
161+
162+
echo "inplace update" >> "$srcbase/dir/file"
163+
sleep 1
164+
touch "$srcbase/dir/file"
165+
166+
(cd "$srcbase" && $RSYNC -KRlptv --inplace --rsync-path="$RSYNC" dir/file localhost:) \
167+
|| test_fail "test 5: inplace update through directory symlink failed"
168+
169+
diff "$srcbase/dir/file" "$HOME/real-dir/file" >/dev/null \
170+
|| test_fail "test 5: inplace update content mismatch"
171+
172+
######################################################################
173+
# Test 6: Top-level file (no dirname, no split needed)
174+
#
175+
# Ensures the dirname/basename split is not attempted for files
176+
# at the top level (file->dirname is NULL).
177+
######################################################################
178+
179+
make_testfile "$srcbase/topfile"
180+
mkdir -p "$HOME"
181+
182+
(cd "$srcbase" && $RSYNC -Rlptv --rsync-path="$RSYNC" topfile localhost:) \
183+
|| test_fail "test 6: initial top-level transfer failed"
184+
185+
echo "toplevel update" >> "$srcbase/topfile"
186+
sleep 1
187+
touch "$srcbase/topfile"
188+
189+
(cd "$srcbase" && $RSYNC -Rlptv --rsync-path="$RSYNC" topfile localhost:) \
190+
|| test_fail "test 6: top-level update failed"
191+
192+
diff "$srcbase/topfile" "$HOME/topfile" >/dev/null \
193+
|| test_fail "test 6: top-level update content mismatch"
194+
195+
######################################################################
196+
# Test 7: --partial-dir with protocol < 29
197+
#
198+
# For protocol < 29, fnamecmp_type stays FNAMECMP_FNAME even when
199+
# fnamecmp is set to partialptr. The dirname/basename split must
200+
# NOT trigger in this case (guarded by "fnamecmp == fname").
201+
######################################################################
202+
203+
rm -f "$HOME/real-dir/file"
204+
make_testfile "$srcbase/dir/file"
205+
206+
(cd "$srcbase" && $RSYNC -KRlptv --protocol=28 --partial-dir=.rsync-partial \
207+
--rsync-path="$RSYNC" dir/file localhost:) \
208+
|| test_fail "test 7: initial proto28 partial-dir transfer failed"
209+
210+
echo "partial-dir update" >> "$srcbase/dir/file"
211+
sleep 1
212+
touch "$srcbase/dir/file"
213+
214+
(cd "$srcbase" && $RSYNC -KRlptv --protocol=28 --partial-dir=.rsync-partial \
215+
--rsync-path="$RSYNC" dir/file localhost:) \
216+
|| test_fail "test 7: proto28 partial-dir update through dirlink failed"
217+
218+
diff "$srcbase/dir/file" "$HOME/real-dir/file" >/dev/null \
219+
|| test_fail "test 7: proto28 partial-dir update content mismatch"
220+
221+
######################################################################
222+
# Test 8: Protocol < 29 basic directory symlink update
223+
#
224+
# Exercises the protocol < 29 code path and its fallback logic
225+
# (clearing basedir on retry).
226+
######################################################################
227+
228+
rm -f "$HOME/real-dir/file"
229+
make_testfile "$srcbase/dir/file"
230+
231+
(cd "$srcbase" && $RSYNC -KRlptv --protocol=28 \
232+
--rsync-path="$RSYNC" dir/file localhost:) \
233+
|| test_fail "test 8: initial proto28 transfer failed"
234+
235+
echo "proto28 update" >> "$srcbase/dir/file"
236+
sleep 1
237+
touch "$srcbase/dir/file"
238+
239+
(cd "$srcbase" && $RSYNC -KRlptv --protocol=28 \
240+
--rsync-path="$RSYNC" dir/file localhost:) \
241+
|| test_fail "test 8: proto28 update through directory symlink failed"
242+
243+
diff "$srcbase/dir/file" "$HOME/real-dir/file" >/dev/null \
244+
|| test_fail "test 8: proto28 update content mismatch"
245+
246+
# The script would have aborted on error, so getting here means we've won.
247+
exit 0

0 commit comments

Comments
 (0)