wallet, bdbro: Validate btree page levels
What the developer wrote
Authored by Ava Chow
wallet, bdbro: Validate btree page levels
BTree pages contain the level in the BTree that the page is supposed to
be at. The root starts at some level between 1 and 255, leaves are
always level 1. Internal pages must be a level that is one less than its
parent. Validating that pages are at their expected level (except for
the root page) enforces that no cycles can occur.
What changed, and why it matters
This change adds a safety check while reading old Berkeley DB (BDB) wallet files during migration. It verifies that each page in the database's tree structure is at the expected depth level, which prevents malformed or attacker-crafted files from causing infinite loops or confusion during wallet import. The patch is defensive hardening rather than a fix for an active exploit, and it includes a new fuzz-test error path so the security check can be automatically exercised.
Treat as a security-hardening patch for the wallet migration path. Backport to maintained branches that include BDB-to-SQLite migration support, and ensure the updated fuzz target is run in CI.
Security signals we found
Defensive validation of parsed file structure
Cycle prevention in tree traversal
New runtime error thrown on malformed input
Fuzz target updated to cover new error paths
Evidence from the diff
BerkeleyRODatabase::Open() now reads the root page header before the DFS traversal and records each page’s expected BTree level. As it walks internal pages it pushes child pages with expected_level = parent_level - 1, and rejects any page whose header.level does not match. Leaves are additionally required to be level 1. This closes a class of issues where a malicious BDB file could create cycles or place internal/leaf pages at inconsistent depths, potentially causing hangs or incorrect wallet state during BDB-to-SQLite migration. The fuzz harness wallet_bdb_parser is updated to treat the two new error strings as expected.
Changed components
src/wallet/migrate.cppsrc/wallet/test/fuzz/wallet_bdb_parser.cppInspect captured patch +25 / −11
diff --git a/src/wallet/migrate.cpp b/src/wallet/migrate.cpp
index 8cfb74f0..0ca4a70c 100644
--- a/src/wallet/migrate.cpp
+++ b/src/wallet/migrate.cpp
@@ -633,10 +633,17 @@ void BerkeleyRODatabase::Open()
throw std::runtime_error("BDB builtin encryption is not supported");
}
+ // Read the root's level from its header
+ // Note that we will read the root page twice in order to process it.
+ SeekToPage(db_file, inner_meta.root, page_size);
+ PageHeader root_header(inner_meta.root, inner_meta.other_endian);
+ db_file >> root_header;
+
// Do a DFS through the BTree, starting at root
- std::vector<uint32_t> pages{inner_meta.root};
+ // We track the expected level of each page in order to avoid loops
+ std::vector<std::pair<uint32_t, uint32_t>> pages{{inner_meta.root, root_header.level}};
while (pages.size() > 0) {
- uint32_t curr_page = pages.back();
+ auto [curr_page, expected_level] = pages.back();
// It turns out BDB completely ignores this last_page field and doesn't actually update it to the correct
// last page. While we should be checking this, we can't.
// This is left commented out as a reminder to not accidentally implement this in the future.
@@ -647,17 +654,23 @@ void BerkeleyRODatabase::Open()
SeekToPage(db_file, curr_page, page_size);
PageHeader header(curr_page, inner_meta.other_endian);
db_file >> header;
+ if (header.level != expected_level) {
+ throw std::runtime_error("BTree page has an unexpected level");
+ }
switch (header.type) {
case PageType::BTREE_INTERNAL: {
InternalPage int_page(header);
db_file >> int_page;
for (const InternalRecord& rec : int_page.records) {
if (rec.m_header.deleted) continue;
- pages.push_back(rec.page_num);
+ pages.emplace_back(rec.page_num, header.level - 1);
}
break;
}
case PageType::BTREE_LEAF: {
+ if (header.level != 1) {
+ throw std::runtime_error("BTree Leaf page is not at level 1");
+ }
RecordsPage rec_page(header);
db_file >> rec_page;
if (rec_page.records.size() % 2 != 0) {
diff --git a/src/wallet/test/fuzz/wallet_bdb_parser.cpp b/src/wallet/test/fuzz/wallet_bdb_parser.cpp
index f15548d4..37c9869b 100644
--- a/src/wallet/test/fuzz/wallet_bdb_parser.cpp
+++ b/src/wallet/test/fuzz/wallet_bdb_parser.cpp
@@ -74,14 +74,15 @@ FUZZ_TARGET(wallet_bdb_parser, .init = initialize_wallet_bdb_parser)
error.original == "Internal record position not in page" ||
error.original == "LSNs are not reset, this database is not completely flushed. Please reopen then close the database with a version that has BDB support" ||
error.original == "Records page has odd number of records" ||
- error.original == "Bad overflow record page type") {
- // Do nothing
- } else if (error.original == "Subdatabase last page is greater than database last page" ||
- error.original == "Page number is greater than database last page" ||
- error.original == "Last page number could not fit in file" ||
- error.original == "Subdatabase has an unexpected name" ||
- error.original == "Unsupported BDB data file version number" ||
- error.original == "BDB builtin encryption is not supported") {
+ error.original == "Bad overflow record page type" ||
+ error.original == "BTree page has an unexpected level" ||
+ error.original == "BTree Leaf page is not at level 1" ||
+ error.original == "Subdatabase last page is greater than database last page" ||
+ error.original == "Page number is greater than database last page" ||
+ error.original == "Last page number could not fit in file" ||
+ error.original == "Subdatabase has an unexpected name" ||
+ error.original == "Unsupported BDB data file version number" ||
+ error.original == "BDB builtin encryption is not supported") {
} else {
throw std::runtime_error(error.original);
}
Why this scored 59/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.