pngcp has a segfault

[tianxiaogu]

Is it a bug of glibc? Not sure whether it is important. The crash can be reproduced with the HEAD or the one in pngtools released in Ubuntu 18.04.

test-case.zip

test-case: error(libpng): read: IEND: out of place
AddressSanitizer:DEADLYSIGNAL
=================================================================
==11158==ERROR: AddressSanitizer: SEGV on unknown address 0x1000982d4c88 (pc 0x7fd677330fac bp 0x7ffcc1727990 sp 0x7ffcc1726258 T0)
==11158==The signal is caused by a WRITE memory access.
    #0 0x7fd677330fab  /build/glibc-OTsEL5/glibc-2.27/string/../sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S:185
    #1 0x52fb4f in cppng /home/t/Projects/afl/fuzzing-experiments/subjects/libpng/contrib/tools/pngcp.c:2302:1
    #2 0x529195 in main /home/t/Projects/afl/fuzzing-experiments/subjects/libpng/contrib/tools/pngcp.c:2351:16
    #3 0x7fd6771c3b96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310
    #4 0x41b789 in _start (/home/t/Projects/afl/fuzzing-experiments/subjects/libpng/.libs/pngcp+0x41b789)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /build/glibc-OTsEL5/glibc-2.27/string/../sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S:185 
==11158==ABORTING

>>> apt-cache show pngtools
Package: pngtools
Architecture: amd64
Version: 0.4-1.3
Priority: optional
Section: universe/graphics
Origin: Ubuntu
Maintainer: Ubuntu Developers <ubuntu-devel-discuss@lists.ubuntu.com>
Original-Maintainer: Nelson A. de Oliveira <naoliv@debian.org>
Bugs: https://bugs.launchpad.net/ubuntu/+filebug
Installed-Size: 64
Depends: libc6 (>= 2.11), libpng16-16 (>= 1.6.2-1)
Suggests: optipng | pngcrush
Filename: pool/universe/p/pngtools/pngtools_0.4-1.3_amd64.deb
Size: 16326
MD5sum: 1e0cc30cfeead44e1b48a2f902d299c9
SHA1: 9316449888ceb108515e09846c64883732669b83
SHA256: ee824468c1296aa4f9ca3a0074ae0ec7bf95346588451974b5590ec64fe4b917
Homepage: http://www.stillhq.com/pngtools/
Description-en: series of tools for PNG (Portable Network Graphics) images
 pngtools is a suite of utilities to work with PNG (Portable Network
 Graphics) files, equivalents to libtiff's tiffinfo, and tiffcp commands.
 These commands are called pnginfo, pngcp. tiffdump is replaced by pngchunks
 and pngchunkdesc as well.
Description-md5: 7946629d3cfba2d00deb521390a5c1c2

[tianxiaogu]

This crash may be related to the compiler. When I build pngcp with plain clang-8, the crash disappeared.
The output of the one released in Ubuntu 18.04 contains more info.

>>> pngcp ../libpng/test-case
libpng error: IEND: out of place
Could not set PNG jump valueSegmentation fault

[tianxiaogu]

https://wiki.sei.cmu.edu/confluence/display/c/MSC22-C.+Use+the+setjmp%28%29%2C+longjmp%28%29+facility+securely

The usage of setjmp seems to be an undefined behavior.

https://github.com/glennrp/libpng/blob/a54a0562c53daf2d627f1631be0dd6531cca1694/contrib/tools/pngcp.c#L2277-L2302

[tangyaofang]

I agree with tianxiaogu.
This is a bug that uses setjmp inappropriately.
This problem may be triggered in different compilers.

[ctruta]

@jbowler FYI.

I rejected the above-mentioned commit tangyaofang@3bfadc2 because it doesn't actually solve the problem. It merely moves it to a different place.

I wonder if making all function arguments (and not just the last one) volatile will solve the problem.

[jbowler]

I don't know what was being run here because the "apt-cache" screenshot is for the pngtools package; the one with the real pngcp, not the libpng one.

It looks like the test code has problems with longjmp/setjmp; as I said in #424 mixing C++ code with C code may or may not work...

The test file works just plain fine with a regular build of pngcp:

$ pngcp bug255.png ttt.png
bug255.png: error(libpng): read: IEND: out of place

"error(libpng)" means that pngerror was called, which is why the call stack in the original report is totally broken. It looks like the longjmp crashed, possibly it encountered a C++ unwind (at least in a recent GCC12 g++ build for #424 I saw that the latest version is apparently doing unwinds, which looks dangerous unless libpng is built with -fexceptions.)

[jbowler]

I suggest adding -fexceptions to all libpng builds with compilers that support this. This was the way I always built libpng anyway. It will change the default behavior on almost all builds.

If you don't do this then people who try to throw in the callbacks are likely to suffer crashes or worse (though it is 15 years since I encountered these problems so maybe GCC has got better...) The GCC problem is that it reported a clobber on values that were unmodified; see the comments on the definition of "gv" in pngcp.c and ANSI-C (X3.159-1989) section 4.6.2.1, the second paragraph, lines 11-14 in the original ANSI standard.

Also note that the compiler actually being used is not explicitly stated, but it looks like clang, not gcc.

Also I suggest not accepting bugs like this; @tangyaofang was testing two COMPLETELY DIFFERENT programs. Any report on pngcp needs to be checked very carefully to make sure which pngcp it is; fortunately the output messages have a completely different format so I can tell from that.