Merge pull request #122 from berkeleydb/security/pentest-final

fix(security): harden untrusted .db parse/verify surface

Author: gburd

.gitignore

@@ -1,5 +1,6 @@
 build_unix/**
 build_asan/**
+build_asan_gate/**
 compile_commands.json
 test/tcl/tclIndex
 

src/btree/bt_search.c

@@ -1066,6 +1066,24 @@ skip_lock:		stack = set_stack;
 		if ((ret = __memp_fget(mpf, &pg,
 		     dbc->thread_info, dbc->txn, get_mode, &h)) != 0)
 			goto err;
+		/*
+		 * On an untrusted/corrupt file a BINTERNAL child pointer can
+		 * point back up the tree (to itself, a sibling, or an ancestor)
+		 * at the same or a higher level.  The descent then never reaches
+		 * LEAFLEVEL and this loop spins forever (a denial of service).
+		 * A valid Btree always has strictly decreasing levels from root
+		 * to leaf, so a child whose level is not below its parent's is
+		 * corruption -- reject it as a clean page error rather than loop.
+		 * (The lock-retry path above already enforces LEVEL(h)==level-1;
+		 * this guards the common latch-coupling fast path.)
+		 */
+		if (LEVEL(h) >= level) {
+			(void)__memp_fput(mpf,
+			    dbc->thread_info, h, dbc->priority);
+			h = NULL;
+			ret = DB_PAGE_NOTFOUND;
+			goto err;
+		}
 		/* Release the parent. */
 		if (parent_h != NULL && (ret = __memp_fput(mpf,
 		    dbc->thread_info, parent_h, dbc->priority)) != 0)

src/db/partition.c

@@ -1822,16 +1822,32 @@ __part_verify(dbp, vdp, fname, handle, callback, flags)
 	dbc = NULL;
 	ip = vdp->thread_info;
 
-	if (dbp->type == DB_BTREE) {
+	if (dbp->type == DB_BTREE || dbp->type == DB_RECNO) {
 		if ((ret = __bam_open(dbp, ip,
 		    NULL, fname, PGNO_BASE_MD, flags)) != 0)
 			goto err;
 	}
 #ifdef HAVE_HASH
-	else if ((ret = __ham_open(dbp, ip,
-	    NULL, fname, PGNO_BASE_MD, flags)) != 0)
-		goto err;
+	else if (dbp->type == DB_HASH) {
+		if ((ret = __ham_open(dbp, ip,
+		    NULL, fname, PGNO_BASE_MD, flags)) != 0)
+			goto err;
+	}
 #endif
+	else {
+		/*
+		 * Only Btree/Recno and Hash databases can be partitioned.  A
+		 * corrupt/hostile file whose meta page claims another type (e.g.
+		 * Heap or Queue) while setting the partition flag must not be
+		 * opened with the Hash access method: __db_cursor would allocate
+		 * a cursor sized for dbp->type, which __ham_get_meta then casts
+		 * to HASH_CURSOR, writing its hlock field past the end of the
+		 * smaller allocation (a heap-buffer-overflow / type confusion).
+		 * Reject the unexpected type instead.
+		 */
+		ret = __db_unknown_type(env, "__part_verify", dbp->type);
+		goto err;
+	}
 
 	/*
 	 * Initalize partition db handles and get the names. Set DB_RDWRMASTER

src/heap/heap_open.c

@@ -172,6 +172,24 @@ __heap_read_meta(dbp, ip, txn, meta_pgno, flags)
 	 * metadata page will be created/initialized elsewhere.
 	 */
 	if (meta->dbmeta.magic == DB_HEAPMAGIC) {
+		/*
+		 * region_size comes from the (possibly corrupt) on-disk meta
+		 * page and is used as a divisor via HEAP_REGION_SIZE(dbp)+1 in
+		 * HEAP_REGION_PGNO / HEAP_REGION_NUM.  A region_size of 0 or of
+		 * UINT32_MAX (so the +1 wraps to 0) would divide by zero; any
+		 * value larger than the per-page region count is impossible for
+		 * this page size.  Reject it before it is trusted, mirroring the
+		 * bound __heap_new_file already enforces on creation.
+		 */
+		if (meta->region_size == 0 ||
+		    meta->region_size > HEAP_REGION_COUNT(dbp, dbp->pgsize)) {
+			__db_errx(dbp->env, DB_STR_A("1169",
+			    "region size may not be larger than %lu",
+			    "%lu"),
+			    (u_long)HEAP_REGION_COUNT(dbp, dbp->pgsize));
+			ret = EINVAL;
+			goto err;
+		}
 		h->curregion = meta->curregion;
 		h->curpgindx = 0;
 		h->gbytes = meta->gbytes;

src/heap/heap_verify.c

@@ -71,6 +71,19 @@ __heap_vrfy_meta(dbp, vdp, meta, pgno, flags)
 	h = (HEAP *)dbp->heap_internal;
 	h->region_size = meta->region_size;
 	last_pgno = meta->dbmeta.last_pgno;
+	/*
+	 * region_size is used as a divisor (HEAP_REGION_SIZE(dbp)+1) below and
+	 * in the structure pass; a corrupt 0 or UINT32_MAX value would divide
+	 * by zero (the +1 wraps).  Reject it as bad rather than crash.
+	 */
+	if (meta->region_size == 0 ||
+	    meta->region_size > HEAP_REGION_COUNT(dbp, dbp->pgsize)) {
+		EPRINT((dbp->env, DB_STR_A("1174",
+		    "Page %lu: invalid heap region size %lu",
+		    "%lu %lu"), (u_long)pgno, (u_long)meta->region_size));
+		isbad = 1;
+		goto err;
+	}
 	if (meta->nregions != HEAP_REGION_NUM(dbp, last_pgno)) {
 		EPRINT((dbp->env, DB_STR_A("1157",
 		    "Page %lu: Number of heap regions incorrect",
@@ -124,6 +137,15 @@ __heap_vrfy(dbp, vdp, h, pgno, flags)
 	int cnt, i, j, ret;
 	db_indx_t *offsets, *offtbl, end;
 
+	/*
+	 * offsets is freed unconditionally at the err label.  If
+	 * __db_vrfy_datapage below fails on a corrupt page we jump there
+	 * before offsets is assigned, so it must start NULL (a free of an
+	 * indeterminate pointer is otherwise undefined behavior / a wild
+	 * free on a hostile heap file).
+	 */
+	offsets = NULL;
+
 	if ((ret = __db_vrfy_datapage(dbp, vdp, h, pgno, flags)) != 0)
 		goto err;
 

src/qam/qam_open.c

@@ -106,6 +106,20 @@ __qam_open(dbp, ip, txn, name, base_pgno, mode, flags)
 	t->re_len = qmeta->re_len;
 	t->rec_page = qmeta->rec_page;
 
+	/*
+	 * rec_page (records per page) is trusted from the on-disk meta page
+	 * and used as a divisor throughout the queue access method via
+	 * QAM_RECNO_PAGE (records/page).  A corrupt 0 divides by zero (SIGFPE).
+	 * A valid queue always has at least one record per page.
+	 */
+	if (t->rec_page == 0) {
+		__db_errx(env, DB_STR_A("1136",
+		    "__qam_open: %s: unexpected file type or format", "%s"),
+		    name);
+		ret = EINVAL;
+		goto err;
+	}
+
 	t->q_meta = base_pgno;
 	t->q_root = base_pgno + 1;
 

src/qam/qam_verify.c

@@ -81,9 +81,12 @@ __qam_vrfy_meta(dbp, vdp, meta, pgno, flags)
 
 	/*
 	 * re_len:  If this is bad, we can't safely verify queue data pages, so
-	 * return DB_VERIFY_FATAL
+	 * return DB_VERIFY_FATAL.  rec_page (records per page) must be non-zero:
+	 * it is used as a divisor via QAM_RECNO_PAGE below and throughout the
+	 * queue AM, so a corrupt 0 would divide by zero (SIGFPE).
 	 */
-	if (DB_ALIGN(meta->re_len + sizeof(QAMDATA) - 1, sizeof(u_int32_t)) *
+	if (meta->rec_page == 0 ||
+	    DB_ALIGN(meta->re_len + sizeof(QAMDATA) - 1, sizeof(u_int32_t)) *
 	    meta->rec_page + QPAGE_SZ(dbp) > dbp->pgsize) {
 		EPRINT((env, DB_STR_A("1147",
     "Page %lu: queue record length %lu too high for page size and recs/page",

test/fuzz/check-crashes.sh

@@ -17,8 +17,18 @@
 # DB_ASSERT in __memp_fopen, or a recovery-failure panic), which is by-design
 # diagnostic behavior, not the OOB/FPE crash class this gate guards against.
 #
+# libdb ASan instrumentation:
+#   Some crash classes (a heap-buffer-overflow / use-after-free / double-free
+#   *inside* libdb's own allocations -- e.g. the __part_verify type-confusion
+#   OOB write) are only observable when libdb itself is compiled with
+#   AddressSanitizer; a harness-only ASan build (libdb.a plain) cannot see
+#   them.  If a build_unix built with `CFLAGS=-fsanitize=address` (ASan only,
+#   NOT undefined -- UBSan flags libdb's pervasive base+offset pointer idioms)
+#   is available, point LIBDB_BUILD at it to catch those.  This gate
+#   auto-builds one under build_asan_gate/ when LIBDB_ASAN=1 (default on).
+#
 # Usage:  ./check-crashes.sh
-# Env:    CC, LIBDB_BUILD (see run.sh)
+# Env:    CC, LIBDB_BUILD (see run.sh), LIBDB_ASAN (1=build+use an ASan libdb)
 #
 # Run from test/fuzz/ inside a `nix develop` shell.
 
@@ -27,6 +37,27 @@ set -eu
 HERE=$(CDPATH= cd -- "$(dirname -- "$0")" && pwd)
 cd "$HERE"
 
+CC=${CC:-clang}
+LIBDB_ASAN=${LIBDB_ASAN:-1}
+
+# Build (once) an ASan-instrumented libdb so a memory fault *inside* libdb is
+# caught, then link the standalone harnesses against it.  ASan only -- UBSan
+# would fire on libdb's legitimate base+offset pointer arithmetic.  The
+# harness's own SAN flags in run.sh still add UBSan to the harness .c, so we
+# neutralise it for the lib by exporting an ASan-only LIBDB build here.
+if [ "$LIBDB_ASAN" = "1" ] && [ -z "${LIBDB_BUILD:-}" ]; then
+	GATE_BUILD="$HERE/../../build_asan_gate"
+	if [ ! -f "$GATE_BUILD/libdb.a" ]; then
+		mkdir -p "$GATE_BUILD"
+		( cd "$GATE_BUILD" &&
+		  ../dist/configure --enable-debug \
+		      CC="$CC" CFLAGS="-fsanitize=address -g -O1" >configure.log 2>&1 &&
+		  make -j4 >build.log 2>&1 ) ||
+		{ echo "warning: ASan libdb build failed; falling back to plain lib" >&2; }
+	fi
+	[ -f "$GATE_BUILD/libdb.a" ] && export LIBDB_BUILD="$GATE_BUILD"
+fi
+
 # Build the standalone (no-libFuzzer) drivers for every harness once.
 FUZZ_STANDALONE=1 ./run.sh build