From 69ce70866d7498eb6efadca0cf57785c5cefd386 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sat, 11 Feb 2012 18:01:24 +0100 Subject: [PATCH 01/10] Extra wallet locking fixes * Fix sign error in calculation of seconds to sleep * Do not mix GetTime() (seconds) and Sleep() (milliseconds) * Do not sleep forever if walletlock() is called * Do locking within critical section --- src/rpc.cpp | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/src/rpc.cpp b/src/rpc.cpp index 5326cdc2b79..da3384e398a 100644 --- a/src/rpc.cpp +++ b/src/rpc.cpp @@ -316,7 +316,7 @@ Value getinfo(const Array& params, bool fHelp) obj.push_back(Pair("keypoolsize", pwalletMain->GetKeyPoolSize())); obj.push_back(Pair("paytxfee", ValueFromAmount(nTransactionFee))); if (pwalletMain->IsCrypted()) - obj.push_back(Pair("unlocked_until", (boost::int64_t)nWalletUnlockTime)); + obj.push_back(Pair("unlocked_until", (boost::int64_t)nWalletUnlockTime / 1000)); obj.push_back(Pair("errors", GetWarnings("statusbar"))); return obj; } @@ -1277,7 +1277,7 @@ void ThreadTopUpKeyPool(void* parg) void ThreadCleanWalletPassphrase(void* parg) { - int64 nMyWakeTime = GetTime() + *((int*)parg); + int64 nMyWakeTime = GetTimeMillis() + *((int*)parg) * 1000; ENTER_CRITICAL_SECTION(cs_nWalletUnlockTime); @@ -1285,17 +1285,25 @@ void ThreadCleanWalletPassphrase(void* parg) { nWalletUnlockTime = nMyWakeTime; - while (GetTime() < nWalletUnlockTime) + do { - int64 nToSleep = GetTime() - nWalletUnlockTime; + if (nWalletUnlockTime==0) + break; + int64 nToSleep = nWalletUnlockTime - GetTimeMillis(); + if (nToSleep <= 0) + break; LEAVE_CRITICAL_SECTION(cs_nWalletUnlockTime); Sleep(nToSleep); ENTER_CRITICAL_SECTION(cs_nWalletUnlockTime); - } - nWalletUnlockTime = 0; - pwalletMain->Lock(); + } while(1); + + if (nWalletUnlockTime) + { + nWalletUnlockTime = 0; + pwalletMain->Lock(); + } } else { @@ -1408,9 +1416,9 @@ Value walletlock(const Array& params, bool fHelp) if (!pwalletMain->IsCrypted()) throw JSONRPCError(-15, "Error: running with an unencrypted wallet, but walletlock was called."); - pwalletMain->Lock(); CRITICAL_BLOCK(cs_nWalletUnlockTime) { + pwalletMain->Lock(); nWalletUnlockTime = 0; } From caad1add4f383960ca09bb466ddd16652245befe Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sat, 18 Feb 2012 15:36:40 +0100 Subject: [PATCH 02/10] Free pwalletdbEncryption after encryping wallet Fixes a memory leak. --- src/wallet.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wallet.cpp b/src/wallet.cpp index 43fb6b6da39..9f7422d1fbc 100644 --- a/src/wallet.cpp +++ b/src/wallet.cpp @@ -183,7 +183,7 @@ bool CWallet::EncryptWallet(const string& strWalletPassphrase) 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. - pwalletdbEncryption->Close(); + delete pwalletdbEncryption; pwalletdbEncryption = NULL; } From 471e0bdc7cc79a993230672dadbd193218b6103c Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sun, 19 Feb 2012 18:44:51 +0100 Subject: [PATCH 03/10] Fix #650: CKey::SetSecret BIGNUM leak --- src/key.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/key.h b/src/key.h index 8b033a029f0..89d82fc8036 100644 --- a/src/key.h +++ b/src/key.h @@ -153,10 +153,13 @@ public: if (vchSecret.size() != 32) throw key_error("CKey::SetSecret() : secret must be 32 bytes"); BIGNUM *bn = BN_bin2bn(&vchSecret[0],32,BN_new()); - if (bn == NULL) + if (bn == NULL) throw key_error("CKey::SetSecret() : BN_bin2bn failed"); if (!EC_KEY_regenerate_key(pkey,bn)) + { + BN_clear_free(bn); throw key_error("CKey::SetSecret() : EC_KEY_regenerate_key failed"); + } BN_clear_free(bn); fSet = true; return true; From 54fee2d0ce1e4b389700cd9d043f9475d567cc25 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sun, 19 Feb 2012 19:05:41 +0100 Subject: [PATCH 04/10] Fix #626: RecvLine wrong error message --- src/irc.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/irc.cpp b/src/irc.cpp index b632b965461..5dfab06bacd 100644 --- a/src/irc.cpp +++ b/src/irc.cpp @@ -110,14 +110,14 @@ bool RecvLine(SOCKET hSocket, string& strLine) if (nBytes == 0) { // socket closed - printf("IRC socket closed\n"); + printf("socket closed\n"); return false; } else { // socket error int nErr = WSAGetLastError(); - printf("IRC recv failed: %d\n", nErr); + printf("recv failed: %d\n", nErr); return false; } } From ab2be34059126d2eff11dd2bd20d740aea33f67d Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sun, 19 Feb 2012 19:12:41 +0100 Subject: [PATCH 05/10] Fix #616: remove base_uint::operator&=(uint64 b) --- src/uint256.h | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/uint256.h b/src/uint256.h index 3e20201387b..ae263346a86 100644 --- a/src/uint256.h +++ b/src/uint256.h @@ -100,13 +100,6 @@ public: return *this; } - base_uint& operator&=(uint64 b) - { - pn[0] &= (unsigned int)b; - pn[1] &= (unsigned int)(b >> 32); - return *this; - } - base_uint& operator|=(uint64 b) { pn[0] |= (unsigned int)b; From feb3c15335d5279931c1e0f713400fb616459d0b Mon Sep 17 00:00:00 2001 From: Gavin Andresen Date: Thu, 23 Feb 2012 13:33:30 -0500 Subject: [PATCH 06/10] Checkpoint block 168,000 --- src/checkpoints.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/checkpoints.cpp b/src/checkpoints.cpp index f78712ef4ba..f5ce053870f 100644 --- a/src/checkpoints.cpp +++ b/src/checkpoints.cpp @@ -30,6 +30,7 @@ namespace Checkpoints (118000, uint256("0x000000000000774a7f8a7a12dc906ddb9e17e75d684f15e00f8767f9e8f36553")) (134444, uint256("0x00000000000005b12ffd4cd315cd34ffd4a594f430ac814c91184a0d42d2b0fe")) (140700, uint256("0x000000000000033b512028abb90e1626d8b346fd0ed598ac0a3c371138dce2bd")) + (168000, uint256("0x000000000000099e61ea72015e79632f216fe6cb33d7899acb35b75c8303b763")) ; bool CheckBlock(int nHeight, const uint256& hash) From 3108aed0f210fe6cb698237c7eeac8715fb95e31 Mon Sep 17 00:00:00 2001 From: Gavin Andresen Date: Wed, 29 Feb 2012 10:14:18 -0500 Subject: [PATCH 07/10] DoS fix for mapOrphanTransactions --- src/main.cpp | 25 ++++++++++++++++++++++++- src/main.h | 1 + 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/main.cpp b/src/main.cpp index 9f12829042b..8b11c2bbf06 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -159,13 +159,14 @@ void static ResendWalletTransactions() // mapOrphanTransactions // -void static AddOrphanTx(const CDataStream& vMsg) +void AddOrphanTx(const CDataStream& vMsg) { CTransaction tx; CDataStream(vMsg) >> tx; uint256 hash = tx.GetHash(); if (mapOrphanTransactions.count(hash)) return; + CDataStream* pvMsg = mapOrphanTransactions[hash] = new CDataStream(vMsg); BOOST_FOREACH(const CTxIn& txin, tx.vin) mapOrphanTransactionsByPrev.insert(make_pair(txin.prevout.hash, pvMsg)); @@ -193,6 +194,23 @@ void static EraseOrphanTx(uint256 hash) mapOrphanTransactions.erase(hash); } +int LimitOrphanTxSize(int nMaxOrphans) +{ + int nEvicted = 0; + while (mapOrphanTransactions.size() > nMaxOrphans) + { + // Evict a random orphan: + std::vector randbytes(32); + RAND_bytes(&randbytes[0], 32); + uint256 randomhash(randbytes); + map::iterator it = mapOrphanTransactions.lower_bound(randomhash); + if (it == mapOrphanTransactions.end()) + it = mapOrphanTransactions.begin(); + EraseOrphanTx(it->first); + ++nEvicted; + } + return nEvicted; +} @@ -2183,6 +2201,11 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv) { printf("storing orphan tx %s\n", inv.hash.ToString().substr(0,10).c_str()); AddOrphanTx(vMsg); + + // DoS prevention: do not allow mapOrphanTransactions to grow unbounded + int nEvicted = LimitOrphanTxSize(MAX_ORPHAN_TRANSACTIONS); + if (nEvicted > 0) + printf("mapOrphan overflow, removed %d tx\n", nEvicted); } } diff --git a/src/main.h b/src/main.h index 25cf0790134..3e381b060b2 100644 --- a/src/main.h +++ b/src/main.h @@ -30,6 +30,7 @@ class CBlockIndex; static const unsigned int MAX_BLOCK_SIZE = 1000000; static const unsigned int MAX_BLOCK_SIZE_GEN = MAX_BLOCK_SIZE/2; static const int MAX_BLOCK_SIGOPS = MAX_BLOCK_SIZE/50; +static const int MAX_ORPHAN_TRANSACTIONS = MAX_BLOCK_SIZE/100; static const int64 COIN = 100000000; static const int64 CENT = 1000000; static const int64 MIN_TX_FEE = 50000; From d7962747c4bfe90f9991d121fd22ac625ae2de30 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Fri, 17 Feb 2012 17:58:02 +0100 Subject: [PATCH 08/10] Do not allow overwriting unspent transactions (BIP 30) Introduce the following network rule: * a block is not valid if it contains a transaction whose hash already exists in the block chain, unless all that transaction's outputs were already spent before said block. Warning: this is effectively a network rule change, with potential risk for forking the block chain. Leaving this unfixed carries the same risk however, for attackers that can cause a reorganisation in part of the network. Thanks to Russell O'Connor and Ben Reeves. --- src/main.cpp | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 6a3bacc78e9..812a55110ff 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -792,8 +792,10 @@ bool CTransaction::DisconnectInputs(CTxDB& txdb) } // Remove transaction from index - if (!txdb.EraseTxIndex(*this)) - return error("DisconnectInputs() : EraseTxPos failed"); + // This can fail if a duplicate of this transaction was in a chain that got + // reorganized away. This is only possible if this transaction was completely + // spent, so erasing it would be a no-op anway. + txdb.EraseTxIndex(*this); return true; } @@ -982,6 +984,26 @@ bool CBlock::ConnectBlock(CTxDB& txdb, CBlockIndex* pindex) if (!CheckBlock()) return false; + // Do not allow blocks that contain transactions which 'overwrite' older transactions, + // unless those are already completely spent. + // If such overwrites are allowed, coinbases and transactions depending upon those + // can be duplicated to remove the ability to spend the first instance -- even after + // being sent to another address. + // See BIP30 and http://r6.ca/blog/20120206T005236Z.html for more information. + // This logic is not necessary for memory pool transactions, as AcceptToMemoryPool + // already refuses previously-known transaction id's entirely. + // This rule applies to all blocks whose timestamp is after March 15, 2012, 0:00 UTC. + // On testnet it is enabled as of februari 20, 2012, 0:00 UTC. + if (pindex->nTime > 1331769600 || (fTestNet && pindex->nTime > 1329696000)) + BOOST_FOREACH(CTransaction& tx, vtx) + { + CTxIndex txindexOld; + if (txdb.ReadTxIndex(tx.GetHash(), txindexOld)) + BOOST_FOREACH(CDiskTxPos &pos, txindexOld.vSpent) + if (pos.IsNull()) + return false; + } + //// issue here: it doesn't know the version unsigned int nTxPos = pindex->nBlockPos + ::GetSerializeSize(CBlock(), SER_DISK) - 1 + GetSizeOfCompactSize(vtx.size()); From 4fc8c042a2f80ce0a1a277a2dcc1240c015ed400 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Sat, 3 Mar 2012 13:44:42 -0500 Subject: [PATCH 09/10] Bugfix: Check return value of SHGetSpecialFolderPath in MyGetSpecialFolderPath Upstream commit: 21ae37d (partial) --- src/util.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/util.cpp b/src/util.cpp index 85ca02f0aae..e2e104cc88b 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -643,13 +643,17 @@ string MyGetSpecialFolderPath(int nFolder, bool fCreate) { PSHGETSPECIALFOLDERPATHA pSHGetSpecialFolderPath = (PSHGETSPECIALFOLDERPATHA)GetProcAddress(hShell32, "SHGetSpecialFolderPathA"); + bool fSuccess = false; if (pSHGetSpecialFolderPath) + fSuccess = (*pSHGetSpecialFolderPath)(NULL, pszPath, nFolder, fCreate); FreeModule(hShell32); + if (fSuccess) + return pszPath; } // Backup option - if (pszPath[0] == '\0') + pszPath[0] = '\0'; { if (nFolder == CSIDL_STARTUP) { From 88aa771536014919e955c4f7b2cada9a0dcf8561 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Sat, 3 Mar 2012 13:51:10 -0500 Subject: [PATCH 10/10] Bugfix: Fix possible buffer overflow (#901) Upstream commit: 21ae37d (partial) --- src/util.cpp | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/util.cpp b/src/util.cpp index e2e104cc88b..0f496bc455a 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -653,20 +653,25 @@ string MyGetSpecialFolderPath(int nFolder, bool fCreate) } // Backup option - pszPath[0] = '\0'; + std::string strPath; { + const char *pszEnv; if (nFolder == CSIDL_STARTUP) { - strcpy(pszPath, getenv("USERPROFILE")); - strcat(pszPath, "\\Start Menu\\Programs\\Startup"); + pszEnv = getenv("USERPROFILE"); + if (pszEnv) + strPath = pszEnv; + strPath += "\\Start Menu\\Programs\\Startup"; } else if (nFolder == CSIDL_APPDATA) { - strcpy(pszPath, getenv("APPDATA")); + pszEnv = getenv("APPDATA"); + if (pszEnv) + strPath = pszEnv; } } - return pszPath; + return strPath; } #endif