diff --git a/.gitignore b/.gitignore index 3ee23ede1..aaa2a6ebe 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,6 @@ build_unix/** build_asan/** +build_asan_gate/** compile_commands.json test/tcl/tclIndex diff --git a/src/btree/bt_search.c b/src/btree/bt_search.c index 236e32dba..19ac4db5d 100644 --- a/src/btree/bt_search.c +++ b/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) diff --git a/src/db/partition.c b/src/db/partition.c index 9c43951ef..f6c6b990b 100644 --- a/src/db/partition.c +++ b/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 diff --git a/src/heap/heap_open.c b/src/heap/heap_open.c index 6eee3dfa8..9f0c1f8be 100644 --- a/src/heap/heap_open.c +++ b/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; diff --git a/src/heap/heap_verify.c b/src/heap/heap_verify.c index 4f121d106..6b85c16c3 100644 --- a/src/heap/heap_verify.c +++ b/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; diff --git a/src/qam/qam_open.c b/src/qam/qam_open.c index 68617ef5c..3c0618c56 100644 --- a/src/qam/qam_open.c +++ b/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; diff --git a/src/qam/qam_verify.c b/src/qam/qam_verify.c index b887bedf1..85b2509b6 100644 --- a/src/qam/qam_verify.c +++ b/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", diff --git a/test/fuzz/check-crashes.sh b/test/fuzz/check-crashes.sh index d154ce83a..7b221f269 100755 --- a/test/fuzz/check-crashes.sh +++ b/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 diff --git a/test/fuzz/corpus/dbfile/valid_heap.db b/test/fuzz/corpus/dbfile/valid_heap.db new file mode 100644 index 000000000..364aaf723 Binary files /dev/null and b/test/fuzz/corpus/dbfile/valid_heap.db differ diff --git a/test/fuzz/corpus/dbfile/valid_queue.db b/test/fuzz/corpus/dbfile/valid_queue.db new file mode 100644 index 000000000..5af96bc7d Binary files /dev/null and b/test/fuzz/corpus/dbfile/valid_queue.db differ diff --git a/test/fuzz/crashes/README.md b/test/fuzz/crashes/README.md index 537d8a911..6b15db50c 100644 --- a/test/fuzz/crashes/README.md +++ b/test/fuzz/crashes/README.md @@ -58,6 +58,31 @@ masking them. Both are the same trust-a-length-from-the-file class and are | `dbfile_fpe_bam_minkey.seed` | SIGFPE (divide-by-zero) | `__bamc_refresh` `src/btree/bt_cursor.c:285`, via `B_MINKEY_TO_OVFLSIZE` on btree open/cursor-init | The btree meta page's `minkey` field is 0; it is used as a divisor. | Reject `minkey < 2` when loaded in `__bam_read_root` (`bt_open.c`), matching what verify and the public setter already require. | | `recover_oob_read_log_chksum.seed` | ASan OOB read | `__ham_func4` `src/hash/hash_func.c:171`, via `__db_check_chksum` ← `__log_valid` `src/log/log.c:818` | A corrupt log header's `hdr->len` makes the checksum hash read past the fixed-size `persist` record buffer. The crypto path already bounded this; the non-crypto path did not. | Add the same `hdr->len - hdrsize == recsize` bound (and underflow guard) on the non-crypto path in `__log_valid`. | +## Findings from the security / pentest review (2026-07) + +Five more, all on the untrusted `.db`-file parse/verify surface, found by +fuzzing an **ASan-instrumented** libdb (a heap-buffer-overflow *inside* libdb's +own allocations is invisible to a plain-lib harness -- see the ASan-gate note +below). All are **FIXED** in this PR; each ships a regression seed. + +| Seed | Fault | Site | Fix | +|------|-------|------|-----| +| `dbfile_typeconf_part_verify.seed` | ASan heap-buffer-overflow (8-byte WRITE) | `__db_lget` via `__ham_get_meta` <- `__ham_open` <- `__part_verify` `partition.c` | Type confusion: a non-Btree (e.g. Heap) file with the partition flag was opened with the Hash AM but a Heap-sized cursor internal -> `LOCK_INIT(&hcp->hlock)` writes past the 88B alloc. Dispatch by exact type; reject non-Btree/Recno/Hash. | +| `dbfile_doublefree_heap_vrfy.seed` | ASan double-free / free of indeterminate ptr | `__heap_vrfy` `heap_verify.c` | `offsets` freed at `err:` while uninitialized (early `__db_vrfy_datapage` failure jumps there). `offsets = NULL;` at decl. (was the OPEN item in `fuzz-found-bugs.md`.) | +| `dbfile_infloop_bam_search.seed` | DoS -- infinite loop | `__bam_search` `bt_search.c` fast-path child fetch | A `P_IBTREE` child pointer to itself/an ancestor at same-or-higher level spins the descent forever. Guard `LEVEL(child) >= LEVEL(parent)` -> `DB_PAGE_NOTFOUND` (levels must strictly decrease). | +| `dbfile_fpe_heap_region_size.seed` | SIGFPE (divide-by-zero) | `__heap_vrfy_meta` `heap_verify.c` (`HEAP_REGION_NUM`) | heap meta `region_size` 0 or UINT32_MAX (`+1` wraps) used as divisor. Reject at open+verify. | +| `dbfile_fpe_qam_recpage.seed` | SIGFPE (divide-by-zero) | `__qam_vrfy_meta` `qam_verify.c` (`QAM_RECNO_PAGE`) | queue meta `rec_page` 0 used as divisor. Reject at open+verify. | + +> **ASan gate:** `check-crashes.sh` now builds an ASan-only libdb under +> `build_asan_gate/` (gitignored) and links the standalone harnesses against +> it, so a memory fault *inside* libdb (e.g. the `__part_verify` OOB write) is +> caught -- a plain-lib harness cannot see it. Set `LIBDB_ASAN=0` to skip. + +A sixth finding (a **bounded** queue extent-scan DoS in `__qam_vrfy_walkqueue` +on a crafted huge `cur_recno`) is documented in `.agents/security-review.md` +and DEFERRED: it terminates, and a safe fix must be extent-aware so it does not +reject valid large/wrapped queues. + ### Still open (documented, not fixed here) `fuzz_recover` / verify-on-a-corrupt-file additionally show a **memory leak on diff --git a/test/fuzz/crashes/dbfile_doublefree_heap_vrfy.seed b/test/fuzz/crashes/dbfile_doublefree_heap_vrfy.seed new file mode 100644 index 000000000..d63fb73a0 Binary files /dev/null and b/test/fuzz/crashes/dbfile_doublefree_heap_vrfy.seed differ diff --git a/test/fuzz/crashes/dbfile_fpe_heap_region_size.seed b/test/fuzz/crashes/dbfile_fpe_heap_region_size.seed new file mode 100644 index 000000000..2095b80f5 Binary files /dev/null and b/test/fuzz/crashes/dbfile_fpe_heap_region_size.seed differ diff --git a/test/fuzz/crashes/dbfile_fpe_qam_recpage.seed b/test/fuzz/crashes/dbfile_fpe_qam_recpage.seed new file mode 100644 index 000000000..e31341404 Binary files /dev/null and b/test/fuzz/crashes/dbfile_fpe_qam_recpage.seed differ diff --git a/test/fuzz/crashes/dbfile_infloop_bam_search.seed b/test/fuzz/crashes/dbfile_infloop_bam_search.seed new file mode 100644 index 000000000..2aa08c9bb Binary files /dev/null and b/test/fuzz/crashes/dbfile_infloop_bam_search.seed differ diff --git a/test/fuzz/crashes/dbfile_typeconf_part_verify.seed b/test/fuzz/crashes/dbfile_typeconf_part_verify.seed new file mode 100644 index 000000000..5ae29e656 Binary files /dev/null and b/test/fuzz/crashes/dbfile_typeconf_part_verify.seed differ