Fuzzing fixes

src/descriptors.cpp

@@ -505,7 +505,7 @@ void MP4ContentIdDescriptor::Read(MP4File& file)
     ReadProperties(file, 0, 1);
 
     /* if compatiblity != 0 */
-    if (((MP4Integer8Property*)m_pProperties[0])->GetValue() != 0) {
+    if (((MP4BitfieldProperty*)m_pProperties[0])->GetValue() != 0) {
         /* we don't understand it */
         log.verbose1f("incompatible content id descriptor");
         return;

src/itmf/Tags.cpp

@@ -420,7 +420,7 @@ Tags::fetchGenre( const CodeItemMap& cim, uint16_t& cpp, const uint16_t*& c )
         return;
 
     MP4ItmfData& data = f->second->dataList.elements[0];
-    if( NULL == data.value )
+    if( NULL == data.value || data.valueSize < 2 )
         return;
 
     cpp = (uint16_t(data.value[0]) <<  8)
@@ -444,7 +444,7 @@ Tags::fetchDisk( const CodeItemMap& cim, MP4TagDisk& cpp, const MP4TagDisk*& c )
 
     MP4ItmfData& data = f->second->dataList.elements[0];
 
-    if( NULL == data.value )
+    if( NULL == data.value || data.valueSize < 6 )
         return;
 
     cpp.index = (uint16_t(data.value[2]) <<  8)
@@ -471,7 +471,7 @@ Tags::fetchTrack( const CodeItemMap& cim, MP4TagTrack& cpp, const MP4TagTrack*&
 
     MP4ItmfData& data = f->second->dataList.elements[0];
 
-    if( NULL == data.value )
+    if( NULL == data.value || data.valueSize < 6 )
         return;
 
     cpp.index = (uint16_t(data.value[2]) <<  8)
@@ -517,7 +517,7 @@ Tags::fetchInteger( const CodeItemMap& cim, const string& code, uint16_t& cpp, c
 
     MP4ItmfData& data = f->second->dataList.elements[0];
 
-    if( NULL == data.value )
+    if( NULL == data.value || data.valueSize < 2 )
         return;
 
     cpp = (uint16_t(data.value[0]) <<  8)
@@ -540,7 +540,7 @@ Tags::fetchInteger( const CodeItemMap& cim, const string& code, uint32_t& cpp, c
 
     MP4ItmfData& data = f->second->dataList.elements[0];
 
-    if( NULL == data.value )
+    if( NULL == data.value || data.valueSize < 4 )
         return;
 
     cpp = (uint32_t(data.value[0]) << 24)
@@ -565,7 +565,7 @@ Tags::fetchInteger( const CodeItemMap& cim, const string& code, uint64_t& cpp, c
 
     MP4ItmfData& data = f->second->dataList.elements[0];
 
-    if( NULL == data.value )
+    if( NULL == data.value || data.valueSize < 8 )
         return;
 
     cpp = (uint64_t(data.value[0]) << 56)

src/mp4.cpp

@@ -2567,14 +2567,18 @@ MP4FileHandle MP4ReadProvider( const char* fileName, const MP4FileProvider* file
     {
         uint32_t ix;
 
-        for (ix = 0; pSeqHeaderSize[ix] != 0; ++ix) {
-            free(pSeqHeaders[ix]);
+        if (pSeqHeaderSize) {
+            for (ix = 0; pSeqHeaderSize[ix] != 0; ++ix) {
+                free(pSeqHeaders[ix]);
+            }
         }
         free(pSeqHeaders);
         free(pSeqHeaderSize);
 
-        for (ix = 0; pPictHeaderSize[ix] != 0; ++ix) {
-            free(pPictHeader[ix]);
+        if (pPictHeaderSize) {
+            for (ix = 0; pPictHeaderSize[ix] != 0; ++ix) {
+                free(pPictHeader[ix]);
+            }
         }
         free(pPictHeader);
         free(pPictHeaderSize);

src/mp4array.h

@@ -77,9 +77,8 @@ class MP4Array {
                   throw new PlatformException("illegal array index", ERANGE, __FILE__, __LINE__, __FUNCTION__); \
             } \
             if (m_numElements == m_maxNumElements) { \
-                m_maxNumElements = max(m_maxNumElements, (MP4ArrayIndex)1) * 2; \
-                m_elements = (type*)MP4Realloc(m_elements, \
-                    m_maxNumElements * sizeof(type)); \
+                MP4ArrayIndex newCapacity = max(m_maxNumElements, (MP4ArrayIndex)1) * 2; \
+                Reserve(newCapacity); \
             } \
             memmove(&m_elements[newIndex + 1], &m_elements[newIndex], \
                 (m_numElements - newIndex) * sizeof(type)); \
@@ -99,13 +98,19 @@ class MP4Array {
                   (m_numElements - index) * sizeof(type)); \
             } \
         } \
+        \
+        void Reserve(MP4ArrayIndex newSize) { \
+            if (newSize > m_maxNumElements) { \
+                if ( (uint64_t) newSize * sizeof(type) > 0xFFFFFFFF ) \
+                    throw new PlatformException("requested array size exceeds 4GB", ERANGE, __FILE__, __LINE__, __FUNCTION__); /* prevent overflow */ \
+                m_elements = (type*)MP4Realloc(m_elements, newSize * sizeof(type)); \
+                m_maxNumElements = newSize; \
+            } \
+        } \
+        \
         void Resize(MP4ArrayIndex newSize) { \
+            Reserve(newSize); \
             m_numElements = newSize; \
-            m_maxNumElements = newSize; \
-            if ( (uint64_t) m_maxNumElements * sizeof(type) > 0xFFFFFFFF ) \
-               throw new PlatformException("requested array size exceeds 4GB", ERANGE, __FILE__, __LINE__, __FUNCTION__); /* prevent overflow */ \
-            m_elements = (type*)MP4Realloc(m_elements, \
-                m_maxNumElements * sizeof(type)); \
         } \
         \
         type& operator[](MP4ArrayIndex index) { \

src/mp4atom.cpp

@@ -155,7 +155,10 @@ MP4Atom* MP4Atom::ReadAtom(MP4File& file, MP4Atom* pParentAtom)
     log.verbose1f("\"%s\": type = \"%s\" data-size = %" PRIu64 " (0x%" PRIx64 ") hdr %u",
                   file.GetFilename().c_str(), type, dataSize, dataSize, hdrSize);
 
-    if (pos + hdrSize + dataSize > pParentAtom->GetEnd()) {
+    // |dataSize + hdrSize| is what |dataSize| was when it was read.
+    // Rather than adding |pos| to it (which can overflow), compare it against how much is left in the file,
+    // otherwise the |SetEnd()| call below can jump backwards and result in an infinite loop of parsing the same atom.
+    if (dataSize + hdrSize > pParentAtom->GetEnd() - pos) {
         log.errorf("%s: \"%s\": invalid atom size, extends outside parent atom - skipping to end of \"%s\" \"%s\" %" PRIu64 " vs %" PRIu64,
                    __FUNCTION__, file.GetFilename().c_str(), pParentAtom->GetType(), type,
                    pos + hdrSize + dataSize,
@@ -173,32 +176,32 @@ MP4Atom* MP4Atom::ReadAtom(MP4File& file, MP4Atom* pParentAtom)
     }
 
     MP4Atom* pAtom = CreateAtom(file, pParentAtom, type);
-    //    pAtom->SetFile(pFile);
-    pAtom->SetStart(pos);
-    pAtom->SetEnd(pos + hdrSize + dataSize);
-    pAtom->SetLargesizeMode(largesizeMode);
-    pAtom->SetSize(dataSize);
-    if (ATOMID(type) == ATOMID("uuid")) {
-        pAtom->SetExtendedType(extendedType);
-    }
-    if (pAtom->IsUnknownType()) {
-        if (!IsReasonableType(pAtom->GetType())) {
-            log.warningf("%s: \"%s\": atom type %s is suspect", __FUNCTION__, file.GetFilename().c_str(),
-                         pAtom->GetType());
-        } else {
-            log.verbose1f("\"%s\": Info: atom type %s is unknown", file.GetFilename().c_str(),
-                          pAtom->GetType());
+	try {
+        //    pAtom->SetFile(pFile);
+        pAtom->SetStart(pos);
+        pAtom->SetEnd(pos + hdrSize + dataSize);
+        pAtom->SetLargesizeMode(largesizeMode);
+        pAtom->SetSize(dataSize);
+        if (ATOMID(type) == ATOMID("uuid")) {
+            pAtom->SetExtendedType(extendedType);
         }
+        if (pAtom->IsUnknownType()) {
+            if (!IsReasonableType(pAtom->GetType())) {
+                log.warningf("%s: \"%s\": atom type %s is suspect", __FUNCTION__, file.GetFilename().c_str(),
+                            pAtom->GetType());
+            } else {
+                log.verbose1f("\"%s\": Info: atom type %s is unknown", file.GetFilename().c_str(),
+                            pAtom->GetType());
+            }
 
-        if (dataSize > 0) {
-            pAtom->AddProperty(
-                new MP4BytesProperty(*pAtom, "data", dataSize));
+            if (dataSize > 0) {
+                pAtom->AddProperty(
+                    new MP4BytesProperty(*pAtom, "data", dataSize));
+            }
         }
-    }
 
-    pAtom->SetParentAtom(pParentAtom);
+        pAtom->SetParentAtom(pParentAtom);
 
-	try {
 		pAtom->Read();
 	}
 	catch (Exception* x) {

src/mp4file.cpp

@@ -2562,7 +2562,7 @@ MP4ChapterType MP4File::GetChapters(MP4Chapter_t ** chapterList, uint32_t * chap
             if (0 < counter)
             {
                 uint32_t timescale = pChapterTrack->GetTimeScale();
-                MP4Chapter_t * chapters = (MP4Chapter_t*)MP4Malloc(sizeof(MP4Chapter_t) * counter);
+                UniqueArray<MP4Chapter_t> chapters(counter);
 
                 // process all chapter sample
                 for (uint32_t i = 0; i < counter; ++i)
@@ -2588,7 +2588,7 @@ MP4ChapterType MP4File::GetChapters(MP4Chapter_t ** chapterList, uint32_t * chap
                     sample = 0;
                 }
 
-                *chapterList = chapters;
+                *chapterList = chapters.release();
                 *chapterCount = counter;
 
                 // we got chapters so we are done
@@ -2647,7 +2647,7 @@ MP4ChapterType MP4File::GetChapters(MP4Chapter_t ** chapterList, uint32_t * chap
             return MP4ChapterTypeNone;
         }
 
-        MP4Chapter_t * chapters = (MP4Chapter_t*)MP4Malloc(sizeof(MP4Chapter_t) * counter);
+        UniqueArray<MP4Chapter_t> chapters(counter);
 
         // get the name of the first chapter
         name = pName->GetValue();
@@ -2685,7 +2685,7 @@ MP4ChapterType MP4File::GetChapters(MP4Chapter_t ** chapterList, uint32_t * chap
             chapters[i].duration = duration;
         }
 
-        *chapterList = chapters;
+        *chapterList = chapters.release();
         *chapterCount = counter;
 
         return MP4ChapterTypeNero;

src/mp4info.cpp

@@ -577,7 +577,7 @@ char* MP4Info(
         try {
             if (trackId == MP4_INVALID_TRACK_ID) {
                 uint32_t buflen = 4 * 1024;
-                info = (char*)MP4Calloc(buflen);
+                info = (char*)MP4Calloc(buflen + 1);
 
                 buflen -= snprintf(info, buflen,
                                    "Track\tType\tInfo\n");

src/mp4util.h

@@ -84,6 +84,23 @@ inline void* MP4Realloc(void* p, uint32_t newSize) {
     return temp;
 }
 
+// C++98 unique_ptr-like that allocates on construction.
+template <typename T>
+class UniqueArray {
+    T *m_ptr;
+    UniqueArray(const UniqueArray&);
+    UniqueArray&operator=(const UniqueArray&);
+public:
+    explicit UniqueArray(size_t count) : m_ptr((T*)MP4Malloc(sizeof(T) * count)) {}
+    ~UniqueArray() { CHECK_AND_FREE(m_ptr); }
+    T & operator[](size_t idx) { return m_ptr[idx]; }
+    T* release() {
+        T * ptr = m_ptr;
+        m_ptr = NULL;
+        return ptr;
+    }
+};
+
 uint32_t STRTOINT32( const char* );
 void     INT32TOSTR( uint32_t, char* );