diff --git a/receiver.c b/receiver.c index 28d663acc..a39108704 100644 --- a/receiver.c +++ b/receiver.c @@ -124,10 +124,15 @@ static int secure_basis_open(const char *basedir, const char *relpath, int flags * and traverse a symlink the secure_relative_open path can't confine: resolve * it with the ownership walk, which follows a uid0/euid-owned symlink but * refuses a foreign one AND (via abspath_excluded_by_module) refuses a target - * the module's exclude hides -- closing the partial-dir exclude bypass. */ + * the module's exclude hides -- closing the partial-dir exclude bypass. The + * final component is opened with O_NOFOLLOW so an operator-path leaf symlink + * cannot be selected as an alternate basis. */ +#if defined AT_FDCWD && defined O_NOFOLLOW && defined O_DIRECTORY if (operator_path_resolve) { char fullpath[MAXPATHLEN]; const char *p = relpath; + const char *leaf; + int dfd, fd, saved_errno; if (basedir) { if (pathjoin(fullpath, sizeof fullpath, basedir, relpath) >= sizeof fullpath) { errno = ENAMETOOLONG; @@ -135,8 +140,16 @@ static int secure_basis_open(const char *basedir, const char *relpath, int flags } p = fullpath; } - return open_no_attacker_symlinks(p, flags, mode); + dfd = owner_walk_parent(p, &leaf); + if (dfd < 0) + return -1; + fd = do_open_atfd(dfd, leaf, flags, mode); + saved_errno = fd < 0 ? errno : 0; + close(dfd); + errno = saved_errno; + return fd; } +#endif /* The confined resolver is needed for the sanitizing daemon * (am_daemon && !am_chrooted) and for a /./ inner-module chroot @@ -1134,7 +1147,8 @@ int recv_files(int f_in, int f_out, char *local_name) /* A peer's basis selector cannot enable direct output through a path * that the confined basis open did not validate. */ - one_inplace = inplace_partial && fnamecmp_type == FNAMECMP_PARTIAL_DIR + one_inplace = inplace_partial && partial_dir + && fnamecmp_type == FNAMECMP_PARTIAL_DIR && fd1 != -1; updating_basis_or_equiv = one_inplace || (inplace && (fnamecmp == fname || fnamecmp_type == FNAMECMP_BACKUP)); diff --git a/testsuite/strict-basis_test.py b/testsuite/strict-basis_test.py new file mode 100644 index 000000000..100a5673b --- /dev/null +++ b/testsuite/strict-basis_test.py @@ -0,0 +1,253 @@ +#!/usr/bin/env python3 +"""Ensure forged partial-dir basis tokens cannot enable in-place writes.""" + +import hashlib +import os +import socket +import struct +import subprocess + +from rsyncfns import ( + TODIR, + hands_setup, + makepath, + rmtree, + rsync_argv, + test_fail, + test_skipped, +) +import rsync_proto as rp + +hands_setup() + +CF_INPLACE_PARTIAL_DIR = 1 << 6 +FNAMECMP_BASIS_DIR_LOW = 0x00 +FNAMECMP_PARTIAL_DIR = 0x81 +ITEM_BASIS_TYPE_FOLLOWS = 1 << 11 +ITEM_XNAME_FOLLOWS = 1 << 12 + +ORIGINAL_DATA = b"ORIGINAL_DEST_BYTES_KEEP_ME\n" +MODIFIED_DATA = b"MODIFIED_INPLACE_DESPITE_BAD_CHECKSUM\n" +INVALID_MD5 = b"\x00" * 16 +MODTIME = 1_700_000_000 +EXPECTED_PROTOCOL_ERROR = "error in rsync protocol data stream" +TEST_DATA = b"STRICT_NOFOLLOW_REQUIRED\n" + + +def drain_argv(peer): + nul_run = 0 + while nul_run < 2: + byte = peer._recv_exact(1) + nul_run = nul_run + 1 if byte == b"\0" else 0 + + +def read_mux_bytes(peer, buf, count): + while len(buf) < count: + word = struct.unpack("> 24) - rp.MPLEX_BASE == rp.MSG_DATA: + buf.extend(payload) + result = bytes(buf[:count]) + del buf[:count] + return result + + +def read_mux_int(peer, buf): + return struct.unpack("