Skip to content

Commit

Permalink
Merge pull request bitcoin#3 from gandrewstone/xthinchecks
Browse files Browse the repository at this point in the history 
Xthinchecks
  • Loading branch information
Neil committed May 11, 2017
2 parents ea94bed + 7b5b04d commit 52b4b4f
Show file tree
Hide file tree
Showing 8 changed files with 433 additions and 42 deletions.
53 changes: 32 additions & 21 deletions src/main.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -480,23 +480,6 @@ void ProcessBlockAvailability(NodeId nodeid) {
}
}

/** Update tracking information about which blocks a peer is assumed to have. */
void UpdateBlockAvailability(NodeId nodeid, const uint256 &hash) {
CNodeState *state = State(nodeid);
DbgAssert(state != NULL, return); // node already destructed, nothing to do in production mode

ProcessBlockAvailability(nodeid);

BlockMap::iterator it = mapBlockIndex.find(hash);
if (it != mapBlockIndex.end() && it->second->nChainWork > 0) {
// An actually better block was announced.
if (state->pindexBestKnownBlock == NULL || it->second->nChainWork >= state->pindexBestKnownBlock->nChainWork)
state->pindexBestKnownBlock = it->second;
} else {
// An unknown block was announced; just assume that the latest one is the best one.
state->hashLastUnknownBlock = hash;
}
}

// Requires cs_main
bool PeerHasHeader(CNodeState *state, CBlockIndex *pindex)
Expand Down Expand Up @@ -614,7 +597,23 @@ void FindNextBlocksToDownload(NodeId nodeid, unsigned int count, std::vector<CBl

} // anon namespace

/** Update tracking information about which blocks a peer is assumed to have. */
void UpdateBlockAvailability(NodeId nodeid, const uint256 &hash) {
CNodeState *state = State(nodeid);
DbgAssert(state != NULL, return); // node already destructed, nothing to do in production mode

ProcessBlockAvailability(nodeid);

BlockMap::iterator it = mapBlockIndex.find(hash);
if (it != mapBlockIndex.end() && it->second->nChainWork > 0) {
// An actually better block was announced.
if (state->pindexBestKnownBlock == NULL || it->second->nChainWork >= state->pindexBestKnownBlock->nChainWork)
state->pindexBestKnownBlock = it->second;
} else {
// An unknown block was announced; just assume that the latest one is the best one.
state->hashLastUnknownBlock = hash;
}
}

void MarkBlockAsInFlight(NodeId nodeid, const uint256& hash, const Consensus::Params& consensusParams, CBlockIndex *pindex = NULL) {
LOCK(cs_main);
Expand Down Expand Up @@ -5978,10 +5977,10 @@ bool ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vRecv, in
CheckAndRequestExpeditedBlocks(pfrom);
}

else if (strCommand == NetMsgType::XTHINBLOCK && !fImporting && !fReindex && !IsInitialBlockDownload()
&& IsThinBlocksEnabled())
else if (strCommand == NetMsgType::XTHINBLOCK && !fImporting && !fReindex && !IsInitialBlockDownload() &&
IsThinBlocksEnabled())
{
return CXThinBlock::HandleMessage(vRecv, pfrom, strCommand, 0);
return CXThinBlock::HandleMessage(vRecv, pfrom, strCommand, 0);
}


Expand Down Expand Up @@ -6184,6 +6183,9 @@ bool ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vRecv, in
mapMissingTx[tx.GetHash().GetCheapHash()] = tx;

int count = 0;
uint64_t maxAllowedSize = maxMessageSizeMultiplier * excessiveBlockSize;
CTransaction nulltx;
uint64_t nSizeNullTx = RecursiveDynamicUsage(nulltx);
for (size_t i = 0; i < pfrom->thinBlock.vtx.size(); i++)
{
if (pfrom->thinBlock.vtx[i].IsNull())
Expand All @@ -6193,6 +6195,15 @@ bool ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vRecv, in
{
pfrom->thinBlock.vtx[i] = val->second;
pfrom->thinBlockWaitingForTxns--;

// In order to prevent a memory exhaustion attack we track transaction bytes used to create Block
// to see if we've exceeded any limits and if so clear out data and return.
uint64_t nTxSize = RecursiveDynamicUsage(val->second) - nSizeNullTx;
if (thindata.AddThinBlockBytes(nTxSize, pfrom) > maxAllowedSize)
{
if (ClearLargestThinBlockAndDisconnect(pfrom))
return error("xthin block has exceeded memory limits of %ld bytes", maxAllowedSize);
}
}
count++;
}
Expand Down Expand Up @@ -6674,7 +6685,7 @@ bool SendMessages(CNode* pto)
std::map<uint256, int64_t>::iterator iter = pto->mapThinBlocksInFlight.begin();
while (iter != pto->mapThinBlocksInFlight.end())
{
if ((GetTime() - (*iter).second) > THINBLOCK_DOWNLOAD_TIMEOUT)
if ((*iter).second != -1 && (GetTime() - (*iter).second) > THINBLOCK_DOWNLOAD_TIMEOUT)
{
if (!pto->fWhitelisted && Params().NetworkIDString() != "regtest")
{
Expand Down
4 changes: 3 additions & 1 deletion src/main.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,9 @@ bool SequenceLocks(const CTransaction &tx, int flags, std::vector<int>* prevHeig
*/
bool CheckSequenceLocks(const CTransaction &tx, int flags, LockPoints* lp = NULL, bool useExistingLockPoints = false);

// BU: This was all moved to parallel.cpp
/** Update tracking information about which blocks a peer is assumed to have. */
void UpdateBlockAvailability(NodeId nodeid, const uint256 &hash);

/**
* Class that keeps track of number of signature operations
* and bytes hashed to compute signature hashes.
Expand Down
1 change: 1 addition & 0 deletions src/net.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2810,6 +2810,7 @@ CNode::CNode(SOCKET hSocketIn, const CAddress& addrIn, const std::string& addrNa
nMinPingUsecTime = std::numeric_limits<int64_t>::max();
thinBlockWaitingForTxns = -1; // BUIP010 Xtreme Thinblocks
addrFromPort = 0; // BU
nLocalThinBlockBytes = 0;

// BU instrumentation
std::string xmledName;
Expand Down
1 change: 1 addition & 0 deletions src/net.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,7 @@ class CNode
CBlock thinBlock;
std::vector<uint256> thinBlockHashes;
std::vector<uint64_t> xThinBlockHashes;
uint64_t nLocalThinBlockBytes; // the bytes used in creating this thinblock, updated dynamically
int nSizeThinBlock; // Original on-wire size of the block. Just used for reporting
int thinBlockWaitingForTxns; // if -1 then not currently waiting
CCriticalSection cs_mapthinblocksinflight; // lock mapThinBlocksInFlight
Expand Down
20 changes: 17 additions & 3 deletions src/parallel.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -496,6 +496,18 @@ void HandleBlockMessageThread(CNode *pfrom, const string &strCommand, const CBlo
CValidationState state;
uint64_t nSizeBlock = ::GetSerializeSize(block, SER_NETWORK, PROTOCOL_VERSION);

// At this point we have either a block or a fully reconstructed thinblock but we still need to
// maintain a mapThinBlocksInFlight entry so that we don't re-request a full block from
// the same node while the block is processing. Furthermore by setting the time = -1 we prevent
// the timeout from triggering and inadvertently disconnecting the node in the event that the block
// takes a longer time to process than the THINBLOCK_DOWNLOAD_TIMEOUT interval.
{
LOCK(pfrom->cs_mapthinblocksinflight);
if (pfrom->mapThinBlocksInFlight.count(inv.hash))
pfrom->mapThinBlocksInFlight[inv.hash] = -1;
}


boost::thread::id this_id(boost::this_thread::get_id());
PV.InitThread(this_id, pfrom, block, inv); // initialize the mapBlockValidationThread entries

Expand Down Expand Up @@ -557,8 +569,8 @@ void HandleBlockMessageThread(CNode *pfrom, const string &strCommand, const CBlo
// Erase this thinblock from the tracking map now that we're done with it.
if (pfrom->mapThinBlocksInFlight.erase(inv.hash))
{
pfrom->thinBlockWaitingForTxns = -1;
pfrom->thinBlock.SetNull();
// Clear out and reset thinblock data
thindata.ClearThinBlockData(pfrom);
}

// Count up any other remaining nodes with thinblocks in flight.
Expand All @@ -570,10 +582,12 @@ void HandleBlockMessageThread(CNode *pfrom, const string &strCommand, const CBlo
pfrom->firstBlock += 1; // update statistics, requires cs_vNodes
}

// When we no longer have any thinblocks in flight then clear the set
// When we no longer have any thinblocks in flight then clear our any data
// just to make sure we don't somehow get growth over time.
if (nTotalThinBlocksInFlight == 0)
{
thindata.ResetThinBlockBytes();

LOCK(cs_xval);
setPreVerifiedTxHash.clear();
setUnVerifiedOrphanTxHash.clear();
Expand Down
195 changes: 195 additions & 0 deletions src/test/exploit_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,201 @@ BOOST_AUTO_TEST_CASE(thinblock_tests)
SendMessages(&dummyNode5a);
BOOST_CHECK(inv2.type != MSG_THINBLOCK && inv2.type != MSG_XTHINBLOCK);
BOOST_CHECK(CNode::IsBanned(addr5));


/* Thinblock memory exhaustion attack 1 */

// test a single valid thinblock reconstruction that goes over the limit.
// result: the peer should have it data cleared and node should be disconnected.
CNode::ClearBanned();
CXThinBlock xthin2 = xthinblock;
CThinBlock thin2 = thinblock;

CNode dummyNode6(INVALID_SOCKET, addr1, "", true);

// The number of tx bytes in this block is 3784 bytes. In order for the node to be disconnected
// we have to make the maxAllowedSize be less than 3784/maxMessageSizeMultiplier (maxMessageSizeMultiplier = 16).
// Therefore the excesssiveBlockSize must be less than 236.5 inorder to trigger an oversized block and susequent
// disconnection.
uint64_t old_excessiveBlockSize = excessiveBlockSize;
excessiveBlockSize = 234;

// Add the node to vNodes and also we need a thinblockinflight entry
dummyNode6.mapThinBlocksInFlight[TestBlock1().GetHash()] = GetTime();
vNodes.push_back(&dummyNode6);

// Process an xthinblock
vRecv1.clear();
vRecv1 << xthin;
dummyNode6.fDisconnect = false;
xthin2.process(&dummyNode6, vRecv1.size(), NetMsgType::XTHINBLOCK);
BOOST_CHECK(dummyNode6.fDisconnect); // node should be disconnected
BOOST_CHECK_EQUAL(0, dummyNode6.nLocalThinBlockBytes);
BOOST_CHECK_EQUAL(-1, dummyNode6.thinBlockWaitingForTxns);
BOOST_CHECK(dummyNode6.thinBlock.IsNull());
BOOST_CHECK(dummyNode6.xThinBlockHashes.empty());
BOOST_CHECK(dummyNode6.thinBlockHashes.empty());

// Process a regular thinblock
vRecv1.clear();
vRecv1 << thin;
dummyNode6.fDisconnect = false;
thin2.process(&dummyNode6, vRecv1.size(), NetMsgType::THINBLOCK);
BOOST_CHECK(dummyNode6.fDisconnect); // node should be disconnected
BOOST_CHECK_EQUAL(0, dummyNode6.nLocalThinBlockBytes);
BOOST_CHECK_EQUAL(-1, dummyNode6.thinBlockWaitingForTxns);
BOOST_CHECK(dummyNode6.thinBlock.IsNull());
BOOST_CHECK(dummyNode6.xThinBlockHashes.empty());
BOOST_CHECK(dummyNode6.thinBlockHashes.empty());

// clean up vNodes and mapthinblocksinflight
vNodes.pop_back();
dummyNode6.mapThinBlocksInFlight.erase(TestBlock1().GetHash());

/* Thinblock memory exhaustion attack 2 */

// test correct disconnection of a multiple valid thinblock reconstruction that goes over the limit.
// result: the peer with largest thinblock set of data should have it data cleared
// and node should be disconnected.

CNode::ClearBanned();
CXThinBlock xthin3 = xthinblock;

CNode dummyNode7(INVALID_SOCKET, addr2, "", true);
CNode dummyNode8(INVALID_SOCKET, addr3, "", true);
CNode dummyNode9(INVALID_SOCKET, addr4, "", true);

// The number of tx bytes in this block is 3784 bytes. In order for the node to be disconnected
// we have to make the maxAllowedSize be less than 3784/maxMessageSizeMultiplier (maxMessageSizeMultiplier = 16).
// Therefore the excesssiveBlockSize must be less than 236.5 inorder to trigger an oversized block and susequent
// disconnection.
excessiveBlockSize = 234;

// Add the node to vNodes and also we need a thinblockinflight entry
dummyNode6.mapThinBlocksInFlight[TestBlock1().GetHash()] = GetTime();
vNodes.push_back(&dummyNode6);
dummyNode7.mapThinBlocksInFlight[TestBlock1().GetHash()] = GetTime();
vNodes.push_back(&dummyNode7);
dummyNode8.mapThinBlocksInFlight[TestBlock1().GetHash()] = GetTime();
vNodes.push_back(&dummyNode8);
dummyNode9.mapThinBlocksInFlight[TestBlock1().GetHash()] = GetTime();
vNodes.push_back(&dummyNode9);

// manually set the nLocalThinBlockBytes to be lower than the actual bytes of the thinblock that we will
// use to test the over limit condition. Also set the global bytes to be the sum of all current nodes.
thindata.ResetThinBlockBytes();
thindata.AddThinBlockBytes(100, &dummyNode7);
thindata.AddThinBlockBytes(110, &dummyNode8);
thindata.AddThinBlockBytes(120, &dummyNode9);

// Process an xthinblock which will be the largest over limit and will be the one that gets disconnected.
vRecv1.clear();
vRecv1 << xthin;
dummyNode6.fDisconnect = false;
xthin3.process(&dummyNode6, vRecv1.size(), NetMsgType::XTHINBLOCK);

BOOST_CHECK(!dummyNode7.fDisconnect); // node should *not* be disconnected
BOOST_CHECK_EQUAL(100, dummyNode7.nLocalThinBlockBytes);
BOOST_CHECK(!dummyNode8.fDisconnect); // node should *not* be disconnected
BOOST_CHECK_EQUAL(110, dummyNode8.nLocalThinBlockBytes);
BOOST_CHECK(!dummyNode9.fDisconnect); // node should *not* be disconnected
BOOST_CHECK_EQUAL(120, dummyNode9.nLocalThinBlockBytes);

BOOST_CHECK(dummyNode6.fDisconnect); // node should be disconnected
BOOST_CHECK_EQUAL(0, dummyNode6.nLocalThinBlockBytes);
BOOST_CHECK_EQUAL(-1, dummyNode6.thinBlockWaitingForTxns);
BOOST_CHECK(dummyNode6.thinBlock.IsNull());
BOOST_CHECK(dummyNode6.xThinBlockHashes.empty());
BOOST_CHECK(dummyNode6.thinBlockHashes.empty());

// clean up vNodes and mapthinblocksinflight
vNodes.pop_back();
dummyNode6.mapThinBlocksInFlight.erase(TestBlock1().GetHash());
vNodes.pop_back();
dummyNode7.mapThinBlocksInFlight.erase(TestBlock1().GetHash());
vNodes.pop_back();
dummyNode8.mapThinBlocksInFlight.erase(TestBlock1().GetHash());
vNodes.pop_back();
dummyNode9.mapThinBlocksInFlight.erase(TestBlock1().GetHash());

/* Thinblock memory exhaustion attack 3 */

// test correct disconnection of a multiple valid thinblock reconstruction that goes over the limit.
// However here, the last thinblock although causing the limit to be exceeded is not the largest.
// result: the peer with largest thinblock set of data should have it data cleared
// and node should be disconnected.

CNode::ClearBanned();
CXThinBlock xthin4 = xthinblock;

// This time we don't want the xthinblock to cause an overlimit but have some other node disconnected.
// So we use a 1000 excessive size which gives us a 16 * 1000 byte limit.
excessiveBlockSize = 234;

// Add the node to vNodes and also we need a thinblockinflight entry
dummyNode6.mapThinBlocksInFlight[TestBlock1().GetHash()] = GetTime();
vNodes.push_back(&dummyNode6);
dummyNode7.mapThinBlocksInFlight[TestBlock1().GetHash()] = GetTime();
vNodes.push_back(&dummyNode7);
dummyNode8.mapThinBlocksInFlight[TestBlock1().GetHash()] = GetTime();
vNodes.push_back(&dummyNode8);
dummyNode9.mapThinBlocksInFlight[TestBlock1().GetHash()] = GetTime();
vNodes.push_back(&dummyNode9);

// manually set two of the nLocalThinBlockBytes to be higher than the actual bytes of the thinblock that we will
// use to test the over limit condition. Also set the global bytes to be the sum of all current nodes.
thindata.ResetThinBlockBytes();
dummyNode7.nLocalThinBlockBytes = 0;
dummyNode8.nLocalThinBlockBytes = 0;
dummyNode9.nLocalThinBlockBytes = 0;
thindata.AddThinBlockBytes(3000, &dummyNode7);
thindata.AddThinBlockBytes(600, &dummyNode8);
thindata.AddThinBlockBytes(100, &dummyNode9);

// Process an xthinblock which will also be the over limit and will cause the largest block to disconnect
// which in this case is dummyNode7. As it continues to process it (dummyNode6) will also go over the limit
// and cause itself to be disconnected.
vRecv1.clear();
vRecv1 << xthin4;
dummyNode6.fDisconnect = false;
xthin4.process(&dummyNode6, vRecv1.size(), NetMsgType::XTHINBLOCK);
BOOST_CHECK(!dummyNode8.fDisconnect); // node should *not* be disconnected
BOOST_CHECK_EQUAL(600, dummyNode8.nLocalThinBlockBytes);
BOOST_CHECK(!dummyNode9.fDisconnect); // node should *not* be disconnected
BOOST_CHECK_EQUAL(100, dummyNode9.nLocalThinBlockBytes);

BOOST_CHECK(dummyNode6.fDisconnect); // node should *not* be disconnected
BOOST_CHECK_EQUAL(0, dummyNode6.nLocalThinBlockBytes);
BOOST_CHECK_EQUAL(-1, dummyNode6.thinBlockWaitingForTxns);
BOOST_CHECK(dummyNode6.thinBlock.IsNull());
BOOST_CHECK(dummyNode6.xThinBlockHashes.empty());
BOOST_CHECK(dummyNode6.thinBlockHashes.empty());

BOOST_CHECK(dummyNode7.fDisconnect); // node should be disconnected
BOOST_CHECK_EQUAL(0, dummyNode7.nLocalThinBlockBytes);
BOOST_CHECK_EQUAL(-1, dummyNode7.thinBlockWaitingForTxns);
BOOST_CHECK(dummyNode7.thinBlock.IsNull());
BOOST_CHECK(dummyNode7.xThinBlockHashes.empty());
BOOST_CHECK(dummyNode7.thinBlockHashes.empty());

// clean up vNodes and mapthinblocksinflight
vNodes.pop_back();
dummyNode6.mapThinBlocksInFlight.erase(TestBlock1().GetHash());
vNodes.pop_back();
dummyNode7.mapThinBlocksInFlight.erase(TestBlock1().GetHash());
vNodes.pop_back();
dummyNode8.mapThinBlocksInFlight.erase(TestBlock1().GetHash());
vNodes.pop_back();
dummyNode9.mapThinBlocksInFlight.erase(TestBlock1().GetHash());

excessiveBlockSize = old_excessiveBlockSize; // reset

// cleanup received queues
vRecv1.clear();
vRecv2.clear();
vRecv3.clear();
vRecv4.clear();
vRecv5.clear();
}

BOOST_AUTO_TEST_SUITE_END()

0 comments on commit 52b4b4f

Please sign in to comment.