CVE-2009-1186: Linux udev NETLINK Missing Checks

This is the second bug discovered by stealth. This was also patched on 1.4.1 release of udev package. Here is the code from 1.4.0 release of the popular management daemon:

228 /**
229  * udev_monitor_receive_device:
230  * @udev_monitor: udev monitor
231  *
232  * Receive data from the udev monitor socket, allocate a new udev
233  * device, fill in the received data, and return the device.
234  *
235  * Only socket connections with uid=0 are accepted. The caller
236  * needs to make sure that there is data to read from the socket.
237  * The call will block until the socket becomes readable.
238  *
239  * The initial refcount is 1, and needs to be decremented to
240  * release the resources of the udev device.
241  *
242  * Returns: a new udev device, or #NULL, in case of an error
243  **/
244 struct udev_device *udev_monitor_receive_device(struct udev_monitor *udev_monitor)
245 {

This function can be found at udev/lib/libudev-monitor.c. The pointer passed to it is a udev_monitor structure which includes these members:

35 struct udev_monitor {
36         struct udev *udev;
37         int refcount;
38         int sock;
39         struct sockaddr_nl snl;
40         struct sockaddr_un sun;
41         socklen_t addrlen;
42 };

Here are the checks performed in this function:

246         struct udev_device *udev_device;
247         struct msghdr smsg;
     ...
258         if (udev_monitor == NULL)
     ...
264         smsg.msg_iov = &iov;
265         smsg.msg_iovlen = 1;
266         smsg.msg_control = cred_msg;
267         smsg.msg_controllen = sizeof(cred_msg);
268
269         if (recvmsg(udev_monitor->sock, &smsg, 0) < 0) {
     ...
275         if (udev_monitor->sun.sun_family != 0) {
     ...
279                 if (cmsg == NULL || cmsg->cmsg_type != SCM_CREDENTIALS) {
     ...
284                 if (cred->uid != 0) {
     ...
288         }
     ...
308         while (bufpos < sizeof(buf)) {
     ...
318                 if (strncmp(key, "DEVPATH=", 8) == 0) {
319                         char path[UTIL_PATH_SIZE];
     ...
380         return udev_device;
381 }

A malicous user can send a message using NETLINK socket (member of udev_monitor, line 39) with family type not equal to zero. This will lead in an invalid message passed to recvmsg() at line 269. This was patched by adding a temporary local variable:

        struct cmsghdr *cmsg;
+       struct sockaddr_nl snl;
        struct ucred *cred;

And performing an additional check before recvmsg() call like this:

        smsg.msg_controllen = sizeof(cred_msg);

+       if (udev_monitor->snl.nl_family != 0) {
+               smsg.msg_name = &snl;
+               smsg.msg_namelen = sizeof snl;
+       }
+
        if (recvmsg(udev_monitor->sock, &smsg, 0) < 0) {

The same condition could be reached later by sending a NETLINK message with UDEV_MONITOR_KERNEL NETLINK group set. This multicast packet could bypass various checks on udev implementation and it was fixed like this:

+       if (udev_monitor->snl.nl_family != 0) {
+               if (snl.nl_groups == 0) {
+                       info(udev_monitor->udev, "unicast netlink message ignored\n");
+                       return NULL;
+               }
+               if ((snl.nl_groups == UDEV_MONITOR_KERNEL) && (snl.nl_pid > 0)) {
+                       info(udev_monitor->udev, "multicast kernel netlink message from pid %d ignored\n", snl.nl_pid);
+                       return NULL;
+               }
+       }
+
        cmsg = CMSG_FIRSTHDR(&smsg);

Moreover, function udev_monitor_enable_receiving() was patched in its NETLINK family handling operations so that it fixes this bypass vulnerability like this:

-       if (udev_monitor->snl.nl_family != 0) {
-               err = bind(udev_monitor->sock,
-                          (struct sockaddr *)&udev_monitor->snl, sizeof(struct sockaddr_nl));
-               if (err < 0) {
-                       err(udev_monitor->udev, "bind failed: %m\n");
-                       return err;
-               }
-               dbg(udev_monitor->udev, "monitor %p listening on netlink\n", udev_monitor);
-       } else if (udev_monitor->sun.sun_family != 0) {
+       if (udev_monitor->sun.sun_family != 0)
                err = bind(udev_monitor->sock,
                           (struct sockaddr *)&udev_monitor->sun, udev_monitor->addrlen);
-               if (err < 0) {
-                       err(udev_monitor->udev, "bind failed: %m\n");
-                       return err;
-               }
-               /* enable receiving of the sender credentials */
-               setsockopt(udev_monitor->sock, SOL_SOCKET, SO_PASSCRED, &on, sizeof(on));
-               dbg(udev_monitor->udev, "monitor %p listening on socket\n", udev_monitor);
+       else if (udev_monitor->snl.nl_family != 0)
+               err = bind(udev_monitor->sock,
+                          (struct sockaddr *)&udev_monitor->snl, sizeof(struct sockaddr_nl));
+       else
+               return -EINVAL;
+
+       if (err < 0) {
+               err(udev_monitor->udev, "bind failed: %m\n");
+               return err;
        }
+
+       /* enable receiving of sender credentials */
+       setsockopt(udev_monitor->sock, SOL_SOCKET, SO_PASSCRED, &on, sizeof(on));
        return 0;

And of course, the above code requires two additional local variables which where added:

        char cred_msg[CMSG_SPACE(sizeof(struct ucred))];
+       struct cmsghdr *cmsg;
+       struct ucred *cred;
        char buf[4096];

And its credentials checking block changed to this:

-       if (udev_monitor->sun.sun_family != 0) {
-               struct cmsghdr *cmsg = CMSG_FIRSTHDR(&smsg);
-               struct ucred *cred = (struct ucred *)CMSG_DATA (cmsg);
-
-               if (cmsg == NULL || cmsg->cmsg_type != SCM_CREDENTIALS) {
-                       info(udev_monitor->udev, "no sender credentials received, message ignored");
-                       return NULL;
-               }
+       cmsg = CMSG_FIRSTHDR(&smsg);
+       if (cmsg == NULL || cmsg->cmsg_type != SCM_CREDENTIALS) {
+               info(udev_monitor->udev, "no sender credentials received, message ignored");
+               return NULL;
+       }

-               if (cred->uid != 0) {
-                       info(udev_monitor->udev, "sender uid=%d, message ignored", cred->uid);
-                       return NULL;
-               }
+       cred = (struct ucred *)CMSG_DATA(cmsg);
+       if (cred->uid != 0) {
+               info(udev_monitor->udev, "sender uid=%d, message ignored", cred->uid);
+               return NULL;
        }