lunes, abril 14, 2008

Rsync remote code execution

Rync service with xattr support is vulnerable to a remote code execution in versions between 2.6.9 and 3.0.1
--- a/util.c
+++ b/util.c
@@ -1329,7 +1329,7 @@ void *_new_array(unsigned long num, unsigned int size, int use_calloc)
  return use_calloc ? calloc(num, size) : malloc(num * size);
 }
 
-void *_realloc_array(void *ptr, unsigned int size, unsigned long num)
+void *_realloc_array(void *ptr, unsigned int size, size_t num)
 {
  if (num >= MALLOC_MAX/size)
   return NULL;
@@ -1550,7 +1550,10 @@ void *expand_item_list(item_list *lp, size_t item_size,
    new_size += incr;
   else
    new_size *= 2;
-  new_ptr = realloc_array(lp->items, char, new_size * item_size);
+  if (new_size < lp->malloced)
+   overflow_exit("expand_item_list");
+  /* Using _realloc_array() lets us pass the size, not a type. */
+  new_ptr = _realloc_array(lp->items, item_size, new_size);
   if (verbose >= 4) {
    rprintf(FINFO, "[%s] expand %s to %.0f bytes, did%s move\n",
     who_am_i(), desc, (double)new_size * item_size,


1. size_t


First there is the check if (num >= MALLOC_MAX/size)
num*size has to be less than MALLOC_MAX

Have this check a sign problem?

rsync.h redefines size_t to unsigned int, there is not problem, and xattr patch doesn't change this to a signed one.

There is not sign problem here.

2. new_size < malloced


+ if (new_size < lp->malloced)
Now is not possible use this realloc to reduce the heap variable lp->items!!!

3. supply the fixed size instead of the type char


char should *1 I dont think this was a problem.

- new_ptr = realloc_array(lp->items, char, new_size * item_size);
+ new_ptr = _realloc_array(lp->items, item_size, new_size);

The first will use a macro to do:
_realloc_array (ptr, sizeof(char), new_size*item_size)

rsync.h:#define realloc_array(ptr, type, num) ((type*)_realloc_array((ptr), sizeof(type), (num)))

The second will realloc item_size*new_size

The Problem


Then, the problem seems the reduction of size, that was not controlled and lets overflow the lp->items array.

The Explotation


More details about explotation, soon.