On Sun, 2003-10-05 at 02:56, Wayne Davison wrote:
> On Sat, Oct 04, 2003 at 11:38:48PM +0300, Timo Sirainen wrote:
> > for (i=0; i < (int) s->count;i++) {
>
> Yeah, that's pretty bad. Attached is a patch that should fix this and a
> number of other related problems where the code assumed that size_t
> would fit into an int.
The main problem wasn't int vs. size_t. malloc() call would have
overflowed even if i had been size_t.
Included a patch that fixes all the potential malloc()/realloc()
overflows that I found. I'd feel a bit safer with them included :)
diff -ru rsync-2.5.6-old/checksum.c rsync-2.5.6/checksum.c
--- rsync-2.5.6-old/checksum.c 2002-04-08 11:29:04.000000000 +0300
+++ rsync-2.5.6/checksum.c 2003-10-07 00:40:20.000000000 +0300
@@ -56,6 +56,11 @@
static int len1;
struct mdfour m;
+ if (len <= 0 || len > INT_MAX-4) {
+ rprintf(FERROR, "overflow: len=%d\n", len);
+ overflow("get_checksum2");
+ }
+
if (len > len1) {
if (buf1) free(buf1);
buf1 = (char *)malloc(len+4);
diff -ru rsync-2.5.6-old/exclude.c rsync-2.5.6/exclude.c
--- rsync-2.5.6-old/exclude.c 2003-01-26 22:10:23.000000000 +0200
+++ rsync-2.5.6/exclude.c 2003-10-07 00:31:12.000000000 +0300
@@ -186,6 +186,11 @@
if (list && *list)
for (; (*list)[len]; len++) ;
+ if (len > INT_MAX/2) {
+ rprintf(FERROR, "overflow: len=%d\n", len);
+ overflow("add_exclude_list");
+ }
+
if (strcmp(pattern,"!") == 0) {
if (verbose > 2)
rprintf(FINFO,"clearing exclude list\n");
diff -ru rsync-2.5.6-old/flist.c rsync-2.5.6/flist.c
--- rsync-2.5.6-old/flist.c 2003-10-04 22:08:23.000000000 +0300
+++ rsync-2.5.6/flist.c 2003-10-07 00:42:40.000000000 +0300
@@ -307,7 +307,13 @@
if (flist->count >= flist->malloced) {
size_t new_bytes;
void *new_ptr;
-
+
+ if (flist->malloced >= INT_MAX/2) {
+ rprintf(FERROR,"overflow: flist->malloced=%d\n",
+ flist->malloced);
+ overflow("flist_expand");
+ }
+
if (flist->malloced < 1000)
flist->malloced += 1000;
else
@@ -543,7 +549,7 @@
if (preserve_links && S_ISLNK(file->mode)) {
int l = read_int(f);
- if (l < 0) {
+ if (l < 0 || l >= INT_MAX) {
rprintf(FERROR, "overflow: l=%d\n", l);
overflow("receive_file_entry");
}
diff -ru rsync-2.5.6-old/hlink.c rsync-2.5.6/hlink.c
--- rsync-2.5.6-old/hlink.c 2002-03-25 09:07:03.000000000 +0200
+++ rsync-2.5.6/hlink.c 2003-10-07 00:45:29.000000000 +0300
@@ -57,6 +57,11 @@
if (hlink_list)
free(hlink_list);
+ if (flist->count > INT_MAX/sizeof(hlink_list[0])) {
+ rprintf(FERROR, "overflow: flist->count=%d\n", flist->count);
+ overflow("init_hard_links");
+ }
+
if (!(hlink_list =
(struct file_struct *) malloc(sizeof(hlink_list[0]) *
flist->count)))
diff -ru rsync-2.5.6-old/log.c rsync-2.5.6/log.c
--- rsync-2.5.6-old/log.c 2002-12-24 09:42:04.000000000 +0200
+++ rsync-2.5.6/log.c 2003-10-07 00:45:19.000000000 +0300
@@ -90,6 +90,12 @@
static void err_list_add(int code, char *buf, int len)
{
struct err_list *el;
+
+ if (len < 0 || len > INT_MAX-4) {
+ rprintf(FERROR, "overflow: len=%d\n", len);
+ overflow("err_list_add");
+ }
+
el = (struct err_list *)malloc(sizeof(*el));
if (!el) exit_cleanup(RERR_MALLOC);
el->next = NULL;
diff -ru rsync-2.5.6-old/match.c rsync-2.5.6/match.c
--- rsync-2.5.6-old/match.c 2002-04-09 09:11:06.000000000 +0300
+++ rsync-2.5.6/match.c 2003-10-07 00:46:34.000000000 +0300
@@ -67,6 +67,11 @@
if (!tag_table)
tag_table = (int *)malloc(sizeof(tag_table[0])*TABLESIZE);
+ if (s->count > INT_MAX/sizeof(targets[0])) {
+ rprintf(FERROR, "overflow: s->count=%d\n", s->count);
+ overflow("build_hash_table");
+ }
+
targets = (struct target *)malloc(sizeof(targets[0])*s->count);
if (!tag_table || !targets)
out_of_memory("build_hash_table");
diff -ru rsync-2.5.6-old/receiver.c rsync-2.5.6/receiver.c
--- rsync-2.5.6-old/receiver.c 2003-10-04 22:08:23.000000000 +0300
+++ rsync-2.5.6/receiver.c 2003-10-07 00:42:03.000000000 +0300
@@ -66,6 +66,11 @@
static void add_delete_entry(struct file_struct *file)
{
if (dlist_len == dlist_alloc_len) {
+ if (dlist_alloc_len >= INT_MAX - 1024) {
+ rprintf(FERROR,"overflow: dlist_alloc_len=%d\n",
+ dlist_alloc_len);
+ overflow("add_delete_entry");
+ }
dlist_alloc_len += 1024;
delete_list = (struct delete_list *)Realloc(delete_list, sizeof(delete_list[0])*dlist_alloc_len);
if (!delete_list) out_of_memory("add_delete_entry");
diff -ru rsync-2.5.6-old/sender.c rsync-2.5.6/sender.c
--- rsync-2.5.6-old/sender.c 2003-10-05 02:12:14.000000000 +0300
+++ rsync-2.5.6/sender.c 2003-10-07 00:47:14.000000000 +0300
@@ -62,6 +61,11 @@
if (s->count == 0)
return(s);
+ if (s->count >= INT_MAX/sizeof(s->sums[0])) {
+ rprintf(FERROR, "overflow: s->count=%d\n", s->count);
+ overflow("receive_sums");
+ }
+
s->sums = (struct sum_buf *)malloc(sizeof(s->sums[0])*s->count);
if (!s->sums) out_of_memory("receive_sums");
diff -ru rsync-2.5.6-old/util.c rsync-2.5.6/util.c
--- rsync-2.5.6-old/util.c 2003-01-19 23:37:11.000000000 +0200
+++ rsync-2.5.6/util.c 2003-10-07 00:37:43.000000000 +0300
@@ -540,6 +540,11 @@
void *Realloc(void *p, int size)
{
+ if (size <= 0) {
+ /* Should never happen - potential security hole */
+ rprintf(FERROR,"Realloc() called with size=%d\n", size);
+ abort();
+ }
if (!p) return (void *)malloc(size);
return (void *)realloc(p, size);
}
--
To unsubscribe or change options: http://lists.samba.org/mailman/listinfo/rsync
Before posting, read: http://www.catb.org/~esr/faqs/smart-questions.html
