SymbX/systemd
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

catalog: bound item offsets against the mmap in the binary reader

Browse Source 
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.
This commit is contained in:
jmestwa-coder
2026-06-19 16:54:46 +09:00
committed by Yu Watanabe
parent 7aaa26571d
commit 3a6059e9c0
3 changed files with 118 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/libsystemd/sd-journal/catalog.c
+33 -29
View File
@@ -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);
src/libsystemd/sd-journal/catalog.h
+21
View File
@@ -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);
src/libsystemd/sd-journal/test-catalog.c
+64
View File
@@ -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();