From nobody Fri Sep 18 10:31:01 2026 X-Original-To: dev-commits-src-main@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 4hmTRp1cP4z6sGMN for ; Fri, 18 Sep 2026 10:31:02 +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 4hmTRn75B0z3Mv7 for ; Fri, 18 Sep 2026 10:31:01 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1789727462; 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=LF7gcQ3LCxC8sVglwfAjtlBp3PNjIYC1Npfq5SBRxo8JcC+88237rZDlyphF1MJrc8Lvo6 NVdcb/q/M9CAA98A12EdGCEHVKt7IC4pSzn//qUuHztCGWSy25WLZvHvl2e5C8TxruZnJi oamf9D4jk7skSpMyAmbjiD4foQKR6h18wXjPhkV9SL5gq0NweJr4xTAHhVIpOjBZojeWxJ pG13uqFnF3gw8YkubgFJw0Hi9ZYK2XAwrm7WVYq+kt8LQQywbyISbLjpwSh/JveD43S2rZ kgImDHSi81OWNF/KzIndPePmcvYb9D1ljcP3P4L3yD/oCr/Z3MwttODk8//1hQ== ARC-Seal: i=1; a=rsa-sha256; d=freebsd.org; s=dkim; cv=none; t=1789727462; b=SIugYadweIEI6szcD0ICOsNKAMDXckJ2EYhKmO0IR1JZCvRR8sJyjMOf7ZUmJUQoYUxsLv xnOcotgnO+SrP3KuP1RrzXKwj+hS0RTW8gK3aCkpYzB2q/KahLwxu+y4WUG56saIt/u1QK AHc0ZMzZCUv8laZ0uMjug/tEUC3YHkL0UVWrhGo2ezmeFG5vqZk/CE2jGPBNZzoHKhGq8P CvDu6BfS/zy6i5AsymLJ9G+8E/8OskmhcQ/kQVnDtOgG8XGkX6QVT86h8q3KcO/kcurPui Woos2X5Lu/RflpWqoobaqw62qnjWwbv+QQXAcYy4ClTUj6UJ/LKDwX9JXDTocQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1789727462; 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=nLJTlex1VXFi0ALnH/bX3gT7bFPg7KuR9lxMqWJSD1BHVz/pIa/IZBVhjyJYcn0BMoUWli Tb6EypMWz/SxZO7za3WjvXPXxzIhUBSOdFyYJVfoa9dRJOGRlakGr3bgsjJBhEtx38qMPa 0CRyMas33OfYR9nFr4du3/PZ4pYV6Ft15itk52tZC1xGCzHjo2kT1CbhVPaHtZqLY/dlCW PJAS37ngF8CFvmvtTQsgJ3WDlRBOFNXo9+dKNe/JwDCdB1Jr8gzrTb/JdpC06AM/ZsbFcU RfVY0ILsVAv+yK3AmqTUY5+HWuB3FE1sMjkz5pRTrke2CFoZnrg2Dzzvpj9QPA== 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 4hmTRn67RjzFTc for ; Fri, 18 Sep 2026 10:31:01 +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 the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-main@freebsd.org Sender: owner-dev-commits-src-main@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()); +}