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

1. 
   shuv-amp commented at 9:15 PM on August 16, 2026: contributor

   The read-only BDB parser in migrate.cpp runs on any file passed to migratewallet (RPC or GUI) or bitcoin-wallet. #34959 added checks for btree levels and overflow lengths, which stop simple cycles, but a crafted file can still make the parser hang or use a lot of memory:

   • A btree page can be the child of more than one internal page. Its level still checks out, so it gets parsed again for every path that reaches it. On master an ~8 KB file built this way doesn't finish.
   • An overflow page with no data doesn't count toward the length #34959 checks, so two such pages pointing at each other loop forever. Separately, any number of records can point at the same overflow chain, and each one reads and stores its own copy.
   • All of a page's index entries can point at the same record, which then gets read and kept once per entry. 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.

   The first two are fixed by tracking visited pages (the approach from #34946): btree pages in one set, and overflow pages in another that covers the whole tree. For the third, a page is rejected once its records add up to more than the page size.

   This shouldn't affect real wallets. In a valid btree every page has a single parent and every overflow page belongs to a single record. Records on a page can't overlap either, since the parser already rejects databases that allow duplicates.

   Each fix comes with a test in db_tests.cpp that builds the bad file by hand, plus a test that a page filled to the last byte still parses. wallet_migration.py passes with v28.2.

   The last commit adds two errors from #34959 to the wallet_bdb_parser fuzz target. They were missing, so the target aborted on inputs that reach them. I ran into this while feeding mutated copies of the test files through the target.

   Supersedes #35992.

2. DrahtBot added the label Wallet on Aug 16, 2026
3. 
   DrahtBot commented at 9:16 PM on August 16, 2026: contributor

   <!--e57a25ab6845829454e8d69fc972939a-->

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

   <!--006a51241073e994b41acfe9ec718e94-->

   External sites

   • Code coverage & benchmarks: https://corecheck.dev/bitcoin/bitcoin/pulls/35990
   • Ralph review <img src=https://review.fish.foo/assets/bitcoin/bitcoin/35990.png alt="Ralph review status"> https://git.fish.foo/bitcoin/bitcoin/pulls/35990#author-ralph

   <!--021abf342d371248e50ceaed478a90ca-->

   Reviews

   See the guideline and AI policy for information on the review process.

   Type	Reviewers
   ACK	fametrano

   If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

   <!--5faf32d7da4f0f540f40219e4f7537a3-->

4. shuv-amp marked this as ready for review on Aug 16, 2026
5. shuv-amp marked this as a draft on Aug 17, 2026
6. shuv-amp force-pushed on Aug 17, 2026
7. shuv-amp renamed this:
   wallet: Reject revisited pages in the read-only BDB parser
   wallet: harden the read-only BDB parser against crafted files
   on Aug 17, 2026
8. shuv-amp marked this as ready for review on Aug 17, 2026
9. shuv-amp force-pushed on Aug 17, 2026
10. sedited requested review from achow101 on Oct 2, 2026
11. wallet, bdbro: Reject revisited btree pages

    Validating btree page levels stops a page from being its own ancestor,
    but a page can still be reachable from more than one parent while
    keeping a consistent level. Such a page, and everything below it, is
    parsed again for every path that reaches it, so a small file can
    describe an exponential amount of work. A valid btree references each
    page once, so keep track of the pages already visited and reject any
    that is seen again.

    436ba919bd
12. wallet, bdbro: Reject revisited overflow pages

    Enforcing overflow data lengths stops a chain whose pages carry data,
    but overflow pages that carry no data never advance the accumulated
    length, so a chain that cycles through empty overflow pages loops
    forever. Any number of records can also point at the same chain, which
    is then read and stored once per record.
    
    In a valid database each overflow page belongs to one record, so keep
    track of the overflow pages visited across the whole tree and reject
    one that is seen again.

    6baaa9c4c4
13. wallet, bdbro: Enforce that a page's records fit in the page

    A page's index entries each point at a record that is read and kept, but
    the offsets are not checked against each other. A page whose entries all
    point at the same record is therefore read into entries * len bytes of
    memory, so a single page can be read into far more than its own size. In
    a valid page the records do not overlap and cannot be larger than the
    page, so track their total size while reading and reject a page whose
    records do not fit.

    c5b11dc9e6
14. fuzz: Add missing BDB overflow errors to wallet_bdb_parser

    #34959 added two errors for invalid overflow records, but did not add
    them to the errors the wallet_bdb_parser target expects, so the target
    aborts on inputs that reach them.

    2dfce13f9f
15. shuv-amp force-pushed on Oct 6, 2026
16. 
    fametrano commented at 4:39 AM on October 15, 2026: contributor

    tACK 2dfce13f9f

    Tested on macOS arm64: 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. Details on request.

Contributors

shuv-ampDrahtBotfametrano

Review Requested
achow101