Heap-buffer-overflow in pngwrite.c:856:4 in png_write_png

[PromptFuzz]

Summary

A heap-buffer-overflow found in png_write_png, it could cause 200 bytes out-of-bound read on heap!
If the transform flags of png_write_png could be controlled by remote attackers, it could cause information leak or further exploition.

POC

#include <png.h>
#include <stdlib.h>
#include <string.h>
#include <stdint.h>
#include <vector>
#include <fstream>
#include <iostream>

extern "C" int LLVMFuzzerTestOneInput(const uint8_t* f_data, size_t f_size) {

    FILE *in_file = fmemopen((void *)f_data, f_size, "rb");
    FILE *out_file = fopen("output_file", "wb");

    // Create libpng read and write structures
    png_structp read_ptr = png_create_read_struct(PNG_LIBPNG_VER_STRING, NULL, NULL, NULL);
    png_structp write_ptr = png_create_write_struct(PNG_LIBPNG_VER_STRING, NULL, NULL, NULL);


    png_infop info_ptr = png_create_info_struct(read_ptr);


    // Set up error handling
    if (setjmp(png_jmpbuf(read_ptr))) {
        png_destroy_read_struct(&read_ptr, &info_ptr, NULL);
        png_destroy_write_struct(&write_ptr, NULL);
        fclose(in_file);
        fclose(out_file);
	    return 0;
    }

    // Set up the input/output functions
    png_set_read_fn(read_ptr, (png_voidp)in_file, [](png_structp png_ptr, png_bytep data, size_t size){
        fread(data, 1, size, (FILE *)png_get_io_ptr(png_ptr));
    });

    png_set_write_fn(write_ptr, (png_voidp)out_file, [](png_structp png_ptr, png_bytep data, size_t size){
        fwrite(data, 1, size, (FILE *)png_get_io_ptr(png_ptr));
    }, NULL);
    
    // Read the PNG data
    png_read_png(read_ptr, info_ptr, PNG_TRANSFORM_IDENTITY, NULL);

    // Write the PNG data
    png_write_png(write_ptr, info_ptr, 0x7089, NULL);
    return 0;
}

POC Input

Crash input

Version

Found on version of 2023/06/07. Reproducible on the master branch.

Compile commands

Build the libpng

# export the flags.
    SANITIZER_FLAGS="-O2 -fsanitize=address,undefined -fsanitize-address-use-after-scope -g "
    FUZZER_FLAGS="-fsanitize=fuzzer-no-link -fno-omit-frame-pointer -g -DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION $SANITIZER_FLAGS"
    export CFLAGS="${CFLAGS:-} $FUZZER_FLAGS"
    export CXXFLAGS="${CXXFLAGS:-} $FUZZER_FLAGS"
# build the libpng library.
    cd $SRC/libpng
    autoreconf -f -i
    ./configure
    make -j$(nproc) clean
    make -j$(nproc) libpng16.la

Compile the poc program

clang++ -fsanitize=fuzzer -O0 -g -fsanitize=address,undefined -ftrivial-auto-var-init=zero -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang -I/src/libpng/include  poc.cc -o test.out /src/libpng/lib/libpng16.so

Reproduce Step

./test.out triger_input

ASAN INFO

==389375==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x615000000780 at pc 0x55a5feb098d7 bp 0x7ffcd74093d0 sp 0x7ffcd7408ba0
READ of size 1024 at 0x615000000780 thread T0
    #0 0x55a5feb098d6 in __asan_memcpy ../llvm/llvm-project/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp:22:3
    #1 0x7fda0942515d in png_write_row ../libpng/src/libpng/pngwrite.c:856:4
    #2 0x7fda0942703b in png_write_image ../src/libpng/pngwrite.c:640:10
    #3 0x7fda0942fb08 in png_write_png ../libpng/src/libpng/pngwrite.c:1469:4
    #4 0x55a5feb47330 in LLVMFuzzerTestOneInput ../id_005327.cc:57:5
    #5 0x55a5fea12e72 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /llvm/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:612:13
    #6 0x55a5fe9fe513 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /llvm/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:324:6

[PromptFuzz]

Here are the PoC program and PoC input:
crash_poc.tar.gz

You can reproduce this issue by running:

./poc.out triger_input

[jbowler]

Try using png_init_io

[jbowler]

No prob without the transform.

[jbowler]

No problem without either. I think I'll do this once more. Here's the simplified test case:

#include <png.h>

int main(void) {
    png_structp r = png_create_read_struct(PNG_LIBPNG_VER_STRING, 0, 0, 0);
    png_structp w = png_create_write_struct(PNG_LIBPNG_VER_STRING, 0, 0, 0);
    png_infop i = png_create_info_struct(r);
    png_init_io(r, stdin);
    png_init_io(w, stdout);
    png_read_png(r, i, 0, 0);
    png_write_png(w, i, 0x7089, 0);

    return 0;
}

That's correct apart from the bogus transform but no problem.

[jbowler]

@ctruta: for your benefit, I feel the OP knows all this, the APIs png_(read|write)_png take transforms from the same set but "read" transforms transform the input PNG into the desired state while "write" transforms make statements about the input.

In this case the write transform is called with data that doesn't require any transforms (it's already in the format specified by the IHDR) however the programmer has requested transforms:

PNG_TRANSFORM_STRIP_16       0x0001    /* read only */
PNG_TRANSFORM_PACKSWAP       0x0008    /* read and write */
PNG_TRANSFORM_BGR            0x0080    /* read and write */
PNG_TRANSFORM_STRIP_FILLER_AFTER 0x1000 /* write only */
PNG_TRANSFORM_GRAY_TO_RGB   0x2000      /* read only */
PNG_TRANSFORM_EXPAND_16     0x4000      /* read only */

The supplied test case is 16bit gray with some spurious data at the end (i.e. apart from the extra data it is valid).

Some of the transforms are valid and some are no-op. All the requested "read only" transforms should be no-ops, but it's still a programmer error specifying them.

"packswap" is fine; it just borks the image, but I think its one of Willem's test images and the high and low bytes are equal (so no effect). BGR should be ignored (requested output is 16-bit gray).

STRIP_FILLER_AFTER is, however, a big problem: it states that the input data has "Gf" format, two samples per pixel. Consequently png_write_png reads twice as much data from each row as is available.

@PromptFuzz even states this. In fact the argument to png_write_png is an array of 256 pointers to 512 bytes of memory and png_write_png reads 512 bytes beyond the end of each. It doesn't do that at the memcpy; it happens earlier in the transforms.

So this is clearly a programmer error. The argument is that this might be done by a remote attacker is ridiculous; that's an app bug, not a libpng bug. The app needs to pass a consistent set of arguments to png_write_png.