xorl %eax, %eax

acpid UNIX Domain Socket Name Buffer Overflow

with 7 comments

First of all, this is probably not a security issue since as Kurt Seifried of Red Hat Security Response Team mentioned you need administrative access to trigger this overflow. However, there might be some other way to reach this bug using an unprivileged account and thus make it a vulnerability.

So, the bug resides in ud_socket.c file and more specifically in the routine you see here from acpid 2.0.12.

int
ud_create_socket(const char *name)
{
	int fd;
	int r;
	struct sockaddr_un uds_addr;

	/* JIC */
	unlink(name);

	fd = socket(AF_UNIX, SOCK_STREAM, 0);
	if (fd < 0) {
		return fd;
	}

	/* setup address struct */
	memset(&uds_addr, 0, sizeof(uds_addr));
	uds_addr.sun_family = AF_UNIX;
	strcpy(uds_addr.sun_path, name);
	
	/* bind it to the socket */
	r = bind(fd, (struct sockaddr *)&uds_addr, sizeof(uds_addr));
	if (r < 0) {
		return r;
	}

	/* listen - allow 10 to queue */
	r = listen(fd, 10);
	if (r < 0) {
		return r;
	}

	return fd;
}

You can quickly see that there is a common strcpy(3) stack based buffer overflow. The problem is that it copies the user supplied ‘name’ to ‘uds_addr.sun_path’ which is defined in /usr/include/sys/un.h header file to have the size you see below.

/* Structure describing the address of an AF_LOCAL (aka AF_UNIX) socket.  */
struct sockaddr_un
  {
    __SOCKADDR_COMMON (sun_);
    char sun_path[108];         /* Path name.  */
  };

As a result, any given name larger than this would result in a classic stack based buffer overflow.
The fix was to check the name length and log any long socket filenames.

-       /* JIC */
+    if (strnlen(name, sizeof(uds_addr.sun_path)) >
+        sizeof(uds_addr.sun_path) - 1) {
+        acpid_log(LOG_ERR, "ud_create_socket(): "
+            "socket filename longer than %u characters: %s",
+            sizeof(uds_addr.sun_path) - 1, name);
+        errno = EINVAL;
+        return -1;
+    }
+
+    /* JIC */
        unlink(name);

Of course, replace the strcpy(3) call with strncpy(3) as you can see here.

        uds_addr.sun_family = AF_UNIX;
-       strcpy(uds_addr.sun_path, name);
+    strncpy(uds_addr.sun_path, name, sizeof(uds_addr.sun_path) - 1);
       
        /* bind it to the socket */

And finally, perform some similar bound checking on ud_connect() from the same source code file which also included a sprintf(3) stack based buffer overflow.

int
ud_connect(const char *name)
{
	int fd;
	int r;
	struct sockaddr_un addr;

	fd = socket(AF_UNIX, SOCK_STREAM, 0);
	if (fd < 0) {
		return fd;
	}

	memset(&addr, 0, sizeof(addr));
	addr.sun_family = AF_UNIX;
	sprintf(addr.sun_path, "%s", name);

	r = connect(fd, (struct sockaddr *)&addr, sizeof(addr));
	if (r < 0) {
		close(fd);
		return r;
	}

	return fd;
}

And the equivalent diff from the patch file is the following.

@@ -85,6 +95,14 @@
        int r;
        struct sockaddr_un addr;
 
+    if (strnlen(name, sizeof(addr.sun_path)) > sizeof(addr.sun_path) - 1) {
+        acpid_log(LOG_ERR, "ud_connect(): "
+            "socket filename longer than %u characters: %s",
+            sizeof(addr.sun_path) - 1, name);
+        errno = EINVAL;
+        return -1;
+    }
+   
        fd = socket(AF_UNIX, SOCK_STREAM, 0);
        if (fd < 0) {
                return fd;
@@ -93,6 +111,8 @@
        memset(&addr, 0, sizeof(addr));
        addr.sun_family = AF_UNIX;
        sprintf(addr.sun_path, "%s", name);
+    /* safer: */
+    /*strncpy(addr.sun_path, name, sizeof(addr.sun_path) - 1);*/
 
        r = connect(fd, (struct sockaddr *)&addr, sizeof(addr));
        if (r < 0) {

Written by xorl

December 18, 2011 at 01:00

Posted in bugs

7 Responses

Subscribe to comments with RSS.

  1. Pretty same vulnerability was found this year in PHP: http://www.securityfocus.com/bid/47950/info , nice write up about exploiting it is available at http://seclists.org/fulldisclosure/2011/May/472 .

    whokilledlaura

    December 18, 2011 at 11:01

  2. Hello, ive been reading your blog a bit and you seem really experienced with security and such, i was wondering if you might wanna take me in as an “apprentice” and teach me?

    handleless

    December 23, 2011 at 07:21

  3. I guess that this is a joke or something.
    The truth is that in the past I had plenty of time to play with security related stuff but currently I can barely find a couple of hours per week for coding and code auditing. I don’t think that I am able by any means to teach anyone about security.

    Sorry man.

    xorl

    December 23, 2011 at 10:17

  4. well, find a -s argument you can control is not quite easy :)

    xi4oyu

    December 27, 2011 at 09:05

  5. It’s quite hard to find a controllable -s argument :)

    xi4oyu

    December 27, 2011 at 11:26

  6. Can you reply to my email on OSS-sec with this information please so we have it as part of the discussion thread? Thanks!

    Kurt Seifried

    December 28, 2011 at 03:26

  7. First of all sorry for the delayed response but I was really busy lately.
    No, I cannot reply to your thread on oss-sec but you’re free to use anything published on this blog as you wish.

    I’m not replying because I really don’t have any time for discussions on mailing lists. I’m sorry.

    xorl

    January 2, 2012 at 08:48


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