xorl %eax, %eax

CVE-2009-1251: OpenAFS RX Response Heap Overflow

leave a comment »

OpenAFS is a multi-platform and open-source implementation of the AFS (Andrew File System) used for remote storage sharing. So, this nice little bug was identified by Simon Wilkinson, with assistance from Derrick Brashear and Jeffrey Altman as the security advisory says, and affects OpenAFS 1.0 through 1.4.8 and 1.5.0 through 1.5.58 only on UNIX clients, not including MAC OS X. Here I’ll be using OpenAFS 1.5.58 to demonstrate the bug. Here is the vulnerable routine from src/sys/rmtsysc.c:

207 /* Remote pioctl(2) client routine */
208 #ifdef AFS_DUX40_ENV
209 #pragma weak pioctl = afs_pioctl
210 int
211 afs_pioctl(char *path, afs_int32 cmd, struct ViceIoctl *data,
212            afs_int32 follow)
213 #else
214 int
215 pioctl(char *path, afs_int32 cmd, struct ViceIoctl *data, afs_int32 follow)
216 #endif
217 {
218     struct rx_connection *conn;
219     clientcred creds;
220     afs_int32 errorcode, errornumber, ins = data->in_size;
221     afs_uint32 groups[NGROUPS_MAX];
222     rmtbulk InData, OutData;
223     char pathname[256], *pathp = pathname, *inbuffer;

This is part of the UNIX cache manager and this is the reason why it only affects UNIX clients. Keep an eye on InData and outData and let’s now move one…

235     if (!(inbuffer = (char *)malloc(ins)))
236         return (-1);            /* helpless here */
237     if (data->in_size)
238         memcpy(inbuffer, data->in, data->in_size);
239     InData.rmtbulk_len = data->in_size;
240     InData.rmtbulk_val = inbuffer;
241     inparam_conversion(cmd, InData.rmtbulk_val, 0);
242     OutData.rmtbulk_len = data->out_size;
243     OutData.rmtbulk_val = data->out;

Here it allocates space for the incoming buffer using malloc() and then stores the new input buffer size using memcpy() at line 238. Now, it updates the InData.rmtbulk_len to contain this user controlled size and sets its value to this buffer. At lines 242 and 243 it performs the exact same steps for the output data stored into OutData. Of course, by know all of these have user controlled data, now let’s continue:

244     /* We always need to pass absolute pathnames to the remote pioctl since we
245      * lose the current directory value when doing an rpc call. Below we
246      * prepend the current absolute path directory, if the name is relative */
247     if (path) {
266     errorcode =
267         RMTSYS_Pioctl(conn, &creds, pathp, cmd, follow, &InData, &OutData,
268                       &errornumber);
277     if (!errorcode) {
278         /* Do the conversions back to the host order; store the results back
279          * on the same buffer */
280         outparam_conversion(cmd, OutData.rmtbulk_val, 1);
281     }
282     free(inbuffer);
283     return errorcode;
284 }

At line 267 it actually executes the PIOCTL and at lines 277-281 it performs some basic error checking which results in a call to outparam_conversion() and finally freeing the allocated space. The problem is that if the OutData.rmtbulk_val might not have sufficient space to store data->out and lead to a heap overflow. To fix this fairly straightforward memory corruption the patch was this:

     inparam_conversion(cmd, InData.rmtbulk_val, 0);
-    OutData.rmtbulk_len = data->out_size;
-    OutData.rmtbulk_val = data->out;
+    OutData.rmtbulk_len = MAXBUFFERLEN * sizeof(*OutData.rmtbulk_val);
+    OutData.rmtbulk_val = malloc(OutData.rmtbulk_len); 
+    if (!OutData.rmtbulk_val) {
+	free(inbuffer);
+	return -1;
+    }
     /* We always need to pass absolute pathnames to the remote pioctl since we

Which stores the appropriate length on OutData.rmtbukn_len and then, attempts to dynamically allocate this requested space using malloc() and the last part of the patch was this:

 	 * on the same buffer */
-	outparam_conversion(cmd, OutData.rmtbulk_val, 1);
+	if (data->out_size < OutData.rmtbulk_len) {
+	    errno = EINVAL;
+	    errorcode = -1;
+	} else {
+	    memcpy(data->out, OutData.rmtbulk_val, data->out_size);
+	    outparam_conversion(cmd, data->out, 1);
+	}
+    free(OutData.rmtbulk_val);

Which copies the OutData.rmtbukn_val to data->out and uses this one on outparam_conversion(). This can be reached by constructing a malicious RX responce containing more data than those specified in the request.

Written by xorl

April 11, 2009 at 19:03

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