The branch, master has been updated
       via  43f70b96 The latest NEWS.
       via  b7231c7d Some extra file-list safety checks.
       via  15c34f0a A few more minor doc tweaks.
      from  d1e42ffa A few minor fixes.

https://git.samba.org/?p=rsync.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 43f70b961e92d88387fecd3c7f6dbefc044ce4a3
Author: Wayne Davison <wa...@opencoder.net>
Date:   Sun Jul 31 16:59:52 2022 -0700

    The latest NEWS.

commit b7231c7d02cfb65d291af74ff66e7d8c507ee871
Author: Wayne Davison <wa...@opencoder.net>
Date:   Sun Jul 31 16:55:34 2022 -0700

    Some extra file-list safety checks.

commit 15c34f0a8c76e250f5487997849ab31e40e556e9
Author: Wayne Davison <wa...@opencoder.net>
Date:   Mon Jul 11 13:31:30 2022 -0700

    A few more minor doc tweaks.

-----------------------------------------------------------------------

Summary of changes:
 NEWS.md    |  27 ++++++++++++-
 exclude.c  | 130 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 flist.c    |  17 +++++++-
 io.c       |   4 ++
 main.c     |   7 +++-
 receiver.c |  11 ++++--
 rsync.1.md |  57 ++++++++++++++++++++++-----
 7 files changed, 235 insertions(+), 18 deletions(-)


Changeset truncated at 500 lines:

diff --git a/NEWS.md b/NEWS.md
index ecaac28c..4cb98a63 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -2,9 +2,34 @@
 
 ## Changes in this version:
 
+### SECURITY FIXES:
+
+- Added some file-list safety checking that helps to ensure that a rogue
+  sending rsync can't add unrequested top-level names and/or include recursive
+  names that should have been excluded by the sender.  This extra safety check
+  only requires the client side rsync to be udateed.  When dealing with an
+  untrusted sending host using an older rsync, it is safest to copy into a
+  dedicated destination directory for the remote content (i.e. don't copy into
+  a destination directory that contains files that aren't from the remote
+  host unless you trust the remote host). Fixes CVE-2022-29154.
+
 ### BUG FIXES:
 
-- ...
+- Fixed the configure check for signed char that was causing a host that
+  defaults to unsigned characters to generate bogus rolling checksums. This
+  made rsync send mostly literal data for a copy instead of finding matching
+  data in the receiver's basis file.
+
+### PACKAGING RELATED:
+
+- The build date that goes into the manpages is now based on the developer's
+  release date, not on the build's local-timezone interpretation of the date.
+
+### DEVELOPER RELATED:
+
+- Configure now looks for the bsd/string.h include file in order to fix the
+  build on a host that has strlcpy() in the main libc but not defined in the
+  main string.h file.
 
 ------------------------------------------------------------------------------
 
diff --git a/exclude.c b/exclude.c
index 39073a0c..b670c8ba 100644
--- a/exclude.c
+++ b/exclude.c
@@ -27,16 +27,22 @@ extern int am_server;
 extern int am_sender;
 extern int eol_nulls;
 extern int io_error;
+extern int xfer_dirs;
+extern int recurse;
 extern int local_server;
 extern int prune_empty_dirs;
 extern int ignore_perishable;
+extern int old_style_args;
+extern int relative_paths;
 extern int delete_mode;
 extern int delete_excluded;
 extern int cvs_exclude;
 extern int sanitize_paths;
 extern int protocol_version;
+extern int list_only;
 extern int module_id;
 
+extern char *filesfrom_host;
 extern char curr_dir[MAXPATHLEN];
 extern unsigned int curr_dir_len;
 extern unsigned int module_dirlen;
@@ -44,8 +50,10 @@ extern unsigned int module_dirlen;
 filter_rule_list filter_list = { .debug_type = "" };
 filter_rule_list cvs_filter_list = { .debug_type = " [global CVS]" };
 filter_rule_list daemon_filter_list = { .debug_type = " [daemon]" };
+filter_rule_list implied_filter_list = { .debug_type = " [implied]" };
 
 int saw_xattr_filter = 0;
+int trust_sender_filter = 0;
 
 /* Need room enough for ":MODS " prefix plus some room to grow. */
 #define MAX_RULE_PREFIX (16)
@@ -292,6 +300,125 @@ static void add_rule(filter_rule_list *listp, const char 
*pat, unsigned int pat_
        }
 }
 
+/* Each arg the client sends to the remote sender turns into an implied include
+ * that the receiver uses to validate the file list from the sender. */
+void add_implied_include(const char *arg)
+{
+       filter_rule *rule;
+       int arg_len, saw_wild = 0, backslash_cnt = 0;
+       int slash_cnt = 1; /* We know we're adding a leading slash. */
+       const char *cp;
+       char *p;
+       if (old_style_args || list_only || filesfrom_host != NULL)
+               return;
+       if (relative_paths) {
+               cp = strstr(arg, "/./");
+               if (cp)
+                       arg = cp+3;
+       } else {
+               if ((cp = strrchr(arg, '/')) != NULL)
+                       arg = cp + 1;
+       }
+       arg_len = strlen(arg);
+       if (arg_len) {
+               if (strpbrk(arg, "*[?")) {
+                       /* We need to add room to escape backslashes if 
wildcard chars are present. */
+                       cp = arg;
+                       while ((cp = strchr(cp, '\\')) != NULL) {
+                               arg_len++;
+                               cp++;
+                       }
+                       saw_wild = 1;
+               }
+               arg_len++; /* Leave room for the prefixed slash */
+               rule = new0(filter_rule);
+               if (!implied_filter_list.head)
+                       implied_filter_list.head = implied_filter_list.tail = 
rule;
+               else {
+                       rule->next = implied_filter_list.head;
+                       implied_filter_list.head = rule;
+               }
+               rule->rflags = FILTRULE_INCLUDE + (saw_wild ? FILTRULE_WILD : 
0);
+               p = rule->pattern = new_array(char, arg_len + 1);
+               *p++ = '/';
+               cp = arg;
+               while (*cp) {
+                       switch (*cp) {
+                         case '\\':
+                               backslash_cnt++;
+                               if (saw_wild)
+                                       *p++ = '\\';
+                               *p++ = *cp++;
+                               break;
+                         case '/':
+                               if (p[-1] == '/') /* This is safe because of 
the initial slash. */
+                                       break;
+                               if (relative_paths) {
+                                       filter_rule const *ent;
+                                       int found = 0;
+                                       *p = '\0';
+                                       for (ent = implied_filter_list.head; 
ent; ent = ent->next) {
+                                               if (ent != rule && 
strcmp(ent->pattern, rule->pattern) == 0)
+                                                       found = 1;
+                                       }
+                                       if (!found) {
+                                               filter_rule *R_rule = 
new0(filter_rule);
+                                               R_rule->rflags = 
FILTRULE_INCLUDE + (saw_wild ? FILTRULE_WILD : 0);
+                                               R_rule->pattern = 
strdup(rule->pattern);
+                                               R_rule->u.slash_cnt = slash_cnt;
+                                               R_rule->next = 
implied_filter_list.head;
+                                               implied_filter_list.head = 
R_rule;
+                                       }
+                               }
+                               slash_cnt++;
+                               *p++ = *cp++;
+                               break;
+                         default:
+                               *p++ = *cp++;
+                               break;
+                       }
+               }
+               *p = '\0';
+               rule->u.slash_cnt = slash_cnt;
+               arg = (const char *)rule->pattern;
+       }
+
+       if (recurse || xfer_dirs) {
+               /* Now create a rule with an added "/" & "**" or "*" at the end 
*/
+               rule = new0(filter_rule);
+               if (recurse)
+                       rule->rflags = FILTRULE_INCLUDE | FILTRULE_WILD | 
FILTRULE_WILD2;
+               else
+                       rule->rflags = FILTRULE_INCLUDE | FILTRULE_WILD;
+               /* A +4 in the len leaves enough room for / * * \0 or / * \0 \0 
*/
+               if (!saw_wild && backslash_cnt) {
+                       /* We are appending a wildcard, so now the backslashes 
need to be escaped. */
+                       p = rule->pattern = new_array(char, arg_len + 
backslash_cnt + 3 + 1);
+                       cp = arg;
+                       while (*cp) {
+                               if (*cp == '\\')
+                                       *p++ = '\\';
+                               *p++ = *cp++;
+                       }
+               } else {
+                       p = rule->pattern = new_array(char, arg_len + 3 + 1);
+                       if (arg_len) {
+                               memcpy(p, arg, arg_len);
+                               p += arg_len;
+                       }
+               }
+               if (p[-1] != '/')
+                       *p++ = '/';
+               *p++ = '*';
+               if (recurse)
+                       *p++ = '*';
+               *p = '\0';
+               rule->u.slash_cnt = slash_cnt + 1;
+               rule->next = implied_filter_list.head;
+               implied_filter_list.head = rule;
+       }
+}
+
 /* This frees any non-inherited items, leaving just inherited items on the 
list. */
 static void pop_filter_list(filter_rule_list *listp)
 {
@@ -718,7 +845,7 @@ static void report_filter_result(enum logcode code, char 
const *name,
                              : name_flags & NAME_IS_DIR ? "directory"
                              : "file";
                rprintf(code, "[%s] %sing %s %s because of pattern %s%s%s\n",
-                   w, actions[*w!='s'][!(ent->rflags & FILTRULE_INCLUDE)],
+                   w, actions[*w=='g'][!(ent->rflags & FILTRULE_INCLUDE)],
                    t, name, ent->pattern,
                    ent->rflags & FILTRULE_DIRECTORY ? "/" : "", type);
        }
@@ -890,6 +1017,7 @@ static filter_rule *parse_rule_tok(const char 
**rulestr_ptr,
                }
                switch (ch) {
                case ':':
+                       trust_sender_filter = 1;
                        rule->rflags |= FILTRULE_PERDIR_MERGE
                                      | FILTRULE_FINISH_SETUP;
                        /* FALL THROUGH */
diff --git a/flist.c b/flist.c
index 1ba306bc..0e6bf782 100644
--- a/flist.c
+++ b/flist.c
@@ -73,6 +73,7 @@ extern int need_unsorted_flist;
 extern int sender_symlink_iconv;
 extern int output_needs_newline;
 extern int sender_keeps_checksum;
+extern int trust_sender_filter;
 extern int unsort_ndx;
 extern uid_t our_uid;
 extern struct stats stats;
@@ -83,8 +84,7 @@ extern char curr_dir[MAXPATHLEN];
 
 extern struct chmod_mode_struct *chmod_modes;
 
-extern filter_rule_list filter_list;
-extern filter_rule_list daemon_filter_list;
+extern filter_rule_list filter_list, implied_filter_list, daemon_filter_list;
 
 #ifdef ICONV_OPTION
 extern int filesfrom_convert;
@@ -986,6 +986,19 @@ static struct file_struct *recv_file_entry(int f, struct 
file_list *flist, int x
                exit_cleanup(RERR_UNSUPPORTED);
        }
 
+       if (*thisname != '.' || thisname[1] != '\0') {
+               int filt_flags = S_ISDIR(mode) ? NAME_IS_DIR : NAME_IS_FILE;
+               if (!trust_sender_filter /* a per-dir filter rule means we must 
trust the sender's filtering */
+                && filter_list.head && check_filter(&filter_list, FINFO, 
thisname, filt_flags) < 0) {
+                       rprintf(FERROR, "ERROR: rejecting excluded file-list 
name: %s\n", thisname);
+                       exit_cleanup(RERR_PROTOCOL);
+               }
+               if (implied_filter_list.head && 
check_filter(&implied_filter_list, FINFO, thisname, filt_flags) <= 0) {
+                       rprintf(FERROR, "ERROR: rejecting unrequested file-list 
name: %s\n", thisname);
+                       exit_cleanup(RERR_PROTOCOL);
+               }
+       }
+
        if (inc_recurse && S_ISDIR(mode)) {
                if (one_file_system) {
                        /* Room to save the dir's device for -x */
diff --git a/io.c b/io.c
index cf94cee7..a6e3ed30 100644
--- a/io.c
+++ b/io.c
@@ -419,6 +419,7 @@ static void forward_filesfrom_data(void)
                while (s != eob) {
                        if (*s++ == '\0') {
                                ff_xb.len = s - sob - 1;
+                               add_implied_include(sob);
                                if (iconvbufs(ic_send, &ff_xb, &iobuf.out, 
flags) < 0)
                                        exit_cleanup(RERR_PROTOCOL); /* 
impossible? */
                                write_buf(iobuf.out_fd, s-1, 1); /* Send the 
'\0'. */
@@ -450,9 +451,12 @@ static void forward_filesfrom_data(void)
                char *f = ff_xb.buf + ff_xb.pos;
                char *t = ff_xb.buf;
                char *eob = f + len;
+               char *cur = t;
                /* Eliminate any multi-'\0' runs. */
                while (f != eob) {
                        if (!(*t++ = *f++)) {
+                               add_implied_include(cur);
+                               cur = t;
                                while (f != eob && *f == '\0')
                                        f++;
                        }
diff --git a/main.c b/main.c
index 58920a2d..5a7fbdd7 100644
--- a/main.c
+++ b/main.c
@@ -89,6 +89,7 @@ extern int backup_dir_len;
 extern int basis_dir_cnt;
 extern int default_af_hint;
 extern int stdout_format_has_i;
+extern int trust_sender_filter;
 extern struct stats stats;
 extern char *stdout_format;
 extern char *logfile_format;
@@ -104,7 +105,7 @@ extern char curr_dir[MAXPATHLEN];
 extern char backup_dir_buf[MAXPATHLEN];
 extern char *basis_dir[MAX_BASIS_DIRS+1];
 extern struct file_list *first_flist;
-extern filter_rule_list daemon_filter_list;
+extern filter_rule_list daemon_filter_list, implied_filter_list;
 
 uid_t our_uid;
 gid_t our_gid;
@@ -635,6 +636,7 @@ static pid_t do_cmd(char *cmd, char *machine, char *user, 
char **remote_argv, in
 #ifdef ICONV_CONST
                setup_iconv();
 #endif
+               trust_sender_filter = 1;
        } else if (local_server) {
                /* If the user didn't request --[no-]whole-file, force
                 * it on, but only if we're not batch processing. */
@@ -1500,6 +1502,8 @@ static int start_client(int argc, char *argv[])
                char *dummy_host;
                int dummy_port = rsync_port;
                int i;
+               if (filesfrom_fd < 0)
+                       add_implied_include(remote_argv[0]);
                /* For remote source, any extra source args must have either
                 * the same hostname or an empty hostname. */
                for (i = 1; i < remote_argc; i++) {
@@ -1523,6 +1527,7 @@ static int start_client(int argc, char *argv[])
                        if (!rsync_port && !*arg) /* Turn an empty arg into a 
dot dir. */
                                arg = ".";
                        remote_argv[i] = arg;
+                       add_implied_include(arg);
                }
        }
 
diff --git a/receiver.c b/receiver.c
index b3a69da0..93cf8efd 100644
--- a/receiver.c
+++ b/receiver.c
@@ -593,10 +593,13 @@ int recv_files(int f_in, int f_out, char *local_name)
                if (DEBUG_GTE(RECV, 1))
                        rprintf(FINFO, "recv_files(%s)\n", fname);
 
-               if (daemon_filter_list.head && (*fname != '.' || fname[1] != 
'\0')
-                && check_filter(&daemon_filter_list, FLOG, fname, 0) < 0) {
-                       rprintf(FERROR, "attempt to hack rsync failed.\n");
-                       exit_cleanup(RERR_PROTOCOL);
+               if (daemon_filter_list.head && (*fname != '.' || fname[1] != 
'\0')) {
+                       int filt_flags = S_ISDIR(file->mode) ? NAME_IS_DIR : 
NAME_IS_FILE;
+                       if (check_filter(&daemon_filter_list, FLOG, fname, 
filt_flags) < 0) {
+                               rprintf(FERROR, "ERROR: rejecting file transfer 
request for daemon excluded file: %s\n",
+                                       fname);
+                               exit_cleanup(RERR_PROTOCOL);
+                       }
                }
 
 #ifdef SUPPORT_XATTRS
diff --git a/rsync.1.md b/rsync.1.md
index 1ab014f1..576dd90b 100644
--- a/rsync.1.md
+++ b/rsync.1.md
@@ -167,6 +167,33 @@ separate the files into different rsync calls, or consider 
using
 [`--delay-updates`](#opt) (which doesn't affect the sorted transfer order, but
 does make the final file-updating phase happen much more rapidly).
 
+## MULTI-HOST SECURITY
+
+Rsync takes steps to ensure that the file requests that are shared in a
+transfer are protected against various security issues.  Most of the potential
+problems arise on the receiving side where rsync takes steps to ensure that the
+list of files being transferred remains within the bounds of what was
+requested.
+
+Toward this end, rsync 3.1.2 and later have aborted when a file list contains
+an absolute or relative path that tries to escape out of the top of the
+transfer.  Also, beginning with version 3.2.5, rsync does two more safety
+checks of the file list to (1) ensure that no extra source arguments were added
+into the transfer other than those that the client requested and (2) ensure
+that the file list obeys the exclude rules that we sent to the sender.
+
+For those that don't yet have a 3.2.5 client rsync, it is safest to do a copy
+into a dedicated destination directory for the remote files rather than
+requesting the remote content get mixed in with other local content.  For
+example, doing an rsync copy into your home directory is potentially unsafe on
+an older rsync if the remote rsync is being controlled by a bad actor:
+
+>     rsync -aiv host1:dir1 ~
+
+A safer command would be:
+
+>     rsync -aiv host1:dir1 ~/host1-files
+
 ## ADVANCED USAGE
 
 The syntax for requesting multiple files from a remote host is done by
@@ -187,7 +214,7 @@ Starting in 3.2.4, filenames are passed to a remote shell 
in such a way as to
 preserve the characters you give it. Thus, if you ask for a file with spaces
 in the name, that's what the remote rsync looks for:
 
->     rsync -aiv host:'a simple file.pdf' /dest/                               
                                                 
+>     rsync -aiv host:'a simple file.pdf' /dest/
 
 If you use scripts that have been written to manually apply extra quoting to
 the remote rsync args (or to require remote arg splitting), you can ask rsync
@@ -2343,6 +2370,12 @@ option name from the pathname using a space if you want 
the shell to expand it.
     behavior.  The environment is always overridden by manually specified
     positive or negative options (the negative is `--no-old-args`).
 
+    Note that this option also disables the extra safety check added in 3.2.5
+    that ensures that a remote sender isn't including extra top-level items in
+    the file-list that you didn't request.  This side-effect is necessary
+    because we can't know for sure what names to expect when the remote shell
+    is interpreting the args.
+
     This option conflicts with the [`--protect-args`](#opt) option.
 
 0.  `--protect-args`, `-s`
@@ -3795,7 +3828,7 @@ also warn if a filter rule has trailing whitespace, since 
an exclude of "foo "
 (with a trailing space) will not exclude a file named "foo".
 
 Exclude and include rules can specify wildcard [PATTERN MATCHING RULES](#)
-(similar to shell wilcards) that allow you to match things like a file suffix
+(similar to shell wildcards) that allow you to match things like a file suffix
 or a portion of a filename.
 
 A rule can be limited to only affecting a directory by putting a trailing slash
@@ -3837,7 +3870,8 @@ it contains:
 
 ### FILTER RULES WHEN DELETING
 
-By default a filter rule affects both the sender (as it creates its file list)
+By default the include & exclude filter rules affect both the sender
+(as it creates its file list)
 and the receiver (as it creates its file lists for calculating deletions).  If
 no delete option is in effect, the receiver skips creating the delete-related
 file lists.  This two-sided default can be manually overridden so that you are
@@ -3846,8 +3880,8 @@ RULES IN DEPTH](#) section.
 
 When deleting, an exclude protects a file from being removed on the receiving
 side while an include overrides that protection (putting the file at risk of
-deletion). The default is for a file to be at risk (its safety depends on it
-matching a corresponding file from the sender).
+deletion). The default is for a file to be at risk -- its safety depends on it
+matching a corresponding file from the sender.
 
 An example of the two-sided exclude effect can be illustrated by the copying of
 a C development directory between 2 systems.  When doing a touch-up copy, you
@@ -3893,8 +3927,13 @@ Here are the available rule prefixes:
     `hide` and a `protect`.
 0.  `include, '+'` specifies an include pattern that (by default) is both a
     `show` and a `risk`.
-0.  `merge, '.'` specifies a merge-file to read for more rules.
-0.  `dir-merge, ':'` specifies a per-directory merge-file.
+0.  `merge, '.'` specifies a merge-file on the client side to read for more
+    rules.
+0.  `dir-merge, ':'` specifies a per-directory merge-file.  Using this kind of
+    filter rule requires that you trust the sending side's filter checking, and
+    thus it disables the receiver's verification of the file-list names against
+    the filter rules (since only the sender can know for sure if it obeyed all
+    the filter rules when some are per-dir merged from the sender's files).
 0.  `hide, 'H'` specifies a pattern for hiding files from the transfer.
     Equivalent to a sender-only exclude, so `-f'H foo'` could also be specified
     as `-f'-s foo'`.
@@ -3957,7 +3996,7 @@ checking if the pattern contains one of these three 
wildcard characters: '`*`',
   must match one character.
 - a trailing `***` in the pattern is a shorthand that allows you to match a
   directory and all its contents using a single rule.  For example, specifying
-  "`dir_name/***`" will match both the "dir_name" directory (as if "dir_name/"
+  "`dir_name/***`" will match both the "dir_name" directory (as if 
"`dir_name/`"
   had been specified) and everything in the directory (as if "`dir_name/**`"
   had been specified).
 - a backslash can be used to escape a wildcard character, but it is only
@@ -4284,7 +4323,7 @@ Given that the files are still in the sender's file list, 
the
 [`--prune-empty-dirs`](#opt) option will not judge a directory as being empty
 even if it contains only files that the transfer rules omitted.
 
-Similarly, a transfer rule does not have any extra affect on which files are
+Similarly, a transfer rule does not have any extra effect on which files are
 deleted on the receiving side, so setting a maximum file size for the transfer
 does not prevent big files from being deleted.
 


-- 
The rsync repository.

_______________________________________________
rsync-cvs mailing list
rsync-cvs@lists.samba.org
https://lists.samba.org/mailman/listinfo/rsync-cvs

Reply via email to