selmf / unarr

A decompression library for rar, tar, zip and 7z archives
GNU Lesser General Public License v3.0
73 stars 13 forks source link

memory leak if 7z is not valid #7

Closed comicfans closed 5 years ago

comicfans commented 5 years ago

_7z.c:170 memory allocated by

    _7z->look_stream.buf = ISzAlloc_Alloc(&gSzAlloc, 1 << 18);

but in following error condition line 181 it returned without free it

    if (res != SZ_OK) {
        if (res != SZ_ERROR_NO_ARCHIVE)
            warn("Invalid 7z archive (failed with error %d)", res);
        //----------------> miss   ISzAlloc_Free(&gSzAlloc, _7z->look_stream.buf);  ?
        free(ar);
        return NULL;
    }

leak sanitizer report

=================================================================
==22571==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 262144 byte(s) in 1 object(s) allocated from:
    #0 0x564dd5826879 in malloc (/home/wangxinyu/unarr/fuz/test/fuzzer+0x132879)
    #1 0x7f451a5288a0 in gSzAlloc_Alloc /home/wangxinyu/unarr/fuz/../_7z/_7z.c:8:81
    #2 0x7f451a526855 in ar_open_7z_archive /home/wangxinyu/unarr/fuz/../_7z/_7z.c:170:28
    #3 0x564dd58649c4 in ar_open_any_archive /home/wangxinyu/unarr/fuz/../test/fuzzer.c:5:20
    #4 0x564dd5864b5b in read_test /home/wangxinyu/unarr/fuz/../test/fuzzer.c:20:20
    #5 0x564dd5865021 in LLVMFuzzerTestOneInput /home/wangxinyu/unarr/fuz/../test/fuzzer.c:61:3
    #6 0x564dd572d225 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/ho
me/wangxinyu/unarr/fuz/test/fuzzer+0x39225)
    #7 0x564dd57327d3 in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::vector<std::__cxx11::basic
_string<char, std::char_traits<char>, std::allocator<char> >, fuzzer::fuzzer_allocator<std::__cxx1
1::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&) (/home/wangxinyu/
unarr/fuz/test/fuzzer+0x3e7d3)
    #8 0x564dd5734072 in fuzzer::Fuzzer::Loop(std::vector<std::__cxx11::basic_string<char, std::ch
ar_traits<char>, std::allocator<char> >, fuzzer::fuzzer_allocator<std::__cxx11::basic_string<char,
 std::char_traits<char>, std::allocator<char> > > > const&) (/home/wangxinyu/unarr/fuz/test/fuzzer
+0x40072)
    #9 0x564dd5724d75 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigne
d long)) (/home/wangxinyu/unarr/fuz/test/fuzzer+0x30d75)
    #10 0x564dd5718293 in main (/home/wangxinyu/unarr/fuz/test/fuzzer+0x24293)
    #11 0x7f4519f69222 in __libc_start_main (/usr/lib/libc.so.6+0x24222)

Direct leak of 262144 byte(s) in 1 object(s) allocated from:
    #0 0x564dd5826879 in malloc (/home/wangxinyu/unarr/fuz/test/fuzzer+0x132879)
    #1 0x7f451a5288a0 in gSzAlloc_Alloc /home/wangxinyu/unarr/fuz/../_7z/_7z.c:8:81
    #2 0x7f451a526855 in ar_open_7z_archive /home/wangxinyu/unarr/fuz/../_7z/_7z.c:170:28
    #3 0x564dd58649c4 in ar_open_any_archive /home/wangxinyu/unarr/fuz/../test/fuzzer.c:5:20
    #4 0x564dd5864b5b in read_test /home/wangxinyu/unarr/fuz/../test/fuzzer.c:20:20
    #5 0x564dd5865021 in LLVMFuzzerTestOneInput /home/wangxinyu/unarr/fuz/../test/fuzzer.c:61:3
    #6 0x564dd572d225 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/ho
me/wangxinyu/unarr/fuz/test/fuzzer+0x39225)
    #7 0x564dd572fa20 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer:
:InputInfo*, bool*) (/home/wangxinyu/unarr/fuz/test/fuzzer+0x3ba20)
    #8 0x564dd573287f in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::vector<std::__cxx11::basic
_string<char, std::char_traits<char>, std::allocator<char> >, fuzzer::fuzzer_allocator<std::__cxx1
1::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&) (/home/wangxinyu/
unarr/fuz/test/fuzzer+0x3e87f)
    #9 0x564dd5734072 in fuzzer::Fuzzer::Loop(std::vector<std::__cxx11::basic_string<char, std::ch
ar_traits<char>, std::allocator<char> >, fuzzer::fuzzer_allocator<std::__cxx11::basic_string<char,
 std::char_traits<char>, std::allocator<char> > > > const&) (/home/wangxinyu/unarr/fuz/test/fuzzer
+0x40072)
    #10 0x564dd5724d75 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsign
ed long)) (/home/wangxinyu/unarr/fuz/test/fuzzer+0x30d75)
    #11 0x564dd5718293 in main (/home/wangxinyu/unarr/fuz/test/fuzzer+0x24293)
    #12 0x7f4519f69222 in __libc_start_main (/usr/lib/libc.so.6+0x24222)

SUMMARY: AddressSanitizer: 524288 byte(s) leaked in 2 allocation(s).
selmf commented 5 years ago

Thanks for noticing this. I've checked the LZMA sdk and calling ISzAlloc_Free on the buffer is the right way to go. There is just one thing that bothers me - it seems that this isn't needed when the 7z archive is valid.