fix(db): heap page-item bounds check used btree/hash inp[] semantics - #67
Merged
Conversation
Regression from the malformed-file hardening (PR #58): __db_ret_okitem gated every access-method cursor return through one bounds check, but it applied btree/hash inp[] semantics to HEAP pages, which use inp[] as an offset table with different rules: - valid heap slot indexes run up to HEAP_HIGHINDX, which can exceed NUM_ENT (deleted slots leave holes), so the generic `indx >= NUM_ENT(h)` reject was wrong for heap; - a deleted/empty heap slot has offset 0, and valid heap offsets don't have to fall after the inp[] array, so the generic offset lower-bound reject was wrong. The result: db_dump (and any DB_NEXT scan) on a heap DB that had a non-last record deleted returned DB_PAGE_NOTFOUND at end-of-scan instead of DB_NOTFOUND, so db_dump exited non-zero -- reproduced by test046/049/139 on the heap method (found by the full-suite coverage run; see test/coverage/COVERAGE-TEST-TRIAGE.md). Fix: validate P_HEAP up front against HEAP_HIGHINDX and the heap record-header bounds, then return -- do not fall through to the btree/hash generic checks. The btree/hash/recno hardening is unchanged. Verified: db_dump on the heap DB now succeeds; test046/049/139 pass on heap; test001/003 pass on all five access methods + verify/salvage + logverify001; all four fuzz crash reproducers (incl. the db_retcopy OOB the #58 guard protects) still pass.
Coccinelle convention checksNo new violations. ✅ Resolved since baseline (2) -- update dist/cocci/baseline.txt to lock these in. |
ABI diff vs
|
This was referenced Jul 28, 2026
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.
Regression from PR #58 (malformed-file hardening).
__db_ret_okitemgates every access-method cursor return through one bounds check, but it applied btree/hashinp[]semantics to HEAP pages, which useinp[]as an offset table with different rules:HEAP_HIGHINDX, which can exceedNUM_ENT(deleted slots leave holes) → the genericindx >= NUM_ENT(h)reject was wrong for heap;inp[]array → the generic offset lower-bound reject was wrong.Impact:
db_dump(and anyDB_NEXTscan) on a heap DB with a deleted non-last record returnedDB_PAGE_NOTFOUNDat end-of-scan instead ofDB_NOTFOUND, sodb_dumpexited non-zero. Reproduced by test046/049/139 on the heap method — surfaced by the full-suite coverage run (seetest/coverage/COVERAGE-TEST-TRIAGE.md, PR #66).Fix: validate
P_HEAPup front againstHEAP_HIGHINDX+ the heap record-header bounds, then return — don't fall through to the btree/hash generic checks. The btree/hash/recno hardening is unchanged.Verification
db_dump -kon the heap DB now succeeds (was rc=1)db_retcopyOOB the harden: bounds-check untrusted page/log fields (OOB read + SIGFPE) on open/scan/recover #58 guard protects) — hardening intactHow it was found
This is exactly the kind of latent bug deeper testing surfaces: the coverage run exercised the heap AM (previously untested), which hit the dump/load round-trip; triage isolated it to a clean-build heap failure; root-caused to my own #58 hardening being too btree-centric.