From: jmestwa-coder Date: Sat, 13 Jun 2026 17:58:08 +0000 (+0530) Subject: catalog: bound item offsets against the mmap in the binary reader X-Git-Tag: v261~11 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=3a6059e9c04c7146d9c8115abc2d38f98ffd0116;p=thirdparty%2Fsystemd.git catalog: bound item offsets against the mmap in the binary reader The binary catalog reader trusted two values straight from a (possibly hostile) database: open_mmap() summed header_size + n_items * catalog_item_size in uint64 with no overflow check, and find_id() added the matched item's offset to the map base with no upper bound. Reachable through sd_journal_get_catalog() with $SYSTEMD_CATALOG set, this let catalog_get()/catalog_list() strdup() a string starting outside the mapping. Guard the size math with MUL_SAFE/INC_SAFE and reject item offsets that fall outside the file. --- diff --git a/src/libsystemd/sd-journal/catalog.c b/src/libsystemd/sd-journal/catalog.c index 5d91b6a0e53..433288b4abb 100644 --- a/src/libsystemd/sd-journal/catalog.c +++ b/src/libsystemd/sd-journal/catalog.c @@ -34,24 +34,6 @@ static const char * const catalog_file_dirs[] = { NULL }; -#define CATALOG_SIGNATURE { 'R', 'H', 'H', 'H', 'K', 'S', 'L', 'P' } - -typedef struct CatalogHeader { - uint8_t signature[8]; /* "RHHHKSLP" */ - le32_t compatible_flags; - le32_t incompatible_flags; - le64_t header_size; - le64_t n_items; - le64_t catalog_item_size; -} CatalogHeader; - -typedef struct CatalogItem { - sd_id128_t id; - 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; - static void catalog_hash_func(const CatalogItem *i, struct siphash *state) { assert(i); assert(state); @@ -514,11 +496,12 @@ int catalog_update(const char *database, const char *root, const char* const *di return 0; } -static int open_mmap(const char *database, int *ret_fd, struct stat *ret_st, void **ret_map) { +static int open_mmap(const char *database, int *ret_fd, struct stat *ret_st, void **ret_map, uint64_t *ret_strings_offset) { assert(database); assert(ret_fd); assert(ret_st); assert(ret_map); + assert(ret_strings_offset); _cleanup_close_ int fd = open(database, O_RDONLY|O_CLOEXEC); if (fd < 0) @@ -536,12 +519,15 @@ static int open_mmap(const char *database, int *ret_fd, struct stat *ret_st, voi return -errno; const CatalogHeader *h = p; + uint64_t total; if (memcmp(h->signature, (const uint8_t[]) CATALOG_SIGNATURE, sizeof(h->signature)) != 0 || le64toh(h->header_size) < sizeof(CatalogHeader) || le64toh(h->catalog_item_size) < sizeof(CatalogItem) || h->incompatible_flags != 0 || le64toh(h->n_items) <= 0 || - st.st_size < (off_t) (le64toh(h->header_size) + le64toh(h->catalog_item_size) * le64toh(h->n_items))) { + !MUL_SAFE(&total, le64toh(h->catalog_item_size), le64toh(h->n_items)) || + !INC_SAFE(&total, le64toh(h->header_size)) || + (uint64_t) st.st_size < total) { munmap(p, st.st_size); return -EBADMSG; } @@ -549,10 +535,11 @@ static int open_mmap(const char *database, int *ret_fd, struct stat *ret_st, voi *ret_fd = TAKE_FD(fd); *ret_st = st; *ret_map = p; + *ret_strings_offset = total; /* start of the string store, already validated to fit in the file */ return 0; } -static const char* find_id(const void *p, sd_id128_t id) { +static const char* find_id(const void *p, uint64_t file_size, uint64_t strings_offset, sd_id128_t id) { CatalogItem key = { .id = id }; const CatalogItem *f = NULL; const CatalogHeader *h = ASSERT_PTR(p); @@ -602,10 +589,20 @@ static const char* find_id(const void *p, sd_id128_t id) { if (!f) return NULL; - return (const char*) p + - le64toh(h->header_size) + - le64toh(h->n_items) * le64toh(h->catalog_item_size) + - le64toh(f->offset); + /* f->offset is attacker-controlled in a hostile database; make sure the string start, plus a + * terminating NUL, stay inside the mapping before handing the pointer out for strlen()/strdup(). + * strings_offset (the start of the string store) was already bounded against the file in + * open_mmap(), so only f->offset needs to be added and checked here. */ + uint64_t off = strings_offset; + if (!INC_SAFE(&off, le64toh(f->offset)) || + off >= file_size) + return NULL; + + const char *s = (const char*) p + off; + if (!memchr(s, 0, file_size - off)) + return NULL; + + return s; } int catalog_get(const char *database, sd_id128_t id, char **ret_text) { @@ -617,11 +614,12 @@ int catalog_get(const char *database, sd_id128_t id, char **ret_text) { _cleanup_close_ int fd = -EBADF; struct stat st; void *p; - r = open_mmap(database, &fd, &st, &p); + uint64_t strings_offset; + r = open_mmap(database, &fd, &st, &p, &strings_offset); if (r < 0) return r; - const char *s = find_id(p, id); + const char *s = find_id(p, st.st_size, strings_offset, id); if (!s) r = -ENOENT; else @@ -678,7 +676,8 @@ int catalog_list(FILE *f, const char *database, bool oneline) { _cleanup_close_ int fd = -EBADF; struct stat st; void *p; - r = open_mmap(database, &fd, &st, &p); + uint64_t strings_offset; + r = open_mmap(database, &fd, &st, &p, &strings_offset); if (r < 0) return r; @@ -693,7 +692,12 @@ int catalog_list(FILE *f, const char *database, bool oneline) { if (last_id_set && sd_id128_equal(last_id, items[n].id)) continue; - assert_se(s = find_id(p, items[n].id)); + s = find_id(p, st.st_size, strings_offset, items[n].id); + if (!s) { + log_debug("Skipping catalog item " SD_ID128_FORMAT_STR " with out-of-bounds string offset.", + SD_ID128_FORMAT_VAL(items[n].id)); + continue; + } dump_catalog_entry(f, items[n].id, s, oneline); diff --git a/src/libsystemd/sd-journal/catalog.h b/src/libsystemd/sd-journal/catalog.h index 41dab41fbbd..1e5b3b5477d 100644 --- a/src/libsystemd/sd-journal/catalog.h +++ b/src/libsystemd/sd-journal/catalog.h @@ -1,7 +1,28 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #pragma once +#include "sd-id128.h" + #include "sd-forward.h" +#include "sparse-endian.h" + +#define CATALOG_SIGNATURE { 'R', 'H', 'H', 'H', 'K', 'S', 'L', 'P' } + +typedef struct CatalogHeader { + uint8_t signature[8]; /* "RHHHKSLP" */ + le32_t compatible_flags; + le32_t incompatible_flags; + le64_t header_size; + le64_t n_items; + le64_t catalog_item_size; +} CatalogHeader; + +typedef struct CatalogItem { + sd_id128_t id; + 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; int catalog_import_file(OrderedHashmap **h, int fd, const char *path); int catalog_update(const char *database, const char *root, const char* const *dirs); diff --git a/src/libsystemd/sd-journal/test-catalog.c b/src/libsystemd/sd-journal/test-catalog.c index 09f05a6b905..c4b63319d3d 100644 --- a/src/libsystemd/sd-journal/test-catalog.c +++ b/src/libsystemd/sd-journal/test-catalog.c @@ -10,8 +10,10 @@ #include "fd-util.h" #include "hashmap.h" #include "log.h" +#include "memstream-util.h" #include "tests.h" #include "tmpfile-util.h" +#include "unaligned.h" static char** catalog_dirs = NULL; static const char *no_catalog_dirs[] = { @@ -179,6 +181,66 @@ static void test_catalog_file_lang(void) { ASSERT_STREQ(lang4, "ru_RU"); } +static void test_catalog_oob_offset_one(uint64_t item_offset, size_t strings_size) { + /* Builds a hostile single-item catalog database and verifies the reader rejects it instead of + * chasing the item's string offset out of the mapping. The blob is laid out from the real struct + * offsets so it keeps matching open_mmap() if CatalogHeader/CatalogItem ever change. */ + _cleanup_(unlink_tempfilep) char db[] = "/tmp/test-catalog.XXXXXX"; + _cleanup_close_ int fd = -EBADF; + _cleanup_free_ char *text = NULL; + _cleanup_free_ uint8_t *blob = NULL; + size_t blob_size = sizeof(CatalogHeader) + sizeof(CatalogItem) + strings_size; + + ASSERT_NOT_NULL(blob = new0(uint8_t, blob_size)); + uint8_t *item = blob + sizeof(CatalogHeader); + + memcpy(blob + offsetof(CatalogHeader, signature), + (const uint8_t[]) CATALOG_SIGNATURE, sizeof_field(CatalogHeader, signature)); + unaligned_write_le64(blob + offsetof(CatalogHeader, header_size), sizeof(CatalogHeader)); + unaligned_write_le64(blob + offsetof(CatalogHeader, n_items), 1); + unaligned_write_le64(blob + offsetof(CatalogHeader, catalog_item_size), sizeof(CatalogItem)); + + memset(item + offsetof(CatalogItem, id), 0x42, sizeof_field(CatalogItem, id)); + /* item language left zero so the C-locale lookup matches */ + unaligned_write_le64(item + offsetof(CatalogItem, offset), item_offset); + + /* Any trailing string store is filled with non-NUL bytes, so an in-bounds offset still has no + * terminator before EOF. */ + memset(blob + sizeof(CatalogHeader) + sizeof(CatalogItem), 0x41, strings_size); + + ASSERT_OK(fd = mkostemp_safe(db)); + ASSERT_OK_EQ_ERRNO(write(fd, blob, blob_size), (ssize_t) blob_size); + + sd_id128_t id; + memset(&id, 0x42, sizeof(id)); + + ASSERT_ERROR(catalog_get(db, id, &text), ENOENT); + ASSERT_NULL(text); + + /* Listing the same database must walk every item without dereferencing the bad offset, and the + * corrupt item must be skipped rather than emitted. Capture the output and assert its id is absent + * so a regressed guard is caught here and not only under a sanitizer. */ + bool oneline; + FOREACH_ARGUMENT(oneline, true, false) { + _cleanup_(memstream_done) MemStream m = {}; + _cleanup_free_ char *out = NULL; + FILE *f; + + ASSERT_NOT_NULL(f = memstream_init(&m)); + ASSERT_OK(catalog_list(f, db, oneline)); + ASSERT_OK(memstream_finalize(&m, &out, NULL)); + ASSERT_NULL(strstr(out, SD_ID128_TO_STRING(id))); + } +} + +static void test_catalog_oob_offset(void) { + /* Offset lands far past EOF: rejected by the bounds check. */ + test_catalog_oob_offset_one(/* item_offset= */ UINT64_C(0x4000000000), /* strings_size= */ 0); + + /* Offset is in bounds but its string runs to EOF with no terminator: rejected by memchr(). */ + test_catalog_oob_offset_one(/* item_offset= */ 0, /* strings_size= */ 16); +} + int main(int argc, char *argv[]) { _cleanup_(unlink_tempfilep) char database[] = "/tmp/test-catalog.XXXXXX"; _cleanup_close_ int fd = -EBADF; @@ -197,6 +259,8 @@ int main(int argc, char *argv[]) { test_catalog_file_lang(); + test_catalog_oob_offset(); + test_catalog_import_invalid(); test_catalog_import_badid(); test_catalog_import_one();