aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBojan Novković <bnovkov@FreeBSD.org>2026-07-07 12:28:50 +0000
committerBojan Novković <bnovkov@FreeBSD.org>2026-09-18 10:27:47 +0000
commit7f5f07b139a5fd456087804ee96802b54c151703 (patch)
tree4c48b9d10b7b6e17e2f8d1206f1a50177c4b2dd2
parentd749076e80a2f2627fea5dfd3d328cc4fb767ec9 (diff)
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
-rw-r--r--lib/libc/db/hash/hash.c47
-rw-r--r--lib/libc/db/hash/hash_buf.c16
-rw-r--r--lib/libc/db/hash/hash_page.c16
-rw-r--r--lib/libc/tests/db/Makefile3
-rw-r--r--lib/libc/tests/db/db_hash_tamper_test.c227
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 <sys/types.h>
+#include <sys/wait.h>
+
+#include <netinet/in.h>
+
+#include <atf-c.h>
+#include <db.h>
+#include <fcntl.h>
+#include <signal.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+/*
+ * 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());
+}