madler / zlib

A massively spiffy yet delicately unobtrusive compression library.
http://zlib.net/
Other
5.55k stars 2.42k forks source link

issue with gzfread #887

Closed tregua87 closed 8 months ago

tregua87 commented 8 months ago

Hi. I found an unexpected behavior in gzfread while fuzzing zlib with libFuzzer.

I prepared a PoC to replicate the crash. I also include building details.

Zlib commit: 09155eaa2f9270dc4ed1fa13e2b4b2613e6e4851 clang:

$clang --version
Ubuntu clang version 12.0.0-3ubuntu1~20.04.5
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

Env: Docker + ubuntu:20.04

The driver code (poc.cc):

#include <zlib.h>

#include <string.h>
#include <stdlib.h>
#include <stdio.h>
#include <time.h>
#include <stdint.h>

#define MIN_SEED_SIZE 1160

extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t Size) {
    if (Size < MIN_SEED_SIZE) return 0;
    size_t size_t_s0[1];
    char *filename = "tmp_file.bin";
    size_t in_buffer_size[128];
    char mode[128];
    char *in_buffer[1] = { 0 };
    gzFile_s *afile[1] = { 0 };
    memcpy(mode, data, sizeof(mode));data += sizeof(mode);
    mode[sizeof(mode) - 1] = 0;

    //file init
    memcpy(size_t_s0, data, sizeof(size_t_s0));data += sizeof(size_t_s0);
    FILE *tmp_file = fopen(filename, "w");
    fwrite(data, size_t_s0[0], 1, tmp_file);
    fclose(tmp_file);data += size_t_s0[0];

    //dyn array init
    memcpy(in_buffer_size, data, sizeof(in_buffer_size));data += sizeof(in_buffer_size);
    in_buffer[0] = (char*)malloc(in_buffer_size[0]);
    memcpy(in_buffer[0], data, in_buffer_size[0]);
    data += in_buffer_size[0];
    if (in_buffer_size[0] > 0) in_buffer[0][in_buffer_size[0] - 1] = 0;

    afile[0] =  gzopen64(filename, mode);
    if (afile[0] == 0) goto clean_up;
    size_t_s0[0] =  gzfread(in_buffer[0], in_buffer_size[0], in_buffer_size[0], afile[0]);

clean_up:
    if (in_buffer[0] != 0) free(in_buffer[0]);
    if (afile[0] != 0) gzclearerr(afile[0]);

    return 0;
}

Build library:

cmake .. -DCMAKE_INSTALL_PREFIX=${WORK} -DBUILD_SHARED_LIBS=off \
        -DENABLE_STATIC=on -DCMAKE_BUILD_TYPE=Debug \
        -DCMAKE_C_FLAGS_DEBUG="-fsanitize=fuzzer-no-link,address -g" \
        -DCMAKE_CXX_FLAGS_DEBUG="-fsanitize=fuzzer-no-link,address -g" \
        -DBENCHMARK_ENABLE_GTEST_TESTS=off \
        -DBENCHMARK_ENABLE_INSTALL=off

make -j$(nproc) clean
make -j$(nproc)
make install

Build driver:

clang++ -g -std=c++11 -fsanitize=fuzzer,address -I/${WORK}/include \
        poc.cc -Wl,--whole-archive ${WORK}/lib/libz.a -Wl,--no-whole-archive \
        -lz -ljpeg -Wl,-Bstatic -llzma -Wl,-Bdynamic -lstdc++ -o poc

Replicate the crash:

./poc  ./crash-916f7b73270144575b04587bae5684e27e44d5c1

The crash seed: crash.zip

madler commented 8 months ago

Your code has a bug, where the same value is used in the second and third parameters of gzfread(), resulting in a request to read data into memory beyond its allocation. Those two parameters being the same buffer size makes no sense, as, just like fread(), they are multiplied to get the length of the buffer to read into. Your code squares the buffer size and tells gzread() that that's the size of the buffer at the pointer in the first parameter. In this particular case, gzfread() is getting a two-byte buffer, and being told to read four bytes into it. If I correct the second parameter of gzfread() to 1, then all is well.

There is no issue here in zlib.