diff --git a/src/descriptors.cpp b/src/descriptors.cpp index e83fc66..062620a 100644 --- a/src/descriptors.cpp +++ b/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; diff --git a/src/itmf/Tags.cpp b/src/itmf/Tags.cpp index e8ca481..c2e90d8 100644 --- a/src/itmf/Tags.cpp +++ b/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) diff --git a/src/mp4.cpp b/src/mp4.cpp index 626a802..10a3d0f 100644 --- a/src/mp4.cpp +++ b/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); diff --git a/src/mp4array.h b/src/mp4array.h index 69d470a..a3396bc 100644 --- a/src/mp4array.h +++ b/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) { \ diff --git a/src/mp4atom.cpp b/src/mp4atom.cpp index 0945748..542e6d0 100644 --- a/src/mp4atom.cpp +++ b/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) { diff --git a/src/mp4file.cpp b/src/mp4file.cpp index cd4ba83..6aef5be 100644 --- a/src/mp4file.cpp +++ b/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 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 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; diff --git a/src/mp4info.cpp b/src/mp4info.cpp index 676a5be..ac1ed41 100644 --- a/src/mp4info.cpp +++ b/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"); diff --git a/src/mp4util.h b/src/mp4util.h index b33bb44..8c8b51e 100644 --- a/src/mp4util.h +++ b/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 +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* );