From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michael Tremer To: development@lists.ipfire.org Subject: Re: [PATCH] rsync: Update to version 3.2.6 and fix Bug#12947 Date: Tue, 04 Oct 2022 13:51:16 +0100 Message-ID: <270B43E8-4686-49C7-B564-AAFF60D03823@ipfire.org> In-Reply-To: <3e169cf8-b42d-0347-987c-c78db64ec01b@ipfire.org> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1064262779263396078==" List-Id: --===============1064262779263396078== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hello, > On 4 Oct 2022, at 12:07, Adolf Belka wrote: >=20 > Hi All, >=20 > Below is a patch to implement rsync-3.2.6 which fixes a bug that was in the= CVE-2022-29154 patch that was merged into CU170. >=20 > This causes rsync to stop with an error. There is a person on the community= forum that is suffering from this and of course as it is an addon, rolling b= ack the CU doesn't help as the addon server always has the latest version. Rolling back would also make us vulnerable again. > I know that CU171 is due to close shortly or maybe has already closed. Is t= here any possibility to get this patch merged into CU171, otherwise it would = not get issued until CU172. I think this should be fine as rsync is kind of broken as it is. Reviewed-by: Michael Tremer >=20 > Regards, > Adolf. >=20 > On 04/10/2022 12:54, Adolf Belka wrote: >> - Update from version 3.2.4 plus CVE-2022-29154 patch to 3.2.6 >> - Patch for CVE-2022-29154 applied in CU170 turned out to have a bug with= in it causing >> rsync to fail with an error. Four additional commits were done to fix t= his bug and >> its consequences but these were all applied in the rsync git repo after= the patch had >> been merged into CU170. >> - Version 3.2.5 onwards contains the CVE-2022-29154 fix and associated com= mits. >> - No update of rootfile required. >> - Changelog >> NEWS for rsync 3.2.6 (9 Sep 2022) >> BUG FIXES: >> More path-cleaning improvements in the file-list validation code to a= void >> rejecting of valid args. >> A file-list validation fix for a --files-from file that ends without a >> line-terminating character. >> Added a safety check that prevents the sender from removing destinati= on >> files when a local copy using --remove-source-files has some = files that are >> shared between the sending & receiving hierarchies, including= the case >> where the source dir & destination dir are identical. >> Fixed a bug in the internal MD4 checksum code that could cause the di= gest to >> be sporadically incorrect (the openssl version was/is fine). >> A minor tweak to rrsync added "copy-devices" to the list of known arg= s, but >> left it disabled by default. >> ENHANCEMENTS: >> Rename --protect-args to --secluded-args to make it clearer how it di= ffers >> from the default backslash-escaped arg-protecting behavior of= rsync. The >> old option names are still accepted. The environment-variable= override did >> not change its name. >> PACKAGING RELATED: >> The configure option --with-protected-args was renamed to >> --with-secluded-args. This option makes --secluded-args the d= efault rsync >> behavior instead of using backslash escaping for protecting a= rgs. >> The mkgitver script now makes sure that a .git dir/file is in the top= -level >> source dir before calling git describe. It also runs a basic = check on the >> version value. This should avoid using an unrelated git descr= iption for >> rsync's version. >> DEVELOPER RELATED: >> The configure script no longer sets the -=E2=81=A0pedantic-errors CFL= AG (which it >> used to try to do only for gcc). >> The name_num_obj struct was modified to allow its dynamic name_num_it= em list >> to be initialized in a better way. >> NEWS for rsync 3.2.5 (14 Aug 2022) >> 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 in= clude >> recursive names that should have been excluded by the sender.= These extra >> safety checks only require the receiver rsync to be updated. = When dealing >> with an untrusted sending host, it is safest to copy into a d= edicated >> destination directory for the remote content (i.e. don't copy= into a >> destination directory that contains files that aren't from th= e remote host >> unless you trust the remote host). Fixes CVE-2022-29154. >> A fix for CVE-2022-37434 in the bundled zlib (buffer overflow issue). >> BUG FIXES: >> Fixed the handling of filenames specified with backslash-quoted wildc= ards >> when the default remote-arg-escaping is enabled. >> Fixed the configure check for signed char that was causing a host that >> defaults to unsigned characters to generate bogus rolling che= cksums. This >> made rsync send mostly literal data for a copy instead of fin= ding matching >> data in the receiver's basis file (for a file that contains h= igh-bit >> characters). >> Lots of manpage improvements, including an attempt to better describe= how >> include/exclude filters work. >> If rsync is compiled with an xxhash 0.8 library and then moved to a s= ystem >> with a dynamically linked xxhash 0.7 library, we now detect t= his and >> disable the XX3 hashes (since these routines didn't stabilize= until 0.8). >> ENHANCEMENTS: >> The --trust-sender option was added as a way to bypass the extra file= -list >> safety checking (should that be required). >> PACKAGING RELATED: >> A note to those wanting to patch older rsync versions: the changes in= this >> release requires the quoted argument change from 3.2.4. Then,= you'll want >> every single code change from 3.2.5 since there is no fluff i= n this release. >> The build date that goes into the manpages is now based on the develo= per's >> release date, not on the build's local-timezone interpretatio= n of the date. >> DEVELOPER RELATED: >> Configure now defaults GETGROUPS_T to gid_t when cross compiling. >> 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 d= efined in the >> main string.h file. >> Signed-off-by: Adolf Belka >> --- >> lfs/rsync | 9 +- >> src/patches/rsync-CVE-2022-29154.patch | 322 ------------------------- >> 2 files changed, 3 insertions(+), 328 deletions(-) >> delete mode 100644 src/patches/rsync-CVE-2022-29154.patch >> diff --git a/lfs/rsync b/lfs/rsync >> index c27258929..07a56f96d 100644 >> --- a/lfs/rsync >> +++ b/lfs/rsync >> @@ -26,7 +26,7 @@ include Config >> SUMMARY =3D Versatile tool for fast incremental file transfer >> -VER =3D 3.2.4 >> +VER =3D 3.2.6 >> THISAPP =3D rsync-$(VER) >> DL_FILE =3D $(THISAPP).tar.gz >> @@ -34,7 +34,7 @@ DL_FROM =3D $(URL_IPFIRE) >> DIR_APP =3D $(DIR_SRC)/$(THISAPP) >> TARGET =3D $(DIR_INFO)/$(THISAPP) >> PROG =3D rsync >> -PAK_VER =3D 15 >> +PAK_VER =3D 16 >> DEPS =3D >> @@ -48,7 +48,7 @@ objects =3D $(DL_FILE) >> $(DL_FILE) =3D $(DL_FROM)/$(DL_FILE) >> -$(DL_FILE)_BLAKE2 =3D a67fcb9619874f1c5346a876138e59f4bf508a90736f830fb2= b4eaf180ab11f15a0a7db9b3b28c3b990b77c2b0973d8e668bf509e4134f464159ed3172f53d80 >> +$(DL_FILE)_BLAKE2 =3D fa0c4aa9cdffbc9ffd4f81e8c3cdc1fda7080f80c1923084c6d= 705e6872caaba31c13de4603c9462f312dbbdae76520c27d3f4f40b327f1e66c7127b1d05ea73 >> install : $(TARGET) >> @@ -85,9 +85,6 @@ $(TARGET) : $(patsubst %,$(DIR_DL)/%,$(objects)) >> # Replace shebang in rsync-ssl >> cd $(DIR_APP) && sed -i -e "s@^#!.*@#!/bin/bash@" rsync-ssl >> - # Fix for CVE-2022-29154 >> - cd $(DIR_APP) && patch -Np1 < $(DIR_SRC)/src/patches/rsync-CVE-2022-2915= 4.patch >> - >> cd $(DIR_APP) && ./configure \ >> --prefix=3D/usr \ >> --without-included-popt \ >> diff --git a/src/patches/rsync-CVE-2022-29154.patch b/src/patches/rsync-CV= E-2022-29154.patch >> deleted file mode 100644 >> index d3b4499a4..000000000 >> --- a/src/patches/rsync-CVE-2022-29154.patch >> +++ /dev/null >> @@ -1,322 +0,0 @@ >> -commit b7231c7d02cfb65d291af74ff66e7d8c507ee871 >> -Author: Wayne Davison >> -Date: Sun Jul 31 16:55:34 2022 -0700 >> - >> - Some extra file-list safety checks. >> - >> -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 =3D { .debug_type =3D "" }; >> - filter_rule_list cvs_filter_list =3D { .debug_type =3D " [global CVS]" }; >> - filter_rule_list daemon_filter_list =3D { .debug_type =3D " [daemon]" }; >> -+filter_rule_list implied_filter_list =3D { .debug_type =3D " [implied]" = }; >> - >> - int saw_xattr_filter =3D 0; >> -+int trust_sender_filter =3D 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 =3D 0, backslash_cnt =3D 0; >> -+ int slash_cnt =3D 1; /* We know we're adding a leading slash. */ >> -+ const char *cp; >> -+ char *p; >> -+ if (old_style_args || list_only || filesfrom_host !=3D NULL) >> -+ return; >> -+ if (relative_paths) { >> -+ cp =3D strstr(arg, "/./"); >> -+ if (cp) >> -+ arg =3D cp+3; >> -+ } else { >> -+ if ((cp =3D strrchr(arg, '/')) !=3D NULL) >> -+ arg =3D cp + 1; >> -+ } >> -+ arg_len =3D strlen(arg); >> -+ if (arg_len) { >> -+ if (strpbrk(arg, "*[?")) { >> -+ /* We need to add room to escape backslashes if wildcard chars are pr= esent. */ >> -+ cp =3D arg; >> -+ while ((cp =3D strchr(cp, '\\')) !=3D NULL) { >> -+ arg_len++; >> -+ cp++; >> -+ } >> -+ saw_wild =3D 1; >> -+ } >> -+ arg_len++; /* Leave room for the prefixed slash */ >> -+ rule =3D new0(filter_rule); >> -+ if (!implied_filter_list.head) >> -+ implied_filter_list.head =3D implied_filter_list.tail =3D rule; >> -+ else { >> -+ rule->next =3D implied_filter_list.head; >> -+ implied_filter_list.head =3D rule; >> -+ } >> -+ rule->rflags =3D FILTRULE_INCLUDE + (saw_wild ? FILTRULE_WILD : 0); >> -+ p =3D rule->pattern =3D new_array(char, arg_len + 1); >> -+ *p++ =3D '/'; >> -+ cp =3D arg; >> -+ while (*cp) { >> -+ switch (*cp) { >> -+ case '\\': >> -+ backslash_cnt++; >> -+ if (saw_wild) >> -+ *p++ =3D '\\'; >> -+ *p++ =3D *cp++; >> -+ break; >> -+ case '/': >> -+ if (p[-1] =3D=3D '/') /* This is safe because of the initial slash. = */ >> -+ break; >> -+ if (relative_paths) { >> -+ filter_rule const *ent; >> -+ int found =3D 0; >> -+ *p =3D '\0'; >> -+ for (ent =3D implied_filter_list.head; ent; ent =3D ent->next) { >> -+ if (ent !=3D rule && strcmp(ent->pattern, rule->pattern) =3D=3D 0) >> -+ found =3D 1; >> -+ } >> -+ if (!found) { >> -+ filter_rule *R_rule =3D new0(filter_rule); >> -+ R_rule->rflags =3D FILTRULE_INCLUDE + (saw_wild ? FILTRULE_WILD : = 0); >> -+ R_rule->pattern =3D strdup(rule->pattern); >> -+ R_rule->u.slash_cnt =3D slash_cnt; >> -+ R_rule->next =3D implied_filter_list.head; >> -+ implied_filter_list.head =3D R_rule; >> -+ } >> -+ } >> -+ slash_cnt++; >> -+ *p++ =3D *cp++; >> -+ break; >> -+ default: >> -+ *p++ =3D *cp++; >> -+ break; >> -+ } >> -+ } >> -+ *p =3D '\0'; >> -+ rule->u.slash_cnt =3D slash_cnt; >> -+ arg =3D (const char *)rule->pattern; >> -+ } >> -+ >> -+ if (recurse || xfer_dirs) { >> -+ /* Now create a rule with an added "/" & "**" or "*" at the end */ >> -+ rule =3D new0(filter_rule); >> -+ if (recurse) >> -+ rule->rflags =3D FILTRULE_INCLUDE | FILTRULE_WILD | FILTRULE_WILD2; >> -+ else >> -+ rule->rflags =3D 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 esc= aped. */ >> -+ p =3D rule->pattern =3D new_array(char, arg_len + backslash_cnt + 3 += 1); >> -+ cp =3D arg; >> -+ while (*cp) { >> -+ if (*cp =3D=3D '\\') >> -+ *p++ =3D '\\'; >> -+ *p++ =3D *cp++; >> -+ } >> -+ } else { >> -+ p =3D rule->pattern =3D new_array(char, arg_len + 3 + 1); >> -+ if (arg_len) { >> -+ memcpy(p, arg, arg_len); >> -+ p +=3D arg_len; >> -+ } >> -+ } >> -+ if (p[-1] !=3D '/') >> -+ *p++ =3D '/'; >> -+ *p++ =3D '*'; >> -+ if (recurse) >> -+ *p++ =3D '*'; >> -+ *p =3D '\0'; >> -+ rule->u.slash_cnt =3D slash_cnt + 1; >> -+ rule->next =3D implied_filter_list.head; >> -+ implied_filter_list.head =3D rule; >> -+ } >> -+} >> -+ >> - /* This frees any non-inherited items, leaving just inherited items on t= he list. */ >> - static void pop_filter_list(filter_rule_list *listp) >> - { >> -@@ -718,7 +845,7 @@ static void report_filter_result(enum logcode code, c= har 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!=3D's'][!(ent->rflags & FILTRULE_INCLUDE)], >> -+ w, actions[*w=3D=3D'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 **rule= str_ptr, >> - } >> - switch (ch) { >> - case ':': >> -+ trust_sender_filter =3D 1; >> - rule->rflags |=3D 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, st= ruct file_list *flist, int x >> - exit_cleanup(RERR_UNSUPPORTED); >> - } >> - >> -+ if (*thisname !=3D '.' || thisname[1] !=3D '\0') { >> -+ int filt_flags =3D 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, fi= lt_flags) < 0) { >> -+ rprintf(FERROR, "ERROR: rejecting excluded file-list name: %s\n", thi= sname); >> -+ exit_cleanup(RERR_PROTOCOL); >> -+ } >> -+ if (implied_filter_list.head && check_filter(&implied_filter_list, FIN= FO, thisname, filt_flags) <=3D 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 !=3D eob) { >> - if (*s++ =3D=3D '\0') { >> - ff_xb.len =3D 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 =3D ff_xb.buf + ff_xb.pos; >> - char *t =3D ff_xb.buf; >> - char *eob =3D f + len; >> -+ char *cur =3D t; >> - /* Eliminate any multi-'\0' runs. */ >> - while (f !=3D eob) { >> - if (!(*t++ =3D *f++)) { >> -+ add_implied_include(cur); >> -+ cur =3D t; >> - while (f !=3D eob && *f =3D=3D '\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 *u= ser, char **remote_argv, in >> - #ifdef ICONV_CONST >> - setup_iconv(); >> - #endif >> -+ trust_sender_filter =3D 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 =3D 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 =3D 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 =3D "."; >> - remote_argv[i] =3D 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_nam= e) >> - if (DEBUG_GTE(RECV, 1)) >> - rprintf(FINFO, "recv_files(%s)\n", fname); >> - >> -- if (daemon_filter_list.head && (*fname !=3D '.' || fname[1] !=3D '\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 !=3D '.' || fname[1] !=3D '\0')= ) { >> -+ int filt_flags =3D 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 e= xcluded file: %s\n", >> -+ fname); >> -+ exit_cleanup(RERR_PROTOCOL); >> -+ } >> - } >> - >> - #ifdef SUPPORT_XATTRS --===============1064262779263396078==--