Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
openSUSE:Step:15
rsync
rsync-CVE-2022-29154.patch
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File rsync-CVE-2022-29154.patch of Package rsync
From b7231c7d02cfb65d291af74ff66e7d8c507ee871 Mon Sep 17 00:00:00 2001 From: Wayne Davison <wayne@opencoder.net> 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(-) Index: rsync-3.2.3/exclude.c =================================================================== --- rsync-3.2.3.orig/exclude.c +++ rsync-3.2.3/exclude.c @@ -27,16 +27,23 @@ 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 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 int local_server; + +extern char *filesfrom_host; extern char curr_dir[MAXPATHLEN]; extern unsigned int curr_dir_len; extern unsigned int module_dirlen; @@ -44,8 +51,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) @@ -288,6 +297,143 @@ 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; + + int old_style_args = -1; + + /* + A local transfer doesn't unbackslash anything, so leave the args alone. + + This check comes from 3.2.4+ main.c check, where the CVE is fixed. In + these versions, the protection against the CVE is ran only when a remote + server is involved. + + In this version, there is no "old_style_args", as this was not + implemented yet, so we relay on detecting if a local server is + involved directly and we set a local variable "old_style_args" + to not to modify the original patch too much. + */ + if (local_server) + old_style_args = 2; + + 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) { @@ -714,7 +860,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); } @@ -886,6 +1032,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 */ Index: rsync-3.2.3/flist.c =================================================================== --- rsync-3.2.3.orig/flist.c +++ rsync-3.2.3/flist.c @@ -72,6 +72,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; @@ -82,8 +83,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; @@ -971,6 +971,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 */ Index: rsync-3.2.3/io.c =================================================================== --- rsync-3.2.3.orig/io.c +++ rsync-3.2.3/io.c @@ -414,6 +414,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'. */ @@ -445,9 +446,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++; } Index: rsync-3.2.3/main.c =================================================================== --- rsync-3.2.3.orig/main.c +++ rsync-3.2.3/main.c @@ -87,6 +87,7 @@ extern BOOL shutting_down; extern int backup_dir_len; extern int basis_dir_cnt; extern int default_af_hint; +extern int trust_sender_filter; extern struct stats stats; extern char *stdout_format; extern char *logfile_format; @@ -102,7 +103,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; @@ -642,6 +643,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. */ @@ -1516,6 +1518,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++) { @@ -1539,6 +1543,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); } } Index: rsync-3.2.3/receiver.c =================================================================== --- rsync-3.2.3.orig/receiver.c +++ rsync-3.2.3/receiver.c @@ -584,10 +584,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 Index: rsync-3.2.3/rsync.1.md =================================================================== --- rsync-3.2.3.orig/rsync.1.md +++ rsync-3.2.3/rsync.1.md @@ -304,6 +304,35 @@ separate the files into different rsync `--delay-updates` (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 (or a version patched against +CVE-2022-29154), 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 (or a version patched against +CVE-2022-29154) 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 + # EXAMPLES Here are some examples of how I use rsync. @@ -3493,8 +3522,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.
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
