Unvalidated Negative `buffer_size` in `GzipOutputStream::Init` Causes Allocation-Size-Too-Big Crash

[FuzzAnything-ORG]

Unvalidated Negative buffer_size in GzipOutputStream::Init Causes Allocation-Size-Too-Big Crash

Summary

GzipOutputStream::Init assigns options.buffer_size (type int) directly to
input_buffer_length_ (type size_t) without any validation. Passing -1 silently
wraps to 0xFFFFFFFFFFFFFFFF, causing operator new to request ~16 EB of memory and
aborting the process. The symmetric API GzipInputStream correctly guards against this
with an explicit -1 sentinel check, making this an API-consistency bug and a missing
input-validation defect.

Version

$ git describe
v35-dev-76-ga57778883

Description

GzipOutputStream::Options::buffer_size is declared as int (header line 108). Its
default constructor (gzip_stream.cc:194) initialises it to kDefaultBufferSize
(65536). No documentation or contract for GzipOutputStream states that -1 is a
valid sentinel value.

However, GzipInputStream's constructor explicitly documents and handles -1 as
"use default" (gzip_stream.h:54, gzip_stream.cc:48–51):

// GzipInputStream — gzip_stream.cc:48
if (buffer_size == -1) {
    output_buffer_length_ = kDefaultBufferSize;   // ← safe path
} else {
    output_buffer_length_ = buffer_size;
}

GzipOutputStream::Init lacks the equivalent guard (gzip_stream.cc:213–214):

// GzipOutputStream::Init — gzip_stream.cc:213
input_buffer_length_ = options.buffer_size;            // int(-1) → size_t(0xFFFFFFFFFFFFFFFF)
input_buffer_ = operator new(input_buffer_length_);    // requests 18446744073709551615 bytes → abort

The unsafe implicit conversion int → size_t turns -1 into SIZE_MAX, and
operator new(SIZE_MAX) exceeds the ASAN allocator ceiling
(0x10000000000, 64 GB), aborting the process. Without ASAN the same call
throws std::bad_alloc, crashing any caller that does not catch it.

PoC Code

// Minimized PoC for allocation-size-too-big crash in GzipOutputStream::Init
// Root cause: buffer_size=-1 is cast to size_t (0xffffffffffffffff), causing
// operator new to request an impossibly large allocation.

#include <cstddef>
#include <cstdint>

#include "google/protobuf/io/gzip_stream.h"
#include "google/protobuf/io/zero_copy_stream_impl_lite.h"

using google::protobuf::io::GzipOutputStream;
using google::protobuf::io::ArrayOutputStream;

extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
    uint8_t buf[256];
    ArrayOutputStream array_output(buf, sizeof(buf));

    GzipOutputStream::Options opts;
    opts.buffer_size = -1;  // -1 wraps to SIZE_MAX when cast to size_t

    // CRASH: GzipOutputStream::Init attempts new char[SIZE_MAX]
    GzipOutputStream gzip_output(&array_output, opts);

    return 0;
}

Reproduction Steps

export OUTPUT=<path-to-protobuf-sanitizer-build>

clang++ -g -O0 -fstandalone-debug -fno-omit-frame-pointer \
    -fsanitize=address,undefined,fuzzer \
    -I$OUTPUT/build/sanitizer/include \
    poc.cpp -o poc \
    -Wl,--start-group $OUTPUT/build/sanitizer/lib/lib*.a -Wl,--end-group -lz

./poc

Stack Trace

==20997==ERROR: AddressSanitizer: requested allocation size 0xffffffffffffffff (0x800 after adjustments for alignment, red zones etc.) exceeds maximum supported size of 0x10000000000 (thread T0)
    #0 0x5aeba77d8841 in operator new(unsigned long) (/root/FuzzAgent/output/protobuf/crash_reports/crash_005/poc+0x2ca841) (BuildId: ef1a049216f2bb47ddf4898a65333515cc3d1171)
    #1 0x5aeba77de8b5 in google::protobuf::io::GzipOutputStream::Init(google::protobuf::io::ZeroCopyOutputStream*, google::protobuf::io::GzipOutputStream::Options const&) /root/src/protobuf/src/google/protobuf/io/gzip_stream.cc:214:19
    #2 0x5aeba77df99d in google::protobuf::io::GzipOutputStream::GzipOutputStream(google::protobuf::io::ZeroCopyOutputStream*, google::protobuf::io::GzipOutputStream::Options const&) /root/src/protobuf/src/google/protobuf/io/gzip_stream.cc:204:3
    #3 0x5aeba77d9f9e in LLVMFuzzerTestOneInput /root/FuzzAgent/output/protobuf/crash_reports/crash_005/poc.cpp:22:22

Suggested Fix

Mirror the -1 sentinel handling from GzipInputStream in GzipOutputStream::Init
(gzip_stream.cc:213):

 void GzipOutputStream::Init(ZeroCopyOutputStream* sub_stream,
                             const Options& options) {
   sub_stream_ = sub_stream;
   sub_data_ = nullptr;
   sub_data_size_ = 0;

-  input_buffer_length_ = options.buffer_size;
+  input_buffer_length_ = (options.buffer_size == -1)
+                             ? kDefaultBufferSize
+                             : static_cast<size_t>(options.buffer_size);
   input_buffer_ = operator new(input_buffer_length_);

---

Signed-off-by: FuzzAnything fuzzanything@gmail.com

[esrauchg]

Thanks for raising this topic.

The code looks correct to me: the user really shouldn't explicitly pass a -1 through any of these paths, the asymmetry between the reader and writer is just because the reader has a default-argument int.

explicit GzipInputStream(ZeroCopyInputStream* sub_stream,
                           Format format = AUTO, int buffer_size = -1);

The user should either pass a non-negative buffer_size or not write one at all. It happens to use a sentinel value of -1 here and a user could type it but it wouldn't make much sense to do so.

On the the OutputStream it has an Options struct instead, the Options struct has a default constructor instead which initializes it. In this case again the user should never set a sentinel, if they want to use a default and override one value they should just use the default ctor and then change a corresponding other one of the arguments.

And then from there, application code passing negative values for int size is just UB here in general. After the proposed change that would special case -1, they also could just write -2 and see a crash as well.

So I think this is working as intended and not a bug.