[vlc-devel] [vlc-commits] demux: mp4: skip failed boxes in containers

[Filip Roséen]

Hi Francois,

On 16/08/22 10:56, Francois Cartegnie wrote:

> vlc | branch: master | Francois Cartegnie <fcvlcdev at free.fr> | Mon Aug 22 10:18:52 2016 +0800| [fa8d43327488b3d205188aeacc76349e9d6f1efa] | committer: Francois Cartegnie
> 
> demux: mp4: skip failed boxes in containers
> 
> > http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=fa8d43327488b3d205188aeacc76349e9d6f1efa

--------------------------------------------------------------------------

Potential *use-after-free* + redundant check
---------------------------------------------------------------------

> @@ -269,7 +268,8 @@ static MP4_Box_t *MP4_ReadBoxRestricted( stream_t *p_stream, MP4_Box_t *p_father
>          MP4_Seek( p_stream, i_next );
>      }
>  
> -    MP4_BoxAddChild( p_father, p_box );
> +    if ( p_box )
> +        MP4_BoxAddChild( p_father, p_box );
>  

Two things:

 - `p_box` is never `NULL` at this stage given the *if-statement* at
   `libmp4.c:252`.

 - If `MP4_Box_Read_Specific` fails, `p_box` will point to memory
   which has already been freed (which probably means that it should
   not be added to `p_father` through `MP4_BoxAddChild`, nor
   returned).

--------------------------------------------------------------------------

Potential fix
--------------------------------------------------------------------------

> @@ -259,7 +259,6 @@ static MP4_Box_t *MP4_ReadBoxRestricted( stream_t *p_stream, MP4_Box_t *p_father
>      {
>          msg_Warn( p_stream, "Failed reading box %4.4s", (char*) &peekbox.i_type );
>          MP4_BoxFree( p_box );
> -        return NULL;
>      }

 - Did you mean to set `p_box` to `NULL` instead of only removing the
   *return-statement*?

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20160822/b54f2b74/attachment.html>