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 leaks in Ap4DescriptorFactory.cpp:89 at Bento4 1.5.1-627 when running mp42hls #343

Open
PikaQQQ opened this Issue Dec 19, 2018 · 3 comments

Comments

Projects
None yet
4 participants
@PikaQQQ
Copy link

PikaQQQ commented Dec 19, 2018

A crafted input will lead to memroy leaks in Ap4DescriptorFactory.cpp at Bento4 1.5.1-627.

Triggered by
./mp42hls crash3.mp4

Poc
crash3.zip

Bento4 Version 1.5.1-627
The ASAN information is as follows:

ERROR: failed to write samples (-18)

=================================================================
==52749==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 88 byte(s) in 1 object(s) allocated from:
    #0 0x7f6969285532 in operator new(unsigned long) (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x99532)
    #1 0x53fd71 in AP4_DescriptorFactory::CreateDescriptorFromStream(AP4_ByteStream&, AP4_Descriptor*&) /home/jas/Downloads/Bento4-SRC-1-5-1-627/Source/C++/Core/Ap4DescriptorFactory.cpp:89

Direct leak of 48 byte(s) in 1 object(s) allocated from:
    #0 0x7f6969285532 in operator new(unsigned long) (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x99532)
    #1 0x4de961 in AP4_StdcFileByteStream::Create(AP4_FileByteStream*, char const*, AP4_FileByteStream::Mode, AP4_ByteStream*&) /home/jas/Downloads/Bento4-SRC-1-5-1-627/Source/C++/System/StdC/Ap4StdCFileByteStream.cpp:175
    #2 0x4de961 in AP4_FileByteStream::Create(char const*, AP4_FileByteStream::Mode, AP4_ByteStream*&) /home/jas/Downloads/Bento4-SRC-1-5-1-627/Source/C++/System/StdC/Ap4StdCFileByteStream.cpp:332

Indirect leak of 2097544 byte(s) in 10 object(s) allocated from:
    #0 0x7f69692856b2 in operator new[](unsigned long) (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x996b2)
    #1 0x4abb54 in AP4_DataBuffer::ReallocateBuffer(unsigned int) /home/jas/Downloads/Bento4-SRC-1-5-1-627/Source/C++/Core/Ap4DataBuffer.cpp:210
    #2 0x4abb54 in AP4_DataBuffer::SetDataSize(unsigned int) /home/jas/Downloads/Bento4-SRC-1-5-1-627/Source/C++/Core/Ap4DataBuffer.cpp:151

Indirect leak of 896 byte(s) in 16 object(s) allocated from:
    #0 0x7f6969285532 in operator new(unsigned long) (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x99532)
    #1 0x53fec9 in AP4_DescriptorFactory::CreateDescriptorFromStream(AP4_ByteStream&, AP4_Descriptor*&) /home/jas/Downloads/Bento4-SRC-1-5-1-627/Source/C++/Core/Ap4DescriptorFactory.cpp:126

Indirect leak of 408 byte(s) in 17 object(s) allocated from:
    #0 0x7f6969285532 in operator new(unsigned long) (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x99532)
    #1 0x55b961 in AP4_List<AP4_Descriptor>::Add(AP4_Descriptor*) /home/jas/Downloads/Bento4-SRC-1-5-1-627/Source/C++/Core/Ap4List.h:160
    #2 0x55b961 in AP4_InitialObjectDescriptor::AP4_InitialObjectDescriptor(AP4_ByteStream&, unsigned char, unsigned int, unsigned int) /home/jas/Downloads/Bento4-SRC-1-5-1-627/Source/C++/Core/Ap4ObjectDescriptor.cpp:260

Indirect leak of 32 byte(s) in 1 object(s) allocated from:
    #0 0x7f6969285532 in operator new(unsigned long) (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x99532)
    #1 0x53fd32 in AP4_DescriptorFactory::CreateDescriptorFromStream(AP4_ByteStream&, AP4_Descriptor*&) /home/jas/Downloads/Bento4-SRC-1-5-1-627/Source/C++/Core/Ap4DescriptorFactory.cpp:114

SUMMARY: AddressSanitizer: 2099016 byte(s) leaked in 46 allocation(s).

FoundBy: yjiiit@aliyun.com

@fgeek

This comment has been minimized.

Copy link

fgeek commented Jan 1, 2019

I'm not developer of Bento4 nor expert in this area, but isn't it normal that there are memory leaks if program doesn't exit successfully? Correct me if I'm wrong. I'm interested to learn more about this area :)

Someone apparently requested a CVE for this, which got assigned CVE-2018-20407.

@orivej

This comment has been minimized.

Copy link
Contributor

orivej commented Jan 4, 2019

Such issues have to be fixed in order to support meaningful testing with ASAN or Valgrind: otherwise it is difficult to distinguish a false positive (when the memory is leaked in the application on purpose) from true errors (when the memory is leaked in a library, or when the application encounters an unexpected error).

Since C++11 it is easier to write code that does not leak memory (thanks to unique_ptr and shared_ptr) than code that leaks it on error but does not on success. However, Bento predates C++11 and almost does not use C++ STL or even C++ templates (except in its List and Vector). (There is some beauty in this.)

I have investigated this issue a bit, and after plugging a small possibly intentional leak in the application it looks like a true leak in the library, but I have not yet figured the exact cause. Yet it seems that the cause of this issue and #333 is the same.

@barbibulle barbibulle self-assigned this Jan 12, 2019

@barbibulle barbibulle added the fuzzing label Jan 12, 2019

@barbibulle

This comment has been minimized.

Copy link
Contributor

barbibulle commented Jan 12, 2019

Things would be definitely easier by leveraging some of the more recent features of "modern" C++. But the code base started in 2002, 17 years ago! C++ was quite different then. Since the code base has been pretty stable over the years, there's little motivation to go back and re-write in with STL classes. I do, however, welcome any bug report like this, as any memory leak should be an error that's fixable. The class library itself should have 0 memory leaks. Some of the command line apps are a bit more relaxed that way, since exiting on error keeps the code simple, relying on the OS to reclaim the memory when the process exits.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment