[vlc-commits] Check element size before reading it

Denis Charmet git at videolan.org
Thu Jul 11 00:21:36 CEST 2013


vlc/vlc-2.1 | branch: master | Denis Charmet <typx at dinauz.org> | Wed Jul 10 23:09:59 2013 +0200| [c37ab274d859c33cbcfb10a6879c70ff9345857d] | committer: Denis Charmet

Check element size before reading it

This should avoid integer overflows inside the libebml causing heap buffer overflow. Since new called by the lib is limited to SIZE_MAX bytes.
(cherry picked from commit 027380966251622435288af5b0f9bacfec549288)

> http://git.videolan.org/gitweb.cgi/vlc/vlc-2.1.git/?a=commit;h=c37ab274d859c33cbcfb10a6879c70ff9345857d
---

 modules/demux/mkv/demux.cpp                  |    5 +++
 modules/demux/mkv/matroska_segment.cpp       |   42 ++++++++++++++++++++++++--
 modules/demux/mkv/matroska_segment_parse.cpp |   36 ++++++++++++++++++++++
 3 files changed, 80 insertions(+), 3 deletions(-)

diff --git a/modules/demux/mkv/demux.cpp b/modules/demux/mkv/demux.cpp
index 8a913df..be20476 100644
--- a/modules/demux/mkv/demux.cpp
+++ b/modules/demux/mkv/demux.cpp
@@ -527,6 +527,11 @@ 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 ) )
+                    {
+                        msg_Err( p_demux, "KaxInfo too big aborting" );
+                        break;
+                    }
                     try
                     {
                         p_info->Read(*p_estream, EBML_CLASS_CONTEXT(KaxInfo), i_upper_lvl, p_l2, true);
diff --git a/modules/demux/mkv/matroska_segment.cpp b/modules/demux/mkv/matroska_segment.cpp
index fa477fe..9f00f42 100644
--- a/modules/demux/mkv/matroska_segment.cpp
+++ b/modules/demux/mkv/matroska_segment.cpp
@@ -138,6 +138,12 @@ void matroska_segment_c::LoadCues( KaxCues *cues )
                     KaxCueTime &ctime = *(KaxCueTime*)el;
                     try
                     {
+                        if( unlikely( ctime.GetSize() >= SIZE_MAX ) )
+                        {
+                            msg_Err( &sys.demuxer, "CueTime size too big");
+                            b_invalid_cue = true;
+                            break;
+                        }
                         ctime.ReadData( es.I_O() );
                     }
                     catch(...)
@@ -155,10 +161,17 @@ void matroska_segment_c::LoadCues( KaxCues *cues )
                     {
                         while( ( el = ep->Get() ) != NULL )
                         {
+                            if( unlikely( el->GetSize() >= SIZE_MAX ) )
+                            {
+                                ep->Up();
+                                msg_Err( &sys.demuxer, "Error %s too big, aborting", typeid(*el).name() );
+                                b_invalid_cue = true;
+                                break;
+                            }
+
                             if( MKV_IS_ID( el, KaxCueTrack ) )
                             {
                                 KaxCueTrack &ctrack = *(KaxCueTrack*)el;
-
                                 ctrack.ReadData( es.I_O() );
                                 idx.i_track = uint16( ctrack );
                             }
@@ -270,6 +283,13 @@ SimpleTag * matroska_segment_c::ParseSimpleTags( KaxTagSimple *tag, int target_t
     {
         while( ( el = ep->Get() ) != NULL )
         {
+            if( unlikely( el->GetSize() >= SIZE_MAX ) )
+            {
+                msg_Err( &sys.demuxer, "Error %s too big ignoring the tag", typeid(*el).name() );
+                delete ep;
+                delete p_simple;
+                return NULL;
+            }
             if( MKV_IS_ID( el, KaxTagName ) )
             {
                 KaxTagName &key = *(KaxTagName*)el;
@@ -376,6 +396,11 @@ void matroska_segment_c::LoadTags( KaxTags *tags )
                     {
                         try
                         {
+                            if( unlikely( el->GetSize() >= SIZE_MAX ) )
+                            {
+                                msg_Err( &sys.demuxer, "Invalid size while reading tag");
+                                break;
+                            }
                             if( MKV_IS_ID( el, KaxTagTargetTypeValue ) )
                             {
                                 KaxTagTargetTypeValue &value = *(KaxTagTargetTypeValue*)el;
@@ -421,11 +446,10 @@ void matroska_segment_c::LoadTags( KaxTags *tags )
                         catch(...)
                         {
                             msg_Err( &sys.demuxer, "Error while reading tag");
-                            ep->Up();
                             break;
                         }
-                        ep->Up();
                     }
+                    ep->Up();
                 }
                 else if( MKV_IS_ID( el, KaxTagSimple ) )
                 {
@@ -1296,6 +1320,12 @@ int matroska_segment_c::BlockGet( KaxBlock * & pp_block, KaxSimpleBlock * & pp_s
                     }
                     break;
                 case 2:
+                    if( unlikely( el->GetSize() >= SIZE_MAX ) )
+                    {
+                        msg_Err( &sys.demuxer, "Error while reading %s... upping level", typeid(*el).name());
+                        ep->Up();
+                        break;
+                    }
                     if( MKV_IS_ID( el, KaxClusterTimecode ) )
                     {
                         KaxClusterTimecode &ctc = *(KaxClusterTimecode*)el;
@@ -1327,6 +1357,12 @@ int matroska_segment_c::BlockGet( KaxBlock * & pp_block, KaxSimpleBlock * & pp_s
                     }
                     break;
                 case 3:
+                    if( unlikely( el->GetSize() >= SIZE_MAX ) )
+                    {
+                        msg_Err( &sys.demuxer, "Error while reading %s... upping level", typeid(*el).name());
+                        ep->Up();
+                        break;
+                    }
                     if( MKV_IS_ID( el, KaxBlock ) )
                     {
                         pp_block = (KaxBlock*)el;
diff --git a/modules/demux/mkv/matroska_segment_parse.cpp b/modules/demux/mkv/matroska_segment_parse.cpp
index 42019f3..5e8d608 100644
--- a/modules/demux/mkv/matroska_segment_parse.cpp
+++ b/modules/demux/mkv/matroska_segment_parse.cpp
@@ -90,6 +90,11 @@ void matroska_segment_c::ParseSeekHead( KaxSeekHead *seekhead )
             {
                 while( ( l = ep->Get() ) != NULL )
                 {
+                    if( unlikely( l->GetSize() >= SIZE_MAX ) )
+                    {
+                        msg_Err( &sys.demuxer,"%s too big... skipping it",  typeid(*l).name() );
+                        continue;
+                    }
                     if( MKV_IS_ID( l, KaxSeekID ) )
                     {
                         KaxSeekID &sid = *(KaxSeekID*)l;
@@ -720,6 +725,11 @@ void matroska_segment_c::ParseTracks( KaxTracks *tracks )
     int i_upper_level = 0;
 
     /* Master elements */
+    if( unlikely( tracks->GetSize() >= SIZE_MAX ) )
+    {
+        msg_Err( &sys.demuxer, "Track too big, aborting" );
+        return;
+    }
     try
     {
         tracks->Read( es, EBML_CONTEXT(tracks), i_upper_level, el, true );
@@ -756,6 +766,11 @@ void matroska_segment_c::ParseInfo( KaxInfo *info )
 
     /* Master elements */
     m = static_cast<EbmlMaster *>(info);
+    if( unlikely( m->GetSize() >= SIZE_MAX ) )
+    {
+        msg_Err( &sys.demuxer, "Info too big, aborting" );
+        return;
+    }
     try
     {
         m->Read( es, EBML_CONTEXT(info), i_upper_level, el, true );
@@ -879,6 +894,12 @@ void matroska_segment_c::ParseInfo( KaxInfo *info )
             KaxChapterTranslate *p_trans = static_cast<KaxChapterTranslate*>( l );
             try
             {
+                if( unlikely( p_trans->GetSize() >= SIZE_MAX ) )
+                {
+                    msg_Err( &sys.demuxer, "Chapter translate too big, aborting" );
+                    continue;
+                }
+
                 p_trans->Read( es, EBML_CONTEXT(p_trans), i_upper_level, el, true );
                 chapter_translation_c *p_translate = new chapter_translation_c();
 
@@ -1069,6 +1090,11 @@ void matroska_segment_c::ParseAttachments( KaxAttachments *attachments )
     EbmlElement *el;
     int i_upper_level = 0;
 
+    if( unlikely( attachments->GetSize() >= SIZE_MAX ) )
+    {
+        msg_Err( &sys.demuxer, "Attachments too big, aborting" );
+        return;
+    }
     try
     {
         attachments->Read( es, EBML_CONTEXT(attachments), i_upper_level, el, true );
@@ -1127,6 +1153,11 @@ void matroska_segment_c::ParseChapters( KaxChapters *chapters )
     int i_upper_level = 0;
 
     /* Master elements */
+    if( unlikely( chapters->GetSize() >= SIZE_MAX ) )
+    {
+        msg_Err( &sys.demuxer, "Chapters too big, aborting" );
+        return;
+    }
     try
     {
         chapters->Read( es, EBML_CONTEXT(chapters), i_upper_level, el, true );
@@ -1196,6 +1227,11 @@ void matroska_segment_c::ParseCluster( bool b_update_start_time )
 
     /* Master elements */
     m = static_cast<EbmlMaster *>( cluster );
+    if( unlikely( m->GetSize() >= SIZE_MAX ) )
+    {
+        msg_Err( &sys.demuxer, "Cluster too big, aborting" );
+        return;
+    }
     try
     {
         m->Read( es, EBML_CONTEXT(cluster), i_upper_level, el, true );



More information about the vlc-commits mailing list