From d5d1425657d0dd2dc76f4938c8141a387a81a5a8 Mon Sep 17 00:00:00 2001 From: Gavin Andresen Date: Tue, 29 Oct 2013 11:16:27 +1000 Subject: [PATCH 1/2] Bug fix: CDataStream::GetAndClear() when nReadPos > 0 Changed CDataStream::GetAndClear() to use the most obvious get get and clear instead of a tricky swap(). Added a unit test for CDataStream insert/erase/GetAndClear. Note: GetAndClear() is not performance critical, it is used only by the send-a-message-to-the-network code. Bug was not noticed before now because the send-a-message code never erased from the stream. --- src/serialize.h | 4 +-- src/test/serialize_tests.cpp | 48 ++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/src/serialize.h b/src/serialize.h index 4d9aec342..32f386b36 100644 --- a/src/serialize.h +++ b/src/serialize.h @@ -1104,8 +1104,8 @@ public: } void GetAndClear(CSerializeData &data) { - vch.swap(data); - CSerializeData().swap(vch); + data.insert(data.end(), begin(), end()); + clear(); } }; diff --git a/src/test/serialize_tests.cpp b/src/test/serialize_tests.cpp index 50139df09..7e193c88c 100644 --- a/src/test/serialize_tests.cpp +++ b/src/test/serialize_tests.cpp @@ -102,4 +102,52 @@ BOOST_AUTO_TEST_CASE(noncanonical) BOOST_CHECK_EXCEPTION(ReadCompactSize(ss), std::ios_base::failure, isCanonicalException); } +BOOST_AUTO_TEST_CASE(insert_delete) +{ + // Test inserting/deleting bytes. + CDataStream ss(SER_DISK, 0); + BOOST_CHECK_EQUAL(ss.size(), 0); + + ss.write("\x00\x01\x02\xff", 4); + BOOST_CHECK_EQUAL(ss.size(), 4); + + char c = (char)11; + + // Inserting at beginning/end/middle: + ss.insert(ss.begin(), c); + BOOST_CHECK_EQUAL(ss.size(), 5); + BOOST_CHECK_EQUAL(ss[0], c); + BOOST_CHECK_EQUAL(ss[1], 0); + + ss.insert(ss.end(), c); + BOOST_CHECK_EQUAL(ss.size(), 6); + BOOST_CHECK_EQUAL(ss[4], (char)0xff); + BOOST_CHECK_EQUAL(ss[5], c); + + ss.insert(ss.begin()+2, c); + BOOST_CHECK_EQUAL(ss.size(), 7); + BOOST_CHECK_EQUAL(ss[2], c); + + // Delete at beginning/end/middle + ss.erase(ss.begin()); + BOOST_CHECK_EQUAL(ss.size(), 6); + BOOST_CHECK_EQUAL(ss[0], 0); + + ss.erase(ss.begin()+ss.size()-1); + BOOST_CHECK_EQUAL(ss.size(), 5); + BOOST_CHECK_EQUAL(ss[4], (char)0xff); + + ss.erase(ss.begin()+1); + BOOST_CHECK_EQUAL(ss.size(), 4); + BOOST_CHECK_EQUAL(ss[0], 0); + BOOST_CHECK_EQUAL(ss[1], 1); + BOOST_CHECK_EQUAL(ss[2], 2); + BOOST_CHECK_EQUAL(ss[3], (char)0xff); + + // Make sure GetAndClear does the right thing: + CSerializeData d; + ss.GetAndClear(d); + BOOST_CHECK_EQUAL(ss.size(), 0); +} + BOOST_AUTO_TEST_SUITE_END() From 9038b18f4655a5b8ad119d768decd1c693ebd7dd Mon Sep 17 00:00:00 2001 From: Gavin Andresen Date: Mon, 28 Oct 2013 16:28:00 +1000 Subject: [PATCH 2/2] -fuzzmessagestest=N : randomly corrupt 1-of-N sent messages I needed this to test the new "reject" p2p message, but it should be generally useful for fuzz-testing network message handling code. --- src/net.cpp | 35 +++++++++++++++++++++++++++++++++++ src/net.h | 10 +++++++++- 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/src/net.cpp b/src/net.cpp index d223b3999..de8543da5 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1894,3 +1894,38 @@ uint64 CNode::GetTotalBytesSent() LOCK(cs_totalBytesSent); return nTotalBytesSent; } + +void CNode::Fuzz(int nChance) +{ + if (!fSuccessfullyConnected) return; // Don't fuzz initial handshake + if (GetRand(nChance) != 0) return; // Fuzz 1 of every nChance messages + + switch (GetRand(3)) + { + case 0: + // xor a random byte with a random value: + if (!ssSend.empty()) { + CDataStream::size_type pos = GetRand(ssSend.size()); + ssSend[pos] ^= (unsigned char)(GetRand(256)); + } + break; + case 1: + // delete a random byte: + if (!ssSend.empty()) { + CDataStream::size_type pos = GetRand(ssSend.size()); + ssSend.erase(ssSend.begin()+pos); + } + break; + case 2: + // insert a random byte at a random position + { + CDataStream::size_type pos = GetRand(ssSend.size()); + char ch = (char)GetRand(256); + ssSend.insert(ssSend.begin()+pos, ch); + } + break; + } + // Chance of more than one change half the time: + // (more changes exponentially less likely): + Fuzz(2); +} diff --git a/src/net.h b/src/net.h index a1dc19df3..b32178ad4 100644 --- a/src/net.h +++ b/src/net.h @@ -218,6 +218,9 @@ protected: static CCriticalSection cs_setBanned; int nMisbehavior; + // Basic fuzz-testing + void Fuzz(int nChance); // modifies ssSend + public: uint256 hashContinue; CBlockIndex* pindexLastGetBlocksBegin; @@ -434,12 +437,17 @@ public: // TODO: Document the precondition of this function. Is cs_vSend locked? void EndMessage() UNLOCK_FUNCTION(cs_vSend) { - if (mapArgs.count("-dropmessagestest") && GetRand(atoi(mapArgs["-dropmessagestest"])) == 0) + // The -*messagestest options are intentionally not documented in the help message, + // since they are only used during development to debug the networking code and are + // not intended for end-users. + if (mapArgs.count("-dropmessagestest") && GetRand(GetArg("-dropmessagestest", 2)) == 0) { LogPrint("net", "dropmessages DROPPING SEND MESSAGE\n"); AbortMessage(); return; } + if (mapArgs.count("-fuzzmessagestest")) + Fuzz(GetArg("-fuzzmessagestest", 10)); if (ssSend.size() == 0) return;