pngimage: heap-buffer-overflow in ./pngimage.c:1249 in compare_read

[gandalf4a]

from：https://github.com/gandalf4a/crash_report/blob/main/libpng/pngimage/bufferoverflow_compareread.md

Summary

There is a buffer overflow in ./pngimage.c:1249 in compare_read.
Remote attackers could leverage this vulnerability to cause a denial-of-service via a crafted PNG file.

Version

$ git clone https://github.com/glennrp/libpng.git
$ cd libpng && git show
commit e519af8b49f52c4ac400f50f23b48ebe36a5f4df (HEAD -> libpng16, origin/tmp/develop, origin/master, origin/libpng16, origin/HEAD)
Author: Cosmin Truta <[email protected]>
Date:   Sun Feb 12 22:31:11 2023 +0200
...

Platform

uname -a 
Linux user-GE40-2PC-Dragon-Eyes 5.19.0-40-generic #41~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Fri Mar 31 16:00:14 UTC 2 x86_64 x86_64 x86_64 GNU/Linux

ASAN_heap-buffer-overflow

==923000==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000002420 at pc 0x562b7a077fe5 bp 0x7ffce67c64f0 sp 0x7ffce67c64e0
READ of size 1 at 0x602000002420 thread T0
    #0 0x562b7a077fe4 in compare_read ../contrib/libtests/pngimage.c:1249
    #1 0x562b7a078ff2 in test_one_file ../contrib/libtests/pngimage.c:1493
    #2 0x562b7a079299 in do_test ../contrib/libtests/pngimage.c:1573
    #3 0x562b7a079e55 in main ../contrib/libtests/pngimage.c:1677
    #4 0x7f4c8dc29d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #5 0x7f4c8dc29e3f in __libc_start_main_impl ../csu/libc-start.c:392
    #6 0x562b7a073994 in _start (/home/user/fuzzing/libpng/build/pngimage+0xd994)

0x602000002420 is located 0 bytes to the right of 16-byte region [0x602000002410,0x602000002420)
allocated by thread T0 here:
    #0 0x7f4c8e0b4867 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
    #1 0x562b7a092c59 in png_malloc_base ../pngmem.c:95
    #2 0x562b7a092e3e in png_malloc ../pngmem.c:179
    #3 0x562b7a096eca in png_read_png ../pngread.c:1237
    #4 0x562b7a0758c0 in read_png ../contrib/libtests/pngimage.c:905
    #5 0x562b7a078fe1 in test_one_file ../contrib/libtests/pngimage.c:1492
    #6 0x562b7a079299 in do_test ../contrib/libtests/pngimage.c:1573
    #7 0x562b7a079e55 in main ../contrib/libtests/pngimage.c:1677
    #8 0x7f4c8dc29d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58

SUMMARY: AddressSanitizer: heap-buffer-overflow ../contrib/libtests/pngimage.c:1249 in compare_read
Shadow bytes around the buggy address:
  0x0c047fff8430: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fd
  0x0c047fff8440: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fd
  0x0c047fff8450: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fd
  0x0c047fff8460: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fd
  0x0c047fff8470: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fd
=>0x0c047fff8480: fa fa 00 00[fa]fa 00 00 fa fa 00 00 fa fa 00 00
  0x0c047fff8490: fa fa 00 00 fa fa 00 00 fa fa 00 00 fa fa 00 00
  0x0c047fff84a0: fa fa 00 00 fa fa 00 00 fa fa 00 00 fa fa 00 00
  0x0c047fff84b0: fa fa 00 00 fa fa 00 00 fa fa 00 00 fa fa 00 00
  0x0c047fff84c0: fa fa 00 00 fa fa 00 00 fa fa 00 00 fa fa 00 00
  0x0c047fff84d0: fa fa 00 00 fa fa 00 00 fa fa 00 00 fa fa 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==923000==ABORTING

Steps to reproduce

./pngimage $poc_file

POC File

poc_png_overflow

Credit

Gandalf4a@QAXsrc

[ctruta]

I confirm the vulnerability that you reported, and I will develop a fix.

@gandalf4a, it is not required, but it would be appreciated if I may please know your real-world name. Alternatively, I can credit your discovery at the bottom of the commit message, as follows, if that's ok with you:

Reported-by: gandalf4a <[email protected]>

[ctruta]

On a second thought, considering that pngimage.c is an internal test program, not production code, I'd like to defer the investigation and the development of a proper fix. I want to publish the upcoming libpng-1.6.40 with other important fixes, and we'll leave this one for version 1.6.41.

(Unless @jbowler steps in ASAP with a fix...)

The crash occurs at pngimage.c:1249. From what I understood so far (and, mind you, my understanding at this moment may or may not be correct) we have a 4bpp grayscale image, with two pixels packed inside one byte, and it is incorrect to increment *row++ and *orig++ under these conditions.

The crashing POC test case confirms my hypothesis. The POC image width is 32, and everything works fine for the first half, i.e. for the first 16 iterations. The crash occurs after x grows from 15 to 16.

                  if ((*row++ & sig_bits[b]) != (*orig++ & sig_bits[b]))

[jbowler]

The crash occurs at pngimage.c:1249. From what I understood so far (and, mind you, my understanding at this moment may or may not be correct) we have a 4bpp grayscale image, with two pixels packed inside one byte, and it is incorrect to increment *row++ and *orig++ under these conditions.

The code is comparing bytes, not pixels; it's memcmp but with sigbits factored in. Look at line 1222 which converts bits-per-pixel into bytes-per-pixel.

makepng.c in the same directory was made uncompilable by commit e2bb5e7 and that commit also contains quite a lot of similar changes to pngimage.c. @gandalf4a can you verify that the bug existed in b50d5ce (i.e. 1.6.24)?

[jbowler]

pngimage fails in both HEAD and libpng-1.6.24-signed on that file, but it is a regular failure at line 24 (IRC how to decode the error message). I don't know if either version has out-of-bounds error. The file does have an sBIT chunk with a value of '4', which is clearly spurious so the most likely explanation is that pngimage can't handle an image where sBIT==depth; so what? It's more interesting if it is a bug in libpng, but I doubt that.

[jbowler]

The sBIT/SHIFT code was backported from 1.7 and never gets invoked in 1.6. The original path did, indeed, use memcmp on 'rowbytes', the other (else) path should be using rowbytes too but the error message tries to output a pixel coordinate. contrib/pngsuite contains basn3p02 and basn3p04 with sBIT but those are palette images so use the memcmp branch.

The attached patch is simple, safe and keeps the format of the error message.
pngimage.c.patch

[jbowler]

This issue should be closed; the original reporter has dropped the ball.