Browse Source

don't use memset() in privacy/security relevant code parts

As memset() can be optimized out by a compiler it should not be used in
privacy/security relevant code parts. OpenSSL provides the safe
OPENSSL_cleanse() function in crypto.h, which perfectly does the job of
clean and overwrite data.

For details see: http://www.viva64.com/en/b/0178/

- change memset() to OPENSSL_cleanse() where appropriate
- change a hard-coded number from netbase.cpp into a sizeof()
0.8
Philip Kaufmann 12 years ago
parent
commit
0f8a647782
  1. 5
      src/allocators.h
  2. 6
      src/base58.h
  3. 4
      src/crypter.cpp
  4. 4
      src/crypter.h
  5. 2
      src/netbase.cpp
  6. 2
      src/util.cpp

5
src/allocators.h

@ -9,6 +9,7 @@
#include <string> #include <string>
#include <boost/thread/mutex.hpp> #include <boost/thread/mutex.hpp>
#include <map> #include <map>
#include <openssl/crypto.h> // for OPENSSL_cleanse()
#ifdef WIN32 #ifdef WIN32
#ifdef _WIN32_WINNT #ifdef _WIN32_WINNT
@ -212,7 +213,7 @@ struct secure_allocator : public std::allocator<T>
{ {
if (p != NULL) if (p != NULL)
{ {
memset(p, 0, sizeof(T) * n); OPENSSL_cleanse(p, sizeof(T) * n);
LockedPageManager::instance.UnlockRange(p, sizeof(T) * n); LockedPageManager::instance.UnlockRange(p, sizeof(T) * n);
} }
std::allocator<T>::deallocate(p, n); std::allocator<T>::deallocate(p, n);
@ -246,7 +247,7 @@ struct zero_after_free_allocator : public std::allocator<T>
void deallocate(T* p, std::size_t n) void deallocate(T* p, std::size_t n)
{ {
if (p != NULL) if (p != NULL)
memset(p, 0, sizeof(T) * n); OPENSSL_cleanse(p, sizeof(T) * n);
std::allocator<T>::deallocate(p, n); std::allocator<T>::deallocate(p, n);
} }
}; };

6
src/base58.h

@ -17,6 +17,8 @@
#include <string> #include <string>
#include <vector> #include <vector>
#include <openssl/crypto.h> // for OPENSSL_cleanse()
#include "bignum.h" #include "bignum.h"
#include "key.h" #include "key.h"
#include "script.h" #include "script.h"
@ -190,7 +192,7 @@ protected:
{ {
// zero the memory, as it may contain sensitive data // zero the memory, as it may contain sensitive data
if (!vchData.empty()) if (!vchData.empty())
memset(&vchData[0], 0, vchData.size()); OPENSSL_cleanse(&vchData[0], vchData.size());
} }
void SetData(int nVersionIn, const void* pdata, size_t nSize) void SetData(int nVersionIn, const void* pdata, size_t nSize)
@ -221,7 +223,7 @@ public:
vchData.resize(vchTemp.size() - 1); vchData.resize(vchTemp.size() - 1);
if (!vchData.empty()) if (!vchData.empty())
memcpy(&vchData[0], &vchTemp[1], vchData.size()); memcpy(&vchData[0], &vchTemp[1], vchData.size());
memset(&vchTemp[0], 0, vchTemp.size()); OPENSSL_cleanse(&vchTemp[0], vchData.size());
return true; return true;
} }

4
src/crypter.cpp

@ -24,8 +24,8 @@ bool CCrypter::SetKeyFromPassphrase(const SecureString& strKeyData, const std::v
if (i != (int)WALLET_CRYPTO_KEY_SIZE) if (i != (int)WALLET_CRYPTO_KEY_SIZE)
{ {
memset(&chKey, 0, sizeof chKey); OPENSSL_cleanse(chKey, sizeof(chKey));
memset(&chIV, 0, sizeof chIV); OPENSSL_cleanse(chIV, sizeof(chIV));
return false; return false;
} }

4
src/crypter.h

@ -76,8 +76,8 @@ public:
void CleanKey() void CleanKey()
{ {
memset(&chKey, 0, sizeof chKey); OPENSSL_cleanse(chKey, sizeof(chKey));
memset(&chIV, 0, sizeof chIV); OPENSSL_cleanse(chIV, sizeof(chIV));
fKeySet = false; fKeySet = false;
} }

2
src/netbase.cpp

@ -545,7 +545,7 @@ bool ConnectSocketByName(CService &addr, SOCKET& hSocketRet, const char *pszDest
void CNetAddr::Init() void CNetAddr::Init()
{ {
memset(ip, 0, 16); memset(ip, 0, sizeof(ip));
} }
void CNetAddr::SetIP(const CNetAddr& ipIn) void CNetAddr::SetIP(const CNetAddr& ipIn)

2
src/util.cpp

@ -156,7 +156,7 @@ void RandAddSeedPerfmon()
if (ret == ERROR_SUCCESS) if (ret == ERROR_SUCCESS)
{ {
RAND_add(pdata, nSize, nSize/100.0); RAND_add(pdata, nSize, nSize/100.0);
memset(pdata, 0, nSize); OPENSSL_cleanse(pdata, nSize);
printf("RandAddSeed() %lu bytes\n", nSize); printf("RandAddSeed() %lu bytes\n", nSize);
} }
#endif #endif

Loading…
Cancel
Save