From 227ee9f107b5f7f7110a3e30d8b3562407722fa3 Mon Sep 17 00:00:00 2001 From: Max Moroz Date: Fri, 6 Sep 2019 09:47:12 -0700 Subject: [PATCH] [mpg123] Use FuzzedDataProvider instead of byte_stream. (#2810) --- projects/mpg123/Dockerfile | 5 +- projects/mpg123/byte_stream.h | 129 ------------------------------- projects/mpg123/decode_fuzzer.cc | 22 +++--- 3 files changed, 12 insertions(+), 144 deletions(-) delete mode 100644 projects/mpg123/byte_stream.h diff --git a/projects/mpg123/Dockerfile b/projects/mpg123/Dockerfile index 31a407588..81a2ec6df 100644 --- a/projects/mpg123/Dockerfile +++ b/projects/mpg123/Dockerfile @@ -21,7 +21,4 @@ RUN wget https://www.mpg123.de/snapshot RUN tar -xvf snapshot RUN mv mpg123* mpg123 WORKDIR $SRC -COPY read_fuzzer.c $SRC/ -COPY decode_fuzzer.cc $SRC/ -COPY byte_stream.h $SRC/ -COPY build.sh $SRC/ +COPY read_fuzzer.c decode_fuzzer.cc build.sh $SRC/ diff --git a/projects/mpg123/byte_stream.h b/projects/mpg123/byte_stream.h deleted file mode 100644 index 8fb9f3c54..000000000 --- a/projects/mpg123/byte_stream.h +++ /dev/null @@ -1,129 +0,0 @@ -// Copyright 2018 Google Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#ifndef BYTE_STREAM_H_ -#define BYTE_STREAM_H_ - -#include -#include -#include -#include -#include - -// Wrapper for fuzzer input strings that helps consume and interpret the data -// as a sequence of values, such as strings and PODs. -class ByteStream { - public: - // Does not take ownership of data. - ByteStream(const uint8_t* data, size_t size) - : data_(data), size_(size), position_(0) {} - - ByteStream(const ByteStream&) = delete; - ByteStream& operator=(const ByteStream&) = delete; - - // Returns a string. Strings are obtained from the byte stream by reading a - // size_t N followed by N char elements. If there are fewer than N bytes left - // in the stream, this returns as many bytes as are available. - std::string GetNextString(); - - // The following GetNext{integer type} functions all return the next - // sizeof(integer type) bytes in the stream or 0 if there is insufficient - // capacity. - size_t GetNextSizeT() { return ConsumeCopyOrDefault(0); } - int GetNextInt() { return ConsumeCopyOrDefault(0); } - uint8_t GetNextUint8() { return ConsumeCopyOrDefault(0); } - int64_t GetNextInt64() { return ConsumeCopyOrDefault(0); } - - // Returns an integer in the range [0,n) for n > 0 and consumes up to - // sizeof(int) bytes. For n<=0, returns 0 and consumes 0 bytes. - int GetNextInt(int n); - - // The remaining capacity of the ByteStream. - size_t capacity() const { return size_ - position_; } - - // Returns data_ + position_ and then advances position_ by requested bytes. - // - // This is the canonical way for the class to request regions of memory - // or to advance the position by requested bytes. This operation is unchecked - // for maintaining that position_ <= size_. Requesting 0 bytes always - // succeeds. - const uint8_t* UncheckedConsume(size_t requested) { - const uint8_t* region = data_ + position_; - position_ += requested; - return region; - } - - private: - - // Directly initialize T by copying sizeof(T) bytes into results if there is - // sufficient capacity in the stream. If there is not sufficient capacity - // result is unmodified. - template - void ConsumeBytesByCopy(T* result) { - constexpr size_t type_size = sizeof(T); - if (type_size <= capacity()) { - const uint8_t* region = UncheckedConsume(type_size); - memcpy(static_cast(result), region, type_size); - } else { - // Consume the remainder of data_. - UncheckedConsume(capacity()); - } - } - - // A helper function for using ConsumeBytesByCopy and returning a default - // value `t` if there is insufficient capacity to read a full `T`. T should - // probably be a primitive type. - template - T ConsumeCopyOrDefault(T t) { - ConsumeBytesByCopy(&t); - return t; - } - - const uint8_t* data_; - const size_t size_; - size_t position_; -}; - -inline std::string ByteStream::GetNextString() { - const size_t requested_size = GetNextSizeT(); - const size_t consumed_size = std::min(requested_size, capacity()); - const uint8_t* selection = UncheckedConsume(consumed_size); - return std::string(reinterpret_cast(selection), consumed_size); -} - -inline int ByteStream::GetNextInt(int n) { - if (n <= 0) { - return 0; - } - // We grab as few bytes as possible as n will often be fixed. - int selection = 0; - if (n <= std::numeric_limits::max()) { - selection = static_cast(GetNextUint8()); - } else if (n <= std::numeric_limits::max()) { - selection = ConsumeCopyOrDefault(0); - } else { - selection = GetNextInt(); - } - - // Take the absolute value of selection w/o undefined behavior. - // If selection is INT_MIN, return 0. - if (selection == std::numeric_limits::min()) { - selection = 0; - } else if (selection < 0) { - selection = -selection; - } - return selection % n; -} - -#endif // BYTE_STREAM_H_ diff --git a/projects/mpg123/decode_fuzzer.cc b/projects/mpg123/decode_fuzzer.cc index 0f0e662f1..9dafb20f3 100644 --- a/projects/mpg123/decode_fuzzer.cc +++ b/projects/mpg123/decode_fuzzer.cc @@ -1,3 +1,5 @@ +#include + #include #include #include @@ -5,7 +7,6 @@ #include #include "mpg123.h" -#include "byte_stream.h" extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { static bool initialized = false; @@ -32,20 +33,19 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { size_t output_written = 0; // Initially, start by feeding the decoder more data. int decode_ret = MPG123_NEED_MORE; - ByteStream stream(data, size); + FuzzedDataProvider provider(data, size); while ((decode_ret != MPG123_ERR)) { if (decode_ret == MPG123_NEED_MORE) { - if (stream.capacity() == 0) { + if (provider.remaining_bytes() == 0) { break; } - const size_t next_size = std::min(stream.GetNextSizeT(), stream.capacity()); - uint8_t* next_input = (uint8_t*)malloc(sizeof(uint8_t) * next_size); - memcpy(next_input, stream.UncheckedConsume(next_size), next_size); - decode_ret = mpg123_decode( - handle, reinterpret_cast(next_input), - next_size, output_buffer.data(), output_buffer.size(), - &output_written); - free(next_input); + const size_t next_size = provider.ConsumeIntegralInRange( + 0, + provider.remaining_bytes()); + auto next_input = provider.ConsumeBytes(next_size); + decode_ret = mpg123_decode(handle, next_input.data(), next_input.size(), + output_buffer.data(), output_buffer.size(), + &output_written); } else if (decode_ret != MPG123_ERR && decode_ret != MPG123_NEED_MORE) { decode_ret = mpg123_decode(handle, nullptr, 0, output_buffer.data(), output_buffer.size(), &output_written);