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

Jean-Baptiste Kempf jb at videolan.org
Sun Feb 22 22:38:19 CET 2015


Does not work either.

Also, your mailer destroys your patches.

Le 20/02/2015 19:37, Steve Lhomme a écrit :
> 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
> <mailto: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 <mailto: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
>         <tel:%2B33%20672%20704%20734>
>         Sent from my Electronic Device
>         _________________________________________________
>         vlc-devel mailing list
>         To unsubscribe or modify your subscription options:
>         https://mailman.videolan.org/__listinfo/vlc-devel
>         <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
>


-- 
Jean-Baptiste Kempf
http://www.jbkempf.com/ - +33 672 704 734
Sent from my Electronic Device



More information about the vlc-devel mailing list