fix(security): harden untrusted .db parse/verify surface (OOB write, double-free, SIGFPE, infinite-loop DoS) - #121
Closed
gburd wants to merge 5 commits into
Closed
fix(security): harden untrusted .db parse/verify surface (OOB write, double-free, SIGFPE, infinite-loop DoS)#121gburd wants to merge 5 commits into
gburd wants to merge 5 commits into
Conversation
A partitioned-database verify (__part_verify) chose the access-method open with a blind `if (type==DB_BTREE) __bam_open; else __ham_open;`. The else catches every non-Btree type, so a corrupt/hostile file whose meta page declares DB_HEAP (or DB_QUEUE) while setting the partition flag is opened with the Hash AM. __ham_open's __db_cursor then allocates the cursor internal sized for dbp->type (an 88-byte HEAP_CURSOR), which __ham_get_meta casts to HASH_CURSOR and __db_lget writes hcp->hlock past the end of -- an 8-byte heap-buffer-overflow WRITE (type confusion, memory corruption) reachable from DB->verify on an untrusted .db file. Partitioned databases only support Btree/Recno and Hash, so dispatch by exact type and reject anything else via __db_unknown_type(), mirroring the type switch already used in __part_truncate. Regression: test/fuzz/crashes/dbfile_typeconf_part_verify.seed (ASan heap-buffer-overflow before, clean after). Found by fuzzing an ASan-instrumented libdb with test/fuzz/fuzz_dbfile.
…nter) __heap_vrfy declared `db_indx_t *offsets;` uninitialized and freed it unconditionally at the err label. When __db_vrfy_datapage (called first, before offsets is assigned) fails on a corrupt heap page, control jumps to `err: __os_free(env, offsets)` with offsets holding stack garbage -- a free of an indeterminate pointer (ASan observed a double-free where the garbage aliased a freshly-freed 88-byte VRFY_PAGEINFO; in general a wild free / heap corruption) reachable from DB->verify on an untrusted heap file. Initialize offsets = NULL at declaration; __os_free(NULL) is a documented no-op. This is the HEAP-verify double-free left OPEN in .agents/fuzz-found-bugs.md, now root-caused. Regression: test/fuzz/crashes/dbfile_doublefree_heap_vrfy.seed.
…op DoS) __bam_search descends the tree in a for(;;) that terminates only at LEVEL(h)==LEAFLEVEL. The common latch-coupling fast path fetched the child page and re-looped with no check that the child's level is below the parent's (the lock-retry path already enforces LEVEL(h)==level-1, but the fast path did not). A corrupt P_IBTREE page whose BINTERNAL child pointer targets itself, a sibling, or an ancestor at the same-or-higher level makes the descent never reach a leaf, so a read cursor (DB_FIRST/DB_NEXT) on an untrusted .db file spins forever -- a denial of service. A valid Btree always has strictly decreasing levels root->leaf, so guard `LEVEL(child) >= LEVEL(parent)` after the fast-path fetch and return a clean DB_PAGE_NOTFOUND (no __env_panic), bounding the descent to <=255 iterations. Cannot fire on a valid tree. Hot-path safety verified: test001 btree/hash/heap/ queue, test011, test003 recno, recd005/recd015 recovery, and the DST crash-recover capstone all pass. Regression: test/fuzz/crashes/dbfile_infloop_bam_search.seed (hang before, 0.02s clean after).
…ify (SIGFPE) Two divide-by-zero crashes (SIGFPE) reachable from DB->open/verify on an untrusted .db file, both because a meta-page scalar is trusted before being used as a divisor and DB_ASSERT is compiled out of production builds: - heap region_size: used via HEAP_REGION_SIZE(dbp)+1 in HEAP_REGION_PGNO/ HEAP_REGION_NUM. region_size==0 or ==UINT32_MAX (the +1 wraps to 0) divides by zero (SIGFPE __heap_vrfy_meta). Reject region_size==0 || > HEAP_REGION_COUNT at both load points (__heap_read_meta open, __heap_vrfy_meta verify) -- exactly the bound __heap_new_file enforces on creation. - queue rec_page (records/page): used via QAM_RECNO_PAGE ((recno-1)/rec_page) throughout the queue AM. rec_page==0 divides by zero (SIGFPE __qam_vrfy_meta). Reject rec_page==0 at both load points (__qam_open, __qam_vrfy_meta). Regression: test/fuzz/crashes/dbfile_fpe_heap_region_size.seed and dbfile_fpe_qam_recpage.seed.
check-crashes.sh linked a non-ASan libdb.a (only the harness .c was instrumented), so a heap-buffer-overflow / UAF *inside* libdb's own allocations (e.g. the __part_verify type-confusion OOB write) produced no ASan report and passed the gate. Build an ASan-only libdb under build_asan_gate/ (UBSan omitted -- it fires on libdb's legitimate base+offset pointer idioms) and link the standalone harnesses against it so libdb-internal memory faults are now caught. Opt out with LIBDB_ASAN=0. Add valid_heap.db / valid_queue.db corpus seeds so fuzz_dbfile reaches the heap and queue meta/verify paths (which surfaced findings above). Document the five new crash seeds and the deferred queue extent-scan DoS in crashes/README.md. build_asan_gate/ is gitignored.
Coccinelle convention checksNo new violations. ✅ Resolved since baseline (2) -- update dist/cocci/baseline.txt to lock these in. |
ABI diff vs
|
Collaborator
Author
|
Superseded by security/pentest-final — rebased onto master to drop the ~1000 docs-src files that #120 already merged; this PR is now ONLY the 6 engine security fixes + 5 regression crash-seeds. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Security / penetration review — embedded core
Threat-model-scoped review of the embedded engine (untrusted on-disk data is
the primary surface) + ASan/UBSan fuzzing (reused & extended
test/fuzz/).Six confirmed vulnerabilities fixed, each with a before/after ASan proof and
a committed regression seed. One further DoS documented and deferred (see
below). No speculative findings.
All six were found by fuzzing an ASan-instrumented libdb with
test/fuzz/fuzz_dbfile(open/verify/scan of an untrusted.dbfile).Fixed
__part_verifyblindelse→__ham_openon a Heap-typed dbp →LOCK_INIT(&hcp->hlock)past an 88-byte HEAP_CURSORdbfile_typeconf_part_verify.seed__heap_vrfyfrees uninitializedoffsetson early datapage failuredbfile_doublefree_heap_vrfy.seed__bam_searchfast-path descent has no child-level-decreases check; a self/ancestorP_IBTREEpointer spins foreverdbfile_infloop_bam_search.seedregion_size(0 / UINT32_MAX+1) as divisor in__heap_vrfy_meta/__heap_read_metadbfile_fpe_heap_region_size.seedrec_page==0 as divisor in__qam_vrfy_meta/__qam_opendbfile_fpe_qam_recpage.seedEvery fix is a minimal, root-cause validation guard at the load/descent
chokepoint (mirroring the existing
__db_ret_okitempattern): reject bad inputwith
EINVAL/DB_VERIFY_BAD/DB_PAGE_NOTFOUND, no panic, no on-disk / log /region / ABI change, no valid-input rejection.
The HIGH type-confusion OOB write was invisible to the existing crash gate and
fuzz.ymlbecause both link a non-ASanlibdb.a; this PR fixes that gap —check-crashes.shnow builds an ASan-only libdb (build_asan_gate/) and linksthe standalone harnesses against it, so memory faults inside libdb are caught.
Deferred (documented, not force-patched)
__qam_vrfy_walkqueue, mirrored inqam_method/qam_stat): a crafted hugecur_recnomakes the extent-fileprobe loop run for minutes. It terminates (not an infinite loop, not
memory corruption) and a safe fix must be extent-aware so it doesn't reject
valid large/wrapped queues. Root cause + scoped fix in the local review doc.
__db_generate_iv): MT19937 seeded fromhash(gettimeofday). AES-CBC mode/IV-uniqueness are correct; this is IVunpredictability only, a longstanding design characteristic, not a
memory-safety bug.
Validation
check-crashes.sh(ASan gate): 9/9 seeds PASS (4 pre-existing + 5 new); each new seed FAILs the gate when its fix is reverted.--enable-dst):test_sim_crash_recover/_rng/_torn— PASS (recovery/ACID intact; 0 silent-bad).fuzz_dbfile360 s,fuzz_recover240 s,fuzz_api180 s — no new memory faults.--enable-debugbuild, 0 warnings.Posture
The core is solid on the happy path and mature; the concentrated weakness was
the untrusted-file parse/verify surface trusting meta-page scalars
(heap
region_size, queuerec_page, partition type) before using them asdivisors / cursor-size selectors, plus one uninitialized-pointer free. These
extend the existing hardening track's validate-at-the-chokepoint pattern to
heap/queue/partition and add a corruption-termination guard to Btree descent.
After these fixes a single untrusted
.dbfile no longer yields OOB write,double-free, SIGFPE, or an infinite loop on the fuzzed paths.