Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

memory leak in png_malloc_warn and png_create_info_struct #307

Open
zer0yu opened this issue Jul 11, 2019 · 6 comments

Comments

@zer0yu
Copy link

commented Jul 11, 2019

Hi,libpng team. there are memory leaks in the function png_malloc_warn and png_create_info_struct, respectively.

I compiler gif2png to the 32-bit LSB version with ASAN. The software runs in the x86-64 Ubuntu 16.04 services.

the bug is trigered by ./gif2png -r poc.
libpng_poc.zip

the asan debug info is as follows:

=================================================================
==35676==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 64056 byte(s) in 51 object(s) allocated from:
#0 0x7ffff6f02602 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x98602)
#1 0x7ffff6c43c0d in png_malloc_warn (/lib/x86_64-linux-gnu/libpng16.so.16+0xac0d)

Direct leak of 17544 byte(s) in 51 object(s) allocated from:
#0 0x7ffff6f02602 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x98602)
#1 0x7ffff6c3e032 in png_create_info_struct (/lib/x86_64-linux-gnu/libpng16.so.16+0x5032)
#2 0x4039d8 in processfile (/home/zeroyu/target_gif2png/gif2png64+0x4039d8)
#3 0x40406d in main (/home/zeroyu/target_gif2png/gif2png64+0x40406d)
#4 0x7ffff688f82f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)

Direct leak of 6656 byte(s) in 26 object(s) allocated from:
#0 0x7ffff6f02602 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x98602)
#1 0x405df0 in xalloc (/home/zeroyu/target_gif2png/gif2png64+0x405df0)
#2 0x405953 in ReadImage (/home/zeroyu/target_gif2png/gif2png64+0x405953)
#3 0x404a6d in ReadGIF (/home/zeroyu/target_gif2png/gif2png64+0x404a6d)
#4 0x403647 in processfile (/home/zeroyu/target_gif2png/gif2png64+0x403647)
#5 0x40406d in main (/home/zeroyu/target_gif2png/gif2png64+0x40406d)
#6 0x7ffff688f82f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)

Direct leak of 2048 byte(s) in 8 object(s) allocated from:
#0 0x7ffff6f02602 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x98602)
#1 0x405df0 in xalloc (/home/zeroyu/target_gif2png/gif2png64+0x405df0)
#2 0x405953 in ReadImage (/home/zeroyu/target_gif2png/gif2png64+0x405953)
#3 0x4049cc in ReadGIF (/home/zeroyu/target_gif2png/gif2png64+0x4049cc)
#4 0x403647 in processfile (/home/zeroyu/target_gif2png/gif2png64+0x403647)
#5 0x40406d in main (/home/zeroyu/target_gif2png/gif2png64+0x40406d)
#6 0x7ffff688f82f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)

SUMMARY: AddressSanitizer: 90304 byte(s) leaked in 136 allocation(s).
[Inferior 1 (process 35676) exited with code 027]

@abergmann

This comment has been minimized.

Copy link

commented Oct 10, 2019

CVE-2019-17371 was assigned to this issue.

@zer0yu

This comment has been minimized.

Copy link
Author

commented Oct 10, 2019

thx

@willson-chen

This comment has been minimized.

Copy link

commented Oct 21, 2019

@zer0yu Hi, could you please offer more information about how to reproduce this bug.

I have cloned gif2png and checkout to tag 2.5.9, the latest version is 3.0.0 and is ported to Golang.

After compiling gif2png and extracting gif file(renamed to poc.gif) from your attachment, I executed ./gif2png -r poc and got a error message:

libpng error: Invalid palette length

and some files:

poc.p01
poc.p02
...
poc.p50
poc.png

poc.p01...poc.p50 are empty files.

But no message about memory leakage were generated.

@rossburton

This comment has been minimized.

Copy link

commented Oct 21, 2019

Important detail missing: what version of libpng?

@willson-chen

This comment has been minimized.

Copy link

commented Oct 22, 2019

@zer0yu Hi, I had some tests on gif2png. But I need your help.

I used valgrind to check the memory leakage by executing valgrind --leak-check=full ./gif2png -r poc, and here is the output message.

==25395==
==25395== HEAP SUMMARY:
==25395==     in use at exit: 91,120 bytes in 136 blocks
==25395==   total heap usage: 602 allocs, 466 frees, 1,177,468 bytes allocated
==25395==
==25395== 8,704 bytes in 34 blocks are definitely lost in loss record 1 of 3
==25395==    at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==25395==    by 0x10CC48: xalloc (memory.c:17)
==25395==    by 0x10BEF3: ReadImage (gifread.c:662)
==25395==    by 0x10C646: ReadGIF (gifread.c:218)
==25395==    by 0x10B0A1: processfile.part.0 (gif2png.c:707)
==25395==    by 0x109A2B: processfile (stdio2.h:97)
==25395==    by 0x109A2B: main (gif2png.c:987)
==25395==
==25395== 18,360 bytes in 51 blocks are definitely lost in loss record 2 of 3
==25395==    at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==25395==    by 0x4E40F32: png_create_info_struct (in /usr/lib/x86_64-linux-gnu/libpng16.so.16.34.0)
==25395==    by 0x109F9C: writefile (gif2png.c:157)
==25395==    by 0x10B29D: processfile.part.0 (gif2png.c:788)
==25395==    by 0x109A2B: processfile (stdio2.h:97)
==25395==    by 0x109A2B: main (gif2png.c:987)
==25395==
==25395== 64,056 bytes in 51 blocks are definitely lost in loss record 3 of 3
==25395==    at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==25395==    by 0x4E46E81: png_malloc_warn (in /usr/lib/x86_64-linux-gnu/libpng16.so.16.34.0)
==25395==    by 0x4E40E8F: ??? (in /usr/lib/x86_64-linux-gnu/libpng16.so.16.34.0)
==25395==    by 0x4E4A40C: png_create_read_struct_2 (in /usr/lib/x86_64-linux-gnu/libpng16.so.16.34.0)
==25395==    by 0x4E4A460: png_create_read_struct (in /usr/lib/x86_64-linux-gnu/libpng16.so.16.34.0)
==25395==    by 0x109F86: writefile (gif2png.c:151)
==25395==    by 0x10B29D: processfile.part.0 (gif2png.c:788)
==25395==    by 0x109A2B: processfile (stdio2.h:97)
==25395==    by 0x109A2B: main (gif2png.c:987)
==25395==
==25395== LEAK SUMMARY:
==25395==    definitely lost: 91,120 bytes in 136 blocks
==25395==    indirectly lost: 0 bytes in 0 blocks
==25395==      possibly lost: 0 bytes in 0 blocks
==25395==    still reachable: 0 bytes in 0 blocks
==25395==         suppressed: 0 bytes in 0 blocks
==25395==
==25395== For counts of detected and suppressed errors, rerun with: -v
==25395== ERROR SUMMARY: 3 errors from 3 contexts (suppressed: 0 from 0)

We can see the key function png_malloc_warn and png_create_info_struct. Although the program did not crash, the output message was similar to the error information offered by @zer0yu .

So I reviewed the gif2png source code, whose git-tag was 2.5.9 and commit id was b6c6bb. The function wirtefile of gif2png calls png_create_read_struct and png_create_info_struct. That will cause libpng to allocate memory. But png_destroy_read_struct is not called to free the memory after that.

Here is my patch to fixed it.

diff --git a/gif2png.c b/gif2png.c
index 8a3d1c1..25537a5
--- a/gif2png.c
+++ b/gif2png.c
@@ -311,6 +311,7 @@ static int writefile(struct GIFelement *s, struct GIFelement *e,
         (void)free(info_ptr);
         return 1;
     }
+    png_destroy_read_struct(&png_ptr, &info_ptr, NULL);

     /* Create and initialize the png_struct with the desired error handler
      * functions.  If you want to use the default stderr and longjump method,
diff --git a/gifread.c b/gifread.c
index 6995c3a..f48ab65
--- a/gifread.c
+++ b/gifread.c
@@ -727,9 +727,11 @@ ReadImage(FILE *fd, int x_off, int y_off, int width, int height,
                         goto fini;
                     } else {
                         (void)check_recover(true);
+                        (void)free(image);
                         return(true);
                     }
                 } else {
+                    (void)free(image);
                     return(true);
                 }
             }

And here is the new output message

==27072== HEAP SUMMARY:
==27072==     in use at exit: 0 bytes in 0 blocks
==27072==   total heap usage: 602 allocs, 602 frees, 1,177,468 bytes allocated
==27072==
==27072== All heap blocks were freed -- no leaks are possible
==27072==
==27072== For counts of detected and suppressed errors, rerun with: -v
==27072== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

0 error!

So my conclusion is that: the memory leakage is caused by gif2png but NOT libpng.
But I need some more information from @zer0yu to further confirm it.
My env:

Ubuntu 18.04 x86_64
valgrind-3.13.0
gcc 7.4
libpng-dev 1.6.34
@zer0yu

This comment has been minimized.

Copy link
Author

commented Oct 22, 2019

Sorry, it’s my mistake.

My env:

Ubuntu 16.04 x86_64
gcc version 5.4.0 20160609
ASAN
libpng 1.6.37
gif2png 2.5.13

details

gif2png: ELF 64-bit LSB executable, x86-64, version 1 (SYSV), dynamically linked, interpreter /lib64/l, for GNU/Linux 2.6.32,

gif2png-2.5.13.tar.gz

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.