udev-util: bound leading whitespace skip in udev_replace_whitespace

The function documents that at most 'len' bytes are read from 'str',
but the leading whitespace skip used strspn(), which is bounded only
by a non-whitespace byte or a NUL. ata_id passes the space padded,
non-NUL-terminated ATA IDENTIFY fields, so an all-blank model reads
past the buffer. Use strnspn() to cap the skip to 'len'.

diff --git a/src/shared/udev-util.c b/src/shared/udev-util.c
index f6021de56950903ad36755ba560b6b985e1b0155..a54720d9591cb58f4af81ce3639dadd5d94567b6 100644 (file)
--- a/src/shared/udev-util.c
+++ b/src/shared/udev-util.c
@@ -315,7 +315,9 @@ size_t udev_replace_whitespace(const char *str, char *to, size_t len) {
          *
          * Note that only 'len' characters are read from 'str'. */
 
-        i = strspn(str, WHITESPACE);
+        /* Skip leading whitespace, but read at most 'len' bytes: 'str' is not necessarily NUL
+         * terminated within 'len' (e.g. the space padded ATA IDENTIFY fields ata_id passes in). */
+        i = strnspn(str, WHITESPACE, len);
 
         for (j = 0; j < len && i < len && str[i] != '\0'; i++) {
                 if (isspace(str[i])) {

diff --git a/src/test/test-udev-util.c b/src/test/test-udev-util.c
index e6c339bcc7736f50edfa2f30eab47d1d8d510425..8355a94b0d967e6ad9b918588dd843057a09d2b0 100644 (file)
--- a/src/test/test-udev-util.c
+++ b/src/test/test-udev-util.c
@@ -57,4 +57,43 @@ TEST(udev_replace_whitespace) {
         test_udev_replace_whitespace_one_len("    hoge   hoge    ", 0, "");
 }
 
+static void test_udev_replace_whitespace_not_terminated_one(const char *str, size_t len, const char *expected) {
+        _cleanup_free_ char *buf = NULL, *result = NULL;
+        int r;
+
+        /* Copy exactly 'len' bytes with no trailing NUL, so any read past 'len' is caught by the
+         * sanitizers. */
+        assert_se(buf = memdup(str, len));
+
+        assert_se(result = new(char, len + 1));
+        r = udev_replace_whitespace(buf, result, len);
+        assert_se((size_t) r == strlen(expected));
+        ASSERT_STREQ(result, expected);
+}
+
+TEST(udev_replace_whitespace_not_nul_terminated) {
+        /* 'str' is not NUL terminated within 'len' and consists entirely of whitespace, like a
+         * space padded, non-terminated ATA IDENTIFY model/serial field. Only 'len' bytes may be
+         * read from it; otherwise this reads off the end of the buffer (caught by the sanitizers). */
+
+        for (size_t len = 1; len <= 64; len++) {
+                _cleanup_free_ char *str = NULL, *result = NULL;
+
+                assert_se(str = new(char, len));
+                memset(str, ' ', len);
+
+                assert_se(result = new(char, len + 1));
+                assert_se(udev_replace_whitespace(str, result, len) == 0);
+                ASSERT_STREQ(result, "");
+        }
+
+        /* Leading whitespace followed by content, also not NUL terminated within 'len': exercises
+         * the handoff from the bounded leading skip into the copy loop on a non-terminated buffer. */
+        test_udev_replace_whitespace_not_terminated_one("   hoge", 7, "hoge");
+        test_udev_replace_whitespace_not_terminated_one("  hoge   hoge", 13, "hoge_hoge");
+        test_udev_replace_whitespace_not_terminated_one("    hoge   hoge    ", 16, "hoge_hoge");
+        test_udev_replace_whitespace_not_terminated_one("    hoge   hoge    ", 7, "hog");
+        test_udev_replace_whitespace_not_terminated_one(" x", 2, "x");
+}
+
 DEFINE_TEST_MAIN(LOG_INFO);