forked from openwrt/packages
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
The rsync package is vulnerable to CVE-2022-29154[1], which is not yet in a non-preview release. This commit applies the upstream commit to fix it and several subsequent commits needed to fix bugs the initial fix introduced[2]. 1. https://rsync.samba.org/ftp/rsync/NEWS#SECURITY_FIXES-3.2.5 2. https://bugs.archlinux.org/task/75558 Signed-off-by: John Audia <[email protected]>
- Loading branch information
Showing
4 changed files
with
565 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,385 @@ | ||
| From b7231c7d02cfb65d291af74ff66e7d8c507ee871 Mon Sep 17 00:00:00 2001 | ||
| From: Wayne Davison <[email protected]> | ||
| Date: Sun, 31 Jul 2022 16:55:34 -0700 | ||
| Subject: [PATCH] Some extra file-list safety checks. | ||
|
|
||
| --- | ||
| exclude.c | 130 ++++++++++++++++++++++++++++++++++++++++++++++++++++- | ||
| flist.c | 17 ++++++- | ||
| io.c | 4 ++ | ||
| main.c | 7 ++- | ||
| receiver.c | 11 +++-- | ||
| rsync.1.md | 44 ++++++++++++++++-- | ||
| 6 files changed, 202 insertions(+), 11 deletions(-) | ||
|
|
||
| --- 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 *l | ||
| } | ||
| } | ||
|
|
||
| +/* 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 lo | ||
| : 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 | ||
| } | ||
| switch (ch) { | ||
| case ':': | ||
| + trust_sender_filter = 1; | ||
| rule->rflags |= FILTRULE_PERDIR_MERGE | ||
| | FILTRULE_FINISH_SETUP; | ||
| /* FALL THROUGH */ | ||
| --- 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_ent | ||
| 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 */ | ||
| --- 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++; | ||
| } | ||
| --- 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 *mac | ||
| #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 * | ||
| 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 * | ||
| if (!rsync_port && !*arg) /* Turn an empty arg into a dot dir. */ | ||
| arg = "."; | ||
| remote_argv[i] = arg; | ||
| + add_implied_include(arg); | ||
| } | ||
| } | ||
|
|
||
| --- a/receiver.c | ||
| +++ b/receiver.c | ||
| @@ -593,10 +593,13 @@ int recv_files(int f_in, int f_out, char | ||
| 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 | ||
| --- a/rsync.1.md | ||
| +++ b/rsync.1.md | ||
| @@ -154,6 +154,33 @@ rsync daemon by leaving off the module n | ||
|
|
||
| See the following section for more details. | ||
|
|
||
| +## 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 | ||
| @@ -2323,6 +2350,12 @@ your home directory (remove the '=' for | ||
| 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` | ||
| @@ -3754,8 +3787,13 @@ available rule prefixes: | ||
|
|
||
| 0. `exclude, '-'` specifies an exclude pattern. | ||
| 0. `include, '+'` specifies an include pattern. | ||
| -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. | ||
| 0. `show, 'S'` files that match the pattern are not hidden. | ||
| 0. `protect, 'P'` specifies a pattern for protecting files from deletion. |
Oops, something went wrong.