Skip to content

Commit

Permalink
Merge pull request #2221 from barton2526/cherrypicks
Browse files Browse the repository at this point in the history 
wallet: Close DB on error, use memory_cleanse
  • Loading branch information
@jamescowens
jamescowens committed Aug 2, 2021
2 parents f9b5b13 + 5072082 commit b61fad7
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 34 deletions.
4 changes: 2 additions & 2 deletions src/base58.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ class CBase58Data
{
// zero the memory, as it may contain sensitive data
if (!vchData.empty())
memset(&vchData[0], 0, vchData.size());
memory_cleanse(&vchData[0], vchData.size());
}

void SetData(int nVersionIn, const void* pdata, size_t nSize)
Expand Down Expand Up @@ -222,7 +222,7 @@ class CBase58Data
vchData.resize(vchTemp.size() - 1);
if (!vchData.empty())
memcpy(&vchData[0], &vchTemp[1], vchData.size());
memset(&vchTemp[0], 0, vchTemp.size());
memory_cleanse(&vchTemp[0], vchTemp.size());
return true;
}

Expand Down
6 changes: 3 additions & 3 deletions src/util.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ void RandAddSeed()
// Seed with CPU performance counter
int64_t nCounter = GetPerformanceCounter();
RAND_add(&nCounter, sizeof(nCounter), 1.5);
memset(&nCounter, 0, sizeof(nCounter));
memory_cleanse(&nCounter, sizeof(nCounter));
}

void RandAddSeedPerfmon()
Expand All @@ -128,14 +128,14 @@ void RandAddSeedPerfmon()
// Don't need this on Linux, OpenSSL automatically uses /dev/urandom
// Seed with the entire set of perfmon data
unsigned char pdata[250000];
memset(pdata, 0, sizeof(pdata));
memory_cleanse(pdata, sizeof(pdata));
unsigned long nSize = sizeof(pdata);
long ret = RegQueryValueExA(HKEY_PERFORMANCE_DATA, "Global", nullptr, nullptr, pdata, &nSize);
RegCloseKey(HKEY_PERFORMANCE_DATA);
if (ret == ERROR_SUCCESS)
{
RAND_add(pdata, nSize, nSize/100.0);
memset(pdata, 0, nSize);
memory_cleanse(pdata, nSize);
LogPrint(BCLog::LogFlags::NOISY, "rand", "RandAddSeed() %lu bytes", nSize);
}
#endif
Expand Down
15 changes: 9 additions & 6 deletions src/wallet/db.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,10 @@ bool CDBEnv::Open(fs::path pathEnv_)
DB_RECOVER |
nEnvFlags,
S_IRUSR | S_IWUSR);
if (ret != 0)
if (ret != 0) {
dbenv.close(0);
return error("CDB() : error %s (%d) opening database environment", DbEnv::strerror(ret), ret);
}

fDbEnvInit = true;
fMockDb = false;
Expand Down Expand Up @@ -376,9 +378,9 @@ bool CDB::Rewrite(const string& strFile, const char* pszSkip)
DB_BTREE, // Database type
DB_CREATE, // Flags
0);
if (ret > 0)
{
if (ret > 0) {
LogPrintf("Cannot create database file %s", strFileRes);
pdbCopy->close(0);
fSuccess = false;
}

Expand Down Expand Up @@ -415,14 +417,15 @@ bool CDB::Rewrite(const string& strFile, const char* pszSkip)
if (ret2 > 0)
fSuccess = false;
}
if (fSuccess)
{
if (fSuccess) {
db.Close();
bitdb.CloseDb(strFile);
if (pdbCopy->close(0))
fSuccess = false;
delete pdbCopy;
} else {
pdbCopy->close(0);
}
delete pdbCopy;
}
if (fSuccess)
{
Expand Down
45 changes: 22 additions & 23 deletions src/wallet/db.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,24 +127,23 @@ class CDB
Dbt datValue;
datValue.set_flags(DB_DBT_MALLOC);
int ret = pdb->get(activeTxn, &datKey, &datValue, 0);
memset(datKey.get_data(), 0, datKey.get_size());
if (datValue.get_data() == nullptr) {
return false;
}

// Unserialize value
try {
CDataStream ssValue((char*)datValue.get_data(), (char*)datValue.get_data() + datValue.get_size(), SER_DISK, CLIENT_VERSION);
ssValue >> value;
}
catch (std::exception &e) {
return false;
memory_cleanse(datKey.get_data(), datKey.get_size());
bool success = false;
if (datValue.get_data() != nullptr) {
// Unserialize value
try {
CDataStream ssValue((char*)datValue.get_data(), (char*)datValue.get_data() + datValue.get_size(), SER_DISK, CLIENT_VERSION);
ssValue >> value;
success = true;
} catch (const std::exception&) {
// In this case success remains 'false'
}

// Clear and free memory
memory_cleanse(datValue.get_data(), datValue.get_size());
free(datValue.get_data());
}

// Clear and free memory
memset(datValue.get_data(), 0, datValue.get_size());
free(datValue.get_data());
return (ret == 0);
return ret == 0 && success;
}

template<typename K, typename T>
Expand All @@ -171,8 +170,8 @@ class CDB
int ret = pdb->put(activeTxn, &datKey, &datValue, (fOverwrite ? 0 : DB_NOOVERWRITE));

// Clear memory in case it was a private key
memset(datKey.get_data(), 0, datKey.get_size());
memset(datValue.get_data(), 0, datValue.get_size());
memory_cleanse(datKey.get_data(), datKey.get_size());
memory_cleanse(datValue.get_data(), datValue.get_size());
return (ret == 0);
}

Expand All @@ -194,7 +193,7 @@ class CDB
int ret = pdb->del(activeTxn, &datKey, 0);

// Clear memory
memset(datKey.get_data(), 0, datKey.get_size());
memory_cleanse(datKey.get_data(), datKey.get_size());
return (ret == 0 || ret == DB_NOTFOUND);
}

Expand All @@ -214,7 +213,7 @@ class CDB
int ret = pdb->exists(activeTxn, &datKey, 0);

// Clear memory
memset(datKey.get_data(), 0, datKey.get_size());
memory_cleanse(datKey.get_data(), datKey.get_size());
return (ret == 0);
}

Expand Down Expand Up @@ -264,8 +263,8 @@ class CDB
ssValue.write((char*)datValue.get_data(), datValue.get_size());

// Clear and free memory
memset(datKey.get_data(), 0, datKey.get_size());
memset(datValue.get_data(), 0, datValue.get_size());
memory_cleanse(datKey.get_data(), datKey.get_size());
memory_cleanse(datValue.get_data(), datValue.get_size());
free(datKey.get_data());
free(datValue.get_data());
return 0;
Expand Down

0 comments on commit b61fad7

Please sign in to comment.