[vlc-devel] [PATCH 6/7] demux:mkv: don't read further than our parent or its parent

[Steve Lhomme]

On Sat, Nov 18, 2017 at 1:57 PM, Romain Vimont <rom at rom1v.com> wrote:
>
>
> Le lundi 13 novembre 2017 à 18:08 +0100, Steve Lhomme a écrit :
>> ---
>>  modules/demux/mkv/Ebml_parser.cpp | 23 ++++++++++++++++++++++-
>>  1 file changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/modules/demux/mkv/Ebml_parser.cpp b/modules/demux/mkv/Ebml_parser.cpp
>> index 330edd4c7c..5f663c6a45 100644
>> --- a/modules/demux/mkv/Ebml_parser.cpp
>> +++ b/modules/demux/mkv/Ebml_parser.cpp
>> @@ -158,6 +158,27 @@ EbmlElement *EbmlParser::Get( int n_call )
>>      if( p_prev )
>>          p_prev->SkipData( *m_es, EBML_CONTEXT(p_prev) );
>>
>> +    uint64_t i_max_read;
>> +    if (mi_level == 0)
>> +        i_max_read = UINT64_MAX;
>> +    else if (!m_el[mi_level-1]->IsFiniteSize())
>> +        i_max_read = UINT64_MAX;
>> +    else if (!p_prev)
>> +        i_max_read = m_el[mi_level-1]->GetSize();
>
> Here, i_max_read may be set to 0.
>
>> +    else {
>> +        size_t size_lvl = mi_level;
>> +        while ( size_lvl && m_el[size_lvl-1]->IsFiniteSize() &&
>> +                m_el[size_lvl-1]->GetEndPosition() == m_el[size_lvl]->GetEndPosition() )
>> +            size_lvl--;
>> +        if (size_lvl == 0 || !m_el[size_lvl-1]->IsFiniteSize() )
>> +            i_max_read = UINT64_MAX;
>> +        else {
>> +            uint64 top = m_el[size_lvl-1]->GetEndPosition();
>> +            uint64 bom = m_el[mi_level]->GetEndPosition();
>> +            i_max_read = top - bom;
>> +        }
>> +    }
>> +
>>      // If the parent is a segment, use the segment context when creating children
>>      // (to prolong their lifetime), otherwise just continue as normal
>>      EbmlSemanticContext e_context =
>> @@ -167,7 +188,7 @@ EbmlElement *EbmlParser::Get( int n_call )
>>
>>      /* Ignore unknown level 0 or 1 elements */
>>      m_el[mi_level] = m_es->FindNextElement( e_context,
>> -                                            i_ulev, UINT64_MAX,
>> +                                            i_ulev, i_max_read,
>
> When i_max_read is 0, an assertion fails:
>
> vlc: src/EbmlElement.cpp:480: libebml::EbmlElement* libebml::EbmlElement::SkipData(libebml::EbmlStream&, const libebml::EbmlSemanticContext&, libebml::EbmlElement*, bool): Assertion `ElementPosition < SizePosition' failed.
> Aborted
>
> Here is a file to reproduce: https://tmp.rom1v.com/fail.mkv

I do not get an assert with the current master and that file. I
noticed an issue in the EBML parser though. Can you check with this
patch ?
https://patches.videolan.org/patch/18874/

> Replacing i_max_read by UINT64_MAX avoids the problem (but this is not a
> fix).
>
>>                                              (  mb_dummy | (mi_level > 1) ), 1 );
>>      if( i_ulev > 0 )
>>      {
>> --
>> 2.14.2
>>
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel