wallet: reject cyclic page references in legacy BDB parser #35150

pull thomasbuilds wants to merge 2 commits into bitcoin:master from thomasbuilds:fix-berkeleyro-cycle-detection changing 3 files +211 −2

thomasbuilds commented at 12:03 PM on April 24, 2026: none
The read-only BDB parser (BerkeleyRODatabase::Open() in migrate.cpp) walks B-tree pages and overflow chains by following page numbers read directly from the input file. Neither traversal tracked visited pages, so a crafted legacy wallet file containing a cyclic page reference causes the parser to loop forever — and, for cycles on an overflow chain, to grow memory without bound (each iteration appends OverflowPage.data to a std::vector).

Impact

Local denial-of-service / memory exhaustion of the wallet process. Reachable whenever a user feeds an attacker-supplied .dat file to a BDB-reading code path:
- migratewallet RPC
- bitcoin-wallet -wallet=<path> dump
Legacy BDB wallets can no longer be created or loaded since v30.0, but the read-only parser is intentionally retained to power migratewallet and will remain in tree for the foreseeable future, so hardening it still matters.

Not remotely exploitable; no memory corruption.

Fix

Add a single shared std::unordered_set<uint32_t> visited_pages around the B-tree DFS loop, and check it inside the overflow-chain following loop as well. In a valid BDB database every physical page is referenced at most once, so a re-visit indicates corruption or a deliberate cycle and is rejected with "Cyclic page reference in BDB database".

The core change is ~11 lines in migrate.cpp.

Testing

Two new regression tests in db_tests.cpp build minimal hand-crafted BDB files that exercise each cyclic code path:
- bdb_parser_btree_cycle_detection — BTREE_INTERNAL page whose InternalRecord.page_num points back to itself
- bdb_parser_overflow_cycle_detection — OVERFLOW_DATA page whose next_page points back to itself
Verification:

Configuration btree test overflow test

master (no fix) hangs until SIGTERM hangs until SIGTERM

this PR passes in ~1 ms passes in ~1 ms

Full db_tests suite passes with no regressions. The existing wallet_bdb_parser fuzz target is updated to accept the new error string.

Configuration	btree test	overflow test
master (no fix)	hangs until SIGTERM	hangs until SIGTERM
this PR	passes in ~1 ms	passes in ~1 ms

wallet: reject cyclic page references in legacy BDB parser

The read-only BDB parser (BerkeleyRODatabase::Open) walks B-tree pages
via DFS, pushing page numbers read directly from attacker-supplied
InternalRecord.page_num values. It also follows overflow chains by
reading next_page values directly from OverflowPage headers. Neither
traversal tracked visited pages, so a crafted .dat file containing a
cyclic page reference caused:

  - an infinite loop in the B-tree DFS, or
  - an infinite loop plus unbounded heap growth when the cycle was in
    an overflow chain (each iteration appends OverflowPage.data to a
    std::vector).

This is reachable via the `migratewallet` RPC and `bitcoin-wallet
-wallet=X dump` whenever a user loads a crafted legacy wallet file.
Impact is a local denial-of-service / memory exhaustion of the wallet
process.

Track visited page numbers in a single std::unordered_set shared
between the B-tree traversal and every overflow chain. In a valid BDB
database every page is referenced at most once, so re-visiting any
page number indicates corruption or a deliberate cycle and is rejected
with a clear error.

The existing wallet_bdb_parser fuzz target is updated to accept the
new error string.

a64125e90f

test: regression tests for BDB parser cyclic page references

Add two unit tests that build minimal hand-crafted BDB files and
verify that MakeBerkeleyRODatabase rejects them with
DatabaseStatus::FAILED_LOAD rather than hanging:

  - bdb_parser_btree_cycle_detection:    BTREE_INTERNAL page whose
    InternalRecord.page_num points back to itself.
  - bdb_parser_overflow_cycle_detection: OVERFLOW_DATA page whose
    next_page points back to itself.

Without the parent commit's fix, both fixtures cause test_bitcoin to
hang indefinitely; with the fix, both fixtures are rejected in under
a millisecond.

c179d932f0

DrahtBot added the label Wallet on Apr 24, 2026
DrahtBot commented at 12:03 PM on April 24, 2026: contributor



The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Reviews

See the guideline for information on the review process. A summary of reviews will appear here.
maflcko commented at 12:23 PM on April 24, 2026: member

Thx, but how is this different from #34959?
thomasbuilds commented at 12:30 PM on April 24, 2026: none

Yeah I missed #34959 in my search No problem, closing in favor of #34959
thomasbuilds closed this on Apr 24, 2026
thomasbuilds deleted the branch on Apr 24, 2026

Contributors

Labels

wallet: reject cyclic page references in legacy BDB parser #35150

Impact

Fix

Testing

Reviews