oss-security - acpid - possible issue in socket handling

Follow @Openwall on Twitter for new release announcements and other news

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <4EDEA3C9.1000500@redhat.com>
Date: Tue, 06 Dec 2011 16:22:49 -0700
From: Kurt Seifried <kseifried@...hat.com>
To: oss-security@...ts.openwall.com
Subject: acpid - possible issue in socket handling

While reading the acpid ChangeLog I noticed:

* Tue Nov 15 2011  Ted Felix <http://www.tedfelix.com>
  - 2.0.13 release
  - Fix for socket name buffer overflow.  (ud_socket.c)  (Ted Felix)

This doesn't appear to cross a security boundary without an admin doing
something intentionally strange. But just in case someone knows a clever
way to exploit this I thought I'd post to the list and ask (and if so
I'll assign a CVE).

Code below:

--- acpid-2.0.12/ud_socket.c    2009-04-29 08:36:27.000000000 -0600
+++ acpid-2.0.13/ud_socket.c    2011-10-17 17:47:16.000000000 -0600
@@ -15,6 +15,7 @@
 #include <fcntl.h>
 
 #include "acpid.h"
+#include "log.h"
 #include "ud_socket.h"
 
 int
@@ -24,7 +25,16 @@
        int r;
        struct sockaddr_un uds_addr;
 
-       /* 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);
 
        fd = socket(AF_UNIX, SOCK_STREAM, 0);
@@ -35,7 +45,7 @@
        /* setup address struct */
        memset(&uds_addr, 0, sizeof(uds_addr));
        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 */
        r = bind(fd, (struct sockaddr *)&uds_addr, sizeof(uds_addr));
@@ -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) {



-- 

-Kurt Seifried / Red Hat Security Response Team

Please check out the Open Source Software Security Wiki, which is counterpart to this mailing list.

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.