From f606bb9bafafb12bcf9bc0834125c884da97f9e1 Mon Sep 17 00:00:00 2001 From: Philip Kaufmann Date: Sun, 28 Sep 2014 16:09:19 +0200 Subject: [PATCH 1/3] fix a possible memory leak in CWalletDB::Recover - convert pdbCopy into a boost::scoped_ptr to ensure memory gets freed in all cases (e.g. after "ret > 0") --- src/walletdb.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/walletdb.cpp b/src/walletdb.cpp index a84f44db0..e13830a8f 100644 --- a/src/walletdb.cpp +++ b/src/walletdb.cpp @@ -15,11 +15,11 @@ #include #include +#include #include -using namespace std; using namespace boost; - +using namespace std; static uint64_t nAccountingEntryNumber = 0; @@ -926,7 +926,7 @@ bool CWalletDB::Recover(CDBEnv& dbenv, std::string filename, bool fOnlyKeys) LogPrintf("Salvage(aggressive) found %u records\n", salvagedData.size()); bool fSuccess = allOK; - Db* pdbCopy = new Db(&dbenv.dbenv, 0); + boost::scoped_ptr pdbCopy(new Db(&dbenv.dbenv, 0)); int ret = pdbCopy->open(NULL, // Txn pointer filename.c_str(), // Filename "main", // Logical db name @@ -967,7 +967,6 @@ bool CWalletDB::Recover(CDBEnv& dbenv, std::string filename, bool fOnlyKeys) } ptxn->commit(0); pdbCopy->close(0); - delete pdbCopy; return fSuccess; } From 870da77da632501e8eec58ed73e8f30549cc41e9 Mon Sep 17 00:00:00 2001 From: Philip Kaufmann Date: Sun, 28 Sep 2014 16:11:17 +0200 Subject: [PATCH 2/3] fix possible memory leaks in CWallet::EncryptWallet - add missing deletes for pwalletdbEncryption - add an assert before trying to reserve memory for pwalletdbEncryption - add a destructor to CWallet, which ensures deletion of pwalletdbEncryption on object destruction --- src/wallet.cpp | 24 ++++++++++++++++++------ src/wallet.h | 8 ++++++++ 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/src/wallet.cpp b/src/wallet.cpp index e69f59aac..897f53b8b 100644 --- a/src/wallet.cpp +++ b/src/wallet.cpp @@ -426,17 +426,25 @@ bool CWallet::EncryptWallet(const SecureString& strWalletPassphrase) mapMasterKeys[++nMasterKeyMaxID] = kMasterKey; if (fFileBacked) { + assert(!pwalletdbEncryption); pwalletdbEncryption = new CWalletDB(strWalletFile); - if (!pwalletdbEncryption->TxnBegin()) + if (!pwalletdbEncryption->TxnBegin()) { + delete pwalletdbEncryption; + pwalletdbEncryption = NULL; return false; + } pwalletdbEncryption->WriteMasterKey(nMasterKeyMaxID, kMasterKey); } if (!EncryptKeys(vMasterKey)) { - if (fFileBacked) + if (fFileBacked) { pwalletdbEncryption->TxnAbort(); - exit(1); //We now probably have half of our keys encrypted in memory, and half not...die and let the user reload their unencrypted wallet. + delete pwalletdbEncryption; + } + // We now probably have half of our keys encrypted in memory, and half not... + // die and let the user reload their unencrypted wallet. + exit(1); } // Encryption was introduced in version 0.4.0 @@ -444,8 +452,12 @@ bool CWallet::EncryptWallet(const SecureString& strWalletPassphrase) if (fFileBacked) { - if (!pwalletdbEncryption->TxnCommit()) - exit(1); //We now have keys encrypted in memory, but no on disk...die to avoid confusion and let the user reload their unencrypted wallet. + if (!pwalletdbEncryption->TxnCommit()) { + delete pwalletdbEncryption; + // We now have keys encrypted in memory, but no on disk... + // die to avoid confusion and let the user reload their unencrypted wallet. + exit(1); + } delete pwalletdbEncryption; pwalletdbEncryption = NULL; @@ -1068,7 +1080,7 @@ int64_t CWallet::GetWatchOnlyBalance() const nTotal += pcoin->GetAvailableWatchOnlyCredit(); } } - + return nTotal; } diff --git a/src/wallet.h b/src/wallet.h index fde87a8a2..344f9c0e0 100644 --- a/src/wallet.h +++ b/src/wallet.h @@ -143,6 +143,7 @@ public: { SetNull(); } + CWallet(std::string strWalletFileIn) { SetNull(); @@ -150,6 +151,13 @@ public: strWalletFile = strWalletFileIn; fFileBacked = true; } + + ~CWallet() + { + delete pwalletdbEncryption; + pwalletdbEncryption = NULL; + } + void SetNull() { nWalletVersion = FEATURE_BASE; From d0c4197ef6ecfdaff792579810107e2f1b8b319e Mon Sep 17 00:00:00 2001 From: Philip Kaufmann Date: Wed, 1 Oct 2014 08:50:24 +0200 Subject: [PATCH 3/3] change exit(1) to an assert in CWallet::EncryptWallet --- src/wallet.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/wallet.cpp b/src/wallet.cpp index 897f53b8b..be063ccb4 100644 --- a/src/wallet.cpp +++ b/src/wallet.cpp @@ -15,6 +15,8 @@ #include "util.h" #include "utilmoneystr.h" +#include + #include #include @@ -444,7 +446,7 @@ bool CWallet::EncryptWallet(const SecureString& strWalletPassphrase) } // We now probably have half of our keys encrypted in memory, and half not... // die and let the user reload their unencrypted wallet. - exit(1); + assert(false); } // Encryption was introduced in version 0.4.0 @@ -456,7 +458,7 @@ bool CWallet::EncryptWallet(const SecureString& strWalletPassphrase) delete pwalletdbEncryption; // We now have keys encrypted in memory, but no on disk... // die to avoid confusion and let the user reload their unencrypted wallet. - exit(1); + assert(false); } delete pwalletdbEncryption;