From 00e1adf8b1477e8770a49bc2b0ebc2f611f57906 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 27 Jan 2019 09:37:26 +0100 Subject: [PATCH] journal: avoid buffer overread when locale name is too long 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 | 48 ++++++++++++++++++++++++++++++------------- 1 file changed, 34 insertions(+), 14 deletions(-) diff --git a/src/journal/catalog.c b/src/journal/catalog.c index 3556a101bf2..4062f12c2df 100644 --- a/src/journal/catalog.c +++ b/src/journal/catalog.c @@ -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) -- 2.47.3