Skip to content

Commit 30656c5

Browse files
tridgeclaude
andcommitted
syscall: add symlink-race-safe do_*_at() wrappers and harden secure_relative_open
Add the rest of the path-based syscall wrappers and migrate every receiver-side caller: - do_lchown_at, do_rename_at, do_mkdir_at, do_symlink_at, do_mknod_at, do_link_at, do_unlink_at, do_rmdir_at, do_utimensat_at, do_stat_at, do_lstat_at Same shape as do_chmod_at: open each parent under secure_relative_open(), call the *at() variant against the dirfd, fall through to the bare path-based syscall in non-daemon / chrooted / absolute-path / no-parent cases. macOS's setattrlist-based set_times tier is also routed through the utimensat_at path on daemon-no-chroot. Hardenings to secure_relative_open() itself: - confine basedir resolution under the same kernel mechanism used for relpath (basedirs from --copy-dest / --link-dest are sender-controllable in daemon mode) - reject any '..' component (bare '..', 'foo/..', 'subdir/..') so the per-component O_NOFOLLOW fallback can't escape - return the dirfd we built up from the per-component fallback when the caller passed O_DIRECTORY (otherwise every do_*_at failed with EINVAL on platforms without RESOLVE_BENEATH) Adds testsuite/alt-dest-symlink-race.test and testsuite/secure-relpath-validation.test (with t_secure_relpath helper) as regression coverage for the new hardenings. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 15d2964 commit 30656c5

15 files changed

Lines changed: 1165 additions & 51 deletions

Makefile.in

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,12 +58,12 @@ TLS_OBJ = tls.o syscall.o util2.o t_stub.o lib/compat.o lib/snprintf.o lib/perms
5858
# Programs we must have to run the test cases
5959
CHECK_PROGS = rsync$(EXEEXT) tls$(EXEEXT) getgroups$(EXEEXT) getfsdev$(EXEEXT) \
6060
testrun$(EXEEXT) trimslash$(EXEEXT) t_unsafe$(EXEEXT) t_chmod_secure$(EXEEXT) \
61-
wildtest$(EXEEXT) simdtest$(EXEEXT)
61+
t_secure_relpath$(EXEEXT) wildtest$(EXEEXT) simdtest$(EXEEXT)
6262

6363
CHECK_SYMLINKS = testsuite/chown-fake.test testsuite/devices-fake.test testsuite/xattrs-hlink.test
6464

6565
# Objects for CHECK_PROGS to clean
66-
CHECK_OBJS=tls.o testrun.o getgroups.o getfsdev.o t_stub.o t_unsafe.o t_chmod_secure.o trimslash.o wildtest.o
66+
CHECK_OBJS=tls.o testrun.o getgroups.o getfsdev.o t_stub.o t_unsafe.o t_chmod_secure.o t_secure_relpath.o trimslash.o wildtest.o
6767

6868
# note that the -I. is needed to handle config.h when using VPATH
6969
.c.o:
@@ -183,6 +183,10 @@ T_CHMOD_SECURE_OBJ = t_chmod_secure.o syscall.o util1.o util2.o t_stub.o lib/com
183183
t_chmod_secure$(EXEEXT): $(T_CHMOD_SECURE_OBJ)
184184
$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $(T_CHMOD_SECURE_OBJ) $(LIBS)
185185

186+
T_SECURE_RELPATH_OBJ = t_secure_relpath.o syscall.o util1.o util2.o t_stub.o lib/compat.o lib/snprintf.o lib/wildmatch.o lib/permstring.o
187+
t_secure_relpath$(EXEEXT): $(T_SECURE_RELPATH_OBJ)
188+
$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $(T_SECURE_RELPATH_OBJ) $(LIBS)
189+
186190
.PHONY: conf
187191
conf: configure.sh config.h.in
188192

backup.c

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ static int validate_backup_dir(void)
3939
{
4040
STRUCT_STAT st;
4141

42-
if (do_lstat(backup_dir_buf, &st) < 0) {
42+
if (do_lstat_at(backup_dir_buf, &st) < 0) {
4343
if (errno == ENOENT)
4444
return 0;
4545
rsyserr(FERROR, errno, "backup lstat %s failed", backup_dir_buf);
@@ -98,7 +98,7 @@ static BOOL copy_valid_path(const char *fname)
9898
for ( ; b; name = b + 1, b = strchr(name, '/')) {
9999
*b = '\0';
100100

101-
while (do_mkdir(backup_dir_buf, ACCESSPERMS) < 0) {
101+
while (do_mkdir_at(backup_dir_buf, ACCESSPERMS) < 0) {
102102
if (errno == EEXIST) {
103103
val = validate_backup_dir();
104104
if (val > 0)
@@ -197,7 +197,7 @@ static inline int link_or_rename(const char *from, const char *to,
197197
if (IS_SPECIAL(stp->st_mode) || IS_DEVICE(stp->st_mode))
198198
return 0; /* Use copy code. */
199199
#endif
200-
if (do_link(from, to) == 0) {
200+
if (do_link_at(from, to) == 0) {
201201
if (DEBUG_GTE(BACKUP, 1))
202202
rprintf(FINFO, "make_backup: HLINK %s successful.\n", from);
203203
return 2;
@@ -207,7 +207,7 @@ static inline int link_or_rename(const char *from, const char *to,
207207
return 0;
208208
}
209209
#endif
210-
if (do_rename(from, to) == 0) {
210+
if (do_rename_at(from, to) == 0) {
211211
if (stp->st_nlink > 1 && !S_ISDIR(stp->st_mode)) {
212212
/* If someone has hard-linked the file into the backup
213213
* dir, rename() might return success but do nothing! */
@@ -246,7 +246,7 @@ int make_backup(const char *fname, BOOL prefer_rename)
246246
goto success;
247247
if (errno == EEXIST || errno == EISDIR) {
248248
STRUCT_STAT bakst;
249-
if (do_lstat(buf, &bakst) == 0) {
249+
if (do_lstat_at(buf, &bakst) == 0) {
250250
int flags = get_del_for_flag(bakst.st_mode) | DEL_FOR_BACKUP | DEL_RECURSE;
251251
if (delete_item(buf, bakst.st_mode, flags) != 0)
252252
return 0;
@@ -277,7 +277,7 @@ int make_backup(const char *fname, BOOL prefer_rename)
277277
/* Check to see if this is a device file, or link */
278278
if ((am_root && preserve_devices && IS_DEVICE(file->mode))
279279
|| (preserve_specials && IS_SPECIAL(file->mode))) {
280-
if (do_mknod(buf, file->mode, sx.st.st_rdev) < 0)
280+
if (do_mknod_at(buf, file->mode, sx.st.st_rdev) < 0)
281281
rsyserr(FERROR, errno, "mknod %s failed", full_fname(buf));
282282
else if (DEBUG_GTE(BACKUP, 1))
283283
rprintf(FINFO, "make_backup: DEVICE %s successful.\n", fname);
@@ -294,7 +294,7 @@ int make_backup(const char *fname, BOOL prefer_rename)
294294
}
295295
ret = 2;
296296
} else {
297-
if (do_symlink(sl, buf) < 0)
297+
if (do_symlink_at(sl, buf) < 0)
298298
rsyserr(FERROR, errno, "link %s -> \"%s\"", full_fname(buf), sl);
299299
else if (DEBUG_GTE(BACKUP, 1))
300300
rprintf(FINFO, "make_backup: SYMLINK %s successful.\n", fname);

cleanup.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ NORETURN void _exit_cleanup(int code, const char *file, int line)
198198
switch_step++;
199199

200200
if (cleanup_fname)
201-
do_unlink(cleanup_fname);
201+
do_unlink_at(cleanup_fname);
202202
if (exit_code)
203203
kill_all(SIGUSR1);
204204
if (cleanup_pid && cleanup_pid == getpid()) {

delete.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ enum delret delete_item(char *fbuf, uint16 mode, uint16 flags)
160160

161161
if (S_ISDIR(mode)) {
162162
what = "rmdir";
163-
ok = do_rmdir(fbuf) == 0;
163+
ok = do_rmdir_at(fbuf) == 0;
164164
} else {
165165
if (make_backups > 0 && !(flags & DEL_FOR_BACKUP) && (backup_dir || !is_backup_file(fbuf))) {
166166
what = "make_backup";

generator.c

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -984,7 +984,7 @@ static int try_dests_reg(struct file_struct *file, char *fname, int ndx,
984984
if (find_exact_for_existing) {
985985
if (alt_dest_type == LINK_DEST && real_st.st_dev == sxp->st.st_dev && real_st.st_ino == sxp->st.st_ino)
986986
return -1;
987-
if (do_unlink(fname) < 0 && errno != ENOENT)
987+
if (do_unlink_at(fname) < 0 && errno != ENOENT)
988988
goto got_nothing_for_ya;
989989
}
990990
#ifdef SUPPORT_HARD_LINKS
@@ -1112,7 +1112,7 @@ static int try_dests_non(struct file_struct *file, char *fname, int ndx,
11121112
&& !IS_SPECIAL(file->mode) && !IS_DEVICE(file->mode)
11131113
#endif
11141114
&& !S_ISDIR(file->mode)) {
1115-
if (do_link(cmpbuf, fname) < 0) {
1115+
if (do_link_at(cmpbuf, fname) < 0) {
11161116
rsyserr(FERROR_XFER, errno,
11171117
"failed to hard-link %s with %s",
11181118
cmpbuf, fname);
@@ -1315,7 +1315,7 @@ static void recv_generator(char *fname, struct file_struct *file, int ndx,
13151315
}
13161316
}
13171317
if (relative_paths && !implied_dirs && file->mode != 0
1318-
&& do_stat(dn, &sx.st) < 0) {
1318+
&& do_stat_at(dn, &sx.st) < 0) {
13191319
if (dry_run)
13201320
goto parent_is_dry_missing;
13211321
if (make_path(fname, MKP_DROP_NAME | MKP_SKIP_SLASH) < 0) {
@@ -1427,7 +1427,7 @@ static void recv_generator(char *fname, struct file_struct *file, int ndx,
14271427
&& (stype == FT_DIR
14281428
|| delete_item(fname, sx.st.st_mode, del_opts | DEL_FOR_DIR) != 0))
14291429
goto cleanup; /* Any errors get reported later. */
1430-
if (do_mkdir(fname, (file->mode|added_perms) & 0700) == 0)
1430+
if (do_mkdir_at(fname, (file->mode|added_perms) & 0700) == 0)
14311431
file->flags |= FLAG_DIR_CREATED;
14321432
goto cleanup;
14331433
}
@@ -1469,10 +1469,10 @@ static void recv_generator(char *fname, struct file_struct *file, int ndx,
14691469
itemize(fnamecmp, file, ndx, statret, &sx,
14701470
statret ? ITEM_LOCAL_CHANGE : 0, 0, NULL);
14711471
}
1472-
if (real_ret != 0 && do_mkdir(fname,file->mode|added_perms) < 0 && errno != EEXIST) {
1472+
if (real_ret != 0 && do_mkdir_at(fname,file->mode|added_perms) < 0 && errno != EEXIST) {
14731473
if (!relative_paths || errno != ENOENT
14741474
|| make_path(fname, MKP_DROP_NAME | MKP_SKIP_SLASH) < 0
1475-
|| (do_mkdir(fname, file->mode|added_perms) < 0 && errno != EEXIST)) {
1475+
|| (do_mkdir_at(fname, file->mode|added_perms) < 0 && errno != EEXIST)) {
14761476
rsyserr(FERROR_XFER, errno,
14771477
"recv_generator: mkdir %s failed",
14781478
full_fname(fname));
@@ -1808,7 +1808,7 @@ static void recv_generator(char *fname, struct file_struct *file, int ndx,
18081808
;
18091809
else if (quick_check_ok(FT_REG, fnamecmp, file, &sx.st)) {
18101810
if (partialptr) {
1811-
do_unlink(partialptr);
1811+
do_unlink_at(partialptr);
18121812
handle_partial_dir(partialptr, PDIR_DELETE);
18131813
}
18141814
set_file_attrs(fname, file, &sx, NULL, maybe_ATTRS_REPORT | maybe_ATTRS_ACCURATE_TIME);
@@ -2016,7 +2016,7 @@ int atomic_create(struct file_struct *file, char *fname, const char *slnk, const
20162016

20172017
if (slnk) {
20182018
#ifdef SUPPORT_LINKS
2019-
if (do_symlink(slnk, create_name) < 0) {
2019+
if (do_symlink_at(slnk, create_name) < 0) {
20202020
rsyserr(FERROR_XFER, errno, "symlink %s -> \"%s\" failed",
20212021
full_fname(create_name), slnk);
20222022
return 0;
@@ -2032,22 +2032,22 @@ int atomic_create(struct file_struct *file, char *fname, const char *slnk, const
20322032
return 0;
20332033
#endif
20342034
} else {
2035-
if (do_mknod(create_name, file->mode, rdev) < 0) {
2035+
if (do_mknod_at(create_name, file->mode, rdev) < 0) {
20362036
rsyserr(FERROR_XFER, errno, "mknod %s failed",
20372037
full_fname(create_name));
20382038
return 0;
20392039
}
20402040
}
20412041

20422042
if (!skip_atomic) {
2043-
if (do_rename(tmpname, fname) < 0) {
2043+
if (do_rename_at(tmpname, fname) < 0) {
20442044
char *full_tmpname = strdup(full_fname(tmpname));
20452045
if (full_tmpname == NULL)
20462046
out_of_memory("atomic_create");
20472047
rsyserr(FERROR_XFER, errno, "rename %s -> \"%s\" failed",
20482048
full_tmpname, full_fname(fname));
20492049
free(full_tmpname);
2050-
do_unlink(tmpname);
2050+
do_unlink_at(tmpname);
20512051
return 0;
20522052
}
20532053
}

hlink.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -454,7 +454,7 @@ int hard_link_check(struct file_struct *file, int ndx, char *fname,
454454
int hard_link_one(struct file_struct *file, const char *fname,
455455
const char *oldname, int terse)
456456
{
457-
if (do_link(oldname, fname) < 0) {
457+
if (do_link_at(oldname, fname) < 0) {
458458
enum logcode code;
459459
if (terse) {
460460
if (!INFO_GTE(NAME, 1))

receiver.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -442,7 +442,7 @@ static void handle_delayed_updates(char *local_name)
442442
}
443443
/* We don't use robust_rename() here because the
444444
* partial-dir must be on the same drive. */
445-
if (do_rename(partialptr, fname) < 0) {
445+
if (do_rename_at(partialptr, fname) < 0) {
446446
rsyserr(FERROR_XFER, errno,
447447
"rename failed for %s (from %s)",
448448
full_fname(fname), partialptr);
@@ -926,7 +926,7 @@ int recv_files(int f_in, int f_out, char *local_name)
926926
recv_ok = -1;
927927
else if (fnamecmp == partialptr) {
928928
if (!one_inplace)
929-
do_unlink(partialptr);
929+
do_unlink_at(partialptr);
930930
handle_partial_dir(partialptr, PDIR_DELETE);
931931
}
932932
} else if (keep_partial && partialptr && (!one_inplace || delay_updates)) {
@@ -935,7 +935,7 @@ int recv_files(int f_in, int f_out, char *local_name)
935935
"Unable to create partial-dir for %s -- discarding %s.\n",
936936
local_name ? local_name : f_name(file, NULL),
937937
recv_ok ? "completed file" : "partial file");
938-
do_unlink(fnametmp);
938+
do_unlink_at(fnametmp);
939939
recv_ok = -1;
940940
} else if (!finish_transfer(partialptr, fnametmp, fnamecmp, NULL,
941941
file, recv_ok, !partial_dir))
@@ -946,7 +946,7 @@ int recv_files(int f_in, int f_out, char *local_name)
946946
} else
947947
partialptr = NULL;
948948
} else if (!one_inplace)
949-
do_unlink(fnametmp);
949+
do_unlink_at(fnametmp);
950950

951951
cleanup_disable();
952952

rsync.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -547,7 +547,7 @@ int set_file_attrs(const char *fname, struct file_struct *file, stat_x *sxp,
547547
if (am_root >= 0) {
548548
uid_t uid = change_uid ? (uid_t)F_OWNER(file) : sxp->st.st_uid;
549549
gid_t gid = change_gid ? (gid_t)F_GROUP(file) : sxp->st.st_gid;
550-
if (do_lchown(fname, uid, gid) != 0) {
550+
if (do_lchown_at(fname, uid, gid) != 0) {
551551
/* We shouldn't have attempted to change uid
552552
* or gid unless have the privilege. */
553553
rsyserr(FERROR_XFER, errno, "%s %s failed",
@@ -758,7 +758,7 @@ int finish_transfer(const char *fname, const char *fnametmp,
758758
full_fname(fnametmp), fname);
759759
if (!partialptr || (ret == -2 && temp_copy_name)
760760
|| robust_rename(fnametmp, partialptr, NULL, file->mode) < 0)
761-
do_unlink(fnametmp);
761+
do_unlink_at(fnametmp);
762762
return 0;
763763
}
764764
if (ret == 0) {
@@ -774,7 +774,7 @@ int finish_transfer(const char *fname, const char *fnametmp,
774774
ok_to_set_time ? ATTRS_ACCURATE_TIME : ATTRS_SKIP_MTIME | ATTRS_SKIP_ATIME | ATTRS_SKIP_CRTIME);
775775

776776
if (temp_copy_name) {
777-
if (do_rename(fnametmp, fname) < 0) {
777+
if (do_rename_at(fnametmp, fname) < 0) {
778778
rsyserr(FERROR_XFER, errno, "rename %s -> \"%s\"",
779779
full_fname(fnametmp), fname);
780780
return 0;

runtests.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,7 @@ def main():
301301
# would cause many tests to fail with confusing "not found" errors, so
302302
# check up front and point the user at the make target that builds them.
303303
required_helpers = ['tls', 'trimslash', 't_unsafe', 't_chmod_secure',
304+
't_secure_relpath',
304305
'wildtest', 'getgroups', 'getfsdev']
305306
missing = [h for h in required_helpers
306307
if not os.path.isfile(os.path.join(tooldir, h))]

0 commit comments

Comments
 (0)