Skip to content

Commit

Permalink
Merge bitcoin#22029: [fuzz] Improve transport deserialization fuzz te…
Browse files Browse the repository at this point in the history
…st coverage

e337145 [fuzz] Occasional valid magic bytes for transport serialization test (Dhruv Mehta)
35571d8 [fuzz] Occasional valid checksum for transport serialization fuzz test (Dhruv Mehta)
654472a [fuzz] Add serialization to deserialization test (Dhruv Mehta)

Pull request description:

  This PR has 3 commits that increase the fuzz test coverage:

  Before commit 1:
  ```
  #306853 REDUCE cov: 798 ft: 5820 corp: 150/375Kb lim: 68333 exec/s: 1382 rss: 461Mb L: 254/63171 MS: 1 EraseBytes-
  #1453105 REDUCE cov: 798 ft: 5820 corp: 150/369Kb lim: 79613 exec/s: 1467 rss: 461Mb L: 6027/60873 MS: 1 EraseBytes-
  ```

  After commit 1 (adds serialization to de-serialization test):
  ```
  #303389 NEW cov: 1202 ft: 8382 corp: 157/382Kb lim: 68189 exec/s: 1451 rss: 447Mb L: 1386/65459 MS: 1 CopyPart-
  #1428759 REDUCE cov: 1202 ft: 8512 corp: 169/389Kb lim: 78749 exec/s: 1528 rss: 463Mb L: 1627/60488 MS: 1 EraseBytes-
  ```

  After commit 2 (provides an occasional checksum assist to the fuzzer inputs):
  ```
  #304820 NEW cov: 1440 ft: 4452 corp: 92/12551b lim: 2237 exec/s: 3386 rss: 486Mb L: 47/1111 MS: 1 ChangeByte-
  #1416181 REDUCE cov: 1442 ft: 5681 corp: 125/59Kb lim: 4096 exec/s: 3522 rss: 535Mb L: 2164/4049 MS: 1 EraseBytes-
  ```

  After commit 3 (provides an occasional magic bytes assist to the fuzzer inputs):
  ```
  #302684 NEW cov: 1454 ft: 3936 corp: 84/7056b lim: 2424 exec/s: 4146 rss: 477Mb L: 65/1108 MS: 3 CopyPart-CrossOver-CMP- DE: "\x0e\x00\x00\x00"-
  #1383925 REDUCE cov: 1454 ft: 4828 corp: 102/14573b lim: 4096 exec/s: 3954 rss: 534Mb L: 116/4050 MS: 2 EraseBytes-ChangeByte-
  ```

  If reviewers only accept the first commit, the seeds are not invalidated and new seeds are at: bitcoin-core/qa-assets#61. In this case, we can also revert the test name change.

  If reviewers accept all three commits, the existing seeds are invalidated.

ACKs for top commit:
  practicalswift:
    Tested ACK e337145

Tree-SHA512: d37f06eea0249322b00a99c4827359eb53aeb711751e5571f4681eeca06dc257e0c4cd4887150fc37cc2f689e26986112d768066ad274361615ba9b6a522c61a
  • Loading branch information
laanwj committed May 27, 2021
2 parents 7076bba + e337145 commit e20745c
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 44 deletions.
2 changes: 1 addition & 1 deletion src/Makefile.test.include
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ test_fuzz_fuzz_SOURCES = \
test/fuzz/netaddress.cpp \
test/fuzz/netbase_dns_lookup.cpp \
test/fuzz/node_eviction.cpp \
test/fuzz/p2p_transport_deserializer.cpp \
test/fuzz/p2p_transport_serialization.cpp \
test/fuzz/parse_hd_keypath.cpp \
test/fuzz/parse_iso8601.cpp \
test/fuzz/parse_numbers.cpp \
Expand Down
43 changes: 0 additions & 43 deletions src/test/fuzz/p2p_transport_deserializer.cpp

This file was deleted.

85 changes: 85 additions & 0 deletions src/test/fuzz/p2p_transport_serialization.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
// Copyright (c) 2019-2020 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 <chainparams.h>
#include <hash.h>
#include <net.h>
#include <netmessagemaker.h>
#include <protocol.h>
#include <test/fuzz/FuzzedDataProvider.h>
#include <test/fuzz/fuzz.h>

#include <cassert>
#include <cstdint>
#include <limits>
#include <optional>
#include <vector>

void initialize_p2p_transport_serialization()
{
SelectParams(CBaseChainParams::REGTEST);
}

FUZZ_TARGET_INIT(p2p_transport_serialization, initialize_p2p_transport_serialization)
{
// Construct deserializer, with a dummy NodeId
V1TransportDeserializer deserializer{Params(), (NodeId)0, SER_NETWORK, INIT_PROTO_VERSION};
V1TransportSerializer serializer{};
FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};

auto checksum_assist = fuzzed_data_provider.ConsumeBool();
auto magic_bytes_assist = fuzzed_data_provider.ConsumeBool();
std::vector<uint8_t> mutable_msg_bytes;

auto header_bytes_remaining = CMessageHeader::HEADER_SIZE;
if (magic_bytes_assist) {
auto msg_start = Params().MessageStart();
for (size_t i = 0; i < CMessageHeader::MESSAGE_SIZE_SIZE; ++i) {
mutable_msg_bytes.push_back(msg_start[i]);
}
header_bytes_remaining -= CMessageHeader::MESSAGE_SIZE_SIZE;
}

if (checksum_assist) {
header_bytes_remaining -= CMessageHeader::CHECKSUM_SIZE;
}

auto header_random_bytes = fuzzed_data_provider.ConsumeBytes<uint8_t>(header_bytes_remaining);
mutable_msg_bytes.insert(mutable_msg_bytes.end(), header_random_bytes.begin(), header_random_bytes.end());
auto payload_bytes = fuzzed_data_provider.ConsumeRemainingBytes<uint8_t>();

if (checksum_assist && mutable_msg_bytes.size() == CMessageHeader::CHECKSUM_OFFSET) {
CHash256 hasher;
unsigned char hsh[32];
hasher.Write(payload_bytes);
hasher.Finalize(hsh);
for (size_t i = 0; i < CMessageHeader::CHECKSUM_SIZE; ++i) {
mutable_msg_bytes.push_back(hsh[i]);
}
}

mutable_msg_bytes.insert(mutable_msg_bytes.end(), payload_bytes.begin(), payload_bytes.end());
Span<const uint8_t> msg_bytes{mutable_msg_bytes};
while (msg_bytes.size() > 0) {
const int handled = deserializer.Read(msg_bytes);
if (handled < 0) {
break;
}
if (deserializer.Complete()) {
const std::chrono::microseconds m_time{std::numeric_limits<int64_t>::max()};
uint32_t out_err_raw_size{0};
std::optional<CNetMessage> result{deserializer.GetMessage(m_time, out_err_raw_size)};
if (result) {
assert(result->m_command.size() <= CMessageHeader::COMMAND_SIZE);
assert(result->m_raw_message_size <= mutable_msg_bytes.size());
assert(result->m_raw_message_size == CMessageHeader::HEADER_SIZE + result->m_message_size);
assert(result->m_time == m_time);

std::vector<unsigned char> header;
auto msg = CNetMsgMaker{result->m_recv.GetVersion()}.Make(result->m_command, MakeUCharSpan(result->m_recv));
serializer.prepareForTransport(msg, header);
}
}
}
}

0 comments on commit e20745c

Please sign in to comment.