<p>On 16/05/24 20:42, Emeric Grange wrote:</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<p>+/* GoPro HiLight tags support <em>/ +static int MP4_ReadBox_HMMT( stream_t </em>p_stream, MP4_Box_t <em>p_box ) +{ + MP4_Box_data_HMMT_t </em>p_hmmt; + MP4_READBOX_ENTER( MP4_Box_data_HMMT_t, NULL ); + + if ( i_read < 5 ) + MP4_READBOX_EXIT( 0 ); + + p_hmmt = p_box->data.p_hmmt; + + MP4_GET4BYTES( p_hmmt->i_chapter_count ); + + if ( i_read < p_hmmt->i_chapter_count*4 ) + MP4_READBOX_EXIT( 0 );</p>
</blockquote>
<p>I am not truely familiar with the <code>MP4_READBOX_ENTER</code> macro, but after skimming through its implementation I discovered an issue with the above implementation.</p>
<p>See <a href="http://codepad.org/gTFDYk8G">this testcase</a>, or the attached file, for a simulation of the above. Given that the expression <code>p_hmmt->i_chapter_count*4</code> might result in a value that will overflow, the if-statement itself is faulty.</p>
<p>Changing the code to something like the below is a suitable workaround (though it could certainly use a comment or a constant instead of the magic <code>4</code>):</p>
<pre><code>if( ( i_read / 4 ) < p_hmmt->i_chapter_count )
    MP4_READBOX_EXIT( 0 );</code></pre>
<h3 id="disclaimer">Disclaimer</h3>
<ul>
<li><em>I am currently insanely tired and just skimmed the patch in this thread while having a smoke, I in no way claim that I am fit for fight for reviews at this hour.</em></li>
</ul>