From b51d75ea97ee0d01ee586e40a30cb68c0bf7ffd3 Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Wed, 12 Jun 2024 12:51:46 +0200 Subject: fuzz: simplify FuzzedSock::m_peek_data `FuzzedSock::m_peek_data` need not be an optional of a vector. It can be just a vector whereas an empty vector denotes "no peek data". --- src/test/fuzz/util/net.cpp | 6 +++--- src/test/fuzz/util/net.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test/fuzz/util/net.cpp b/src/test/fuzz/util/net.cpp index 5e7aae670e..562fa4ce68 100644 --- a/src/test/fuzz/util/net.cpp +++ b/src/test/fuzz/util/net.cpp @@ -191,11 +191,11 @@ ssize_t FuzzedSock::Recv(void* buf, size_t len, int flags) const } std::vector random_bytes; bool pad_to_len_bytes{m_fuzzed_data_provider.ConsumeBool()}; - if (m_peek_data.has_value()) { + if (!m_peek_data.empty()) { // `MSG_PEEK` was used in the preceding `Recv()` call, return `m_peek_data`. - random_bytes = m_peek_data.value(); + random_bytes = m_peek_data; if ((flags & MSG_PEEK) == 0) { - m_peek_data.reset(); + m_peek_data.clear(); } pad_to_len_bytes = false; } else if ((flags & MSG_PEEK) != 0) { diff --git a/src/test/fuzz/util/net.h b/src/test/fuzz/util/net.h index ed02680676..1a5902329e 100644 --- a/src/test/fuzz/util/net.h +++ b/src/test/fuzz/util/net.h @@ -43,7 +43,7 @@ class FuzzedSock : public Sock * If `MSG_PEEK` is used, then our `Recv()` returns some random data as usual, but on the next * `Recv()` call we must return the same data, thus we remember it here. */ - mutable std::optional> m_peek_data; + mutable std::vector m_peek_data; /** * Whether to pretend that the socket is select(2)-able. This is randomly set in the -- cgit v1.2.3 From 4d81b4de339efbbb68c9785203b699e6e12ecd83 Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Wed, 12 Jun 2024 14:41:50 +0200 Subject: fuzz: FuzzedSock::Recv() don't lose bytes from MSG_PEEK read Problem: If `FuzzedSock::Recv(N, MSG_PEEK)` is called then `N` bytes would be retrieved from the fuzz provider, saved in `m_peek_data` and returned to the caller (ok). If after this `FuzzedSock::Recv(M, 0)` is called where `M < N` then the first `M` bytes from `m_peek_data` would be returned to the caller (ok), but the remaining `N - M` bytes in `m_peek_data` would be discarded/lost (not ok). They must be returned by a subsequent `Recv()`. To resolve this, only remove the head `N` bytes from `m_peek_data`. --- src/test/fuzz/util/net.cpp | 68 +++++++++++++++++++++++----------------------- 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/src/test/fuzz/util/net.cpp b/src/test/fuzz/util/net.cpp index 562fa4ce68..ad69c29d12 100644 --- a/src/test/fuzz/util/net.cpp +++ b/src/test/fuzz/util/net.cpp @@ -182,6 +182,12 @@ ssize_t FuzzedSock::Recv(void* buf, size_t len, int flags) const EWOULDBLOCK, }; assert(buf != nullptr || len == 0); + + // Do the latency before any of the "return" statements. + if (m_fuzzed_data_provider.ConsumeBool() && std::getenv("FUZZED_SOCKET_FAKE_LATENCY") != nullptr) { + std::this_thread::sleep_for(std::chrono::milliseconds{2}); + } + if (len == 0 || m_fuzzed_data_provider.ConsumeBool()) { const ssize_t r = m_fuzzed_data_provider.ConsumeBool() ? 0 : -1; if (r == -1) { @@ -189,47 +195,41 @@ ssize_t FuzzedSock::Recv(void* buf, size_t len, int flags) const } return r; } - std::vector random_bytes; - bool pad_to_len_bytes{m_fuzzed_data_provider.ConsumeBool()}; + + size_t copied_so_far{0}; + if (!m_peek_data.empty()) { - // `MSG_PEEK` was used in the preceding `Recv()` call, return `m_peek_data`. - random_bytes = m_peek_data; + // `MSG_PEEK` was used in the preceding `Recv()` call, copy the first bytes from `m_peek_data`. + const size_t copy_len{std::min(len, m_peek_data.size())}; + std::memcpy(buf, m_peek_data.data(), copy_len); + copied_so_far += copy_len; if ((flags & MSG_PEEK) == 0) { - m_peek_data.clear(); + m_peek_data.erase(m_peek_data.begin(), m_peek_data.begin() + copy_len); } - pad_to_len_bytes = false; - } else if ((flags & MSG_PEEK) != 0) { - // New call with `MSG_PEEK`. - random_bytes = ConsumeRandomLengthByteVector(m_fuzzed_data_provider, len); - if (!random_bytes.empty()) { - m_peek_data = random_bytes; - pad_to_len_bytes = false; - } - } else { - random_bytes = ConsumeRandomLengthByteVector(m_fuzzed_data_provider, len); } - if (random_bytes.empty()) { - const ssize_t r = m_fuzzed_data_provider.ConsumeBool() ? 0 : -1; - if (r == -1) { - SetFuzzedErrNo(m_fuzzed_data_provider, recv_errnos); - } - return r; + + if (copied_so_far == len) { + return copied_so_far; } - // `random_bytes` might exceed the size of `buf` if e.g. Recv is called with - // len=N and MSG_PEEK first and afterwards called with len=M (M < N) and - // without MSG_PEEK. - size_t recv_len{std::min(random_bytes.size(), len)}; - std::memcpy(buf, random_bytes.data(), recv_len); - if (pad_to_len_bytes) { - if (len > random_bytes.size()) { - std::memset((char*)buf + random_bytes.size(), 0, len - random_bytes.size()); - } - return len; + + auto new_data = ConsumeRandomLengthByteVector(m_fuzzed_data_provider, len - copied_so_far); + if (new_data.empty()) return copied_so_far; + + std::memcpy(reinterpret_cast(buf) + copied_so_far, new_data.data(), new_data.size()); + copied_so_far += new_data.size(); + + if ((flags & MSG_PEEK) != 0) { + m_peek_data.insert(m_peek_data.end(), new_data.begin(), new_data.end()); } - if (m_fuzzed_data_provider.ConsumeBool() && std::getenv("FUZZED_SOCKET_FAKE_LATENCY") != nullptr) { - std::this_thread::sleep_for(std::chrono::milliseconds{2}); + + if (copied_so_far == len || m_fuzzed_data_provider.ConsumeBool()) { + return copied_so_far; } - return recv_len; + + // Pad to len bytes. + std::memset(reinterpret_cast(buf) + copied_so_far, 0x0, len - copied_so_far); + + return len; } int FuzzedSock::Connect(const sockaddr*, socklen_t) const -- cgit v1.2.3