From 9dd891bb28f6cc558b87905f1adc5e3d4180b6b3 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Wed, 23 Jan 2002 04:57:18 +0000 Subject: [PATCH] Signedness security patch from Sebastian Krahmer -- in some cases we were not sufficiently careful about reading integers from the network. Also, make sure log messages are always nul-terminated. --- NEWS | 8 ++++++++ exclude.c | 3 ++- fileio.c | 4 ++-- flist.c | 6 +++++- io.c | 26 +++++++++++++------------- log.c | 2 +- receiver.c | 3 ++- rsync.h | 6 +++--- util.c | 6 +++--- 9 files changed, 39 insertions(+), 25 deletions(-) diff --git a/NEWS b/NEWS index 55ee031b..0d6b9c6d 100644 --- a/NEWS +++ b/NEWS @@ -1,7 +1,15 @@ rsync 2.5.2 (???) + SECURITY FIXES: + + * Signedness security patch from Sebastian Krahmer + -- in some cases we were not sufficiently + careful about reading integers from the network. + BUG FIXES: + * Fix possible string mangling in log files. + * Fix for setting local address of outgoing sockets. * Better handling of hardlinks and devices on platforms with diff --git a/exclude.c b/exclude.c index 53852f69..a9d32a3e 100644 --- a/exclude.c +++ b/exclude.c @@ -299,7 +299,8 @@ void send_exclude_list(int f) void recv_exclude_list(int f) { char line[MAXPATHLEN]; - int l; + unsigned int l; + while ((l=read_int(f))) { if (l >= MAXPATHLEN) overflow("recv_exclude_list"); read_sbuf(f,line,l); diff --git a/fileio.c b/fileio.c index c39d6b02..3ed28036 100644 --- a/fileio.c +++ b/fileio.c @@ -36,7 +36,7 @@ int sparse_end(int f) } -static int write_sparse(int f,char *buf,int len) +static int write_sparse(int f,char *buf,size_t len) { int l1=0,l2=0; int ret; @@ -69,7 +69,7 @@ static int write_sparse(int f,char *buf,int len) -int write_file(int f,char *buf,int len) +int write_file(int f,char *buf,size_t len) { int ret = 0; diff --git a/flist.c b/flist.c index 843cf9e4..5d1a2590 100644 --- a/flist.c +++ b/flist.c @@ -375,7 +375,7 @@ static void receive_file_entry(struct file_struct **fptr, static gid_t last_gid; static char lastname[MAXPATHLEN]; char thisname[MAXPATHLEN]; - int l1=0,l2=0; + unsigned int l1=0,l2=0; char *p; struct file_struct *file; @@ -442,6 +442,10 @@ static void receive_file_entry(struct file_struct **fptr, if (preserve_links && S_ISLNK(file->mode)) { int l = read_int(f); + if (l < 0) { + rprintf(FERROR,"overflow: l=%d\n", l); + overflow("receive_file_entry"); + } file->link = (char *)malloc(l+1); if (!file->link) out_of_memory("receive_file_entry 2"); read_sbuf(f,file->link,l); diff --git a/io.c b/io.c index e57d0708..95fb73b6 100644 --- a/io.c +++ b/io.c @@ -49,7 +49,7 @@ int kludge_around_eof = False; static int io_error_fd = -1; -static void read_loop(int fd, char *buf, int len); +static void read_loop(int fd, char *buf, size_t len); static void check_timeout(void) { @@ -163,7 +163,7 @@ static void die_from_readerr (int err) * give a better explanation. We can tell whether the connection has * started by looking e.g. at whether the remote version is known yet. */ -static int read_timeout (int fd, char *buf, int len) +static int read_timeout (int fd, char *buf, size_t len) { int n, ret=0; @@ -236,7 +236,7 @@ static int read_timeout (int fd, char *buf, int len) /*! Continue trying to read len bytes - don't return until len has been read. */ -static void read_loop (int fd, char *buf, int len) +static void read_loop (int fd, char *buf, size_t len) { while (len) { int n = read_timeout(fd, buf, len); @@ -253,7 +253,7 @@ static void read_loop (int fd, char *buf, int len) * * Never returns <= 0. */ -static int read_unbuffered(int fd, char *buf, int len) +static int read_unbuffered(int fd, char *buf, size_t len) { static int remaining; int tag, ret=0; @@ -305,7 +305,7 @@ static int read_unbuffered(int fd, char *buf, int len) /* do a buffered read from fd. don't return until all N bytes have been read. If all N can't be read then exit with an error */ -static void readfd (int fd, char *buffer, int N) +static void readfd (int fd, char *buffer, size_t N) { int ret; int total=0; @@ -356,12 +356,12 @@ int64 read_longint(int f) return ret; } -void read_buf(int f,char *buf,int len) +void read_buf(int f,char *buf,size_t len) { readfd(f,buf,len); } -void read_sbuf(int f,char *buf,int len) +void read_sbuf(int f,char *buf,size_t len) { read_buf (f,buf,len); buf[len] = 0; @@ -375,7 +375,7 @@ unsigned char read_byte(int f) } /* write len bytes to fd */ -static void writefd_unbuffered(int fd,char *buf,int len) +static void writefd_unbuffered(int fd,char *buf,size_t len) { int total = 0; fd_set w_fds, r_fds; @@ -483,7 +483,7 @@ void io_start_buffering(int fd) /* write an message to a multiplexed stream. If this fails then rsync exits */ -static void mplex_write(int fd, enum logcode code, char *buf, int len) +static void mplex_write(int fd, enum logcode code, char *buf, size_t len) { char buffer[4096]; int n = len; @@ -533,7 +533,7 @@ void io_end_buffering(int fd) } } -static void writefd(int fd,char *buf,int len) +static void writefd(int fd,char *buf,size_t len) { stats.total_written += len; @@ -587,7 +587,7 @@ void write_longint(int f, int64 x) writefd(f,b,8); } -void write_buf(int f,char *buf,int len) +void write_buf(int f,char *buf,size_t len) { writefd(f,buf,len); } @@ -606,7 +606,7 @@ void write_byte(int f,unsigned char c) -int read_line(int f, char *buf, int maxlen) +int read_line(int f, char *buf, size_t maxlen) { while (maxlen) { buf[0] = 0; @@ -664,7 +664,7 @@ void io_start_multiplex_in(int fd) } /* write an message to the multiplexed error stream */ -int io_multiplex_write(enum logcode code, char *buf, int len) +int io_multiplex_write(enum logcode code, char *buf, size_t len) { if (!io_multiplexing_out) return 0; diff --git a/log.c b/log.c index 3b53a9ae..421829bd 100644 --- a/log.c +++ b/log.c @@ -466,7 +466,7 @@ static void log_formatted(enum logcode code, l = strlen(n); - if ((l-1) + ((int)(s - &buf[0])) > sizeof(buf)) { + if (l + ((int)(s - &buf[0])) >= sizeof(buf)) { rprintf(FERROR,"buffer overflow expanding %%%c - exiting\n", p[0]); exit_cleanup(RERR_MESSAGEIO); diff --git a/receiver.c b/receiver.c index a574ea63..7628b10e 100644 --- a/receiver.c +++ b/receiver.c @@ -206,7 +206,8 @@ static int get_tmpname(char *fnametmp, char *fname) static int receive_data(int f_in,struct map_struct *buf,int fd,char *fname, OFF_T total_size) { - int i,n,remainder,len,count; + int i; + unsigned int n,remainder,len,count; OFF_T offset = 0; OFF_T offset2; char *data; diff --git a/rsync.h b/rsync.h index d18540f1..9565ed77 100644 --- a/rsync.h +++ b/rsync.h @@ -374,9 +374,9 @@ struct sum_buf { struct sum_struct { OFF_T flength; /* total file length */ - int count; /* how many chunks */ - int remainder; /* flength % block_length */ - int n; /* block_length */ + size_t count; /* how many chunks */ + size_t remainder; /* flength % block_length */ + size_t n; /* block_length */ struct sum_buf *sums; /* points to info for each chunk */ }; diff --git a/util.c b/util.c index c5bd662d..e6acf50e 100644 --- a/util.c +++ b/util.c @@ -275,7 +275,7 @@ int create_directory_path(char *fname) derived from GNU C's cccp.c. */ -static int full_write(int desc, char *ptr, int len) +static int full_write(int desc, char *ptr, size_t len) { int total_written; @@ -301,11 +301,11 @@ static int full_write(int desc, char *ptr, int len) for an error. derived from GNU C's cccp.c. */ -static int safe_read(int desc, char *ptr, int len) +static int safe_read(int desc, char *ptr, size_t len) { int n_chars; - if (len <= 0) + if (len == 0) return len; #ifdef EINTR -- 2.34.1