diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 08e2f6af4..897a7dd4a 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -59,6 +59,7 @@ BITCOIN_TESTS =\ test/merkle_tests.cpp \ test/miner_tests.cpp \ test/multisig_tests.cpp \ + test/net_tests.cpp \ test/netbase_tests.cpp \ test/pmt_tests.cpp \ test/policyestimator_tests.cpp \ diff --git a/src/net.cpp b/src/net.cpp index aa5b47340..abc492e2b 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1950,6 +1950,7 @@ void StartNode(boost::thread_group& threadGroup, CScheduler& scheduler) if (adb.Read(addrman)) LogPrintf("Loaded %i addresses from peers.dat %dms\n", addrman.size(), GetTimeMillis() - nStart); else { + addrman.Clear(); // Addrman can be in an inconsistent state after failure, reset it LogPrintf("Invalid or missing peers.dat; recreating\n"); DumpAddresses(); } @@ -2320,6 +2321,11 @@ bool CAddrDB::Read(CAddrMan& addr) if (hashIn != hashTmp) return error("%s: Checksum mismatch, data corrupted", __func__); + return Read(addr, ssPeers); +} + +bool CAddrDB::Read(CAddrMan& addr, CDataStream& ssPeers) +{ unsigned char pchMsgTmp[4]; try { // de-serialize file header (network specific magic number) and .. @@ -2333,6 +2339,8 @@ bool CAddrDB::Read(CAddrMan& addr) ssPeers >> addr; } catch (const std::exception& e) { + // de-serialization has failed, ensure addrman is left in a clean state + addr.Clear(); return error("%s: Deserialize or I/O error - %s", __func__, e.what()); } diff --git a/src/net.h b/src/net.h index fd80056c6..9ab820ca6 100644 --- a/src/net.h +++ b/src/net.h @@ -794,6 +794,7 @@ public: CAddrDB(); bool Write(const CAddrMan& addr); bool Read(CAddrMan& addr); + bool Read(CAddrMan& addr, CDataStream& ssPeers); }; /** Access to the banlist database (banlist.dat) */ diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp new file mode 100644 index 000000000..6debf6ac5 --- /dev/null +++ b/src/test/net_tests.cpp @@ -0,0 +1,136 @@ +// Copyright (c) 2012-2016 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include "addrman.h" +#include "test/test_bitcoin.h" +#include +#include +#include "hash.h" +#include "serialize.h" +#include "streams.h" +#include "net.h" +#include "chainparams.h" + +using namespace std; + +class CAddrManSerializationMock : public CAddrMan +{ +public: + virtual void Serialize(CDataStream& s, int nType, int nVersionDummy) const = 0; +}; + +class CAddrManUncorrupted : public CAddrManSerializationMock +{ +public: + void Serialize(CDataStream& s, int nType, int nVersionDummy) const + { + CAddrMan::Serialize(s, nType, nVersionDummy); + } +}; + +class CAddrManCorrupted : public CAddrManSerializationMock +{ +public: + void Serialize(CDataStream& s, int nType, int nVersionDummy) const + { + // Produces corrupt output that claims addrman has 20 addrs when it only has one addr. + unsigned char nVersion = 1; + s << nVersion; + s << ((unsigned char)32); + s << nKey; + s << 10; // nNew + s << 10; // nTried + + int nUBuckets = ADDRMAN_NEW_BUCKET_COUNT ^ (1 << 30); + s << nUBuckets; + + CAddress addr = CAddress(CService("252.1.1.1", 7777)); + CAddrInfo info = CAddrInfo(addr, CNetAddr("252.2.2.2")); + s << info; + } +}; + +CDataStream AddrmanToStream(CAddrManSerializationMock& addrman) +{ + CDataStream ssPeersIn(SER_DISK, CLIENT_VERSION); + ssPeersIn << FLATDATA(Params().MessageStart()); + ssPeersIn << addrman; + std::string str = ssPeersIn.str(); + vector vchData(str.begin(), str.end()); + return CDataStream(vchData, SER_DISK, CLIENT_VERSION); +} + +BOOST_FIXTURE_TEST_SUITE(net_tests, BasicTestingSetup) + +BOOST_AUTO_TEST_CASE(caddrdb_read) +{ + CAddrManUncorrupted addrmanUncorrupted; + + CService addr1 = CService("250.7.1.1", 8333); + CService addr2 = CService("250.7.2.2", 9999); + CService addr3 = CService("250.7.3.3", 9999); + + // Add three addresses to new table. + addrmanUncorrupted.Add(CAddress(addr1), CService("252.5.1.1", 8333)); + addrmanUncorrupted.Add(CAddress(addr2), CService("252.5.1.1", 8333)); + addrmanUncorrupted.Add(CAddress(addr3), CService("252.5.1.1", 8333)); + + // Test that the de-serialization does not throw an exception. + CDataStream ssPeers1 = AddrmanToStream(addrmanUncorrupted); + bool exceptionThrown = false; + CAddrMan addrman1; + + BOOST_CHECK(addrman1.size() == 0); + try { + unsigned char pchMsgTmp[4]; + ssPeers1 >> FLATDATA(pchMsgTmp); + ssPeers1 >> addrman1; + } catch (const std::exception& e) { + exceptionThrown = true; + } + + BOOST_CHECK(addrman1.size() == 3); + BOOST_CHECK(exceptionThrown == false); + + // Test that CAddrDB::Read creates an addrman with the correct number of addrs. + CDataStream ssPeers2 = AddrmanToStream(addrmanUncorrupted); + + CAddrMan addrman2; + CAddrDB adb; + BOOST_CHECK(addrman2.size() == 0); + adb.Read(addrman2, ssPeers2); + BOOST_CHECK(addrman2.size() == 3); +} + + +BOOST_AUTO_TEST_CASE(caddrdb_read_corrupted) +{ + CAddrManCorrupted addrmanCorrupted; + + // Test that the de-serialization of corrupted addrman throws an exception. + CDataStream ssPeers1 = AddrmanToStream(addrmanCorrupted); + bool exceptionThrown = false; + CAddrMan addrman1; + BOOST_CHECK(addrman1.size() == 0); + try { + unsigned char pchMsgTmp[4]; + ssPeers1 >> FLATDATA(pchMsgTmp); + ssPeers1 >> addrman1; + } catch (const std::exception& e) { + exceptionThrown = true; + } + // Even through de-serialization failed adddrman is not left in a clean state. + BOOST_CHECK(addrman1.size() == 1); + BOOST_CHECK(exceptionThrown); + + // Test that CAddrDB::Read leaves addrman in a clean state if de-serialization fails. + CDataStream ssPeers2 = AddrmanToStream(addrmanCorrupted); + + CAddrMan addrman2; + CAddrDB adb; + BOOST_CHECK(addrman2.size() == 0); + adb.Read(addrman2, ssPeers2); + BOOST_CHECK(addrman2.size() == 0); +} + +BOOST_AUTO_TEST_SUITE_END()