From nobody Fri Sep 18 10:31:01 2026 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4hmTRv3XCVz6sGH3 for ; Fri, 18 Sep 2026 10:31:07 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange x25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "YR2" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 4hmTRv2cylz3N8k for ; Fri, 18 Sep 2026 10:31:07 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1789727467; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=Jrpu3XhKThJXsPSXFRsAi+mKICWKIeUYGtWzLd/jN0w=; b=EwsQsQNL0E1uhDEkLNvl1fltVErnf8GcJTJwWhOXn1sgaSOOs1//30IP4qnanjwrKUV9vz Is5g99Zq1+N1lbm5526Tn4Gtx02ernzK9/0MYDiH8KHitJpbgoiHfNKlXgUJPgZYBgFG7U DNJUqls8qr8JtfPv8fJPCPZaTLBkpd0YtY8RoM/L0V7qbWw4RELfHiHsUxGPLt3WmjbzTE PYTHBfgBcfv27c11JRtZxbE4Xbd+zaONBQJrvTtQ/CJwRXV8qYJ4Bprm01/6BEIf1fMe1c bP0VRlGRRMPwbHlaJxK1aIIToerSgPoIfVhDlG69Prz84iUlJmTzXiYyWcQWDw== ARC-Seal: i=1; a=rsa-sha256; d=freebsd.org; s=dkim; cv=none; t=1789727467; b=DGy1R4zn0BUbZNj6tzbZaWDTihkutc8cjS9rzviYa8AuUGb7GraCGYuIETbdZi/PbZYmVP gBfT68NPwj7cC+rIfqiZcPqnL1NmJebf2VpklA6juDnE6mTqJiFs/AWnx0phGvAjbuv8Dv ziN4R0p91oJ/FZnedMx98DHxX7QJ+T3TsV4OrIt5zJnvhp+yG59fw82JogX9CB1DA/+LU/ 50YG5gnOPUNKaRBqxZKEB8unnR+7iyLhkI3S6rWzEzWiOutm3e/WP++fP3sIWrASg1CJka vw2uY61G2NSZl0cro/okrv1gPecpaHAE3H/VlzhTsZ0UvO7nG69SVidJG2mJag== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1789727467; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=Jrpu3XhKThJXsPSXFRsAi+mKICWKIeUYGtWzLd/jN0w=; b=A2xfY0SOoJ3NsYVbuSu8DEx5ohh65MZCDkaw8T/0UfmqbB6ViLK/1eHJmHSDXK0pxF/FWa 0KT3HGwxGrfW/ZE+2OY4lBN6TSGeF0ignbCBVK3ejl9w9ztaY/bRd9N71IkHO7ve75uDRa Gqp8pxJ7RcRTHCx4S/GJF0kJgSpRBNLfojzJgosQaBK3Cnptn319MiDOQ0WIYUSIXJWR5S mSUf2yXZvxXznNzQQ7l8t/WnxYKyaBCCdZdyxEodx8NKlGqxv+WB74JrDy/z90EgyZ6Vzn zJbOG6qsR2dOdNabz4CcJ5PVO+98eNSuU/v1x3xueiA7rlHM5PeYbkfWJUWLFg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) by mxrelay.nyi.freebsd.org (Postfix) with ESMTP id 4hmTRv1MGkzF96 for ; Fri, 18 Sep 2026 10:31:07 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from git (uid 1279) (envelope-from git@FreeBSD.org) id 23d4f by gitrepo.freebsd.org (DragonFly Mail Agent v0.13+ on gitrepo.freebsd.org); Fri, 18 Sep 2026 10:31:01 +0000 To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Bojan Novk=?utf-8?Q?ovi=C4=87?= Subject: git: 7f5f07b139a5 - main - db/hash: Harden hash(3) database code List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org List-Id: List-Post: List-Help: List-Subscribe: List-Unsubscribe: List-Owner: Precedence: list MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: bnovkov X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 7f5f07b139a5fd456087804ee96802b54c151703 Auto-Submitted: auto-generated Date: Fri, 18 Sep 2026 10:31:01 +0000 Message-Id: <6aad12e5.23d4f.2fde6039@gitrepo.freebsd.org> The branch main has been updated by bnovkov: URL: https://cgit.FreeBSD.org/src/commit/?id=7f5f07b139a5fd456087804ee96802b54c151703 commit 7f5f07b139a5fd456087804ee96802b54c151703 Author: Bojan Novković AuthorDate: 2026-07-07 12:28:50 +0000 Commit: Bojan Novković CommitDate: 2026-09-18 10:27:47 +0000 db/hash: Harden hash(3) database code The hash(3) database code does not validate the on-disk database header, leaving it open to several OOB read and write vulnerabilities. This change adds basic header validation and array bounds checking to parts of the hash(3) code that can be manipulated by messing with the database header. Reviewed by: kevans Sponsored by: Klara, Inc. MFC after: 1 month Differential Revision: https://reviews.freebsd.org/D58822 --- lib/libc/db/hash/hash.c | 47 +++++-- lib/libc/db/hash/hash_buf.c | 16 ++- lib/libc/db/hash/hash_page.c | 16 ++- lib/libc/tests/db/Makefile | 3 +- lib/libc/tests/db/db_hash_tamper_test.c | 227 ++++++++++++++++++++++++++++++++ 5 files changed, 291 insertions(+), 18 deletions(-) diff --git a/lib/libc/db/hash/hash.c b/lib/libc/db/hash/hash.c index b025a2310e8f..7fab09efc47e 100644 --- a/lib/libc/db/hash/hash.c +++ b/lib/libc/db/hash/hash.c @@ -155,12 +155,32 @@ __hash_open(const char *file, int flags, int mode, RETURN_ERROR(EFTYPE, error1); if ((int32_t)hashp->hash(CHARKEY, sizeof(CHARKEY)) != hashp->H_CHARKEY) RETURN_ERROR(EFTYPE, error1); + /* Validate the remaining header variables. */ + if (hashp->OVFL_POINT < 0 || hashp->OVFL_POINT >= NCACHED) + RETURN_ERROR(EFTYPE, error1); + if (hashp->LAST_FREED < 0 && hashp->LAST_FREED >= NCACHED) + RETURN_ERROR(EFTYPE, error1); + if (hashp->BSIZE < 0 || hashp->BSIZE > MAX_BSIZE) + RETURN_ERROR(EFTYPE, error1); + /* Both masks should be derived from power-of-2 values. */ + if (((hashp->HIGH_MASK + 1) & hashp->HIGH_MASK) != 0 || + ((hashp->LOW_MASK + 1) & hashp->LOW_MASK) != 0) + RETURN_ERROR(EFTYPE, error1); + if (hashp->LOW_MASK >= hashp->HIGH_MASK) + RETURN_ERROR(EFTYPE, error1); + + hashp->BSHIFT = __log2(hashp->BSIZE); + hashp->SGSIZE = DEF_SEGSIZE; + hashp->SSHIFT = DEF_SEGSIZE_SHIFT; /* * Figure out how many segments we need. Max_Bucket is the * maximum bucket number, so the number of buckets is * max_bucket + 1. */ nsegs = howmany(hashp->MAX_BUCKET + 1, hashp->SGSIZE); + /* Verify that DSIZE can hold the required number of segments. */ + if (hashp->DSIZE < nsegs) + RETURN_ERROR(EFTYPE, error1); if (alloc_segs(hashp, nsegs)) /* * If alloc_segs fails, table will have been destroyed @@ -171,6 +191,8 @@ __hash_open(const char *file, int flags, int mode, bpages = (hashp->SPARES[hashp->OVFL_POINT] + (hashp->BSIZE << BYTE_SHIFT) - 1) >> (hashp->BSHIFT + BYTE_SHIFT); + if (bpages < 0 || bpages >= NCACHED) + RETURN_ERROR(EFTYPE, error1); hashp->nmaps = bpages; (void)memset(&hashp->mapp[0], 0, bpages * sizeof(u_int32_t *)); @@ -886,26 +908,27 @@ alloc_segs(HTAB *hashp, int nsegs) int save_errno; - if ((hashp->dir = - calloc(hashp->DSIZE, sizeof(SEGMENT *))) == NULL) { - save_errno = errno; - (void)hdestroy(hashp); - errno = save_errno; - return (-1); + if (nsegs < 0) { + errno = EINVAL; + goto err_out; } + if ((hashp->dir = calloc(hashp->DSIZE, sizeof(SEGMENT *))) == NULL) + goto err_out; hashp->nsegs = nsegs; if (nsegs == 0) return (0); /* Allocate segments */ - if ((store = calloc(nsegs << hashp->SSHIFT, sizeof(SEGMENT))) == NULL) { - save_errno = errno; - (void)hdestroy(hashp); - errno = save_errno; - return (-1); - } + if ((store = calloc(nsegs << hashp->SSHIFT, sizeof(SEGMENT))) == NULL) + goto err_out; for (i = 0; i < nsegs; i++) hashp->dir[i] = &store[i << hashp->SSHIFT]; return (0); + +err_out: + save_errno = errno; + (void)hdestroy(hashp); + errno = save_errno; + return (-1); } #if BYTE_ORDER == LITTLE_ENDIAN diff --git a/lib/libc/db/hash/hash_buf.c b/lib/libc/db/hash/hash_buf.c index 94f95c8c0383..8b66d7165c3c 100644 --- a/lib/libc/db/hash/hash_buf.c +++ b/lib/libc/db/hash/hash_buf.c @@ -100,10 +100,10 @@ __get_buf(HTAB *hashp, u_int32_t addr, BUFHEAD *prev_bp, /* If prev_bp set, indicates a new overflow page. */ int newpage) { - BUFHEAD *bp; + int is_disk, segment_ndx, dir_ndx; u_int32_t is_disk_mask; - int is_disk, segment_ndx; SEGMENT segp; + BUFHEAD *bp; is_disk = 0; is_disk_mask = 0; @@ -116,9 +116,17 @@ __get_buf(HTAB *hashp, u_int32_t addr, } else { /* Grab buffer out of directory */ segment_ndx = addr & (hashp->SGSIZE - 1); - + dir_ndx = addr >> hashp->SSHIFT; + if (dir_ndx >= hashp->nsegs) { + /* + * A bucket address could theoretically have been + * generated using maliciously crafted header values + * aimed at __call_hash. + */ + return (NULL); + } /* valid segment ensured by __call_hash() */ - segp = hashp->dir[addr >> hashp->SSHIFT]; + segp = hashp->dir[dir_ndx]; #ifdef DEBUG assert(segp != NULL); #endif diff --git a/lib/libc/db/hash/hash_page.c b/lib/libc/db/hash/hash_page.c index 2a7b594dc3ea..9772b85cd6e8 100644 --- a/lib/libc/db/hash/hash_page.c +++ b/lib/libc/db/hash/hash_page.c @@ -657,14 +657,29 @@ overflow_page(HTAB *hashp) #ifdef DEBUG2 int tmp1, tmp2; #endif +#define OVMSG "HASH: Out of overflow pages. Increase page size\n" splitnum = hashp->OVFL_POINT; + if (splitnum >= NCACHED) { + (void)_write(STDERR_FILENO, OVMSG, sizeof(OVMSG) - 1); + errno = EFBIG; + return (0); + } max_free = hashp->SPARES[splitnum]; free_page = (max_free - 1) >> (hashp->BSHIFT + BYTE_SHIFT); free_bit = (max_free - 1) & ((hashp->BSIZE << BYTE_SHIFT) - 1); + if (free_page < 0 || free_page >= NCACHED) { + errno = EFTYPE; + return (0); + } /* Look through all the free maps to find the first free block */ first_page = hashp->LAST_FREED >>(hashp->BSHIFT + BYTE_SHIFT); + if (first_page < 0 || first_page >= NCACHED) { + errno = EFTYPE; + return (0); + } + for ( i = first_page; i <= free_page; i++ ) { if (!(freep = (u_int32_t *)hashp->mapp[i]) && !(freep = fetch_bitmap(hashp, i))) @@ -694,7 +709,6 @@ overflow_page(HTAB *hashp) offset = hashp->SPARES[splitnum] - (splitnum ? hashp->SPARES[splitnum - 1] : 0); -#define OVMSG "HASH: Out of overflow pages. Increase page size\n" if (offset > SPLITMASK) { if (++splitnum >= NCACHED) { (void)_write(STDERR_FILENO, OVMSG, sizeof(OVMSG) - 1); diff --git a/lib/libc/tests/db/Makefile b/lib/libc/tests/db/Makefile index 771569183584..73841737ac5a 100644 --- a/lib/libc/tests/db/Makefile +++ b/lib/libc/tests/db/Makefile @@ -10,12 +10,13 @@ ${PACKAGE}FILES+= README ATF_TESTS_C+= dbm_open_test ATF_TESTS_C+= dbm_perm_test ATF_TESTS_C+= dbm_nextkey_test +ATF_TESTS_C+= db_hash_tamper_test NETBSD_ATF_TESTS_C+= db_hash_seq_test NETBSD_ATF_TESTS_SH+= db_test ATF_TESTS_SH_SED_db_test= -e 's,/bin/csh,/bin/cat,g' -CFLAGS+= -I${SRCTOP}/lib/libc/db/btree +CFLAGS+= -I${SRCTOP}/lib/libc/db/btree -I${SRCTOP}/lib/libc/db/hash .include "../Makefile.netbsd-tests" diff --git a/lib/libc/tests/db/db_hash_tamper_test.c b/lib/libc/tests/db/db_hash_tamper_test.c new file mode 100644 index 000000000000..ce16c8071b63 --- /dev/null +++ b/lib/libc/tests/db/db_hash_tamper_test.c @@ -0,0 +1,227 @@ +/*- + * Copyright (c) 2026. Klara, Inc. + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include +#include + +#include + +#include +#include +#include +#include +#include +#include +#include + +/* + * The internal db/hash/hash.h header is needed to + * avoid hardcoding header structure offsets. + */ +#include "hash.h" + +#define SET_HDR_VAR(hdr, field, val) (hdr)->field = htonl((uint32_t)val) +#define GET_HDR_VAR(hdr, field) ((uint32_t)ntohl((hdr)->field);) + +static const char *dbname = "tmp.db"; + +/* Create a database file with one entry. */ +static void +create_db(void) +{ + DB *db; + DBT key, val; + + key.data = "foo"; + key.size = strlen("foo"); + + val.data = "bar"; + val.size = strlen("bar"); + + if (atf_utils_file_exists(dbname)) + unlink(dbname); + db = dbopen(dbname, O_CREAT | O_RDWR | O_TRUNC, 0755, DB_HASH, NULL); + ATF_CHECK(db != NULL); + ATF_REQUIRE(atf_utils_file_exists(dbname)); + + ATF_REQUIRE(db->put(db, &key, &key, 0) == 0); + + db->close(db); +} + +static void +read_hdr(HASHHDR *hdr) +{ + int fd; + + ATF_REQUIRE(atf_utils_file_exists(dbname)); + fd = open(dbname, O_RDONLY); + ATF_CHECK(fd != -1); + ATF_CHECK(read(fd, hdr, sizeof(*hdr)) == sizeof(*hdr)); + close(fd); +} + +static void +write_hdr(HASHHDR *hdr) +{ + int fd; + + ATF_REQUIRE(atf_utils_file_exists(dbname)); + fd = open(dbname, O_WRONLY); + ATF_CHECK(fd != -1); + ATF_CHECK(write(fd, hdr, sizeof(*hdr)) == sizeof(*hdr)); + close(fd); +} + +ATF_TC(db_hash_ovflw_point_test); +ATF_TC_HEAD(db_hash_ovflw_point_test, tc) +{ + atf_tc_set_md_var(tc, "descr", + "Test hash(3) operations with a corrupted 'ovfl_point' header variable."); +} + +ATF_TC_BODY(db_hash_ovflw_point_test, tc) +{ + HASHHDR hdr; + + create_db(); + + read_hdr(&hdr); + /* + * An unvalidated 'ovfl_point' variable may trigger + * an OOB read from the SPARES field. + */ + SET_HDR_VAR(&hdr, ovfl_point, NCACHED + 1); + write_hdr(&hdr); + + ATF_REQUIRE(dbopen(dbname, O_RDONLY, 0755, DB_HASH, NULL) == NULL); +} + +ATF_TC(db_hash_bpages_test); +ATF_TC_HEAD(db_hash_bpages_test, tc) +{ + atf_tc_set_md_var(tc, "descr", + "Test hash(3) operations with a corrupted 'spares' header variable."); +} + +ATF_TC_BODY(db_hash_bpages_test, tc) +{ + HASHHDR hdr; + + create_db(); + + read_hdr(&hdr); + /* + * An unvalidated combination of the 'ovfl_point' variable + * and the 'spares' array may be used to manipulate + * a memset in _hash_open. + */ + SET_HDR_VAR(&hdr, ovfl_point, 0); + hdr.spares[0] = htonl(0x10000000UL); + write_hdr(&hdr); + + ATF_REQUIRE(dbopen(dbname, O_RDONLY, 0755, DB_HASH, NULL) == NULL); +} + +ATF_TC(db_hash_bsize_test); +ATF_TC_HEAD(db_hash_bsize_test, tc) +{ + atf_tc_set_md_var(tc, "descr", + "Test hash(3) operations with a corrupted 'bsize' header variable."); +} + +ATF_TC_BODY(db_hash_bsize_test, tc) +{ + HASHHDR hdr; + + create_db(); + + read_hdr(&hdr); + /* + * An unvalidated 'bsize' variable may be + * used to manipulate a memset in _hash_open. + */ + SET_HDR_VAR(&hdr, bsize, 0x100000); + write_hdr(&hdr); + + ATF_REQUIRE(dbopen(dbname, O_RDONLY, 0755, DB_HASH, NULL) == NULL); +} + +ATF_TC(db_hash_masks_test); +ATF_TC_HEAD(db_hash_masks_test, tc) +{ + atf_tc_set_md_var(tc, "descr", + "Test hash(3) operations with corrupted '{high,low}_mask' header variables."); +} + +ATF_TC_BODY(db_hash_masks_test, tc) +{ + HASHHDR hdr; + + /* 'high_mask' must be greater than 'low_mask'. */ + create_db(); + read_hdr(&hdr); + SET_HDR_VAR(&hdr, high_mask, 0x1); + SET_HDR_VAR(&hdr, low_mask, 0xF); + write_hdr(&hdr); + ATF_REQUIRE(dbopen(dbname, O_RDONLY, 0755, DB_HASH, NULL) == NULL); + + /* 'high_mask' and 'low_mask' must be derived from power-of-2 values. */ + create_db(); + read_hdr(&hdr); + SET_HDR_VAR(&hdr, high_mask, 0x13); + write_hdr(&hdr); + ATF_REQUIRE(dbopen(dbname, O_RDONLY, 0755, DB_HASH, NULL) == NULL); + + create_db(); + read_hdr(&hdr); + SET_HDR_VAR(&hdr, high_mask, 0xFF); + SET_HDR_VAR(&hdr, low_mask, 0x13); + write_hdr(&hdr); + ATF_REQUIRE(dbopen(dbname, O_RDONLY, 0755, DB_HASH, NULL) == NULL); +} + +ATF_TC(db_hash_call_hash_oob_test); +ATF_TC_HEAD(db_hash_call_hash_oob_test, tc) +{ + atf_tc_set_md_var(tc, "descr", + "Attempt to trigger an OOB read with corrupted '{high,low}_mask' header variables."); +} + +ATF_TC_BODY(db_hash_call_hash_oob_test, tc) +{ + DBT key, val; + HASHHDR hdr; + DB *db; + + key.data = "foo"; + key.size = strlen("foo"); + + /* + * Invalid values of the '{high,low}_mask' header variables + * will cause __call_hash to return OOB bucket indices. + */ + create_db(); + read_hdr(&hdr); + SET_HDR_VAR(&hdr, low_mask, 0xFFFF); + SET_HDR_VAR(&hdr, high_mask, 0xFFFFF); + write_hdr(&hdr); + db = dbopen(dbname, O_RDONLY, 0755, DB_HASH, NULL); + ATF_REQUIRE(db != NULL); + /* Attempt to trigger an OOB read. */ + ATF_REQUIRE(db->get(db, &key, &val, 0) != 0); +} + +ATF_TP_ADD_TCS(tp) +{ + ATF_TP_ADD_TC(tp, db_hash_ovflw_point_test); + ATF_TP_ADD_TC(tp, db_hash_bpages_test); + ATF_TP_ADD_TC(tp, db_hash_bsize_test); + ATF_TP_ADD_TC(tp, db_hash_masks_test); + ATF_TP_ADD_TC(tp, db_hash_call_hash_oob_test); + + return (atf_no_error()); +}