xorl %eax, %eax

CVE-2008-1720: rsync “xattr” Integer Overflow

leave a comment »

This bug was reported by Sebastian Krahmer (aka. stealth) and it affects rsync 2.6.9 to 3.0.1. However, this affects only extended attribute support enabled rsync. Here is the vulnerable code from util.c of 3.0.1 release:

void *expand_item_list(item_list *lp, size_t item_size,
		       const char *desc, int incr)
{
	/* First time through, 0 <= 0, so list is expanded. */
	if (lp->malloced <= lp->count) {
		void *new_ptr;
		size_t new_size = lp->malloced;
		if (incr < 0)
			new_size += -incr; /* increase slowly */
		else if (new_size < (size_t)incr)
			new_size += incr;
		else
			new_size *= 2;
		new_ptr = realloc_array(lp->items, char, new_size * item_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,
				new_ptr == lp->items ? " not" : "");
		}
		if (!new_ptr)
			out_of_memory("expand_item_list");

		lp->items = new_ptr;
		lp->malloced = new_size;
	}
	return (char*)lp->items + (lp->count++ * item_size);
}

This function is used internally to expand the item list passed to it as an argument (the first argument of that function). It first checks that allocated space (lp->malloced) is less than or equal to lp->count which means that it needs to expand the item list to fit lp->count additional bytes. For completeness, here is the item_list structure along with a couple C macros that are defined in rsync.h:

#define EMPTY_ITEM_LIST {NULL, 0, 0}

typedef struct {
	void *items;
	size_t count;
	size_t malloced;
} item_list;

#define EXPAND_ITEM_LIST(lp, type, incr) \
	(type*)expand_item_list(lp, sizeof (type), #type, incr)

#define EMPTY_XBUF {NULL, 0, 0, 0}

In case of a count larger than the allocated space, ‘new_size’ is initialized with the current allocated space and a few range checks take place on ‘incr’ and ‘new_size’ which is either incremented by ‘incr’ or multiplied by two. Then, realloc_array() is invoked. This is a wrapper around realloc(3) and as you can read, it attempts to allocate ‘new_size * item_size’ bytes. Obviously, this multiplication can wrap around and result in a small allocation which will subsequently result in heap memory corruption when a function will attempt to access lp->items pointer for lp->malloced bytes.
To fix this, the following patch was applied:

 			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) {

Also, _realloc_array() was updated to use size_t instead of unsigned long integer as size argument. Unsigned long integers are 8 bytes long on 64bit architectures and 4 bytes on 32bit ones. On the other hand, ‘new_size’ of the above routine is size_t. This means that the multiplication could result in a number large enough to fit into an unsigned long integer but not on a size_t on 64bit platforms. This could lead to integer truncation and for that reason, this function was updated like this:

-void *_realloc_array(void *ptr, unsigned int size, unsigned long num)
+void *_realloc_array(void *ptr, unsigned int size, size_t num)
 {

Written by xorl

August 18, 2009 at 13:25

Posted in bugs

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s