Menu

#278 Division by zero causes LibPNG to crash

libpng_code
closed-fixed
None
5
2018-07-16
2018-04-05
Thuan Pham
No

Hi All,

The following bug was found with AFLSmart, a fork of AFL, in a fuzzing session on LibPNG. Thanks also to Andrew Santosa, Alexandru Razvan and Marcel Böhme.

LibPNG crashes because of a division by zero in pngrutil.c. The experiment was run on Ubuntu 16.04 x86_64. Following is LibPNG version information:

commit cde1e1fe79974a37d7ef255a44dae3bfd1b34a0f
Author: Glenn Randers-Pehrson <glennrp at="" users.sourceforge.net="">
Date: Tue Mar 6 13:59:27 2018 -0600
[libpng16] Bump version to 1.6.35beta02

To reproduce:

Compilation:

Disable checksum

sed -i 's/return ((int)(crc != png_ptr->crc));/return (0);/g' pngrutil.c
autoreconf -f -i
./configure --disable-shared
make clean all check

Run:
./pngimage crash1.png

Valgrind says:

==104764== Memcheck, a memory error detector

==104764== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.

==104764== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info

==104764== Command: ./pngimage crash1.png

==104764== 

crash1.png: warning(libpng): original read: tRNS: invalid

crash1.png: warning(libpng): original read: bKGD: invalid

==104764== 

==104764== Process terminating with default action of signal 8 (SIGFPE)

==104764==  Integer divide by zero at address 0x802EDBC9D

==104764==    at 0x4BD907: png_check_chunk_length (pngrutil.c:3171)

==104764==    by 0x4BD907: png_read_chunk_header (pngrutil.c:185)

==104764==    by 0x451407: png_read_info.part.4 (pngread.c:108)

==104764==    by 0x46EA41: png_read_info (pngread.c:1046)

==104764==    by 0x46EA41: png_read_png (pngread.c:1052)

==104764==    by 0x40BF87: read_png (pngimage.c:902)

==104764==    by 0x40BF87: update_display (pngimage.c:931)

==104764==    by 0x40BF87: test_one_file (pngimage.c:1413)

==104764==    by 0x40BF87: do_test (pngimage.c:1564)

==104764==    by 0x40380D: main (pngimage.c:1668)

==104764== 

==104764== HEAP SUMMARY:

==104764==     in use at exit: 2,648 bytes in 3 blocks

==104764==   total heap usage: 5 allocs, 2 frees, 7,296 bytes allocated

Analysis:

1) The width of the PNG is set to 0x55555555.
2) The channels of the PNG is set to 0x3.
3) The bit_depth of the PNG is set to 0x8.
4) Interlaced of the PNG is set to FALSE (0).

When it tries to check the chunk length when reading the chunk header, it executes the following line at pngrutil.c.
3169 size_t row_factor =
3170 (png_ptr->width * png_ptr->channels * (png_ptr->bit_depth > 8? 2: 1)
3171 + 1 + (png_ptr->interlaced? 6: 0));
3172 if (png_ptr->height > PNG_UINT_32_MAX/row_factor)

5) The computation in line 3170 yields 0xFFFFFFFF.
6) The addition in line 3171 rolls it over to 0 (integer overflow). Note that the type size_t in Line 3169 could easily accomodate a larger number.
7) In line 3172, we get the division by 0.

1 Attachments

Discussion

  • Thuan Pham

    Thuan Pham - 2018-04-06

    In the compilation instruction, due to some wrong formatting it looks weird. Unfortunately I cannot edit the original ticket. Following is the correct one

    //Disable checksum
    sed -i 's/return ((int)(crc != png_ptr->crc));/return (0);/g' pngrutil.c
    autoreconf -f -i
    ./configure --disable-shared
    make clean all check
    

    Note that you may not need to disable the checksum check. We disabled it during fuzzing.

     
  • Cosmin Truta

    Cosmin Truta - 2018-06-18

    Hi, Thuan,

    Thank you very much for the detailed report. I pushed a fix to the libpng16 branch at https://github.com/ctruta/libpng

    Sincerely,
    Cosmin

     
  • Cosmin Truta

    Cosmin Truta - 2018-07-16

    Thanks, but png_ptr->width cannot be (size_t)(-1). It wouldn't be allowed by png_check_IHDR.

     
  • Cosmin Truta

    Cosmin Truta - 2018-07-16
    • status: open --> closed-fixed
    • assigned_to: Cosmin Truta
     
  • Cosmin Truta

    Cosmin Truta - 2018-07-16

    Fixed in version 1.6.35.

     

Log in to post a comment.