From 634a20b429840caf50de49eaaf5ecfad99123cfb Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Wed, 30 Sep 2026 09:43:29 +0200 Subject: [PATCH 1/3] GH-51649: [C++][Parquet] Tell encoding fuzzer to skip inputs with invalid parameters `LLVMFuzzerTestOneInput` allows two return values: 0 and -1. While 0 allows for the input to be (potentially) saved in the corpus, -1 tells the fuzzer not to save it. In the words of the [fine documentation](https://llvm.org/docs/LibFuzzer.html#rejecting-unwanted-inputs): > It may be desirable to reject some inputs, i.e. to not add them to the corpus. > For example, when fuzzing an API consisting of parsing and other logic, one may want > to allow only those inputs into the corpus that parse successfully. > If the fuzz target returns -1 on a given input, libFuzzer will not add that > input to the corpus, regardless of what coverage it triggers. --- cpp/src/arrow/csv/fuzz.cc | 3 +- cpp/src/arrow/ipc/file_fuzz.cc | 3 +- cpp/src/arrow/ipc/stream_fuzz.cc | 3 +- cpp/src/arrow/util/fuzz_internal.cc | 22 +++++++--- cpp/src/arrow/util/fuzz_internal.h | 29 ++++++++++++- cpp/src/parquet/arrow/encoding_fuzz.cc | 3 +- cpp/src/parquet/arrow/fuzz.cc | 3 +- .../parquet/arrow/fuzz_encoding_internal.cc | 41 +++++++++++++++---- .../parquet/arrow/fuzz_encoding_internal.h | 4 +- cpp/src/parquet/decoder.cc | 41 +++++++++++++++---- cpp/src/parquet/encoding.h | 8 +++- cpp/src/parquet/encoding_test.cc | 2 + 12 files changed, 126 insertions(+), 36 deletions(-) diff --git a/cpp/src/arrow/csv/fuzz.cc b/cpp/src/arrow/csv/fuzz.cc index 39148f1aacd..2219efa2cb5 100644 --- a/cpp/src/arrow/csv/fuzz.cc +++ b/cpp/src/arrow/csv/fuzz.cc @@ -121,6 +121,5 @@ Status FuzzCsvReader(const uint8_t* data, int64_t size) { extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { auto status = arrow::csv::FuzzCsvReader(data, static_cast(size)); - arrow::internal::LogFuzzStatus(status, data, size); - return 0; + return arrow::internal::LogFuzzStatus(status, data, size); } diff --git a/cpp/src/arrow/ipc/file_fuzz.cc b/cpp/src/arrow/ipc/file_fuzz.cc index c3ce9c2dea6..2d2a924befc 100644 --- a/cpp/src/arrow/ipc/file_fuzz.cc +++ b/cpp/src/arrow/ipc/file_fuzz.cc @@ -24,6 +24,5 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { auto status = arrow::ipc::internal::FuzzIpcFile(data, static_cast(size)); - arrow::internal::LogFuzzStatus(status, data, static_cast(size)); - return 0; + return arrow::internal::LogFuzzStatus(status, data, static_cast(size)); } diff --git a/cpp/src/arrow/ipc/stream_fuzz.cc b/cpp/src/arrow/ipc/stream_fuzz.cc index 04b12863bed..8aaef771260 100644 --- a/cpp/src/arrow/ipc/stream_fuzz.cc +++ b/cpp/src/arrow/ipc/stream_fuzz.cc @@ -24,6 +24,5 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { auto status = arrow::ipc::internal::FuzzIpcStream(data, static_cast(size)); - arrow::internal::LogFuzzStatus(status, data, static_cast(size)); - return 0; + return arrow::internal::LogFuzzStatus(status, data, static_cast(size)); } diff --git a/cpp/src/arrow/util/fuzz_internal.cc b/cpp/src/arrow/util/fuzz_internal.cc index 28d210333dd..813652f49ae 100644 --- a/cpp/src/arrow/util/fuzz_internal.cc +++ b/cpp/src/arrow/util/fuzz_internal.cc @@ -34,7 +34,7 @@ MemoryPool* fuzzing_memory_pool() { return pool.get(); } -void LogFuzzStatus(const Status& st, const uint8_t* data, int64_t size) { +int LogFuzzStatus(const FuzzStatus& st, const uint8_t* data, int64_t size) { static const int kVerbosity = []() { auto maybe_env_value = GetEnvVarInteger("ARROW_FUZZING_VERBOSITY", /*min_value=*/0, /*max_value=*/1); @@ -48,13 +48,23 @@ void LogFuzzStatus(const Status& st, const uint8_t* data, int64_t size) { return 0; }(); - if (!st.ok() && kVerbosity >= 1) { - ARROW_LOG(WARNING) << "Fuzzing input with size=" << size - << " failed: " << st.ToString(); - } else if (st.IsOutOfMemory()) { + const auto& reason = FuzzReason(st); + bool skip_input = std::holds_alternative(st); + if (skip_input) { + if (kVerbosity >= 1) { + ARROW_LOG(WARNING) << "Skipping input with size=" << size << ": " + << reason.ToString(); + } + } else if (reason.IsOutOfMemory()) { ARROW_LOG(WARNING) << "Fuzzing input with size=" << size - << " hit allocation failure: " << st.ToString(); + << " hit allocation failure: " << reason.ToString(); + } else { + if (!reason.ok() && kVerbosity >= 1) { + ARROW_LOG(WARNING) << "Fuzzing input with size=" << size + << " failed: " << reason.ToString(); + } } + return skip_input ? -1 : 0; } } // namespace arrow::internal diff --git a/cpp/src/arrow/util/fuzz_internal.h b/cpp/src/arrow/util/fuzz_internal.h index 5280b7ec1ff..685570e5cf7 100644 --- a/cpp/src/arrow/util/fuzz_internal.h +++ b/cpp/src/arrow/util/fuzz_internal.h @@ -18,12 +18,33 @@ #pragma once #include +#include +#include "arrow/status.h" #include "arrow/type_fwd.h" #include "arrow/util/macros.h" namespace arrow::internal { +// A tag used to signal the fuzzing engine that this input should not be saved +// into the corpus. +struct SkipFuzzInput { + Status reason; +}; + +// The Status alternative holds a regular fuzzing success or failure, +// while the SkipFuzzInput alternative holds a setup failure (e.g. invalid +// fuzzing parameters encoded in the payload). +using FuzzStatus = std::variant; + +inline const Status& FuzzReason(const FuzzStatus& st) { + struct Visitor { + const Status& operator()(const SkipFuzzInput& v) { return v.reason; } + const Status& operator()(const Status& v) { return v; } + }; + return std::visit(Visitor{}, st); +} + // The default rss_limit_mb on OSS-Fuzz is 2560 MB and we want to fail allocations // before that limit is reached, otherwise the fuzz target gets killed (GH-48105). constexpr int64_t kFuzzingMemoryLimit = 2200LL * 1000 * 1000; @@ -32,6 +53,12 @@ constexpr int64_t kFuzzingMemoryLimit = 2200LL * 1000 * 1000; ARROW_EXPORT MemoryPool* fuzzing_memory_pool(); /// Optionally log the outcome of fuzzing an input -ARROW_EXPORT void LogFuzzStatus(const Status&, const uint8_t* data, int64_t size); +/// +/// Returns the integer code to return from LLVMFuzzerTestOneInput. +ARROW_EXPORT int LogFuzzStatus(const FuzzStatus&, const uint8_t* data, int64_t size); + +inline int LogFuzzStatus(const Status& status, const uint8_t* data, int64_t size) { + return LogFuzzStatus(FuzzStatus{status}, data, size); +} } // namespace arrow::internal diff --git a/cpp/src/parquet/arrow/encoding_fuzz.cc b/cpp/src/parquet/arrow/encoding_fuzz.cc index 64095795e28..3a968a3df87 100644 --- a/cpp/src/parquet/arrow/encoding_fuzz.cc +++ b/cpp/src/parquet/arrow/encoding_fuzz.cc @@ -22,6 +22,5 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { auto status = parquet::fuzzing::internal::FuzzEncoding(data, static_cast(size)); - arrow::internal::LogFuzzStatus(status, data, static_cast(size)); - return 0; + return arrow::internal::LogFuzzStatus(status, data, static_cast(size)); } diff --git a/cpp/src/parquet/arrow/fuzz.cc b/cpp/src/parquet/arrow/fuzz.cc index c2d8021a5a5..9cf26eb1f2b 100644 --- a/cpp/src/parquet/arrow/fuzz.cc +++ b/cpp/src/parquet/arrow/fuzz.cc @@ -21,6 +21,5 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { auto status = parquet::fuzzing::internal::FuzzReader(data, static_cast(size)); - arrow::internal::LogFuzzStatus(status, data, static_cast(size)); - return 0; + return arrow::internal::LogFuzzStatus(status, data, static_cast(size)); } diff --git a/cpp/src/parquet/arrow/fuzz_encoding_internal.cc b/cpp/src/parquet/arrow/fuzz_encoding_internal.cc index a0077391167..2984a0e7294 100644 --- a/cpp/src/parquet/arrow/fuzz_encoding_internal.cc +++ b/cpp/src/parquet/arrow/fuzz_encoding_internal.cc @@ -18,13 +18,16 @@ #include "parquet/arrow/fuzz_encoding_internal.h" #include +#include #include #include #include #include #include +#include #include #include +#include #include "arrow/array.h" #include "arrow/array/builder_binary.h" @@ -52,6 +55,7 @@ using ::arrow::MemoryPool; using ::arrow::Result; using ::arrow::Status; using ::arrow::TypedBufferBuilder; +using ::arrow::internal::FuzzStatus; using ::parquet::arrow::FileReader; ColumnDescriptor MakeColumnDescriptor(Type::type type, int type_length) { @@ -86,6 +90,16 @@ ARROW_PACKED_END static_assert(sizeof(PackedEncodingHeader) == kPackedEncodingHeaderSize); +template + requires std::unsigned_integral +Result ToEnum(IntType v) { + if (v < EnumType::UNDEFINED) { + return static_cast(v); + } + return Status::Invalid("Invalid enum value ", static_cast(v), " for ", + typeid(v).name()); +} + } // namespace FuzzEncodingHeader::FuzzEncodingHeader(Encoding::type source_encoding, @@ -136,9 +150,16 @@ ::arrow::Result FuzzEncodingHeader::Parse( ph.type_id >= static_cast(Type::UNDEFINED)) { return invalid_payload(); } - FuzzEncodingHeader header(static_cast(ph.source_encoding_id), - static_cast(ph.roundtrip_encoding_id), - static_cast(ph.type_id), ph.type_length, + ARROW_ASSIGN_OR_RAISE(auto source_encoding, + ToEnum(ph.source_encoding_id)); + ARROW_ASSIGN_OR_RAISE(auto roundtrip_encoding, + ToEnum(ph.roundtrip_encoding_id)); + ARROW_ASSIGN_OR_RAISE(auto type, ToEnum(ph.type_id)); + if (!IsEncodingSupported(type, source_encoding) || + !IsEncodingSupported(type, roundtrip_encoding)) { + return Status::Invalid("Unsupported encoding for type"); + } + FuzzEncodingHeader header(source_encoding, roundtrip_encoding, type, ph.type_length, ph.num_values); if ((header.type == Type::FIXED_LEN_BYTE_ARRAY) ? (header.type_length <= 0) : (header.type_length != -1)) { @@ -492,13 +513,17 @@ struct TypedFuzzEncoding { } // namespace -Status FuzzEncoding(const uint8_t* data, int64_t size) { +FuzzStatus FuzzEncoding(const uint8_t* data, int64_t size) { constexpr auto kInt32Max = std::numeric_limits::max(); - ARROW_ASSIGN_OR_RAISE(const auto parse_result, - FuzzEncodingHeader::Parse(std::span(data, size))); - const auto header = parse_result.first; - const auto encoded_data = parse_result.second; + auto maybe_parse_result = FuzzEncodingHeader::Parse(std::span(data, size)); + if (!maybe_parse_result.ok()) { + // If the fuzz encoding header is invalid, we won't save this input + // in the corpus, because it didn't exercise anything interesting. + return ::arrow::internal::SkipFuzzInput(maybe_parse_result.status()); + } + const auto header = maybe_parse_result->first; + const auto encoded_data = maybe_parse_result->second; if (encoded_data.size() > static_cast(kInt32Max)) { // Unlikely but who knows? return Status::Invalid("Fuzz payload too large"); diff --git a/cpp/src/parquet/arrow/fuzz_encoding_internal.h b/cpp/src/parquet/arrow/fuzz_encoding_internal.h index 92d6b2834cc..8ce17eadc41 100644 --- a/cpp/src/parquet/arrow/fuzz_encoding_internal.h +++ b/cpp/src/parquet/arrow/fuzz_encoding_internal.h @@ -27,6 +27,7 @@ #include "arrow/result.h" #include "arrow/status.h" #include "arrow/type_fwd.h" +#include "arrow/util/fuzz_internal.h" #include "arrow/util/macros.h" #include "parquet/platform.h" #include "parquet/types.h" @@ -76,7 +77,8 @@ struct FuzzEncodingHeader { }; /// Fuzz a payload encoded as explained in FuzzEncodingHeader -PARQUET_EXPORT ::arrow::Status FuzzEncoding(const uint8_t* data, int64_t size); +PARQUET_EXPORT ::arrow::internal::FuzzStatus FuzzEncoding(const uint8_t* data, + int64_t size); PARQUET_EXPORT ColumnDescriptor MakeColumnDescriptor(Type::type type, int type_length = -1); diff --git a/cpp/src/parquet/decoder.cc b/cpp/src/parquet/decoder.cc index 47f2b46ebaa..c63d01bb16d 100644 --- a/cpp/src/parquet/decoder.cc +++ b/cpp/src/parquet/decoder.cc @@ -2557,27 +2557,50 @@ std::unique_ptr MakeDictDecoder(Type::type type_num, } // namespace detail -std::vector SupportedEncodings(Type::type physical_type) { +namespace { + +// Discriminant allows definition of distinct functions for different initializers. +template +const std::vector& ConstVectorRef(std::initializer_list init) { + static const auto vec = std::vector(std::move(init)); + return vec; +} + +const std::vector& SupportedEncodingsRef(Type::type physical_type) { switch (physical_type) { case Type::BOOLEAN: - return {Encoding::PLAIN, Encoding::RLE}; + return ConstVectorRef({Encoding::PLAIN, Encoding::RLE}); case Type::INT32: case Type::INT64: - return {Encoding::PLAIN, Encoding::DELTA_BINARY_PACKED, - Encoding::BYTE_STREAM_SPLIT}; + return ConstVectorRef( + {Encoding::PLAIN, Encoding::DELTA_BINARY_PACKED, Encoding::BYTE_STREAM_SPLIT}); case Type::INT96: - return {Encoding::PLAIN}; + return ConstVectorRef({Encoding::PLAIN}); case Type::FLOAT: case Type::DOUBLE: - return {Encoding::PLAIN, Encoding::BYTE_STREAM_SPLIT}; + return ConstVectorRef({Encoding::PLAIN, Encoding::BYTE_STREAM_SPLIT}); case Type::FIXED_LEN_BYTE_ARRAY: - return {Encoding::PLAIN, Encoding::BYTE_STREAM_SPLIT, Encoding::DELTA_BYTE_ARRAY}; + return ConstVectorRef( + {Encoding::PLAIN, Encoding::BYTE_STREAM_SPLIT, Encoding::DELTA_BYTE_ARRAY}); case Type::BYTE_ARRAY: - return {Encoding::PLAIN, Encoding::DELTA_LENGTH_BYTE_ARRAY, - Encoding::DELTA_BYTE_ARRAY}; + return ConstVectorRef({Encoding::PLAIN, + Encoding::DELTA_LENGTH_BYTE_ARRAY, + Encoding::DELTA_BYTE_ARRAY}); default: throw ParquetException("Invalid physical type"); } } +} // namespace + +const std::vector& SupportedEncodings(Type::type physical_type) { + return SupportedEncodingsRef(physical_type); +} + +bool IsEncodingSupported(Type::type physical_type, Encoding::type encoding) { + const auto& supported_encodings = SupportedEncodingsRef(physical_type); + return std::find(supported_encodings.begin(), supported_encodings.end(), encoding) != + supported_encodings.end(); +} + } // namespace parquet diff --git a/cpp/src/parquet/encoding.h b/cpp/src/parquet/encoding.h index 9a0cc55aa18..157d67264c5 100644 --- a/cpp/src/parquet/encoding.h +++ b/cpp/src/parquet/encoding.h @@ -482,6 +482,12 @@ std::unique_ptr::Decoder> MakeTypedDecoder( /// /// Only non-dictionary encodings are returned. PARQUET_EXPORT -std::vector SupportedEncodings(Type::type physical_type); +const std::vector& SupportedEncodings(Type::type physical_type); + +/// Return whether the encoding is supported for the given physical type +/// +/// Dictionary encodings return false. +PARQUET_EXPORT +bool IsEncodingSupported(Type::type physical_type, Encoding::type encoding); } // namespace parquet diff --git a/cpp/src/parquet/encoding_test.cc b/cpp/src/parquet/encoding_test.cc index 74b422cb606..6aa9256aa8a 100644 --- a/cpp/src/parquet/encoding_test.cc +++ b/cpp/src/parquet/encoding_test.cc @@ -77,8 +77,10 @@ void TestSupportedEncodingsConsistentWith( if (std::find(supported_encodings.begin(), supported_encodings.end(), encoding) != supported_encodings.end()) { ASSERT_NO_THROW(func(type, encoding, descr)); + ASSERT_TRUE(IsEncodingSupported(type, encoding)); } else { ASSERT_THROW(func(type, encoding, descr), ParquetException); + ASSERT_FALSE(IsEncodingSupported(type, encoding)); } } } From 389b79d69c5a31e5e241a0d5ef969087b2342990 Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Wed, 30 Sep 2026 10:46:51 +0200 Subject: [PATCH 2/3] Try to appease macOS compiler --- cpp/src/parquet/arrow/fuzz_encoding_internal.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/parquet/arrow/fuzz_encoding_internal.cc b/cpp/src/parquet/arrow/fuzz_encoding_internal.cc index 2984a0e7294..95808116290 100644 --- a/cpp/src/parquet/arrow/fuzz_encoding_internal.cc +++ b/cpp/src/parquet/arrow/fuzz_encoding_internal.cc @@ -520,7 +520,7 @@ FuzzStatus FuzzEncoding(const uint8_t* data, int64_t size) { if (!maybe_parse_result.ok()) { // If the fuzz encoding header is invalid, we won't save this input // in the corpus, because it didn't exercise anything interesting. - return ::arrow::internal::SkipFuzzInput(maybe_parse_result.status()); + return ::arrow::internal::SkipFuzzInput{maybe_parse_result.status()}; } const auto header = maybe_parse_result->first; const auto encoded_data = maybe_parse_result->second; From fa401e9df00148117cd9afd832cb24b3d31b9a72 Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Wed, 30 Sep 2026 11:10:15 +0200 Subject: [PATCH 3/3] Remove superfluous enum handling --- .../parquet/arrow/fuzz_encoding_internal.cc | 28 +++++-------------- 1 file changed, 7 insertions(+), 21 deletions(-) diff --git a/cpp/src/parquet/arrow/fuzz_encoding_internal.cc b/cpp/src/parquet/arrow/fuzz_encoding_internal.cc index 95808116290..9b8917897c0 100644 --- a/cpp/src/parquet/arrow/fuzz_encoding_internal.cc +++ b/cpp/src/parquet/arrow/fuzz_encoding_internal.cc @@ -18,7 +18,6 @@ #include "parquet/arrow/fuzz_encoding_internal.h" #include -#include #include #include #include @@ -90,16 +89,6 @@ ARROW_PACKED_END static_assert(sizeof(PackedEncodingHeader) == kPackedEncodingHeaderSize); -template - requires std::unsigned_integral -Result ToEnum(IntType v) { - if (v < EnumType::UNDEFINED) { - return static_cast(v); - } - return Status::Invalid("Invalid enum value ", static_cast(v), " for ", - typeid(v).name()); -} - } // namespace FuzzEncodingHeader::FuzzEncodingHeader(Encoding::type source_encoding, @@ -150,17 +139,14 @@ ::arrow::Result FuzzEncodingHeader::Parse( ph.type_id >= static_cast(Type::UNDEFINED)) { return invalid_payload(); } - ARROW_ASSIGN_OR_RAISE(auto source_encoding, - ToEnum(ph.source_encoding_id)); - ARROW_ASSIGN_OR_RAISE(auto roundtrip_encoding, - ToEnum(ph.roundtrip_encoding_id)); - ARROW_ASSIGN_OR_RAISE(auto type, ToEnum(ph.type_id)); - if (!IsEncodingSupported(type, source_encoding) || - !IsEncodingSupported(type, roundtrip_encoding)) { - return Status::Invalid("Unsupported encoding for type"); - } - FuzzEncodingHeader header(source_encoding, roundtrip_encoding, type, ph.type_length, + FuzzEncodingHeader header(static_cast(ph.source_encoding_id), + static_cast(ph.roundtrip_encoding_id), + static_cast(ph.type_id), ph.type_length, ph.num_values); + if (!IsEncodingSupported(header.type, header.source_encoding) || + !IsEncodingSupported(header.type, header.roundtrip_encoding)) { + return invalid_payload(); + } if ((header.type == Type::FIXED_LEN_BYTE_ARRAY) ? (header.type_length <= 0) : (header.type_length != -1)) { return invalid_payload();