sd-device: avoid 32-bit overflow in the monitor properties bounds check

"properties_off + 32 > n" is evaluated in 32-bit arithmetic, so a
properties_off near UINT32_MAX wraps but passes the check.

Follow-up for efbd4b3ca84c0426b6ff98d6352f82f3b7c090b2

diff --git a/src/libsystemd/sd-device/device-monitor-private.h b/src/libsystemd/sd-device/device-monitor-private.h
index 4582108df6163c5428db5a815485e69e609aae27..dc1af6c19ca39e94d6136f0aa169ba3254f2f380 100644 (file)
--- a/src/libsystemd/sd-device/device-monitor-private.h
+++ b/src/libsystemd/sd-device/device-monitor-private.h
@@ -15,3 +15,24 @@ int device_monitor_new_full(sd_device_monitor **ret, MonitorNetlinkGroup group,
 int device_monitor_get_address(sd_device_monitor *m, union sockaddr_union *ret);
 int device_monitor_allow_unicast_sender(sd_device_monitor *m, sd_device_monitor *sender);
 int device_monitor_send(sd_device_monitor *m, const union sockaddr_union *destination, sd_device *device);
+
+#define UDEV_MONITOR_MAGIC                0xfeedcafe
+
+typedef struct monitor_netlink_header {
+        /* "libudev" prefix to distinguish libudev and kernel messages */
+        char prefix[8];
+        /* Magic to protect against daemon <-> Library message format mismatch
+         * Used in the kernel from socket filter rules; needs to be stored in network order */
+        unsigned magic;
+        /* Total length of header structure known to the sender */
+        unsigned header_size;
+        /* Properties string buffer */
+        unsigned properties_off;
+        unsigned properties_len;
+        /* Hashes of primary device properties strings, to let libudev subscribers
+         * use in-kernel socket filters; values need to be stored in network order */
+        unsigned filter_subsystem_hash;
+        unsigned filter_devtype_hash;
+        unsigned filter_tag_bloom_hi;
+        unsigned filter_tag_bloom_lo;
+} monitor_netlink_header;

diff --git a/src/libsystemd/sd-device/device-monitor.c b/src/libsystemd/sd-device/device-monitor.c
index 7d9433920ec35126ae888973f9669648d365b275..9799d9b4daca732bc666a8299833651f673a6551 100644 (file)
--- a/src/libsystemd/sd-device/device-monitor.c
+++ b/src/libsystemd/sd-device/device-monitor.c
@@ -67,27 +67,6 @@ struct sd_device_monitor {
         void *userdata;
 };
 
-#define UDEV_MONITOR_MAGIC                0xfeedcafe
-
-typedef struct monitor_netlink_header {
-        /* "libudev" prefix to distinguish libudev and kernel messages */
-        char prefix[8];
-        /* Magic to protect against daemon <-> Library message format mismatch
-         * Used in the kernel from socket filter rules; needs to be stored in network order */
-        unsigned magic;
-        /* Total length of header structure known to the sender */
-        unsigned header_size;
-        /* Properties string buffer */
-        unsigned properties_off;
-        unsigned properties_len;
-        /* Hashes of primary device properties strings, to let libudev subscribers
-         * use in-kernel socket filters; values need to be stored in network order */
-        unsigned filter_subsystem_hash;
-        unsigned filter_devtype_hash;
-        unsigned filter_tag_bloom_hi;
-        unsigned filter_tag_bloom_lo;
-} monitor_netlink_header;
-
 static int monitor_set_nl_address(sd_device_monitor *m) {
         union sockaddr_union snl;
         socklen_t addrlen;
@@ -642,10 +621,10 @@ _public_ int sd_device_monitor_receive(sd_device_monitor *m, sd_device **ret) {
                                                  "Invalid message signature (%x != %x).",
                                                  message.nlh->magic, htobe32(UDEV_MONITOR_MAGIC));
 
-                if (message.nlh->properties_off + 32 > (size_t) n)
+                if (message.nlh->properties_off > LESS_BY((size_t) n, 32u))
                         return log_monitor_errno(m, SYNTHETIC_ERRNO(EAGAIN),
-                                                 "Invalid offset for properties (%u > %zi).",
-                                                 message.nlh->properties_off + 32, n);
+                                                 "Invalid properties offset (%u) for message of length %zi.",
+                                                 message.nlh->properties_off, n);
 
                 offset = message.nlh->properties_off;
 

diff --git a/src/libsystemd/sd-device/test-sd-device-monitor.c b/src/libsystemd/sd-device/test-sd-device-monitor.c
index 523cc7c6cd19228aed5b8ff0e78772fb8a574955..c4034d27c70460c4c988bc52d06c0cb819155c33 100644 (file)
--- a/src/libsystemd/sd-device/test-sd-device-monitor.c
+++ b/src/libsystemd/sd-device/test-sd-device-monitor.c
@@ -9,6 +9,7 @@
 #include "device-private.h"
 #include "device-util.h"
 #include "io-util.h"
+#include "iovec-util.h"
 #include "mountpoint-util.h"
 #include "path-util.h"
 #include "socket-util.h"
@@ -424,6 +425,31 @@ TEST(sd_device_monitor_receive) {
         ASSERT_STREQ(s, syspath);
 }
 
+TEST(sd_device_monitor_receive_bad_properties_off) {
+        _cleanup_(sd_device_monitor_unrefp) sd_device_monitor *monitor_server = NULL, *monitor_client = NULL;
+        union sockaddr_union sa;
+
+        prepare_monitor(&monitor_server, &monitor_client, &sa);
+
+        monitor_netlink_header nlh = {
+                .prefix = "libudev",
+                .magic = htobe32(UDEV_MONITOR_MAGIC),
+                .header_size = sizeof nlh,
+                .properties_off = UINT32_MAX - 10,
+        };
+        struct iovec iov = IOVEC_MAKE(&nlh, sizeof nlh);
+        struct msghdr smsg = {
+                .msg_iov = &iov,
+                .msg_iovlen = 1,
+                .msg_name = &sa,
+                .msg_namelen = sizeof(struct sockaddr_nl),
+        };
+        ASSERT_OK_ERRNO(sendmsg(sd_device_monitor_get_fd(monitor_server), &smsg, 0));
+
+        _cleanup_(sd_device_unrefp) sd_device *dev = NULL;
+        ASSERT_ERROR(sd_device_monitor_receive(monitor_client, &dev), EAGAIN);
+}
+
 static int intro(void) {
         if (getuid() != 0)
                 return log_tests_skipped("not root");