[vlc-commits] Check element size before reading it

Denis Charmet git at videolan.org
Thu Jul 11 02:28:03 CEST 2013


vlc/vlc-2.0 | branch: master | Denis Charmet <typx at dinauz.org> | Wed Jul 10 23:09:59 2013 +0200| [487cf6e4f5694fcc0c6da9263e56ae67069315b1] | 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)

Conflicts:
	modules/demux/mkv/matroska_segment.cpp

> http://git.videolan.org/gitweb.cgi/vlc/vlc-2.0.git/?a=commit;h=487cf6e4f5694fcc0c6da9263e56ae67069315b1
---

 modules/demux/mkv/demux.cpp                  |    5 ++++
 modules/demux/mkv/matroska_segment.cpp       |   35 ++++++++++++++++++++++++-
 modules/demux/mkv/matroska_segment_parse.cpp |   36 ++++++++++++++++++++++++++
 3 files changed, 75 insertions(+), 1 deletion(-)

diff --git a/modules/demux/mkv/demux.cpp b/modules/demux/mkv/demux.cpp
index 225d34e..2035d82 100644
--- a/modules/demux/mkv/demux.cpp
+++ b/modules/demux/mkv/demux.cpp
@@ -525,6 +525,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 c607db0..e820cfb 100644
--- a/modules/demux/mkv/matroska_segment.cpp
+++ b/modules/demux/mkv/matroska_segment.cpp
@@ -152,6 +152,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(...)
@@ -169,10 +175,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 );
                             }
@@ -279,6 +292,14 @@ void matroska_segment_c::ParseSimpleTags( KaxTagSimple *tag )
     {
         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;
+                free(k);
+                free(v);
+                return ;
+            }
             if( MKV_IS_ID( el, KaxTagName ) )
             {
                 KaxTagName &key = *(KaxTagName*)el;
@@ -1563,6 +1584,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;
@@ -1594,6 +1621,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 862c42c..b7ab94d 100644
--- a/modules/demux/mkv/matroska_segment_parse.cpp
+++ b/modules/demux/mkv/matroska_segment_parse.cpp
@@ -70,6 +70,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;
@@ -675,6 +680,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 );
@@ -711,6 +721,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 );
@@ -834,6 +849,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();
 
@@ -1024,6 +1045,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 );
@@ -1079,6 +1105,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 );
@@ -1148,6 +1179,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