#35990 wallet: harden the read-only BDB parser against crafted files

ReviewabilityReviewsAgreementSize
#35990 shuv-amp wallet: harden the read-only BDB parser against crafted filesReady1Strong35 + 230 tests
  • Harden the read-only Berkeley DB parser against crafted or corrupted files during wallet migration
  • Prevent infinite loops, exponential traversal, and large memory bloat when loading legacy wallets
  • Ready for review; no known blockers or rework needed
  • Substantive support with no open objections
  • Verified bug reproduces on master and resolves here across migration and unit tests (fametrano)
  • 35 lines added or modified outside tests
  • 230 lines added or modified in tests
  • 5 lines removed in total
  • 3 files, 4 commits
Wallet#11 of 92
P2 · DoS protection
  • P2 because malformed BDB files can hang wallet migration indefinitely or inflate memory to ~800 MB
  • Protects users against denial-of-service and crashes when parsing corrupted legacy wallet databases
  • Ranking pass: Moved up: hardening the BDB parser is on the migration path every legacy user must cross, grouping it with the other migration fixes.
Ranking notes
Reviewability: ReadyStalePaused
Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets
Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs.
Size: lines added or modified outside tests, then in tests; greener = smaller.

Goal

Hardens the read-only Berkeley DB parser used by wallet migration against crafted or corrupted databases. It tracks visited B-tree and overflow pages to reject multi-parent DAG traversal and infinite loops across empty overflow pages, and verifies that the sum of record lengths on a page does not exceed the page size. It also updates the BDB fuzz target to expect these new errors along with previously unhandled overflow errors.

Problem: Malformed or malicious BDB files passed to `migratewallet` or `bitcoin-wallet` can trigger infinite loops, exponential traversal time, or huge memory inflation. Specifically, a page referenced by multiple parents causes redundant subtree traversal, empty overflow pages pointing to each other bypass length-based loop detection, and multiple index entries pointing to the same record duplicate memory allocation up to hundreds of megabytes.

Evidence of importance:

  • PR description: 'On master an ~8 KB file built this way doesn't finish.'
  • PR description: 'A single 64 KB page can turn into ~500 MB this way. On master a 256 KB file took migratewallet from 54 MB to ~800 MB.'
  • Commit 436ba919bd notes that page level checks stop direct ancestor cycles but small files can still describe exponential work through DAG structures.

Scope: Adds a byte-level mock BDB file generator (`BDBFileBuilder`) in `db_tests.cpp` to construct raw malformed and full B-tree pages for test cases without relying on Berkeley DB.

Risk of a mistake: medium (wallet, wallet_storage). A mistake in BDB page traversal or record validation could cause valid legacy wallets to fail migration.

Changed since the previous assessment: Added commit 2dfce13f9f updating `wallet_bdb_parser.cpp` to expect the new error strings as well as two previously missing overflow errors from #34959 that caused the fuzzer to abort.

Reviewability

Ready

Nothing known that would invalidate a review now.

Waiting on (from the thread read):

  • nothing

Agreement

Strong

substantive support, no open objection (fametrano)

Thread notes: Initial record. fametrano provided a Tested ACK with detailed testing across unit tests and migration. / Initial record. fametrano tested on macOS arm64 and confirmed the hang reproduces on master and is fixed on the branch; no objections raised.

Objections, suggestions, and questions: none recorded.

Support:

  • fametrano (contributor) (Tested ACK, 2026-10-15) [tested: unit tests, wallet_migration.py]: the new rejection tests fail on master (the self-referencing overflow page hangs) and pass here, as do the unit tests, `wallet_migration.py` and migrating a v28.2 legacy wallet with internal and overflow pages

Participants:

  • fametrano (contributor, support, 1 statements 2026-10-15 to 2026-10-15): Tested ACK verifying rejection tests fail on master and pass here, along with wallet migration

Files

230 lines under test/bench/ci.

  • src/wallet/test/db_tests.cpp +224/-0
  • src/wallet/migrate.cpp +35/-5
  • src/wallet/test/fuzz/wallet_bdb_parser.cpp +6/-0

Card

The short text the ranking pass compares across PRs.

This PR hardens the read-only BDB parser used by `migratewallet` and `bitcoin-wallet` against crafted files that bypass earlier checks. It prevents exponential traversal from multi-parent B-tree pages and infinite loops from cyclic empty overflow pages by tracking visited pages, and stops memory blowup by rejecting pages whose total record sizes exceed page bounds. The author demonstrates that an ~8 KB file can hang indefinitely on master and a 256 KB file can inflate memory usage to ~800 MB. The change includes unit tests with hand-crafted raw BDB binary layouts and updates the fuzz target to handle the new error strings.

Data

dossier JSON · extract JSON · model ledger, generated 2026-10-16T01:16, confidence high, input hash 1e208564e71bf3a5
thread read through 2026-10-15 (1 statements), head 2dfce13f9f
labels: Wallet

Categories

Wallet

P2 · DoS protection (ranking pass: Moved up: hardening the BDB parser is on the migration path every legacy user must cross, grouping it with the other migration fixes.)

Prevents infinite loops and severe memory bloat when parsing crafted or corrupt Berkeley DB files during wallet migration. As noted in the description, 'an ~8 KB file built this way doesn't finish' on master, and a 256 KB file inflated memory usage to ~800 MB through duplicated DAG references and looping overflow pages.

Membership: Touches src/wallet/migrate.cpp and carries the Wallet label.

Factors: security/stability 2, bug 2, performance 1, user value 1, leverage 0