Browse Source

Fix de-serialization bug where AddrMan is corrupted after exception

* CAddrDB modified so that when de-serialization code throws an exception Addrman is reset to a clean state
* CAddrDB modified to make unit tests possible
* Regression test created to ensure bug is fixed
* StartNode modifed to clear adrman if CAddrDB::Read returns an error code.
0.13
EthanHeilman 9 years ago committed by Ethan Heilman
parent
commit
1475ecf611
  1. 1
      src/Makefile.test.include
  2. 8
      src/net.cpp
  3. 1
      src/net.h
  4. 136
      src/test/net_tests.cpp

1
src/Makefile.test.include

@ -60,6 +60,7 @@ BITCOIN_TESTS =\ @@ -60,6 +60,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 \

8
src/net.cpp

@ -1944,6 +1944,7 @@ void StartNode(boost::thread_group& threadGroup, CScheduler& scheduler) @@ -1944,6 +1944,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();
}
@ -2336,6 +2337,11 @@ bool CAddrDB::Read(CAddrMan& addr) @@ -2336,6 +2337,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 ..
@ -2349,6 +2355,8 @@ bool CAddrDB::Read(CAddrMan& addr) @@ -2349,6 +2355,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());
}

1
src/net.h

@ -779,6 +779,7 @@ public: @@ -779,6 +779,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) */

136
src/test/net_tests.cpp

@ -0,0 +1,136 @@ @@ -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 <string>
#include <boost/test/unit_test.hpp>
#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<unsigned char> 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()
Loading…
Cancel
Save