[vlc-devel] [PATCH] MKV: we can read a Cluster with unknown size and more check finite size checking before testing too big sizes

Steve Lhomme robux4 at gmail.com
Fri Feb 20 19:37:59 CET 2015


Here's one over the current master. I'll submit the other one after this
one is in, as it depends on it.

---
 modules/demux/mkv/demux.cpp                  |  2 +-
 modules/demux/mkv/matroska_segment.cpp       | 12 ++++++------
 modules/demux/mkv/matroska_segment_parse.cpp | 14 +++++++-------
 3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/modules/demux/mkv/demux.cpp b/modules/demux/mkv/demux.cpp
index 1feca55..21618f4 100644
--- a/modules/demux/mkv/demux.cpp
+++ b/modules/demux/mkv/demux.cpp
@@ -519,7 +519,7 @@ matroska_stream_c
*demux_sys_t::AnalyseAllSegmentsFound( demux_t *p_demux, EbmlS
                     // find the families of this segment
                     KaxInfo *p_info = static_cast<KaxInfo*>(p_l1);
                     b_keep_segment = b_initial;
-                    if( unlikely( p_info->GetSize() >= SIZE_MAX ) )
+                    if( unlikely( p_info->IsFiniteSize() &&
p_info->GetSize() >= SIZE_MAX ) )
                     {
                         msg_Err( p_demux, "KaxInfo too big aborting" );
                         break;
diff --git a/modules/demux/mkv/matroska_segment.cpp
b/modules/demux/mkv/matroska_segment.cpp
index df24096..f71df70 100644
--- a/modules/demux/mkv/matroska_segment.cpp
+++ b/modules/demux/mkv/matroska_segment.cpp
@@ -139,7 +139,7 @@ void matroska_segment_c::LoadCues( KaxCues *cues )
                     KaxCueTime &ctime = *(KaxCueTime*)el;
                     try
                     {
-                        if( unlikely( ctime.GetSize() >= SIZE_MAX ) )
+                        if( unlikely( ctime.IsFiniteSize() &&
ctime.GetSize() >= SIZE_MAX ) )
                         {
                             msg_Err( &sys.demuxer, "CueTime size too big");
                             b_invalid_cue = true;
@@ -162,7 +162,7 @@ void matroska_segment_c::LoadCues( KaxCues *cues )
                     {
                         while( ( el = ep->Get() ) != NULL )
                         {
-                            if( unlikely( el->GetSize() >= SIZE_MAX ) )
+                            if( unlikely( el->IsFiniteSize() &&
el->GetSize() >= SIZE_MAX ) )
                             {
                                 ep->Up();
                                 msg_Err( &sys.demuxer, "Error %s too big,
aborting", typeid(*el).name() );
@@ -296,7 +296,7 @@ SimpleTag * matroska_segment_c::ParseSimpleTags(
KaxTagSimple *tag, int target_t
     {
         while( ( el = ep->Get() ) != NULL && size < max_size)
         {
-            if( unlikely( el->GetSize() >= SIZE_MAX ) )
+            if( unlikely( el->IsFiniteSize() && el->GetSize() >= SIZE_MAX
) )
             {
                 msg_Err( &sys.demuxer, "Error %s too big ignoring the
tag", typeid(*el).name() );
                 delete ep;
@@ -409,7 +409,7 @@ void matroska_segment_c::LoadTags( KaxTags *tags )
                     {
                         try
                         {
-                            if( unlikely( el->GetSize() >= SIZE_MAX ) )
+                            if( unlikely( el->IsFiniteSize() &&
el->GetSize() >= SIZE_MAX ) )
                             {
                                 msg_Err( &sys.demuxer, "Invalid size while
reading tag");
                                 break;
@@ -1351,7 +1351,7 @@ int matroska_segment_c::BlockGet( KaxBlock * &
pp_block, KaxSimpleBlock * & pp_s
                     }
                     break;
                 case 2:
-                    if( unlikely( el->GetSize() >= SIZE_MAX ) )
+                    if( unlikely( el->IsFiniteSize() && el->GetSize() >=
SIZE_MAX ) )
                     {
                         msg_Err( &sys.demuxer, "Error while reading %s...
upping level", typeid(*el).name());
                         ep->Up();
@@ -1388,7 +1388,7 @@ int matroska_segment_c::BlockGet( KaxBlock * &
pp_block, KaxSimpleBlock * & pp_s
                     }
                     break;
                 case 3:
-                    if( unlikely( el->GetSize() >= SIZE_MAX ) )
+                    if( unlikely( el->IsFiniteSize() && el->GetSize() >=
SIZE_MAX ) )
                     {
                         msg_Err( &sys.demuxer, "Error while reading %s...
upping level", typeid(*el).name());
                         ep->Up();
diff --git a/modules/demux/mkv/matroska_segment_parse.cpp
b/modules/demux/mkv/matroska_segment_parse.cpp
index 751c824..fc80103 100644
--- a/modules/demux/mkv/matroska_segment_parse.cpp
+++ b/modules/demux/mkv/matroska_segment_parse.cpp
@@ -93,7 +93,7 @@ void matroska_segment_c::ParseSeekHead( KaxSeekHead
*seekhead )
             {
                 while( ( l = ep->Get() ) != NULL )
                 {
-                    if( unlikely( l->GetSize() >= SIZE_MAX ) )
+                    if( unlikely( l->IsFiniteSize() && l->GetSize() >=
SIZE_MAX ) )
                     {
                         msg_Err( &sys.demuxer,"%s too big... skipping
it",  typeid(*l).name() );
                         continue;
@@ -745,7 +745,7 @@ void matroska_segment_c::ParseTracks( KaxTracks *tracks
)
     int i_upper_level = 0;

     /* Master elements */
-    if( unlikely( tracks->GetSize() >= SIZE_MAX ) )
+    if( unlikely( tracks->IsFiniteSize() && tracks->GetSize() >= SIZE_MAX
) )
     {
         msg_Err( &sys.demuxer, "Track too big, aborting" );
         return;
@@ -786,7 +786,7 @@ void matroska_segment_c::ParseInfo( KaxInfo *info )

     /* Master elements */
     m = static_cast<EbmlMaster *>(info);
-    if( unlikely( m->GetSize() >= SIZE_MAX ) )
+    if( unlikely( m->IsFiniteSize() && m->GetSize() >= SIZE_MAX ) )
     {
         msg_Err( &sys.demuxer, "Info too big, aborting" );
         return;
@@ -914,7 +914,7 @@ void matroska_segment_c::ParseInfo( KaxInfo *info )
             KaxChapterTranslate *p_trans =
static_cast<KaxChapterTranslate*>( l );
             try
             {
-                if( unlikely( p_trans->GetSize() >= SIZE_MAX ) )
+                if( unlikely( p_trans->IsFiniteSize() &&
p_trans->GetSize() >= SIZE_MAX ) )
                 {
                     msg_Err( &sys.demuxer, "Chapter translate too big,
aborting" );
                     continue;
@@ -1108,7 +1108,7 @@ void matroska_segment_c::ParseAttachments(
KaxAttachments *attachments )
     EbmlElement *el;
     int i_upper_level = 0;

-    if( unlikely( attachments->GetSize() >= SIZE_MAX ) )
+    if( unlikely( attachments->IsFiniteSize() && attachments->GetSize() >=
SIZE_MAX ) )
     {
         msg_Err( &sys.demuxer, "Attachments too big, aborting" );
         return;
@@ -1171,7 +1171,7 @@ void matroska_segment_c::ParseChapters( KaxChapters
*chapters )
     int i_upper_level = 0;

     /* Master elements */
-    if( unlikely( chapters->GetSize() >= SIZE_MAX ) )
+    if( unlikely( chapters->IsFiniteSize() && chapters->GetSize() >=
SIZE_MAX ) )
     {
         msg_Err( &sys.demuxer, "Chapters too big, aborting" );
         return;
@@ -1245,7 +1245,7 @@ void matroska_segment_c::ParseCluster( KaxCluster
*cluster, bool b_update_start_

     /* Master elements */
     m = static_cast<EbmlMaster *>( cluster );
-    if( unlikely( m->GetSize() >= SIZE_MAX ) )
+    if( unlikely( m->IsFiniteSize() && m->GetSize() >= SIZE_MAX ) )
     {
         msg_Err( &sys.demuxer, "Cluster too big, aborting" );
         return;
-- 
1.9.5.msysgit.0



On Fri, Feb 20, 2015 at 7:34 PM, Steve Lhomme <robux4 at gmail.com> wrote:

> What's the symptom ?
>
> On Fri, Feb 20, 2015 at 7:19 PM, Jean-Baptiste Kempf <jb at videolan.org>
> wrote:
>
>> I don't seem able to apply this patch.
>>
>> Le 20/02/2015 16:41, Steve Lhomme a écrit :
>>
>>  Replaces the previous similar patch from a few minutes ago with more
>>> checks.
>>>
>>> ---
>>>   modules/demux/mkv/demux.cpp                  |  2 +-
>>>   modules/demux/mkv/matroska_segment.cpp       | 12 ++++++------
>>>   modules/demux/mkv/matroska_segment_parse.cpp | 14 +++++++-------
>>>   3 files changed, 14 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/modules/demux/mkv/demux.cpp b/modules/demux/mkv/demux.cpp
>>> index 1feca55..21618f4 100644
>>> --- a/modules/demux/mkv/demux.cpp
>>> +++ b/modules/demux/mkv/demux.cpp
>>> @@ -519,7 +519,7 @@ matroska_stream_c
>>> *demux_sys_t::AnalyseAllSegmentsFound( demux_t *p_demux, EbmlS
>>>                       // find the families of this segment
>>>                       KaxInfo *p_info = static_cast<KaxInfo*>(p_l1);
>>>                       b_keep_segment = b_initial;
>>> -                    if( unlikely( p_info->GetSize() >= SIZE_MAX ) )
>>> +                    if( unlikely( p_info->IsFiniteSize() &&
>>> p_info->GetSize() >= SIZE_MAX ) )
>>>                       {
>>>                           msg_Err( p_demux, "KaxInfo too big aborting" );
>>>                           break;
>>> diff --git a/modules/demux/mkv/matroska_segment.cpp
>>> b/modules/demux/mkv/matroska_segment.cpp
>>> index df24096..f71df70 100644
>>> --- a/modules/demux/mkv/matroska_segment.cpp
>>> +++ b/modules/demux/mkv/matroska_segment.cpp
>>> @@ -139,7 +139,7 @@ void matroska_segment_c::LoadCues( KaxCues *cues )
>>>                       KaxCueTime &ctime = *(KaxCueTime*)el;
>>>                       try
>>>                       {
>>> -                        if( unlikely( ctime.GetSize() >= SIZE_MAX ) )
>>> +                        if( unlikely( ctime.IsFiniteSize() &&
>>> ctime.GetSize() >= SIZE_MAX ) )
>>>                           {
>>>                               msg_Err( &sys.demuxer, "CueTime size too
>>> big");
>>>                               b_invalid_cue = true;
>>> @@ -162,7 +162,7 @@ void matroska_segment_c::LoadCues( KaxCues *cues )
>>>                       {
>>>                           while( ( el = ep->Get() ) != NULL )
>>>                           {
>>> -                            if( unlikely( el->GetSize() >= SIZE_MAX ) )
>>> +                            if( unlikely( el->IsFiniteSize() &&
>>> el->GetSize() >= SIZE_MAX ) )
>>>                               {
>>>                                   ep->Up();
>>>                                   msg_Err( &sys.demuxer, "Error %s too
>>> big, aborting", typeid(*el).name() );
>>> @@ -296,7 +296,7 @@ SimpleTag * matroska_segment_c::ParseSimpleTags(
>>> KaxTagSimple *tag, int target_t
>>>       {
>>>           while( ( el = ep->Get() ) != NULL && size < max_size)
>>>           {
>>> -            if( unlikely( el->GetSize() >= SIZE_MAX ) )
>>> +            if( unlikely( el->IsFiniteSize() && el->GetSize() >=
>>> SIZE_MAX ) )
>>>               {
>>>                   msg_Err( &sys.demuxer, "Error %s too big ignoring the
>>> tag", typeid(*el).name() );
>>>                   delete ep;
>>> @@ -409,7 +409,7 @@ void matroska_segment_c::LoadTags( KaxTags *tags )
>>>                       {
>>>                           try
>>>                           {
>>> -                            if( unlikely( el->GetSize() >= SIZE_MAX ) )
>>> +                            if( unlikely( el->IsFiniteSize() &&
>>> el->GetSize() >= SIZE_MAX ) )
>>>                               {
>>>                                   msg_Err( &sys.demuxer, "Invalid size
>>> while reading tag");
>>>                                   break;
>>> @@ -1351,7 +1351,7 @@ int matroska_segment_c::BlockGet( KaxBlock * &
>>> pp_block, KaxSimpleBlock * & pp_s
>>>                       }
>>>                       break;
>>>                   case 2:
>>> -                    if( unlikely( el->GetSize() >= SIZE_MAX ) )
>>> +                    if( unlikely( el->IsFiniteSize() && el->GetSize()
>>>  >= SIZE_MAX ) )
>>>                       {
>>>                           msg_Err( &sys.demuxer, "Error while reading
>>> %s... upping level", typeid(*el).name());
>>>                           ep->Up();
>>> @@ -1388,7 +1388,7 @@ int matroska_segment_c::BlockGet( KaxBlock * &
>>> pp_block, KaxSimpleBlock * & pp_s
>>>                       }
>>>                       break;
>>>                   case 3:
>>> -                    if( unlikely( el->GetSize() >= SIZE_MAX ) )
>>> +                    if( unlikely( el->IsFiniteSize() && el->GetSize()
>>>  >= SIZE_MAX ) )
>>>                       {
>>>                           msg_Err( &sys.demuxer, "Error while reading
>>> %s... upping level", typeid(*el).name());
>>>                           ep->Up();
>>> diff --git a/modules/demux/mkv/matroska_segment_parse.cpp
>>> b/modules/demux/mkv/matroska_segment_parse.cpp
>>> index 751c824..fc80103 100644
>>> --- a/modules/demux/mkv/matroska_segment_parse.cpp
>>> +++ b/modules/demux/mkv/matroska_segment_parse.cpp
>>> @@ -93,7 +93,7 @@ void matroska_segment_c::ParseSeekHead( KaxSeekHead
>>> *seekhead )
>>>               {
>>>                   while( ( l = ep->Get() ) != NULL )
>>>                   {
>>> -                    if( unlikely( l->GetSize() >= SIZE_MAX ) )
>>> +                    if( unlikely( l->IsFiniteSize() && l->GetSize() >=
>>> SIZE_MAX ) )
>>>                       {
>>>                           msg_Err( &sys.demuxer,"%s too big... skipping
>>> it",  typeid(*l).name() );
>>>                           continue;
>>> @@ -745,7 +745,7 @@ void matroska_segment_c::ParseTracks( KaxTracks
>>> *tracks )
>>>       int i_upper_level = 0;
>>>
>>>       /* Master elements */
>>> -    if( unlikely( tracks->GetSize() >= SIZE_MAX ) )
>>> +    if( unlikely( tracks->IsFiniteSize() && tracks->GetSize() >=
>>> SIZE_MAX ) )
>>>       {
>>>           msg_Err( &sys.demuxer, "Track too big, aborting" );
>>>           return;
>>> @@ -786,7 +786,7 @@ void matroska_segment_c::ParseInfo( KaxInfo *info )
>>>
>>>       /* Master elements */
>>>       m = static_cast<EbmlMaster *>(info);
>>> -    if( unlikely( m->GetSize() >= SIZE_MAX ) )
>>> +    if( unlikely( m->IsFiniteSize() && m->GetSize() >= SIZE_MAX ) )
>>>       {
>>>           msg_Err( &sys.demuxer, "Info too big, aborting" );
>>>           return;
>>> @@ -914,7 +914,7 @@ void matroska_segment_c::ParseInfo( KaxInfo *info )
>>>               KaxChapterTranslate *p_trans =
>>> static_cast<KaxChapterTranslate*>( l );
>>>               try
>>>               {
>>> -                if( unlikely( p_trans->GetSize() >= SIZE_MAX ) )
>>> +                if( unlikely( p_trans->IsFiniteSize() &&
>>> p_trans->GetSize() >= SIZE_MAX ) )
>>>                   {
>>>                       msg_Err( &sys.demuxer, "Chapter translate too big,
>>> aborting" );
>>>                       continue;
>>> @@ -1108,7 +1108,7 @@ void matroska_segment_c::ParseAttachments(
>>> KaxAttachments *attachments )
>>>       EbmlElement *el;
>>>       int i_upper_level = 0;
>>>
>>> -    if( unlikely( attachments->GetSize() >= SIZE_MAX ) )
>>> +    if( unlikely( attachments->IsFiniteSize() && attachments->GetSize()
>>>  >= SIZE_MAX ) )
>>>       {
>>>           msg_Err( &sys.demuxer, "Attachments too big, aborting" );
>>>           return;
>>> @@ -1171,7 +1171,7 @@ void matroska_segment_c::ParseChapters(
>>> KaxChapters *chapters )
>>>       int i_upper_level = 0;
>>>
>>>       /* Master elements */
>>> -    if( unlikely( chapters->GetSize() >= SIZE_MAX ) )
>>> +    if( unlikely( chapters->IsFiniteSize() && chapters->GetSize() >=
>>> SIZE_MAX ) )
>>>       {
>>>           msg_Err( &sys.demuxer, "Chapters too big, aborting" );
>>>           return;
>>> @@ -1245,7 +1245,7 @@ void matroska_segment_c::ParseCluster( KaxCluster
>>> *cluster, bool b_update_start_
>>>
>>>       /* Master elements */
>>>       m = static_cast<EbmlMaster *>( cluster );
>>> -    if( unlikely( m->GetSize() >= SIZE_MAX ) )
>>> +    if( unlikely( m->IsFiniteSize() && m->GetSize() >= SIZE_MAX ) )
>>>       {
>>>           msg_Err( &sys.demuxer, "Cluster too big, aborting" );
>>>           return;
>>>
>>
>>
>> --
>> Jean-Baptiste Kempf
>> http://www.jbkempf.com/ - +33 672 704 734
>> Sent from my Electronic Device
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20150220/13687830/attachment.html>


More information about the vlc-devel mailing list