Avoid opening copied wallet databases simultaneously

ryanofsky · ryanofsky · commit e32c2a51b757 · 2017-10-10T15:27:26.000-04:00
Make sure wallet databases have unique fileids. If they don't, throw an error. BDB caches do not work properly when more than one open database has the same fileid, because values written to one database may show up in reads to other databases. Bitcoin will never create different databases with the same fileid, but users can create them by manually copying database files. BDB caching bug was reported by Chris Moore <dooglus@gmail.com> #11429 Fixes #11429
diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp
@@ -401,12 +401,48 @@ CDB::CDB(CWalletDBWrapper& dbw, const char* pszMode, bool fFlushOnCloseIn) : pdb
                             nFlags,                             // Flags
                             0);
 
+            std::string error;
             if (ret != 0) {
+                error = strprintf("CDB: Error %d, can't open database %s", ret, strFilename);
+            }
+
+            u_int8_t fileid[DB_FILE_ID_LEN];
+            if (error.empty()) {
+                ret = pdb->get_mpf()->get_fileid(fileid);
+                if (ret != 0) {
+                    error = strprintf("CDB: Can't open database %s (get_fileid failed with %d)", strFilename, ret);
+                }
+            }
+
+            if (error.empty()) {
+                // Make sure database has a unique fileid. If it doesn't, throw
+                // an error. BDB caches do not work properly when more than one
+                // open database has the same fileid (values written to one
+                // database may show up in reads to other databases).
+                //
+                // Bitcoin will never create different databases with the same
+                // fileid, but this error can be triggered if users manually
+                // copy database files.
+                for (const auto& item : env->mapDb) {
+                    u_int8_t item_fileid[DB_FILE_ID_LEN];
+                    if (item.second && item.second->get_mpf()->get_fileid(item_fileid) == 0 &&
+                        memcmp(fileid, item_fileid, sizeof(fileid)) == 0) {
+                        const char* item_filename = nullptr;
+                        item.second->get_dbname(&item_filename, nullptr);
+                        error = strprintf("CDB: Can't open database %s (duplicates fileid %s from %s)", strFilename,
+                            HexStr(std::begin(item_fileid), std::end(item_fileid)),
+                            item_filename ? item_filename : "(unknown database)");
+                        break;
+                    }
+                }
+            }
+
+            if (!error.empty()) {
                 delete pdb;
                 pdb = nullptr;
                 --env->mapFileUseCount[strFile];
                 strFile = "";
-                throw std::runtime_error(strprintf("CDB: Error %d, can't open database %s", ret, strFilename));
+                throw std::runtime_error(error);
             }
 
             if (fCreate && !Exists(std::string("version"))) {
diff --git a/test/functional/multiwallet.py b/test/functional/multiwallet.py
@@ -7,6 +7,7 @@
 Verify that a bitcoind node can load multiple wallet files
 """
 import os
+import shutil
 
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import assert_equal, assert_raises_jsonrpc
@@ -29,6 +30,11 @@ def run_test(self):
         os.mkdir(os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w11'))
         self.assert_start_raises_init_error(0, ['-wallet=w11'], 'Error loading wallet w11. -wallet filename must be a regular file.')
 
+        # should not initialize if one wallet is a copy of another
+        shutil.copyfile(os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w2'),
+                        os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w22'))
+        self.assert_start_raises_init_error(0, ['-wallet=w2', '-wallet=w22'], 'duplicates fileid')
+
         # should not initialize if wallet file is a symlink
         os.symlink(os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w1'), os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w12'))
         self.assert_start_raises_init_error(0, ['-wallet=w12'], 'Error loading wallet w12. -wallet filename must be a regular file.')
