diff --git a/src/bench/base58.cpp b/src/bench/base58.cpp index 489fc9825..28acd47a4 100644 --- a/src/bench/base58.cpp +++ b/src/bench/base58.cpp @@ -22,7 +22,7 @@ static void Base58Encode(benchmark::State& state) } }; while (state.KeepRunning()) { - EncodeBase58(buff.begin(), buff.end()); + EncodeBase58(buff.data(), buff.data() + buff.size()); } } diff --git a/src/bench/checkqueue.cpp b/src/bench/checkqueue.cpp index c4aacb8a7..596d66870 100644 --- a/src/bench/checkqueue.cpp +++ b/src/bench/checkqueue.cpp @@ -19,7 +19,7 @@ static const int MIN_CORES = 2; static const size_t BATCHES = 101; static const size_t BATCH_SIZE = 30; static const int PREVECTOR_SIZE = 28; -static const int QUEUE_BATCH_SIZE = 128; +static const unsigned int QUEUE_BATCH_SIZE = 128; static void CCheckQueueSpeed(benchmark::State& state) { struct FakeJobNoWork { diff --git a/src/compat.h b/src/compat.h index b8c238f22..4d6027c67 100644 --- a/src/compat.h +++ b/src/compat.h @@ -31,6 +31,7 @@ #include #include #include +#include #else #include #include @@ -72,6 +73,15 @@ typedef u_int SOCKET; #else #define MAX_PATH 1024 #endif +#ifdef _MSC_VER +#if !defined(ssize_t) +#ifdef _WIN64 +typedef int64_t ssize_t; +#else +typedef int32_t ssize_t; +#endif +#endif +#endif // As Solaris does not have the MSG_NOSIGNAL flag for send(2) syscall, it is defined as 0 #if !defined(HAVE_MSG_NOSIGNAL) && !defined(MSG_NOSIGNAL) diff --git a/src/support/cleanse.cpp b/src/support/cleanse.cpp index 7a4da12b4..9fa2fc182 100644 --- a/src/support/cleanse.cpp +++ b/src/support/cleanse.cpp @@ -5,9 +5,34 @@ #include "cleanse.h" -#include +#include + +#if defined(_MSC_VER) +#include // For SecureZeroMemory. +#endif void memory_cleanse(void *ptr, size_t len) { - OPENSSL_cleanse(ptr, len); +#if defined(_MSC_VER) + /* SecureZeroMemory is guaranteed not to be optimized out by MSVC. */ + SecureZeroMemory(ptr, len); +#else + std::memset(ptr, 0, len); + + /* In order to prevent the compiler from optimizing out the memset, this uses an + * unremovable (see https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#Volatile ) + * asm block that the compiler must assume could access arbitary memory, including + * the zero bytes written by std::memset. + * + * Quoting Adam Langley in commit ad1907fe73334d6c696c8539646c21b11178f20f + * in BoringSSL (ISC License): + * As best as we can tell, this is sufficient to break any optimisations that + * might try to eliminate "superfluous" memsets. + * This method is used by memzero_explicit() in the Linux kernel, too. Its advantage is that it + * is pretty efficient because the compiler can still implement the memset() efficiently, + * just not remove it entirely. See "Dead Store Elimination (Still) Considered Harmful" by + * Yang et al. (USENIX Security 2017) for more background. + */ + __asm__ __volatile__("" : : "r"(ptr) : "memory"); +#endif } diff --git a/src/support/cleanse.h b/src/support/cleanse.h index c6fe12912..1b2bdc13e 100644 --- a/src/support/cleanse.h +++ b/src/support/cleanse.h @@ -8,6 +8,8 @@ #include +/** Secure overwrite a buffer (possibly containing secret data) with zero-bytes. The write + * operation will not be optimized out by the compiler. */ void memory_cleanse(void *ptr, size_t len); #endif // BITCOIN_SUPPORT_CLEANSE_H diff --git a/src/wallet/db.h b/src/wallet/db.h index 608723593..5fd512024 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -17,6 +17,7 @@ #include #include +#include #include @@ -128,21 +129,22 @@ protected: 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() == NULL) - return false; + memory_cleanse(datKey.get_data(), datKey.get_size()); + if (datValue.get_data() != NULL) { + BOOST_SCOPE_EXIT_TPL(&datValue) { + // Clear and free memory + memory_cleanse(datValue.get_data(), datValue.get_size()); + free(datValue.get_data()); + } BOOST_SCOPE_EXIT_END - // Unserialize value - try { - CDataStream ssValue((char*)datValue.get_data(), (char*)datValue.get_data() + datValue.get_size(), SER_DISK, CLIENT_VERSION); - ssValue >> value; - } catch (const std::exception&) { - 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 (const std::exception&) { + return false; + } } - - // Clear and free memory - memset(datValue.get_data(), 0, datValue.get_size()); - free(datValue.get_data()); return (ret == 0); } @@ -170,8 +172,8 @@ protected: 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); } @@ -193,7 +195,7 @@ protected: 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); } @@ -213,7 +215,7 @@ protected: 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); } @@ -258,8 +260,8 @@ protected: 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;