Skip to content

Commit

Permalink
[Backport release-24.11] rsync: apply patches for 6 vulnerabilities (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
LeSuisse authored Jan 14, 2025
2 parents b5c5bf4 + 095a971 commit 80361d5
Show file tree
Hide file tree
Showing 13 changed files with 930 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
From 0902b52f6687b1f7952422080d50b93108742e53 Mon Sep 17 00:00:00 2001
From: Wayne Davison <[email protected]>
Date: Tue, 29 Oct 2024 22:55:29 -0700
Subject: [PATCH 1/2] Some checksum buffer fixes.

- Put sum2_array into sum_struct to hold an array of sum2 checksums
that are each xfer_sum_len bytes.
- Remove sum2 buf from sum_buf.
- Add macro sum2_at() to access each sum2 array element.
- Throw an error if a sums header has an s2length larger than
xfer_sum_len.
---
io.c | 3 ++-
match.c | 8 ++++----
rsync.c | 5 ++++-
rsync.h | 4 +++-
sender.c | 4 +++-
5 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/io.c b/io.c
index a99ac0ec..bb60eeca 100644
--- a/io.c
+++ b/io.c
@@ -55,6 +55,7 @@ extern int read_batch;
extern int compat_flags;
extern int protect_args;
extern int checksum_seed;
+extern int xfer_sum_len;
extern int daemon_connection;
extern int protocol_version;
extern int remove_source_files;
@@ -1977,7 +1978,7 @@ void read_sum_head(int f, struct sum_struct *sum)
exit_cleanup(RERR_PROTOCOL);
}
sum->s2length = protocol_version < 27 ? csum_length : (int)read_int(f);
- if (sum->s2length < 0 || sum->s2length > MAX_DIGEST_LEN) {
+ if (sum->s2length < 0 || sum->s2length > xfer_sum_len) {
rprintf(FERROR, "Invalid checksum length %d [%s]\n",
sum->s2length, who_am_i());
exit_cleanup(RERR_PROTOCOL);
diff --git a/match.c b/match.c
index cdb30a15..36e78ed2 100644
--- a/match.c
+++ b/match.c
@@ -232,7 +232,7 @@ static void hash_search(int f,struct sum_struct *s,
done_csum2 = 1;
}

- if (memcmp(sum2,s->sums[i].sum2,s->s2length) != 0) {
+ if (memcmp(sum2, sum2_at(s, i), s->s2length) != 0) {
false_alarms++;
continue;
}
@@ -252,7 +252,7 @@ static void hash_search(int f,struct sum_struct *s,
if (i != aligned_i) {
if (sum != s->sums[aligned_i].sum1
|| l != s->sums[aligned_i].len
- || memcmp(sum2, s->sums[aligned_i].sum2, s->s2length) != 0)
+ || memcmp(sum2, sum2_at(s, aligned_i), s->s2length) != 0)
goto check_want_i;
i = aligned_i;
}
@@ -271,7 +271,7 @@ static void hash_search(int f,struct sum_struct *s,
if (sum != s->sums[i].sum1)
goto check_want_i;
get_checksum2((char *)map, l, sum2);
- if (memcmp(sum2, s->sums[i].sum2, s->s2length) != 0)
+ if (memcmp(sum2, sum2_at(s, i), s->s2length) != 0)
goto check_want_i;
/* OK, we have a re-alignment match. Bump the offset
* forward to the new match point. */
@@ -290,7 +290,7 @@ static void hash_search(int f,struct sum_struct *s,
&& (!updating_basis_file || s->sums[want_i].offset >= offset
|| s->sums[want_i].flags & SUMFLG_SAME_OFFSET)
&& sum == s->sums[want_i].sum1
- && memcmp(sum2, s->sums[want_i].sum2, s->s2length) == 0) {
+ && memcmp(sum2, sum2_at(s, want_i), s->s2length) == 0) {
/* we've found an adjacent match - the RLL coder
* will be happy */
i = want_i;
diff --git a/rsync.c b/rsync.c
index cd288f57..b130aba5 100644
--- a/rsync.c
+++ b/rsync.c
@@ -437,7 +437,10 @@ int read_ndx_and_attrs(int f_in, int f_out, int *iflag_ptr, uchar *type_ptr, cha
*/
void free_sums(struct sum_struct *s)
{
- if (s->sums) free(s->sums);
+ if (s->sums) {
+ free(s->sums);
+ free(s->sum2_array);
+ }
free(s);
}

diff --git a/rsync.h b/rsync.h
index d3709fe0..8ddbe702 100644
--- a/rsync.h
+++ b/rsync.h
@@ -958,12 +958,12 @@ struct sum_buf {
uint32 sum1; /**< simple checksum */
int32 chain; /**< next hash-table collision */
short flags; /**< flag bits */
- char sum2[SUM_LENGTH]; /**< checksum */
};

struct sum_struct {
OFF_T flength; /**< total file length */
struct sum_buf *sums; /**< points to info for each chunk */
+ char *sum2_array; /**< checksums of length xfer_sum_len */
int32 count; /**< how many chunks */
int32 blength; /**< block_length */
int32 remainder; /**< flength % block_length */
@@ -982,6 +982,8 @@ struct map_struct {
int status; /* first errno from read errors */
};

+#define sum2_at(s, i) ((s)->sum2_array + ((OFF_T)(i) * xfer_sum_len))
+
#define NAME_IS_FILE (0) /* filter name as a file */
#define NAME_IS_DIR (1<<0) /* filter name as a dir */
#define NAME_IS_XATTR (1<<2) /* filter name as an xattr */
diff --git a/sender.c b/sender.c
index 3d4f052e..ab205341 100644
--- a/sender.c
+++ b/sender.c
@@ -31,6 +31,7 @@ extern int log_before_transfer;
extern int stdout_format_has_i;
extern int logfile_format_has_i;
extern int want_xattr_optim;
+extern int xfer_sum_len;
extern int csum_length;
extern int append_mode;
extern int copy_links;
@@ -94,10 +95,11 @@ static struct sum_struct *receive_sums(int f)
return(s);

s->sums = new_array(struct sum_buf, s->count);
+ s->sum2_array = new_array(char, s->count * xfer_sum_len);

for (i = 0; i < s->count; i++) {
s->sums[i].sum1 = read_int(f);
- read_buf(f, s->sums[i].sum2, s->s2length);
+ read_buf(f, sum2_at(s, i), s->s2length);

s->sums[i].offset = offset;
s->sums[i].flags = 0;
--
2.34.1

Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
From 42e2b56c4ede3ab164f9a5c6dae02aa84606a6c1 Mon Sep 17 00:00:00 2001
From: Wayne Davison <[email protected]>
Date: Tue, 5 Nov 2024 11:01:03 -0800
Subject: [PATCH 2/2] Another cast when multiplying integers.

---
rsync.h | 2 +-
sender.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/rsync.h b/rsync.h
index 8ddbe702..0f9e277f 100644
--- a/rsync.h
+++ b/rsync.h
@@ -982,7 +982,7 @@ struct map_struct {
int status; /* first errno from read errors */
};

-#define sum2_at(s, i) ((s)->sum2_array + ((OFF_T)(i) * xfer_sum_len))
+#define sum2_at(s, i) ((s)->sum2_array + ((size_t)(i) * xfer_sum_len))

#define NAME_IS_FILE (0) /* filter name as a file */
#define NAME_IS_DIR (1<<0) /* filter name as a dir */
diff --git a/sender.c b/sender.c
index ab205341..2bbff2fa 100644
--- a/sender.c
+++ b/sender.c
@@ -95,7 +95,7 @@ static struct sum_struct *receive_sums(int f)
return(s);

s->sums = new_array(struct sum_buf, s->count);
- s->sum2_array = new_array(char, s->count * xfer_sum_len);
+ s->sum2_array = new_array(char, (size_t)s->count * xfer_sum_len);

for (i = 0; i < s->count; i++) {
s->sums[i].sum1 = read_int(f);
--
2.34.1

Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
From cf620065502f065d4ea44f5df4f81295a738aa21 Mon Sep 17 00:00:00 2001
From: Andrew Tridgell <[email protected]>
Date: Thu, 14 Nov 2024 09:57:08 +1100
Subject: [PATCH] prevent information leak off the stack

prevent leak of uninitialised stack data in hash_search
---
match.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/match.c b/match.c
index 36e78ed2..dfd6af2c 100644
--- a/match.c
+++ b/match.c
@@ -147,6 +147,9 @@ static void hash_search(int f,struct sum_struct *s,
int more;
schar *map;

+ // prevent possible memory leaks
+ memset(sum2, 0, sizeof sum2);
+
/* want_i is used to encourage adjacent matches, allowing the RLL
* coding of the output to work more efficiently. */
want_i = 0;
--
2.34.1

Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
From 3feb8669d875d03c9ceb82e208ef40ddda8eb908 Mon Sep 17 00:00:00 2001
From: Andrew Tridgell <[email protected]>
Date: Sat, 23 Nov 2024 11:08:03 +1100
Subject: [PATCH 1/4] refuse fuzzy options when fuzzy not selected

this prevents a malicious server providing a file to compare to when
the user has not given the fuzzy option
---
receiver.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/receiver.c b/receiver.c
index 6b4b369e..2d7f6033 100644
--- a/receiver.c
+++ b/receiver.c
@@ -66,6 +66,7 @@ extern char sender_file_sum[MAX_DIGEST_LEN];
extern struct file_list *cur_flist, *first_flist, *dir_flist;
extern filter_rule_list daemon_filter_list;
extern OFF_T preallocated_len;
+extern int fuzzy_basis;

extern struct name_num_item *xfer_sum_nni;
extern int xfer_sum_len;
@@ -716,6 +717,10 @@ int recv_files(int f_in, int f_out, char *local_name)
fnamecmp = get_backup_name(fname);
break;
case FNAMECMP_FUZZY:
+ if (fuzzy_basis == 0) {
+ rprintf(FERROR_XFER, "rsync: refusing malicious fuzzy operation for %s\n", xname);
+ exit_cleanup(RERR_PROTOCOL);
+ }
if (file->dirname) {
pathjoin(fnamecmpbuf, sizeof fnamecmpbuf, file->dirname, xname);
fnamecmp = fnamecmpbuf;
--
2.34.1

Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
From 33385aefe4773e7a3982d41995681eb079c92d12 Mon Sep 17 00:00:00 2001
From: Andrew Tridgell <[email protected]>
Date: Sat, 23 Nov 2024 12:26:10 +1100
Subject: [PATCH 2/4] added secure_relative_open()

this is an open that enforces no symlink following for all path
components in a relative path
---
syscall.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 74 insertions(+)

diff --git a/syscall.c b/syscall.c
index d92074aa..a4b7f542 100644
--- a/syscall.c
+++ b/syscall.c
@@ -33,6 +33,8 @@
#include <sys/syscall.h>
#endif

+#include "ifuncs.h"
+
extern int dry_run;
extern int am_root;
extern int am_sender;
@@ -712,3 +714,75 @@ int do_open_nofollow(const char *pathname, int flags)

return fd;
}
+
+/*
+ open a file relative to a base directory. The basedir can be NULL,
+ in which case the current working directory is used. The relpath
+ must be a relative path, and the relpath must not contain any
+ elements in the path which follow symlinks (ie. like O_NOFOLLOW, but
+ applies to all path components, not just the last component)
+*/
+int secure_relative_open(const char *basedir, const char *relpath, int flags, mode_t mode)
+{
+ if (!relpath || relpath[0] == '/') {
+ // must be a relative path
+ errno = EINVAL;
+ return -1;
+ }
+
+#if !defined(O_NOFOLLOW) || !defined(O_DIRECTORY)
+ // really old system, all we can do is live with the risks
+ if (!basedir) {
+ return open(relpath, flags, mode);
+ }
+ char fullpath[MAXPATHLEN];
+ pathjoin(fullpath, sizeof fullpath, basedir, relpath);
+ return open(fullpath, flags, mode);
+#else
+ int dirfd = AT_FDCWD;
+ if (basedir != NULL) {
+ dirfd = openat(AT_FDCWD, basedir, O_RDONLY | O_DIRECTORY);
+ if (dirfd == -1) {
+ return -1;
+ }
+ }
+ int retfd = -1;
+
+ char *path_copy = my_strdup(relpath, __FILE__, __LINE__);
+ if (!path_copy) {
+ return -1;
+ }
+
+ for (const char *part = strtok(path_copy, "/");
+ part != NULL;
+ part = strtok(NULL, "/"))
+ {
+ int next_fd = openat(dirfd, part, O_RDONLY | O_DIRECTORY | O_NOFOLLOW);
+ if (next_fd == -1 && errno == ENOTDIR) {
+ if (strtok(NULL, "/") != NULL) {
+ // this is not the last component of the path
+ errno = ELOOP;
+ goto cleanup;
+ }
+ // this could be the last component of the path, try as a file
+ retfd = openat(dirfd, part, flags | O_NOFOLLOW, mode);
+ goto cleanup;
+ }
+ if (next_fd == -1) {
+ goto cleanup;
+ }
+ if (dirfd != AT_FDCWD) close(dirfd);
+ dirfd = next_fd;
+ }
+
+ // the path must be a directory
+ errno = EINVAL;
+
+cleanup:
+ free(path_copy);
+ if (dirfd != AT_FDCWD) {
+ close(dirfd);
+ }
+ return retfd;
+#endif // O_NOFOLLOW, O_DIRECTORY
+}
--
2.34.1

Loading

0 comments on commit 80361d5

Please sign in to comment.