diff --git a/src/fuzzers/FuzzBuffer.cc b/src/fuzzers/FuzzBuffer.cc index cc25550983..60ae96ca27 100644 --- a/src/fuzzers/FuzzBuffer.cc +++ b/src/fuzzers/FuzzBuffer.cc @@ -17,31 +17,28 @@ bool zeek::FuzzBuffer::Valid() const return true; } -int zeek::FuzzBuffer::Next(const unsigned char** chunk, size_t* len, bool* is_orig) +std::optional zeek::FuzzBuffer::Next() { if ( begin == end ) - { - *chunk = nullptr; - *len = 0; - return 0; - } + return {}; auto pos = (const unsigned char*)memmem(begin, end - begin, PKT_MAGIC, PKT_MAGIC_LEN); if ( ! pos ) - return -1; + return {}; begin += PKT_MAGIC_LEN; auto remaining = end - begin; if ( remaining < 2 ) - return -2; + return {}; - *is_orig = begin[0] & 0x01; + Chunk rval; + rval.is_orig = begin[0] & 0x01; begin += 1; - *chunk = begin; + auto chunk_begin = begin; auto next = (const unsigned char*)memmem(begin, end - begin, PKT_MAGIC, PKT_MAGIC_LEN); @@ -51,6 +48,19 @@ int zeek::FuzzBuffer::Next(const unsigned char** chunk, size_t* len, bool* is_or else begin = end; - *len = begin - *chunk; - return 0; + rval.size = begin - chunk_begin; + + if ( rval.size ) + { + // The point of allocating a new buffer here is to better detect + // analyzers that may over-read within a chunk -- ASan wouldn't + // complain if that happens to land within the full input buffer + // provided by the fuzzing engine, but will if we allocate a new buffer + // for each chunk. + rval.data = std::make_unique(rval.size); + memcpy(rval.data.get(), chunk_begin, rval.size); + return rval; + } + + return {}; } diff --git a/src/fuzzers/FuzzBuffer.h b/src/fuzzers/FuzzBuffer.h index 0406be181c..81770857e6 100644 --- a/src/fuzzers/FuzzBuffer.h +++ b/src/fuzzers/FuzzBuffer.h @@ -1,6 +1,8 @@ #pragma once #include +#include +#include namespace zeek { @@ -15,6 +17,13 @@ namespace zeek { * send along to an analyzers Deliver method. */ class FuzzBuffer { +public: + + struct Chunk { + std::unique_ptr data; + size_t size; + bool is_orig; + }; static constexpr int PKT_MAGIC_LEN = 4; static constexpr unsigned char PKT_MAGIC[PKT_MAGIC_LEN + 1] = "\1PKT"; @@ -35,13 +44,9 @@ class FuzzBuffer { bool Valid() const; /** - * Finds the next chunk of data to pass along to an analyzer. - * @param chunk the data chunk to return - * @param len the size of the chunk returned in *chunk* - * @param is_orig whether returned chunk is from originator or responder - * @return a value less than zero if a chunk could not be extracted + * @return the next chunk to deliver, if one could be extracted */ - int Next(const unsigned char** chunk, size_t* len, bool* is_orig); + std::optional Next(); private: diff --git a/src/fuzzers/pop3-fuzzer.cc b/src/fuzzers/pop3-fuzzer.cc index 89bd1e00b5..394a6f508a 100644 --- a/src/fuzzers/pop3-fuzzer.cc +++ b/src/fuzzers/pop3-fuzzer.cc @@ -53,28 +53,22 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) auto conn = add_connection(); auto a = add_analyzer(conn); - const unsigned char* chunk; - size_t chunk_size; - bool is_orig; - for ( ; ; ) { - auto err = fb.Next(&chunk, &chunk_size, &is_orig); + auto chunk = fb.Next(); - if ( err ) - break; - - if ( chunk_size == 0 ) + if ( ! chunk ) break; try { - a->DeliverStream(chunk_size, chunk, is_orig); + a->DeliverStream(chunk->size, chunk->data.get(), chunk->is_orig); } catch ( const binpac::Exception& e ) { } + chunk = {}; mgr.Drain(); }