<p>Hi Francois,</p>
<p>On 16/08/22 10:56, Francois Cartegnie wrote:</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> vlc | branch: master | Francois Cartegnie <fcvlcdev@free.fr> | Mon Aug 22 10:18:52 2016 +0800| [fa8d43327488b3d205188aeacc76349e9d6f1efa] | committer: Francois Cartegnie

 demux: mp4: skip failed boxes in containers</code></pre>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=fa8d43327488b3d205188aeacc76349e9d6f1efa</code></pre>
</blockquote>
</blockquote>
<hr style="height:1px;margin-bottom:20px;background-color:#ddd;color:#ddd" />
<h3 id="potential-use-after-free-redundant-check">Potential <em>use-after-free</em> + redundant check</h3>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> @@ -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 );
  </code></pre>
</blockquote>
<p>Two things:</p>
<ul>
<li><p><code>p_box</code> is never <code>NULL</code> at this stage given the <em>if-statement</em> at <code>libmp4.c:252</code>.</p></li>
<li><p>If <code>MP4_Box_Read_Specific</code> fails, <code>p_box</code> will point to memory which has already been freed (which probably means that it should not be added to <code>p_father</code> through <code>MP4_BoxAddChild</code>, nor returned).</p></li>
</ul>
<hr style="height:1px;margin-bottom:20px;background-color:#ddd;color:#ddd" />
<h3 id="potential-fix">Potential fix</h3>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> @@ -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;
      }</code></pre>
</blockquote>
<ul>
<li>Did you mean to set <code>p_box</code> to <code>NULL</code> instead of only removing the <em>return-statement</em>?</li>
</ul>