[Pkg-bitcoin-commits] [bitcoin] 80/126: Avoid opening copied wallet databases simultaneously

[Jonas Smedegaard]

This is an automated email from the git hooks/post-receive script.

js pushed a commit to annotated tag debian/0.15.1_dfsg-1
in repository bitcoin.

commit 9c8006dc332139069d798f39406ec67186ecb311
Author: Russell Yanofsky <russ at yanofsky.org>
Date:   Tue Oct 10 15:27:26 2017 -0400

    Avoid opening copied wallet databases simultaneously
    
    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 at gmail.com>
    https://github.com/bitcoin/bitcoin/issues/11429
    
    Fixes #11429
    
    Github-Pull: #11476
    Rebased-From: 478a89c1ef79a75275d1b508122c06eee9386b2d
---
 src/wallet/db.cpp              | 35 +++++++++++++++++++++++++++++++++++
 test/functional/multiwallet.py |  6 ++++++
 2 files changed, 41 insertions(+)

diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp
index f5f9676..fb6e576 100644
--- a/src/wallet/db.cpp
+++ b/src/wallet/db.cpp
@@ -20,6 +20,40 @@
 
 #include <boost/thread.hpp>
 
+namespace {
+//! Make sure database has a unique fileid within the environment. 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).
+//!
+//! BerkeleyDB generates unique fileids by default
+//! (https://docs.oracle.com/cd/E17275_01/html/programmer_reference/program_copy.html),
+//! so bitcoin should never create different databases with the same fileid, but
+//! this error can be triggered if users manually copy database files.
+void CheckUniqueFileid(const CDBEnv& env, const std::string& filename, Db& db)
+{
+    if (env.IsMock()) return;
+
+    u_int8_t fileid[DB_FILE_ID_LEN];
+    int ret = db.get_mpf()->get_fileid(fileid);
+    if (ret != 0) {
+        throw std::runtime_error(strprintf("CDB: Can't open database %s (get_fileid failed with %d)", filename, ret));
+    }
+
+    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);
+            throw std::runtime_error(strprintf("CDB: Can't open database %s (duplicates fileid %s from %s)", filename,
+                HexStr(std::begin(item_fileid), std::end(item_fileid)),
+                item_filename ? item_filename : "(unknown database)"));
+        }
+    }
+}
+} // namespace
+
 //
 // CDB
 //
@@ -403,6 +437,7 @@ CDB::CDB(CWalletDBWrapper& dbw, const char* pszMode, bool fFlushOnCloseIn) : pdb
             if (ret != 0) {
                 throw std::runtime_error(strprintf("CDB: Error %d, can't open database %s", ret, strFilename));
             }
+            CheckUniqueFileid(*env, strFilename, *pdb_temp);
 
             pdb = pdb_temp.release();
             env->mapDb[strFilename] = pdb;
diff --git a/test/functional/multiwallet.py b/test/functional/multiwallet.py
index 6adcc1f..ba6b659 100755
--- 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_rpc_error
@@ -29,6 +30,11 @@ class MultiWalletTest(BitcoinTestFramework):
         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.')

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-bitcoin/bitcoin.git