CVE-2018-14054: LibMP4v2 MP4StringProperty Handling Double Free Vulnerability

Introduction

LibMP4v2 is an open source MP4 processing library, designed to create and modify MP4 files as defined by ISO-IEC:14496-1:2001 MPEG-4 Systems.

Originally discovered by Ruikai Liu, a double free vulnerability was found in the MP4StringProperty code. While parsing MP4 atoms, it is possible to cause a MP4StringProperty’s value to be freed twice due to exception handling, resulting a double free condition. Since this is library code and not actively maintained, many third party applications seem to be affected by this without a fix.

Technical Analysis

In MP4 format, data units are called atoms, which contain information about the video file. One of those is called “mp4v”, and this particular one is related to the vulnerability. To understand the problem, we want to learn how atoms are created and destroyed in code, and eventually the walk-through should reveal the double free condition.

Object Creation

First off, atoms are parsed and read from an MP4 file. Our analysis begins with the following:

// Line 399 (mp4atom.cpp)
void MP4Atom::ReadChildAtoms() {
  // ... code ...
  // Line 428
  MP4Atom* pChildAtom = MP4Atom::ReadAtom(m_File, this);
  // code
}

In MP4Atom::ReadAtom, an atom object is created:

// Line 112 (mp4atom.cpp)
MP4Atom* MP4Atom::ReadAtom(MP4File& file, MP4Atom* pParentAtom) {
  MP4Atom* pAtom = CreateAtom(file, pParentAtom, type);
  // ... code ...

The actual atom object depends on the type specified in the media file. For example, if the type is “mp4v”, then this function should return MP4Mp4vAtom.

When the MP4Mp4vAtom object is being prepared, multiple properties are born in the process. One of those is the compressorName string property:

// Line 46 (atom_mp4v.cpp)
MP4StringProperty* pProp = new MP4StringProperty(*this, "compressorName");
pProp->SetFixedLength(32);
pProp->SetCountedFormat(true);
pProp->SetValue("");
AddProperty(pProp); /* 6 */

Basically what this does is setting the compressName property to a 32+1 byte allocation, which is tracked in an array named m_values. It also sets the default value to empty, and finally that property object is saved by calling AddProperty, which is for another array named m_pProperties:

// Line 350 (mp4property.cpp)
void MP4StringProperty::SetValue(const char* value, uint32_t index)
{
    if (m_readOnly) {
        ostringstream msg;
        msg << "property " << m_name << "is read-only";
        throw new PlatformException(msg.str().c_str(), EACCES, __FILE__, __LINE__, __FUNCTION__ );
    }

    MP4Free(m_values[index]);

    if (m_fixedLength) {
        m_values[index] = (char*)MP4Calloc(m_fixedLength + 1);
        if (value) {
            strncpy(m_values[index], value, m_fixedLength);
        }
    } else {
        if (value) {
            m_values[index] = MP4Stralloc(value);
        } else {
            m_values[index] = NULL;
        }
    }
}

Objection Destruction

The MP4StringProperty class has a destructor that empties all the heap allocations in m_values:

// Line 331 (mp4property.cpp)
MP4StringProperty::~MP4StringProperty()
{
    uint32_t count = GetCount();
    for (uint32_t i = 0; i < count; i++) {
        MP4Free(m_values[i]);
    }
}

In order to trigger that destructor, one way is to destroy the atom object, which triggers its own destructor that clears the property array:

// Line 61 (mp4atom.cpp)
MP4Atom::~MP4Atom()
{
    uint32_t i;

    for (i = 0; i < m_pProperties.Size(); i++) {
        delete m_pProperties[i];
    }

    // ... destroying other things ...
}

OK, so MP4Atom::~MP4Atom triggers MP4BytesProperty::~MP4StringProperty. Got it. And this is where things go wrong.

The First Free

Let’s rewind a bit and examine the MP4Atom::ReadAtom function again (line 112 in mp4atom.cpp). After an atom is created, it also performs a read operation toward the end of the function:

// Line 193 (mp4atom.cpp)
try {
  pAtom->Read();
}
catch (Exception* x) {
  // delete atom and rethrow so we don't leak memory.
  delete pAtom;	
  throw x;
}

The Read is a virtual function, so many atom oriented classes may implement their own. If this isn’t overloaded, then the generic version is also available. In this generic function, we just want to focus on how it loads properties:

// Line 222 (mp4atom.cpp)
void MP4Atom::Read()
{
    // ... code ...
    ReadProperties();
		// ... code ...
}

The ReadProperties basically does this in a loop:

// Line 376 (MP4Atom::ReadProperties in mp4atom.cpp)
m_pProperties[i]->Read(m_File);

As you remember, one of the properties is compressorName, which is a type of MP4StringProperty. In this context, we are looking at this Read function:

// Line 374 (mp4property.cpp)
void MP4StringProperty::Read( MP4File& file, uint32_t index )
{
   // ... code ...

    for( uint32_t i = begin; i < max; i++ ) {
        char*& value = m_values[i];
        MP4Free(value); 
        if( m_useCountedFormat ) {
            value = file.ReadCountedString( (m_useUnicode ? 2 : 1), m_useExpandedCount, m_fixedLength );
        }
    // ... code ...
    }
}

Notice the MP4Free is our first free, which frees the string property value. We know we will go down to the ReadCountedString path, because the m_useCountedFormat flag was set by SetCountedFormat while creating the compressorName property (line 48 in atom_mp4v.cpp).

The Second Free

The problem with ReadCountedString is that it may throw exceptions, which causes the property reading operation to fail, forcing the atom object to be deleted. For example:

Line 383 in MP4File::ReadCountedString (mp4file_io.cpp):

if (ix > 25) throw new PlatformException("Counted string too long 25 * 255",ERANGE, __FILE__, __LINE__, __FUNCTION__);

Line 81 in MP4File::ReadBytes, used by ReadCountedString (mp4file_io.cpp):

if( m_memoryBufferPosition + bufsiz > m_memoryBufferSize ) throw new Exception( "not enough bytes, reached end-of-memory", __FILE__, __LINE__, __FUNCTION__ );

Line 93 in MP4File::ReadBytes (mp4file_io.cpp):

if( file->read( buf, bufsiz, nin )) throw new PlatformException( "read failed", sys::getLastError(), __FILE__, __LINE__, __FUNCTION__ );

Line 95 in MP4File::ReadBytes (mp4file_io.cpp):

if( nin != bufsiz ) throw new Exception( "not enough bytes, reached end-of-file", __FILE__, __LINE__, __FUNCTION__ );

Whatever the exception is, it is handled way back in MP4Atom::ReadAtom, specifically here:

// Line 193 (mp4atom.cpp)
try {
  pAtom->Read();
}
catch (Exception* x) {
  // delete atom and rethrow so we don't leak memory.
  delete pAtom;	
  throw x;
}

Notice the delete operator, which is our second free. Again, if an atom is deleted, the MP4Atom::~MP4Atom destructor is called to clear the properties, and that causes MP4StringProperty::~MP4StringProperty to be called as part of the chain of reaction, resulting the m_values getting cleared.

Summary

In short, the MP4StringProperty handling is doomed here:

void MP4StringProperty::Read( MP4File& file, uint32_t index ) {
  // ... code ...

  // Line 392 (mp4property.cpp)
  MP4Free(value); 
  if( m_useCountedFormat ) {
    value = file.ReadCountedString( (m_useUnicode ? 2 : 1), m_useExpandedCount, m_fixedLength );

	// ... code ...
}

The MP4StringProperty::Read function wants to update the string property, but never gets a replacement. Instead, it could get an exception, and causing the second free.

There are multiple ways to fix this. The easier way is by setting the value to NULL after the first free, so the string reading operation could continue successfully even with the second free. The other way is probably redo the exception handling a bit more strategically.

This vulnerability is neat, but it’s also in a library that has stopped upstream development for some number of years, and more recently Debian/Ubuntu completely expunged it from their repositories. A bigger risk is when software embeds a copy (because upstream is dead) that never gets updated, leading to zombie vulnerabilities rising up from the grave of an obsolete video decoder.