From 843c560003b288a073f528ef16b83541f7a9b0ac Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 10 Jan 2017 14:14:01 -0800 Subject: [PATCH 1/6] Avoid unaligned access in crypto i/o --- src/crypto/common.h | 36 ++++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/src/crypto/common.h b/src/crypto/common.h index 580c72f5a..4a9d1150b 100644 --- a/src/crypto/common.h +++ b/src/crypto/common.h @@ -10,57 +10,73 @@ #endif #include +#include #include "compat/endian.h" uint16_t static inline ReadLE16(const unsigned char* ptr) { - return le16toh(*((uint16_t*)ptr)); + uint16_t x; + memcpy((char*)&x, ptr, 2); + return le16toh(x); } uint32_t static inline ReadLE32(const unsigned char* ptr) { - return le32toh(*((uint32_t*)ptr)); + uint32_t x; + memcpy((char*)&x, ptr, 4); + return le32toh(x); } uint64_t static inline ReadLE64(const unsigned char* ptr) { - return le64toh(*((uint64_t*)ptr)); + uint64_t x; + memcpy((char*)&x, ptr, 8); + return le64toh(x); } void static inline WriteLE16(unsigned char* ptr, uint16_t x) { - *((uint16_t*)ptr) = htole16(x); + uint16_t v = htole16(x); + memcpy(ptr, (char*)&v, 2); } void static inline WriteLE32(unsigned char* ptr, uint32_t x) { - *((uint32_t*)ptr) = htole32(x); + uint32_t v = htole32(x); + memcpy(ptr, (char*)&v, 4); } void static inline WriteLE64(unsigned char* ptr, uint64_t x) { - *((uint64_t*)ptr) = htole64(x); + uint64_t v = htole64(x); + memcpy(ptr, (char*)&v, 8); } uint32_t static inline ReadBE32(const unsigned char* ptr) { - return be32toh(*((uint32_t*)ptr)); + uint32_t x; + memcpy((char*)&x, ptr, 4); + return be32toh(x); } uint64_t static inline ReadBE64(const unsigned char* ptr) { - return be64toh(*((uint64_t*)ptr)); + uint64_t x; + memcpy((char*)&x, ptr, 8); + return be64toh(x); } void static inline WriteBE32(unsigned char* ptr, uint32_t x) { - *((uint32_t*)ptr) = htobe32(x); + uint32_t v = htobe32(x); + memcpy(ptr, (char*)&v, 4); } void static inline WriteBE64(unsigned char* ptr, uint64_t x) { - *((uint64_t*)ptr) = htobe64(x); + uint64_t v = htobe64(x); + memcpy(ptr, (char*)&v, 8); } #endif // BITCOIN_CRYPTO_COMMON_H From f94f3e0df82cd9f1202b0741c6fcc83b3560cee8 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 10 Jan 2017 14:34:20 -0800 Subject: [PATCH 2/6] Avoid integer overflows in scriptnum tests --- src/test/scriptnum_tests.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/test/scriptnum_tests.cpp b/src/test/scriptnum_tests.cpp index 6b6689c7d..1d5893bdc 100644 --- a/src/test/scriptnum_tests.cpp +++ b/src/test/scriptnum_tests.cpp @@ -12,8 +12,10 @@ BOOST_FIXTURE_TEST_SUITE(scriptnum_tests, BasicTestingSetup) -static const int64_t values[] = \ -{ 0, 1, CHAR_MIN, CHAR_MAX, UCHAR_MAX, SHRT_MIN, USHRT_MAX, INT_MIN, INT_MAX, UINT_MAX, LONG_MIN, LONG_MAX }; +/** A selection of numbers that do not trigger int64_t overflow + * when added/subtracted. */ +static const int64_t values[] = { 0, 1, -2, 127, 128, -255, 256, (1LL << 15) - 1, -(1LL << 16), (1LL << 24) - 1, (1LL << 31), 1 - (1LL << 32), 1LL << 40 }; + static const int64_t offsets[] = { 1, 0x79, 0x80, 0x81, 0xFF, 0x7FFF, 0x8000, 0xFFFF, 0x10000}; static bool verify(const CScriptNum10& bignum, const CScriptNum& scriptnum) From 6b03bfb840316d5bd96fd22af065a3998abdbefc Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 10 Jan 2017 14:46:07 -0800 Subject: [PATCH 3/6] Fix memory leak in wallet tests --- src/wallet/test/wallet_tests.cpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index c7ce6fd24..ca086c86a 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -23,6 +23,8 @@ using namespace std; +std::vector> wtxn; + typedef set > CoinSet; BOOST_FIXTURE_TEST_SUITE(wallet_tests, WalletTestingSetup) @@ -42,21 +44,21 @@ static void add_coin(const CAmount& nValue, int nAge = 6*24, bool fIsFromMe = fa // so stop vin being empty, and cache a non-zero Debit to fake out IsFromMe() tx.vin.resize(1); } - CWalletTx* wtx = new CWalletTx(&wallet, MakeTransactionRef(std::move(tx))); + std::unique_ptr wtx(new CWalletTx(&wallet, MakeTransactionRef(std::move(tx)))); if (fIsFromMe) { wtx->fDebitCached = true; wtx->nDebitCached = 1; } - COutput output(wtx, nInput, nAge, true, true); + COutput output(wtx.get(), nInput, nAge, true, true); vCoins.push_back(output); + wtxn.emplace_back(std::move(wtx)); } static void empty_wallet(void) { - BOOST_FOREACH(COutput output, vCoins) - delete output.tx; vCoins.clear(); + wtxn.clear(); } static bool equal_sets(CoinSet a, CoinSet b) @@ -349,6 +351,8 @@ BOOST_AUTO_TEST_CASE(ApproximateBestSubset) BOOST_CHECK(wallet.SelectCoinsMinConf(1003 * COIN, 1, 6, 0, vCoins, setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 1003 * COIN); BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); + + empty_wallet(); } BOOST_AUTO_TEST_SUITE_END() From 5a0b7e4106bc97a7a67bda6bf6fbd7f26d892420 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 10 Jan 2017 14:47:04 -0800 Subject: [PATCH 4/6] Fix memory leak in net_tests --- src/test/net_tests.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index 0bd7869f3..b9ed4952b 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -162,12 +162,12 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test) bool fInboundIn = false; // Test that fFeeler is false by default. - CNode* pnode1 = new CNode(id++, NODE_NETWORK, height, hSocket, addr, 0, 0, pszDest, fInboundIn); + std::unique_ptr pnode1(new CNode(id++, NODE_NETWORK, height, hSocket, addr, 0, 0, pszDest, fInboundIn)); BOOST_CHECK(pnode1->fInbound == false); BOOST_CHECK(pnode1->fFeeler == false); fInboundIn = true; - CNode* pnode2 = new CNode(id++, NODE_NETWORK, height, hSocket, addr, 1, 1, pszDest, fInboundIn); + std::unique_ptr pnode2(new CNode(id++, NODE_NETWORK, height, hSocket, addr, 1, 1, pszDest, fInboundIn)); BOOST_CHECK(pnode2->fInbound == true); BOOST_CHECK(pnode2->fFeeler == false); } From 99f001eb52dda703bd326833430054b108de35a1 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 10 Jan 2017 15:05:35 -0800 Subject: [PATCH 5/6] Fix memory leak in multiUserAuthorized --- src/httprpc.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/httprpc.cpp b/src/httprpc.cpp index 742ff2b8f..93beda09a 100644 --- a/src/httprpc.cpp +++ b/src/httprpc.cpp @@ -113,8 +113,8 @@ static bool multiUserAuthorized(std::string strUserPass) std::string strHash = vFields[2]; unsigned int KEY_SIZE = 32; - unsigned char *out = new unsigned char[KEY_SIZE]; - + unsigned char out[KEY_SIZE]; + CHMAC_SHA256(reinterpret_cast(strSalt.c_str()), strSalt.size()).Write(reinterpret_cast(strPass.c_str()), strPass.size()).Finalize(out); std::vector hexvec(out, out+KEY_SIZE); std::string strHashFromPass = HexStr(hexvec); From 82e8baab3caaadb1d923d1332c577fb91b9c5747 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Thu, 12 Jan 2017 12:06:32 -0800 Subject: [PATCH 6/6] Avoid boost dynamic_bitset in rest_getutxos --- src/rest.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/rest.cpp b/src/rest.cpp index 8a7c985e7..a8f8e753c 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -17,7 +17,6 @@ #include "version.h" #include -#include #include @@ -502,7 +501,8 @@ static bool rest_getutxos(HTTPRequest* req, const std::string& strURIPart) vector bitmap; vector outs; std::string bitmapStringRepresentation; - boost::dynamic_bitset hits(vOutPoints.size()); + std::vector hits; + bitmap.resize((vOutPoints.size() + 7) / 8); { LOCK2(cs_main, mempool.cs); @@ -518,10 +518,11 @@ static bool rest_getutxos(HTTPRequest* req, const std::string& strURIPart) for (size_t i = 0; i < vOutPoints.size(); i++) { CCoins coins; uint256 hash = vOutPoints[i].hash; + bool hit = false; if (view.GetCoins(hash, coins)) { mempool.pruneSpent(hash, coins); if (coins.IsAvailable(vOutPoints[i].n)) { - hits[i] = true; + hit = true; // Safe to index into vout here because IsAvailable checked if it's off the end of the array, or if // n is valid but points to an already spent output (IsNull). CCoin coin; @@ -533,10 +534,11 @@ static bool rest_getutxos(HTTPRequest* req, const std::string& strURIPart) } } - bitmapStringRepresentation.append(hits[i] ? "1" : "0"); // form a binary string representation (human-readable for json output) + hits.push_back(hit); + bitmapStringRepresentation.append(hit ? "1" : "0"); // form a binary string representation (human-readable for json output) + bitmap[i / 8] |= ((uint8_t)hit) << (i % 8); } } - boost::to_block_range(hits, std::back_inserter(bitmap)); switch (rf) { case RF_BINARY: {