SEGV in function png_free_data #238

fouzhe · 2018-07-12T04:16:47Z

Hi,all!
We find a bug in libpng 1.6.34 when using pngwriter,which take libpng as dependence.
You can get more information by inferring this!

ax3l · 2018-07-12T11:05:07Z

Concretely speaking, the recommended error handling for png_read_image via

    if (setjmp(png_jmpbuf(png_ptr)))
    {
       png_destroy_read_struct(&png_ptr, &info_ptr, NULL);
       fclose(fp);
       return (ERROR);
    }

results in a SEGV during png_destroy_read_struct. We traced our specific call stack in this comment.

The error occurs in png_free_data during the free of text-related data with specifically crafted files, reading invalid/unowned memory.

You can test it with simple (non-incremental) reads via @fouzhe's fuzzy-file here.

ax3l · 2018-07-12T11:34:15Z

@fouzhe can you try if you can reproduce that issue also with png2pnm in contrib/pngminus? I can't get it there as well but see a memory leak.

libpng error: Read Error PNG2PNM Error: unsuccessful conversion of PNG-image

=================================================================
==626==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 160 byte(s) in 1 object(s) allocated from:
#0 0x4ded88 in malloc (/home/axel/src/libpng/contrib/pngminus/png2pnm+0x4ded88)
#1 0x51777f in png2pnm /home/axel/src/libpng/contrib/pngminus/png2pnm.c:350:6
#2 0x516668 in main /home/axel/src/libpng/contrib/pngminus/png2pnm.c:160:7
#3 0x7fe9580842e0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202e0)

Indirect leak of 2400 byte(s) in 1 object(s) allocated from:
#0 0x4ded88 in malloc (/home/axel/src/libpng/contrib/pngminus/png2pnm+0x4ded88)
#1 0x5176d4 in png2pnm /home/axel/src/libpng/contrib/pngminus/png2pnm.c:343:6
#2 0x516668 in main /home/axel/src/libpng/contrib/pngminus/png2pnm.c:160:7
#3 0x7fe9580842e0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202e0)

SUMMARY: AddressSanitizer: 2560 byte(s) leaked in 2 allocation(s).

fouzhe · 2018-07-12T11:49:23Z

Yeah,I will try it!
@ax3l Did you use the file I attached?

ax3l · 2018-07-12T11:58:04Z

Did you use the file I attached?

yes, via: ./png2pnm crash_SEGV_readfromfile crash.pnm

here is my diff to get it compiled with clang-6.0:

export CFLAGS="-g -fsanitize=address" LDFLAGS="-fsanitize=address" CC=clang-6.0
./autogen.sh
./configure
cd contrib/pngminus/
make -f makefile.std

./png2pnm crash_SEGV_readfromfile crash.pnm

diff --git a/contrib/pngminus/makefile.std b/contrib/pngminus/makefile.std
index 14e25cd64..553bbe354 100644
--- a/contrib/pngminus/makefile.std
+++ b/contrib/pngminus/makefile.std
@@ -2,7 +2,7 @@
 # Linux / Unix
 
 #CC=cc
-CC=gcc
+CC=clang-6.0
 LD=$(CC)
 
 RM=rm -f
@@ -13,18 +13,19 @@ RM=rm -f
 #PNGLIBS = $(PNGPATH)/lib/libpng16.a
 PNGINC = -I../..
 PNGLIB = -L../.. -lpng
-PNGLIBS = ../../libpng.a
+PNGLIBS = ../../.libs/libpng16.a
 
-#ZPATH = /usr/local
-#ZINC = -I$(ZPATH)/include
-#ZLIB = -L$(ZPATH)/lib -lz
+ZPATH = /usr/local
+ZINC = -I$(ZPATH)/include
+ZLIB = -L$(ZPATH)/lib -lz
 #ZLIBS = $(ZPATH)/lib/libz.a
-ZINC = -I../../../zlib
-ZLIB = -L../../../zlib -lz
:...skipping...
diff --git a/contrib/pngminus/makefile.std b/contrib/pngminus/makefile.std
index 14e25cd64..553bbe354 100644
--- a/contrib/pngminus/makefile.std
+++ b/contrib/pngminus/makefile.std
@@ -2,7 +2,7 @@
 # Linux / Unix
 
 #CC=cc
-CC=gcc
+CC=clang-6.0
 LD=$(CC)
 
 RM=rm -f
@@ -13,18 +13,19 @@ RM=rm -f
 #PNGLIBS = $(PNGPATH)/lib/libpng16.a
 PNGINC = -I../..
 PNGLIB = -L../.. -lpng
-PNGLIBS = ../../libpng.a
+PNGLIBS = ../../.libs/libpng16.a
 
-#ZPATH = /usr/local
-#ZINC = -I$(ZPATH)/include
-#ZLIB = -L$(ZPATH)/lib -lz
+ZPATH = /usr/local
+ZINC = -I$(ZPATH)/include
+ZLIB = -L$(ZPATH)/lib -lz
 #ZLIBS = $(ZPATH)/lib/libz.a
-ZINC = -I../../../zlib
-ZLIB = -L../../../zlib -lz
-ZLIBS = ../../../zlib/libz.a
+#ZINC = -I../../../zlib
+#ZLIB = -L../../../zlib -lz
+#ZLIBS = ../../../zlib/libz.a
 
 CPPFLAGS=$(PNGINC) $(ZINC)
-CFLAGS=
+CFLAGS=-g -fsanitize=address
+LDFLAGS=-fsanitize=address
 LDLIBS=$(PNGLIB) $(ZLIB)
 LDLIBSS=$(PNGLIBS) $(ZLIBS)
 C=.c
@@ -41,19 +42,19 @@ png2pnm$(O): png2pnm$(C)
        $(CC) -c $(CPPFLAGS) $(CFLAGS) png2pnm$(C)
 
 png2pnm$(E): png2pnm$(O)
-       $(LD) $(LDFLAGS) -o png2pnm$(E) png2pnm$(O) $(LDLIBS) -lm
+       $(LD) $(LDFLAGS) -o png2pnm$(E) png2pnm$(O) $(LDLIBS) -lm -lz
 
 png2pnm-static$(E): png2pnm$(O)
-       $(LD) $(LDFLAGS) -o png2pnm-static$(E) png2pnm$(O) $(LDLIBSS) -lm
+       $(LD) $(LDFLAGS) -o png2pnm-static$(E) png2pnm$(O) $(LDLIBSS) -lm -lz
 
 pnm2png$(O): pnm2png$(C)
        $(CC) -c $(CPPFLAGS) $(CFLAGS) pnm2png$(C)
 
 pnm2png$(E): pnm2png$(O)
-       $(LD) $(LDFLAGS) -o pnm2png$(E) pnm2png$(O) $(LDLIBS) -lm
+       $(LD) $(LDFLAGS) -o pnm2png$(E) pnm2png$(O) $(LDLIBS) -lm -lz
 
 pnm2png-static$(E): pnm2png$(O)
-       $(LD) $(LDFLAGS) -o pnm2png-static$(E) pnm2png$(O) $(LDLIBSS) -lm
+       $(LD) $(LDFLAGS) -o pnm2png-static$(E) pnm2png$(O) $(LDLIBSS) -lm -lz
 
 clean:
        $(RM) png2pnm$(O)

fouzhe · 2018-07-13T02:13:14Z

@ax3l Thank you very much for the information provided!
I got the same error with you! I think it may be another issue!
For detail,the command for compiling png2pnm should be:

export CFLAGS="-g -fsanitize=address" LDFLAGS="-fsanitize=address" CC=clang-6.0
./autogen.sh
./configure
make
cd contrib/pngminus/
make -f makefile.std

fouzhe · 2018-07-13T02:56:16Z

I think another issue can be opened to report this issue!

kirotawa · 2018-07-17T17:29:45Z

this issue was assigned as this CVE number :
https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-14048

ax3l · 2018-07-17T18:36:43Z

Ok, but admittedly we still try to reproduce the downstream issue herein. @fouzhe could you reproduce the SEGV with a reduced, pure libpng example? I have a hard time proving anything else but the mem leak in #239 right now.

ctruta · 2018-07-18T04:22:06Z

Hi. I'm following up your conversation. Just to confirm:

I'm using crash_SEGV_readfromfile as test case, SHA256 is 42f717a2d57a0b485d10995e0440daf9ef04c78715e58e5aa6f719fbbc043d01.

I tried png2pnm with ASAN from clang-6.0, and Valgrind on top of that. Other than the memory leak from #239 (which I confirm), everything is running fine.

Are you sure it's a bug in libpng, and not in pngwriter?

ctruta · 2018-07-18T04:56:54Z

I took a closer look at the test case. Looks like IDAT is abnormally large, as well as unfinished. There's nothing else special about that, as far as I can see. Libpng should be able to handle that case, correctly.

Here's an idea: try recompiling pngwriter with either a new clang or a new gcc, but with -Wextra enabled, so that you can catch incorrect uses of setjmp/longjmp. If you see a warning about variables being clobbered by longjmp, that may be your error.

Alternatively, re-engineer pngwriter using C++ exception handling, if you're willing to take that path. throw from your user error handler before it gets to longjmp, and use try and catch instead of setjmp.

ax3l · 2018-07-18T17:14:19Z

Thanks for taking a closer look! I suspect the same since I have a hard time reproducing it in the minimal contrib/pngminus example.

It seems somehow that during png_destroy_read_struct (actually in png_destroy_info_struct calling png_free_data) in PNGwriter (here: pngwriter/pngwriter#129 (comment) and below) the info_ptr or access to its text member might already be invalidated during the frees. Unfortunately, -Wextra does not reveil anything and we are using the recommended way for setjmp error handling.

fouzhe · 2018-07-21T05:14:17Z

@ax3l I'm now reproducing the SEGV in png2pnm using gdb!

fouzhe · 2018-07-21T05:16:30Z

@ctruta We used pure png2pnm in libpng and got memory leak using attached file!

ctruta · 2018-07-21T10:18:57Z

I just renamed that issue, for correctness. The crash is in the 3rd-party pnm2png (importantly not in png2pnm), so it's not about a specially-crafted input that can crash libpng. It's rather about crashing a sample program in a place that's outside of the scope of libpng.

ctruta · 2019-04-15T04:57:49Z

The fix is in the master branch and in the public release libpng-1.6.37.

mdeslaur · 2019-04-30T14:47:03Z

I'm a bit confused what exactly the fix for this issue was.
What was the commit?

xiao623 · 2019-05-08T06:53:56Z

@ctruta, what was the commit?
I only found the commit 1f0221f the fix for issue #246.
Or, the commit can fix this issue??
But, I got the same error in libpng-1.6.37

libpng error: Read Error
PNG2PNM
Error:  unsuccessful conversion of PNG-image

=================================================================
==105569==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 160 byte(s) in 1 object(s) allocated from:
   #0 0x4b8da8  (/home/test/libpng-1.6.37/contrib/pngminus/png2pnm+0x4b8da8)
   #1 0x4eab31  (/home/test/libpng-1.6.37/contrib/pngminus/png2pnm+0x4eab31)
   #2 0x4ea64c  (/home/test/libpng-1.6.37/contrib/pngminus/png2pnm+0x4ea64c)
   #3 0x7fb8d348282f  (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
Indirect leak of 2400 byte(s) in 1 object(s) allocated from:
   #0 0x4b8da8  (/home/test/libpng-1.6.37/contrib/pngminus/png2pnm+0x4b8da8)
   #1 0x4eaaf8  (/home/test/libpng-1.6.37/contrib/pngminus/png2pnm+0x4eaaf8)
   #2 0x4ea64c  (/home/test/libpng-1.6.37/contrib/pngminus/png2pnm+0x4ea64c)
   #3 0x7fb8d348282f  (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)

ctruta · 2019-05-09T01:47:48Z

The commit 1f0221f does indeed fix the SEGV (i.e. this issue). The memory leak is an unrelated issue.

ctruta · 2019-05-09T01:58:35Z

... I see: SEGV is not a crash produced by libpng, or by pngminus. It's a result of ASAN.
So it's a different problem. I'm reopening this defect.

YanAnzouyijun · 2020-08-25T11:48:09Z

maybe, If I do not ship the binary of pnm2png,i will not affected by CVE-2018-14048 ?

zzm-up · 2021-03-30T09:59:47Z

Is CVE-2018-14048 and CVE-2018-14047 the same problem solved?

xteema · 2021-10-21T03:12:45Z

Is CVE-2018-14048 fixed？

renkeEdogawa · 2021-11-05T06:09:50Z

Hi,has the vulnerability CVE-2018-14048 been fixed?

mzia · 2022-10-18T18:12:02Z

asking the same as above? Has CVE-2018-14048 been fixed?

vesalvojdani · 2023-10-05T13:59:39Z

Yes, the SEGV issue that remains here is not a problem within libpng. The problem is that pngwriter has some undefined behavior that confuses ASAN, and this can be fixed by adding an error handler at the right place in pngwriter, see full details.

There are also reports of a memory leak within png2pnm (#239), but those leaks go away if one frees the two pointers of the external program before exiting, so it's a harmless "leak" in the client program.

fouzhe mentioned this issue Jul 13, 2018

Memory Leak in png2pnm #239

Open

fouzhe mentioned this issue Jul 21, 2018

stack-buffer-overflow in pnm2png in function get_token #246

Closed

stu1130 mentioned this issue Apr 4, 2019

[Dependency Update] Upgrade the libpng to 1.6.35 apache/mxnet#14620

Merged

6 tasks

ctruta closed this as completed Apr 15, 2019

ctruta reopened this May 9, 2019

tangyaofang added a commit to tangyaofang/libpng that referenced this issue May 31, 2019

Fix the memory leak mentioned in pnggroup#238 pnggroup#288

6b16211

tangyaofang mentioned this issue May 31, 2019

Fix the memory leak mentioned in #238 #288 #290

Closed

tangyaofang mentioned this issue Jun 10, 2019

Repair of CVE-2018-14048 #238 #295

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SEGV in function png_free_data #238

SEGV in function png_free_data #238

fouzhe commented Jul 12, 2018

ax3l commented Jul 12, 2018 •

edited

Loading

ax3l commented Jul 12, 2018 •

edited

Loading

fouzhe commented Jul 12, 2018

ax3l commented Jul 12, 2018 •

edited

Loading

fouzhe commented Jul 13, 2018 •

edited

Loading

fouzhe commented Jul 13, 2018

kirotawa commented Jul 17, 2018

ax3l commented Jul 17, 2018 •

edited

Loading

ctruta commented Jul 18, 2018

ctruta commented Jul 18, 2018

ax3l commented Jul 18, 2018 •

edited

Loading

fouzhe commented Jul 21, 2018

fouzhe commented Jul 21, 2018

ctruta commented Jul 21, 2018

ctruta commented Apr 15, 2019

mdeslaur commented Apr 30, 2019

xiao623 commented May 8, 2019 •

edited

Loading

ctruta commented May 9, 2019

ctruta commented May 9, 2019

YanAnzouyijun commented Aug 25, 2020

zzm-up commented Mar 30, 2021

xteema commented Oct 21, 2021

renkeEdogawa commented Nov 5, 2021

mzia commented Oct 18, 2022

vesalvojdani commented Oct 5, 2023

SEGV in function png_free_data #238

SEGV in function png_free_data #238

Comments

fouzhe commented Jul 12, 2018

ax3l commented Jul 12, 2018 • edited Loading

ax3l commented Jul 12, 2018 • edited Loading

fouzhe commented Jul 12, 2018

ax3l commented Jul 12, 2018 • edited Loading

fouzhe commented Jul 13, 2018 • edited Loading

fouzhe commented Jul 13, 2018

kirotawa commented Jul 17, 2018

ax3l commented Jul 17, 2018 • edited Loading

ctruta commented Jul 18, 2018

ctruta commented Jul 18, 2018

ax3l commented Jul 18, 2018 • edited Loading

fouzhe commented Jul 21, 2018

fouzhe commented Jul 21, 2018

ctruta commented Jul 21, 2018

ctruta commented Apr 15, 2019

mdeslaur commented Apr 30, 2019

xiao623 commented May 8, 2019 • edited Loading

ctruta commented May 9, 2019

ctruta commented May 9, 2019

YanAnzouyijun commented Aug 25, 2020

zzm-up commented Mar 30, 2021

xteema commented Oct 21, 2021

renkeEdogawa commented Nov 5, 2021

mzia commented Oct 18, 2022

vesalvojdani commented Oct 5, 2023

ax3l commented Jul 12, 2018 •

edited

Loading

ax3l commented Jul 12, 2018 •

edited

Loading

ax3l commented Jul 12, 2018 •

edited

Loading

fouzhe commented Jul 13, 2018 •

edited

Loading

ax3l commented Jul 17, 2018 •

edited

Loading

ax3l commented Jul 18, 2018 •

edited

Loading

xiao623 commented May 8, 2019 •

edited

Loading