What the developer wrote
Authored by Lőrinc
wallet: check BDB last page LSN
The BDB metadata field `last_page` stores the last valid page number, not the number of pages.
The read-only wallet migration parser currently checks reset LSNs with a half-open loop, so it skips the final page and may accept a database whose last page still depends on BDB log files.
What changed, and why it matters
This commit fixes an off-by-one bug in Bitcoin Core's read-only wallet migration tool. The tool checks whether a Berkeley DB (BDB) wallet file is self-contained before migrating it. Due to a half-open loop, the tool was skipping the very last page of the database when verifying that all pages had their log sequence numbers (LSNs) reset. As a result, it could have accepted a wallet whose final page still depended on separate BDB log files, potentially leading to incomplete or inconsistent migration data. The fix changes the loop to include the last page.
Apply the patch. After applying, ensure any wallets previously migrated with the buggy code are considered for re-verification or re-migration, especially if migration occurred without BDB log files present. Users should be advised to migrate wallets only when associated BDB log files are available or to verify migrated wallet integrity.
Security signals we found
Off-by-one loop boundary in security-critical validation
Incomplete verification of database self-consistency before migration
Potential acceptance of database state dependent on external log files
Data integrity risk in wallet migration path
Evidence from the diff
In src/wallet/migrate.cpp, BerkeleyRODatabase::Open() validates that every database page has an LSN pointing to file 0, offset 1, which indicates the LSNs were reset and the database no longer needs BDB log files. The original loop used i < outer_meta.last_page, treating last_page as a page count. However, last_page is the last valid page number (zero-based index), so the loop omitted the final page. The patch changes the condition to i <= outer_meta.last_page, ensuring the final page’s LSN is also verified. A wallet whose last page retains an unreset LSN could be migrated without its most recent transactional state, risking data inconsistency.
Changed components
src/wallet/migrate.cppBerkeleyRODatabase::Open()BDB read-only wallet migration parserInspect captured patch +1 / −1
diff --git a/src/wallet/migrate.cpp b/src/wallet/migrate.cpp
index 1869a456..f42aa2be 100644
--- a/src/wallet/migrate.cpp
+++ b/src/wallet/migrate.cpp
@@ -567,7 +567,7 @@ void BerkeleyRODatabase::Open()
// Check all Log Sequence Numbers (LSN) point to file 0 and offset 1 which indicates that the LSNs were
// reset and that the log files are not necessary to get all of the data in the database.
- for (uint32_t i = 0; i < outer_meta.last_page; ++i) {
+ for (uint32_t i = 0; i <= outer_meta.last_page; ++i) {
// The LSN is composed of 2 32-bit ints, the first is a file id, the second an offset
// It will always be the first 8 bytes of a page, so we deserialize it directly for every page
uint32_t file;
Why this scored 57/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.