journal: avoid buffer overread when locale name is too long
authorZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Sun, 27 Jan 2019 08:37:26 +0000 (09:37 +0100)
committerZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Sun, 27 Jan 2019 17:34:15 +0000 (18:34 +0100)
We could potentially create an unterminated string and then call normal string
operations on it. Let's be more careful: first remove the suffix we ignore anyway,
then find if the string is of acceptable length, and possibly ignore it if it
is too long. The code rejects lengths above 31 bytes. Language names that are
actually used are much shorter, so this doesn't matter much.

src/journal/catalog.c

index 3556a10..4062f12 100644 (file)
@@ -46,7 +46,8 @@ typedef struct CatalogHeader {
 
 typedef struct CatalogItem {
         sd_id128_t id;
-        char language[32];
+        char language[32]; /* One byte is used for termination, so the maximum allowed
+                            * length of the string is actually 31 bytes. */
         le64_t offset;
 } CatalogItem;
 
@@ -556,25 +557,44 @@ static const char *find_id(void *p, sd_id128_t id) {
         const char *loc;
 
         loc = setlocale(LC_MESSAGES, NULL);
-        if (loc && loc[0] && !streq(loc, "C") && !streq(loc, "POSIX")) {
-                strncpy(key.language, loc, sizeof(key.language));
-                key.language[strcspn(key.language, ".@")] = 0;
-
-                f = bsearch(&key, (const uint8_t*) p + le64toh(h->header_size), le64toh(h->n_items), le64toh(h->catalog_item_size), (comparison_fn_t) catalog_compare_func);
-                if (!f) {
-                        char *e;
-
-                        e = strchr(key.language, '_');
-                        if (e) {
-                                *e = 0;
-                                f = bsearch(&key, (const uint8_t*) p + le64toh(h->header_size), le64toh(h->n_items), le64toh(h->catalog_item_size), (comparison_fn_t) catalog_compare_func);
+        if (!isempty(loc) && !STR_IN_SET(loc, "C", "POSIX")) {
+                size_t len;
+
+                len = strcspn(loc, ".@");
+                if (len > sizeof(key.language) - 1)
+                        log_debug("LC_MESSAGES value too long, ignoring: \"%.*s\"", (int) len, loc);
+                else {
+                        strncpy(key.language, loc, len);
+                        key.language[len] = '\0';
+
+                        f = bsearch(&key,
+                                    (const uint8_t*) p + le64toh(h->header_size),
+                                    le64toh(h->n_items),
+                                    le64toh(h->catalog_item_size),
+                                    (comparison_fn_t) catalog_compare_func);
+                        if (!f) {
+                                char *e;
+
+                                e = strchr(key.language, '_');
+                                if (e) {
+                                        *e = 0;
+                                        f = bsearch(&key,
+                                                    (const uint8_t*) p + le64toh(h->header_size),
+                                                    le64toh(h->n_items),
+                                                    le64toh(h->catalog_item_size),
+                                                    (comparison_fn_t) catalog_compare_func);
+                                }
                         }
                 }
         }
 
         if (!f) {
                 zero(key.language);
-                f = bsearch(&key, (const uint8_t*) p + le64toh(h->header_size), le64toh(h->n_items), le64toh(h->catalog_item_size), (comparison_fn_t) catalog_compare_func);
+                f = bsearch(&key,
+                            (const uint8_t*) p + le64toh(h->header_size),
+                            le64toh(h->n_items),
+                            le64toh(h->catalog_item_size),
+                            (comparison_fn_t) catalog_compare_func);
         }
 
         if (!f)