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

There are memory leaks in zziplib <=v0.13.69 which is trigged in __zzip_parse_root_directory(in zzip/zip.c:427) #58

Closed
kky0h opened this issue Sep 5, 2018 · 8 comments

Comments

@kky0h
Copy link

kky0h commented Sep 5, 2018

There are memory leaks in zziplib <=v0.13.69 which is trigged in __zzip_parse_root_directory(in zzip/zip.c:427)
I wrote a demo based on the documentation.

#include <stdio.h>
#include <stdlib.h>
#include <zzip/zzip.h>
static const char usage[] = 
{
    "zzip\n"
};

int main(int argc, char const *argv[])
{
    if (argc  <= 1)
    {
        printf(usage);
        exit(0);
    }

    ZZIP_DIR* dir = zzip_dir_open(argv[1],0);

    if (dir)
    {
        ZZIP_DIRENT dirent;
    if (zzip_dir_read(dir,&dirent))
        {
            printf("%s %i/%i\n", dirent.d_name, dirent.d_csize, dirent.st_size);
        }
    zzip_dir_close(dir);
    }
    return 0;
}

when i use https://github.com/Kingkingyoung/fuzz_test/blob/poc/zzip-memory-leak
memory leak happened

=================================================================
==125971==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 76 byte(s) in 1 object(s) allocated from:
    #0 0x7f92614fc4d0 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xda4d0)
    #1 0x4016f3 in __zzip_parse_root_directory ../../zzip/zip.c:427
    #2 0x7f9260e61f44 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21f44)

It seems hdr0(zzip/zip.c:427) doesn't free correctly in some cases.

@kky0h kky0h changed the title There are memory leaks in zziplib v0.13.68 which is trigged in __zzip_parse_root_directory(in zzip/zip.c:430) There are memory leaks in zziplib v0.13.69 which is trigged in __zzip_parse_root_directory(in zzip/zip.c:427) Sep 5, 2018
@kky0h kky0h changed the title There are memory leaks in zziplib v0.13.69 which is trigged in __zzip_parse_root_directory(in zzip/zip.c:427) There are memory leaks in zziplib <=v0.13.69 which is trigged in __zzip_parse_root_directory(in zzip/zip.c:427) Sep 5, 2018
@kky0h
Copy link
Author

kky0h commented Sep 6, 2018

This was assigned CVE-2018-16548

@jmoellers
Copy link
Contributor

I seem to be unable to reproduce the issue with the latest sources. Maybe the bug is already fixed? There IS a free(dir->hdr0) in zzip_dir_free()!
I tried with valgrind and it says
All heap blocks were freed -- no leaks are possible

@kky0h
Copy link
Author

kky0h commented Sep 7, 2018

I tried with valgrind and it says(latest sources and old version)

~/testapp/zziplib-0.13.69/test$ valgrind --leak-check=full  ./fuzz_zzip zzip-memory-leak
==186527== Memcheck, a memory error detector
==186527== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==186527== Using Valgrind-3.10.1 and LibVEX; rerun with -h for copyright info
==186527== Command: ./fuzz_zzip zzip-memory-leak
==186527== 
==186527== 
==186527== HEAP SUMMARY:
==186527==     in use at exit: 76 bytes in 1 blocks
==186527==   total heap usage: 2 allocs, 1 frees, 196 bytes allocated
==186527== 
==186527== 76 bytes in 1 blocks are definitely lost in loss record 1 of 1
==186527==    at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==186527==    by 0x4012B1: __zzip_parse_root_directory (zip.c:427)
==186527==    by 0x4019D5: __zzip_dir_parse (zip.c:757)
==186527==    by 0x4018BE: zzip_dir_fdopen_ext_io (zip.c:715)
==186527==    by 0x401BBA: zzip_dir_open_ext_io (zip.c:831)
==186527==    by 0x401B3D: zzip_dir_open (zip.c:808)
==186527==    by 0x400B4D: main (fuzz_zzip.c:21)
==186527== 
==186527== LEAK SUMMARY:
==186527==    definitely lost: 76 bytes in 1 blocks
==186527==    indirectly lost: 0 bytes in 0 blocks
==186527==      possibly lost: 0 bytes in 0 blocks
==186527==    still reachable: 0 bytes in 0 blocks
==186527==         suppressed: 0 bytes in 0 blocks
==186527== 
==186527== For counts of detected and suppressed errors, rerun with: -v
==186527== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 2 from 1)

You can try again with the code I post above,and input file https://github.com/Kingkingyoung/fuzz_test/blob/poc/zzip-memory-leak
After debugging with gdb, it seems in this case ,the p_reclen=0(zip.c:576), so the dir->hdr0 =0, zzip_dir_free() don't free hdr0, I 'm not sure whether it is correct,you can try again.

@kky0h
Copy link
Author

kky0h commented Sep 7, 2018

Valgrind said 'a memory error detector' as you post.
My sources are also git clone from the master tree.And I also test released version 0.13.68 and 0.13.69. All of them have this problem.
I just use '-g' and '-static' when I compile the test code.

@jmoellers
Copy link
Contributor

Yes, it was early morning ;-)
I did test with ZIP files downloaded from the 'net, yours is a misformed one, __zzip_parse_root_directory() then takes an error return and the hdr0 is not free()'d.

@carnil
Copy link

carnil commented Oct 4, 2018

@jmoellers are those the complete commits, to fix the reported issue?
0e1dadb, d2e5d5c and 9411bde

@jmoellers
Copy link
Contributor

jmoellers commented Oct 5, 2018 via email

@carnil
Copy link

carnil commented Oct 5, 2018

@jmoellers

On 04.10.2018 23:14, carnil wrote: @jmoellers https://github.com/jmoellers are those the complete commits, to fix the reported issue? 0e1dadb <0e1dadb>, d2e5d5c <d2e5d5c> and 9411bde <9411bde>
Yes. Josef

Ack, thanks for confirming. Possibly so this issue can be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants