Block parsing is now robust

Previously, parsing of a Block element was done inside its
constructor.  Parsing errors were handled via assertion checks,
but this only works in practice if there are no actual errors
in the file.

We did come across a file, however, that used EMBL-style lacing,
but the lacing was done incorrectly and so the parse asserted.
This isn't acceptable for a production system, and more a graceful
handling of parse errors was needed.

The code was restructured such that the Block object's ctor does
only trivial initialization of member variables.  A separate Parse
method was added, that is called after the object is constructed.
If the parse succeeds all is well, otherwise the object is destroyed
and the error is reported to the caller.

This commit fixes bug tracker issue #398, described here:

http://code.google.com/p/webm/issues/detail?id=398

Change-Id: Ib95ca95d0eec08cf670b308c461e42ed8345e890

mkvparser.cpp

@@ -21,7 +21,7 @@ void mkvparser::GetVersion(int& major, int& minor, int& build, int& revision)
     major = 1;
     minor = 0;
     build = 0;
-    revision = 22;
+    revision = 23;
 }
 
 long long mkvparser::ReadUInt(IMkvReader* pReader, long long pos, long& len)
@@ -5404,7 +5404,10 @@ long Track::GetFirst(const BlockEntry*& pBlockEntry) const
             return E_BUFFER_NOT_FULL;
         }
 
-        pBlockEntry = pCluster->GetFirst();
+        long status = pCluster->GetFirst(pBlockEntry);
+
+        if (status < 0)  //error
+            return status;
 
         if (pBlockEntry == 0)  //empty cluster
         {
@@ -5422,10 +5425,17 @@ long Track::GetFirst(const BlockEntry*& pBlockEntry) const
             if ((tn == m_info.number) && VetEntry(pBlockEntry))
                 return 0;
 
-            pBlockEntry = pCluster->GetNext(pBlockEntry);
+            const BlockEntry* pNextEntry;
 
-            if (pBlockEntry == 0)
+            status = pCluster->GetNext(pBlockEntry, pNextEntry);
+
+            if (status < 0)  //error
+                return status;
+
+            if (pNextEntry == 0)
                 break;
+
+            pBlockEntry = pNextEntry;
         }
 
         ++i;
@@ -5459,7 +5469,10 @@ long Track::GetNext(
     assert(pCluster);
     assert(!pCluster->EOS());
 
-    pNextEntry = pCluster->GetNext(pCurrEntry);
+    long status = pCluster->GetNext(pCurrEntry, pNextEntry);
+
+    if (status < 0)  //error
+        return status;
 
     for (int i = 0; ; )
     {
@@ -5471,7 +5484,12 @@ long Track::GetNext(
             if (pNextBlock->GetTrackNumber() == m_info.number)
                 return 0;
 
-            pNextEntry = pCluster->GetNext(pNextEntry);
+            pCurrEntry = pNextEntry;
+
+            status = pCluster->GetNext(pCurrEntry, pNextEntry);
+
+            if (status < 0) //error
+                return status;
         }
 
         pCluster = m_pSegment->GetNext(pCluster);
@@ -5514,7 +5532,10 @@ long Track::GetNext(
             return E_BUFFER_NOT_FULL;
         }
 
-        pNextEntry = pCluster->GetFirst();
+        status = pCluster->GetFirst(pNextEntry);
+
+        if (status < 0)  //error
+            return status;
 
         if (pNextEntry == NULL)  //empty cluster
             continue;
@@ -5632,7 +5653,6 @@ Track::EOSBlock::EOSBlock() :
 {
 }
 
-
 BlockEntry::Kind Track::EOSBlock::GetKind() const
 {
     return kBlockEOS;
@@ -6767,11 +6787,13 @@ long Cluster::Parse(long long& pos, long& len) const
             return E_BUFFER_NOT_FULL;
         }
 
+        Cluster* const this_ = const_cast<Cluster*>(this);
+
         if (id == 0x20)  //BlockGroup
-            return ParseBlockGroup(size, pos, len);
+            return this_->ParseBlockGroup(size, pos, len);
 
         if (id == 0x23)  //SimpleBlock
-            return ParseSimpleBlock(size, pos, len);
+            return this_->ParseSimpleBlock(size, pos, len);
 
         pos += size;  //consume payload
         assert((cluster_stop < 0) || (pos <= cluster_stop));
@@ -6813,7 +6835,7 @@ long Cluster::Parse(long long& pos, long& len) const
 long Cluster::ParseSimpleBlock(
     long long block_size,
     long long& pos,
-    long& len) const
+    long& len)
 {
     const long long block_start = pos;
     const long long block_stop = pos + block_size;
@@ -6915,7 +6937,11 @@ long Cluster::ParseSimpleBlock(
         return E_BUFFER_NOT_FULL;
     }
 
-    CreateBlock(0x23, block_start, block_size);  //simple block id
+    status = CreateBlock(0x23, block_start, block_size);  //simple block id
+
+    if (status != 0)
+        return status;
+
     m_pos = block_stop;
 
     return 0;  //success
@@ -6925,7 +6951,7 @@ long Cluster::ParseSimpleBlock(
 long Cluster::ParseBlockGroup(
     long long payload_size,
     long long& pos,
-    long& len) const
+    long& len)
 {
     const long long payload_start = pos;
     const long long payload_stop = pos + payload_size;
@@ -7133,7 +7159,11 @@ long Cluster::ParseBlockGroup(
 
     assert(pos == payload_stop);
 
-    CreateBlock(0x20, payload_start, payload_size);  //BlockGroup ID
+    status = CreateBlock(0x20, payload_start, payload_size);  //BlockGroup ID
+
+    if (status != 0)
+        return status;
+
     m_pos = payload_stop;
 
     return 0;  //success
@@ -7595,7 +7625,12 @@ long long Cluster::GetTime() const
 
 long long Cluster::GetFirstTime() const
 {
-    const BlockEntry* const pEntry = GetFirst();
+    const BlockEntry* pEntry;
+
+    const long status = GetFirst(pEntry);
+
+    if (status < 0)  //error
+        return status;
 
     if (pEntry == NULL)  //empty cluster
         return GetTime();
@@ -7609,7 +7644,12 @@ long long Cluster::GetFirstTime() const
 
 long long Cluster::GetLastTime() const
 {
-    const BlockEntry* const pEntry = GetLast();
+    const BlockEntry* pEntry;
+
+    const long status = GetLast(pEntry);
+
+    if (status < 0)  //error
+        return status;
 
     if (pEntry == NULL)  //empty cluster
         return GetTime();
@@ -7621,12 +7661,12 @@ long long Cluster::GetLastTime() const
 }
 
 
-void Cluster::CreateBlock(
+long Cluster::CreateBlock(
     long long id,
     long long pos,   //absolute pos of payload
-    long long size) const
+    long long size)
 {
-    BlockEntry** ppEntry;
+    assert((id == 0x20) || (id == 0x23));  //BlockGroup or SimpleBlock
 
     if (m_entries_count < 0)  //haven't parsed anything yet
     {
@@ -7636,14 +7676,12 @@ void Cluster::CreateBlock(
         m_entries_size = 1024;
         m_entries = new BlockEntry*[m_entries_size];
 
-        ppEntry = m_entries;
+        m_entries_count = 0;
-        m_entries_count = 1;
     }
     else
     {
         assert(m_entries);
         assert(m_entries_size > 0);
-        assert(m_entries_count > 0);
         assert(m_entries_count <= m_entries_size);
 
         if (m_entries_count >= m_entries_size)
@@ -7666,36 +7704,23 @@ void Cluster::CreateBlock(
             m_entries = entries;
             m_entries_size = entries_size;
         }
-
-        ppEntry = m_entries + m_entries_count;
-        ++m_entries_count;
     }
 
     if (id == 0x20)  //BlockGroup ID
-        CreateBlockGroup(pos, size, ppEntry);
+        return CreateBlockGroup(pos, size);
-    else
+    else  //SimpleBlock ID
-    {
+        return CreateSimpleBlock(pos, size);
-        assert(id == 0x23);  //SimpleBlock ID
-        CreateSimpleBlock(pos, size, ppEntry);
-    }
 }
 
 
-void Cluster::CreateBlockGroup(
+long Cluster::CreateBlockGroup(
     long long st,
-    long long sz,
+    long long sz)
-    BlockEntry**& ppEntry) const
 {
     assert(m_entries);
     assert(m_entries_size > 0);
-    assert(ppEntry);
+    assert(m_entries_count >= 0);
-    assert(ppEntry >= m_entries);
+    assert(m_entries_count < m_entries_size);
-
-    const ptrdiff_t idx = ppEntry - m_entries;
-    assert(idx >= 0);
-    assert(idx < m_entries_size);
-
-    Cluster* const this_ = const_cast<Cluster*>(this);
 
     IMkvReader* const pReader = m_pSegment->m_pReader;
 
@@ -7769,133 +7794,185 @@ void Cluster::CreateBlockGroup(
     assert(bpos >= 0);
     assert(bsize >= 0);
 
-    *ppEntry++ = new BlockGroup(this_, idx, bpos, bsize, prev, next, duration);
+    const long idx = m_entries_count;
+
+    BlockEntry** const ppEntry = m_entries + idx;
+    BlockEntry*& pEntry = *ppEntry;
+
+    pEntry = new (std::nothrow) BlockGroup(
+                                  this,
+                                  idx,
+                                  bpos,
+                                  bsize,
+                                  prev,
+                                  next,
+                                  duration);
+
+    if (pEntry == NULL)
+        return -1;  //generic error
+
+    BlockGroup* const p = static_cast<BlockGroup*>(pEntry);
+
+    const long status = p->Parse();
+
+    if (status == 0)  //success
+    {
+        ++m_entries_count;
+        return 0;
+    }
+
+    delete pEntry;
+    pEntry = 0;
+
+    return status;
 }
 
 
 
-void Cluster::CreateSimpleBlock(
+long Cluster::CreateSimpleBlock(
     long long st,
-    long long sz,
+    long long sz)
-    BlockEntry**& ppEntry) const
 {
     assert(m_entries);
     assert(m_entries_size > 0);
-    assert(ppEntry);
+    assert(m_entries_count >= 0);
-    assert(ppEntry >= m_entries);
+    assert(m_entries_count < m_entries_size);
 
-    const ptrdiff_t idx = ppEntry - m_entries;
+    const long idx = m_entries_count;
-    assert(idx >= 0);
-    assert(idx < m_entries_size);
 
-    Cluster* const this_ = const_cast<Cluster*>(this);
+    BlockEntry** const ppEntry = m_entries + idx;
-    *ppEntry++ = new SimpleBlock(this_, idx, st, sz);
+    BlockEntry*& pEntry = *ppEntry;
+
+    pEntry = new (std::nothrow) SimpleBlock(this, idx, st, sz);
+
+    if (pEntry == NULL)
+        return -1;  //generic error
+
+    SimpleBlock* const p = static_cast<SimpleBlock*>(pEntry);
+
+    const long status = p->Parse();
+
+    if (status == 0)
+    {
+        ++m_entries_count;
+        return 0;
+    }
+
+    delete pEntry;
+    pEntry = 0;
+
+    return status;
 }
 
 
-const BlockEntry* Cluster::GetFirst() const
+long Cluster::GetFirst(const BlockEntry*& pFirst) const
 {
-#if 0
-    LoadBlockEntries();
-
-    if ((m_entries == NULL) || (m_entries_count <= 0))
-        return NULL;
-#else
     if (m_entries_count <= 0)
     {
         long long pos;
         long len;
 
         const long status = Parse(pos, len);
-        assert(status >= 0);
 
-        if (m_entries_count <= 0)
+        if (status < 0)  //error
-            return NULL;  //empty cluster
+        {
+            pFirst = NULL;
+            return status;
+        }
+
+        if (m_entries_count <= 0)  //empty cluster
+        {
+            pFirst = NULL;
+            return 0;
+        }
+    }
 
     assert(m_entries);
-        assert(m_entries_count > 0);
-    }
-#endif
 
-    const BlockEntry* const pFirst = m_entries[0];
+    pFirst = m_entries[0];
     assert(pFirst);
 
-    return pFirst;
+    return 0;  //success
 }
 
-
+long Cluster::GetLast(const BlockEntry*& pLast) const
-const BlockEntry* Cluster::GetLast() const
 {
-#if 0
-    LoadBlockEntries();
-
-    if ((m_entries == NULL) || (m_entries_count <= 0))
-        return NULL;
-#else
     for (;;)
     {
         long long pos;
         long len;
 
         const long status = Parse(pos, len);
-        assert(status >= 0);
 
-        if (status != 0)  //no new block
+        if (status < 0)  //error
+        {
+            pLast = NULL;
+            return status;
+        }
+
+        if (status > 0)  //no new block
             break;
     }
 
     if (m_entries_count <= 0)
-        return NULL;
+    {
+        pLast = NULL;
+        return 0;
+    }
 
     assert(m_entries);
-#endif
 
     const long idx = m_entries_count - 1;
 
-    const BlockEntry* const pLast = m_entries[idx];
+    pLast = m_entries[idx];
     assert(pLast);
 
-    return pLast;
+    return 0;
 }
 
 
-const BlockEntry* Cluster::GetNext(const BlockEntry* pEntry) const
+long Cluster::GetNext(
+    const BlockEntry* pCurr,
+    const BlockEntry*& pNext) const
 {
-    assert(pEntry);
+    assert(pCurr);
-    assert(m_entries != NULL);
+    assert(m_entries);
     assert(m_entries_count > 0);
 
-    size_t idx = pEntry->GetIndex();
+    size_t idx = pCurr->GetIndex();
     assert(idx < size_t(m_entries_count));
-    assert(m_entries[idx] == pEntry);
+    assert(m_entries[idx] == pCurr);
 
     ++idx;
 
-#if 0
-    if (idx >= size_t(m_entries_count))
-      return NULL;
-#else
     if (idx >= size_t(m_entries_count))
     {
         long long pos;
         long len;
 
         const long status = Parse(pos, len);
-        assert(status >= 0);
 
-        if (status < 0)
+        if (status < 0)  //error
-            return NULL;
+        {
+            pNext = NULL;
+            return status;
+        }
 
         if (status > 0)
-            return NULL;
+        {
+            pNext = NULL;
+            return 0;
+        }
 
         assert(m_entries);
         assert(m_entries_count > 0);
         assert(idx < size_t(m_entries_count));
     }
-#endif
 
-    return m_entries[idx];
+    pNext = m_entries[idx];
+    assert(pNext);
+
+    return 0;
 }
 
 
@@ -8313,11 +8390,17 @@ SimpleBlock::SimpleBlock(
     long long start,
     long long size) :
     BlockEntry(pCluster, idx),
-    m_block(start, size, pCluster->m_pSegment->m_pReader)
+    m_block(start, size)
 {
 }
 
 
+long SimpleBlock::Parse()
+{
+    return m_block.Parse(m_pCluster->m_pSegment->m_pReader);
+}
+
+
 BlockEntry::Kind SimpleBlock::GetKind() const
 {
     return kBlockSimple;
@@ -8339,12 +8422,24 @@ BlockGroup::BlockGroup(
     long long next,
     long long duration) :
     BlockEntry(pCluster, idx),
-    m_block(block_start, block_size, pCluster->m_pSegment->m_pReader),
+    m_block(block_start, block_size),
     m_prev(prev),
     m_next(next),
     m_duration(duration)
 {
-    m_block.SetKey((prev > 0) && (next <= 0));
+}
+
+
+long BlockGroup::Parse()
+{
+    const long status = m_block.Parse(m_pCluster->m_pSegment->m_pReader);
+
+    if (status)
+        return status;
+
+    m_block.SetKey((m_prev > 0) && (m_next <= 0));
+
+    return 0;
 }
 
 
@@ -8389,56 +8484,86 @@ long long BlockGroup::GetNextTimeCode() const
 }
 
 
-Block::Block(long long start, long long size_, IMkvReader* pReader) :
+Block::Block(long long start, long long size_) :
     m_start(start),
-    m_size(size_)
+    m_size(size_),
+    m_track(0),
+    m_timecode(-1),
+    m_flags(0),
+    m_frames(NULL),
+    m_frame_count(-1)
 {
-    long long pos = start;
+}
-    const long long stop = start + size_;
+
+
+Block::~Block()
+{
+    delete[] m_frames;
+}
+
+
+long Block::Parse(IMkvReader* pReader)
+{
+    assert(pReader);
+    assert(m_start >= 0);
+    assert(m_size >= 0);
+    assert(m_track <= 0);
+    assert(m_frames == NULL);
+    assert(m_frame_count <= 0);
+
+    long long pos = m_start;
+    const long long stop = m_start + m_size;
 
     long len;
 
     m_track = ReadUInt(pReader, pos, len);
-    assert(m_track > 0);
+
-    assert((pos + len) <= stop);
+    if (m_track <= 0)
+        return E_FILE_FORMAT_INVALID;
+
+    if ((pos + len) > stop)
+        return E_FILE_FORMAT_INVALID;
 
     pos += len;  //consume track number
-    assert((stop - pos) >= 2);
+
+    if ((stop - pos) < 2)
+        return E_FILE_FORMAT_INVALID;
 
     long status;
-
-#if 0
-    m_timecode = Unserialize2SInt(pReader, pos);
-#else
     long long value;
 
     status = UnserializeInt(pReader, pos, 2, value);
-    assert(status == 0);
+
-    assert(value >= SHRT_MIN);
+    if (status)
-    assert(value <= SHRT_MAX);
+        return E_FILE_FORMAT_INVALID;
+
+    if (value < SHRT_MIN)
+        return E_FILE_FORMAT_INVALID;
+
+    if (value > SHRT_MAX)
+        return E_FILE_FORMAT_INVALID;
 
     m_timecode = static_cast<short>(value);
-#endif
 
     pos += 2;
-    assert((stop - pos) >= 1);
+
+    if ((stop - pos) <= 0)
+        return E_FILE_FORMAT_INVALID;
 
     status = pReader->Read(pos, 1, &m_flags);
-    assert(status == 0);
 
-#if 0
+    if (status)
-    const int invisible = int(m_flags & 0x08) >> 3;
+        return E_FILE_FORMAT_INVALID;
-    invisible;
-    assert(!invisible);  //TODO
-#endif
 
     const int lacing = int(m_flags & 0x06) >> 1;
 
     ++pos;  //consume flags byte
-    assert(pos <= stop);
 
     if (lacing == 0)  //no lacing
     {
+        if (pos > stop)
+            return E_FILE_FORMAT_INVALID;
+
         m_frame_count = 1;
         m_frames = new Frame[m_frame_count];
 
@@ -8446,19 +8571,24 @@ Block::Block(long long start, long long size_, IMkvReader* pReader) :
         f.pos = pos;
 
         const long long frame_size = stop - pos;
-        assert(frame_size <= LONG_MAX);
+
+        if (frame_size > LONG_MAX)
+            return E_FILE_FORMAT_INVALID;
 
         f.len = static_cast<long>(frame_size);
 
-        return;
+        return 0;  //success
     }
 
-    assert(pos < stop);
+    if (pos >= stop)
+        return E_FILE_FORMAT_INVALID;
 
     unsigned char biased_count;
 
     status = pReader->Read(pos, 1, &biased_count);
-    assert(status == 0);
+
+    if (status)
+        return E_FILE_FORMAT_INVALID;
 
     ++pos;  //consume frame count
     assert(pos <= stop);
@@ -8484,8 +8614,13 @@ Block::Block(long long start, long long size_, IMkvReader* pReader) :
             {
                 unsigned char val;
 
+                if (pos >= stop)
+                    return E_FILE_FORMAT_INVALID;
+
                 status = pReader->Read(pos, 1, &val);
-                assert(status == 0);
+
+                if (status)
+                    return E_FILE_FORMAT_INVALID;
 
                 ++pos;  //consume xiph size byte
 
@@ -8498,6 +8633,8 @@ Block::Block(long long start, long long size_, IMkvReader* pReader) :
             Frame& f = *pf++;
             assert(pf < pf_end);
 
+            f.pos = 0;  //patch later
+
             f.len = frame_size;
             size += frame_size;  //contribution of this frame
 
@@ -8505,17 +8642,25 @@ Block::Block(long long start, long long size_, IMkvReader* pReader) :
         }
 
         assert(pf < pf_end);
-        assert(pos < stop);
+        assert(pos <= stop);
 
         {
             Frame& f = *pf++;
-            assert(pf == pf_end);
+
+            if (pf != pf_end)
+                return E_FILE_FORMAT_INVALID;
+
+            f.pos = 0;  //patch later
 
             const long long total_size = stop - pos;
-            assert(total_size > size);
+
+            if (total_size < size)
+                return E_FILE_FORMAT_INVALID;
 
             const long long frame_size = total_size - size;
-            assert(frame_size <= LONG_MAX);
+
+            if (frame_size > LONG_MAX)
+                return E_FILE_FORMAT_INVALID;
 
             f.len = static_cast<long>(frame_size);
         }
@@ -8535,10 +8680,14 @@ Block::Block(long long start, long long size_, IMkvReader* pReader) :
     else if (lacing == 2)  //fixed-size lacing
     {
         const long long total_size = stop - pos;
-        assert((total_size % m_frame_count) == 0);
+
+        if ((total_size % m_frame_count) != 0)
+            return E_FILE_FORMAT_INVALID;
 
         const long long frame_size = total_size / m_frame_count;
-        assert(frame_size <= LONG_MAX);
+
+        if (frame_size > LONG_MAX)
+            return E_FILE_FORMAT_INVALID;
 
         Frame* pf = m_frames;
         Frame* const pf_end = pf + m_frame_count;
@@ -8560,18 +8709,28 @@ Block::Block(long long start, long long size_, IMkvReader* pReader) :
     else
     {
         assert(lacing == 3);  //EBML lacing
-        assert(pos < stop);
+
+        if (pos >= stop)
+            return E_FILE_FORMAT_INVALID;
 
         long size = 0;
         int frame_count = m_frame_count;
 
         long long frame_size = ReadUInt(pReader, pos, len);
-        assert(frame_size > 0);
+
-        assert(frame_size <= LONG_MAX);
+        if (frame_size < 0)
-        assert((pos + len) <= stop);
+            return E_FILE_FORMAT_INVALID;
+
+        if (frame_size > LONG_MAX)
+            return E_FILE_FORMAT_INVALID;
+
+        if ((pos + len) > stop)
+            return E_FILE_FORMAT_INVALID;
 
         pos += len; //consume length of size of first frame
-        assert((pos + frame_size) <= stop);
+
+        if ((pos + frame_size) > stop)
+            return E_FILE_FORMAT_INVALID;
 
         Frame* pf = m_frames;
         Frame* const pf_end = pf + m_frame_count;
@@ -8579,6 +8738,8 @@ Block::Block(long long start, long long size_, IMkvReader* pReader) :
         {
             Frame& curr = *pf;
 
+            curr.pos = 0;  //patch later
+
             curr.len = static_cast<long>(frame_size);
             size += curr.len;  //contribution of this frame
         }
@@ -8587,18 +8748,27 @@ Block::Block(long long start, long long size_, IMkvReader* pReader) :
 
         while (frame_count > 1)
         {
-            assert(pos < stop);
+            if (pos >= stop)
+                return E_FILE_FORMAT_INVALID;
+
             assert(pf < pf_end);
 
             const Frame& prev = *pf++;
-            assert(pf < pf_end);
             assert(prev.len == frame_size);
 
+            assert(pf < pf_end);
+
             Frame& curr = *pf;
 
+            curr.pos = 0;  //patch later
+
             const long long delta_size_ = ReadUInt(pReader, pos, len);
-            assert(delta_size_ >= 0);
+
-            assert((pos + len) <= stop);
+            if (delta_size_ < 0)
+                return E_FILE_FORMAT_INVALID;
+
+            if ((pos + len) > stop)
+                return E_FILE_FORMAT_INVALID;
 
             pos += len;  //consume length of (delta) size
             assert(pos <= stop);
@@ -8608,8 +8778,12 @@ Block::Block(long long start, long long size_, IMkvReader* pReader) :
             const long long delta_size = delta_size_ - bias;
 
             frame_size += delta_size;
-            assert(frame_size > 0);
+
-            assert(frame_size <= LONG_MAX);
+            if (frame_size < 0)
+                return E_FILE_FORMAT_INVALID;
+
+            if (frame_size > LONG_MAX)
+                return E_FILE_FORMAT_INVALID;
 
             curr.len = static_cast<long>(frame_size);
             size += curr.len;  //contribution of this frame
@@ -8618,23 +8792,28 @@ Block::Block(long long start, long long size_, IMkvReader* pReader) :
         }
 
         {
-            assert(pos < stop);
+            assert(pos <= stop);
             assert(pf < pf_end);
 
             const Frame& prev = *pf++;
-            assert(pf < pf_end);
             assert(prev.len == frame_size);
 
+            assert(pf < pf_end);
+
             Frame& curr = *pf++;
             assert(pf == pf_end);
 
+            curr.pos = 0;  //patch later
+
             const long long total_size = stop - pos;
-            assert(total_size > 0);
+
-            assert(total_size > size);
+            if (total_size < size)
+                return E_FILE_FORMAT_INVALID;
 
             frame_size = total_size - size;
-            assert(frame_size > 0);
+
-            assert(frame_size <= LONG_MAX);
+            if (frame_size > LONG_MAX)
+                return E_FILE_FORMAT_INVALID;
 
             curr.len = static_cast<long>(frame_size);
         }
@@ -8651,12 +8830,8 @@ Block::Block(long long start, long long size_, IMkvReader* pReader) :
 
         assert(pos == stop);
     }
-}
 
-
+    return 0;  //success
-Block::~Block()
-{
-    delete[] m_frames;
 }
 
 

mkvparser.hpp

@@ -80,9 +80,11 @@ public:
     const long long m_start;
     const long long m_size;
 
-    Block(long long start, long long size, IMkvReader*);
+    Block(long long start, long long size);
     ~Block();
 
+    long Parse(IMkvReader*);
+
     long long GetTrackNumber() const;
     long long GetTimeCode(const Cluster*) const;  //absolute, but not scaled
     long long GetTime(const Cluster*) const;      //absolute, and scaled (ns)
@@ -126,6 +128,7 @@ protected:
 
 public:
     virtual ~BlockEntry();
+
     bool EOS() const;
     const Cluster* GetCluster() const;
     long GetIndex() const;
@@ -148,6 +151,7 @@ class SimpleBlock : public BlockEntry
 
 public:
     SimpleBlock(Cluster*, long index, long long start, long long size);
+    long Parse();
 
     Kind GetKind() const;
     const Block* GetBlock() const;
@@ -173,6 +177,8 @@ public:
         long long next,
         long long duration);
 
+    long Parse();
+
     Kind GetKind() const;
     const Block* GetBlock() const;
 
@@ -345,9 +351,6 @@ protected:
     public:
         EOSBlock();
 
-        //bool EOS() const;
-        //const Cluster* GetCluster() const;
-        //long GetIndex() const;
         Kind GetKind() const;
         const Block* GetBlock() const;
     };
@@ -680,9 +683,10 @@ public:
     long long GetFirstTime() const;  //time (ns) of first (earliest) block
     long long GetLastTime() const;   //time (ns) of last (latest) block
 
-    const BlockEntry* GetFirst() const;
+    long GetFirst(const BlockEntry*&) const;
-    const BlockEntry* GetLast() const;
+    long GetLast(const BlockEntry*&) const;
-    const BlockEntry* GetNext(const BlockEntry*) const;
+    long GetNext(const BlockEntry* curr, const BlockEntry*& next) const;
+
     const BlockEntry* GetEntry(const Track*, long long ns = -1) const;
     const BlockEntry* GetEntry(
         const CuePoint&,
@@ -731,12 +735,12 @@ private:
     mutable long m_entries_size;
     mutable long m_entries_count;
 
-    long ParseSimpleBlock(long long, long long&, long&) const;
+    long ParseSimpleBlock(long long, long long&, long&);
-    long ParseBlockGroup(long long, long long&, long&) const;
+    long ParseBlockGroup(long long, long long&, long&);
 
-    void CreateBlock(long long id, long long pos, long long size) const;
+    long CreateBlock(long long id, long long pos, long long size);
-    void CreateBlockGroup(long long, long long, BlockEntry**&) const;
+    long CreateBlockGroup(long long, long long);
-    void CreateSimpleBlock(long long, long long, BlockEntry**&) const;
+    long CreateSimpleBlock(long long, long long);
 
 };
 

sample.cpp

@@ -256,7 +256,16 @@ int main(int argc, char* argv[])
         const long long time_ns = pCluster->GetTime();
         printf("\t\tCluster Time (ns)\t: %lld\n", time_ns);
 
-        const BlockEntry* pBlockEntry = pCluster->GetFirst();
+        const BlockEntry* pBlockEntry;
+
+        long status = pCluster->GetFirst(pBlockEntry);
+
+        if (status < 0)  //error
+        {
+            printf("\t\tError parsing first block of cluster\n");
+            fflush(stdout);
+            goto done;
+        }
 
         while ((pBlockEntry != NULL) && !pBlockEntry->EOS())
         {
@@ -281,12 +290,20 @@ int main(int argc, char* argv[])
                 printf("\t\t\t %15ld,%15llx\n", size, offset);
             }
 
-            pBlockEntry = pCluster->GetNext(pBlockEntry);
+            status = pCluster->GetNext(pBlockEntry, pBlockEntry);
+
+            if (status < 0)
+            {
+                printf("\t\t\tError parsing next block of cluster\n");
+                fflush(stdout);
+                goto done;
+            }
         }
 
         pCluster = pSegment->GetNext(pCluster);
     }
 
+done:
     delete pSegment;
     return 0;
 

sample_muxer/sample_muxer.cpp

@@ -317,7 +317,15 @@ int main(int argc, char* argv[]) {
   const mkvparser::Cluster* cluster = parser_segment->GetFirst();
 
   while ((cluster != NULL) && !cluster->EOS()) {
-    const mkvparser::BlockEntry* block_entry = cluster->GetFirst();
+    const mkvparser::BlockEntry* block_entry;
+
+    long status = cluster->GetFirst(block_entry);
+
+    if (status)
+    {
+        printf("\n Could not get first block of cluster.\n");
+        return EXIT_FAILURE;
+    }
 
     while ((block_entry != NULL) && !block_entry->EOS()) {
       const mkvparser::Block* const block = block_entry->GetBlock();
@@ -362,7 +370,13 @@ int main(int argc, char* argv[]) {
         }
       }
 
-      block_entry = cluster->GetNext(block_entry);
+      status = cluster->GetNext(block_entry, block_entry);
+
+      if (status)
+      {
+          printf("\n Could not get next block of cluster.\n");
+          return EXIT_FAILURE;
+      }
     }
 
     cluster = parser_segment->GetNext(cluster);