# 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:

```cpp
// 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:

```cpp
// 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:

```cpp
// 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**:

```cpp
// 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`:

```cpp
// 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:

```cpp
// 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:

```cpp
// 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:

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

The ReadProperties basically does this in a loop:

```cpp
// 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:

```cpp
// 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):

```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):

```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):

```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):

```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:

```cpp
// 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:

```cpp
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.