[FFmpeg-devel] [PATCH] rmdec.c: correctly skip indexes
Tue Dec 30 21:09:21 CET 2008
On Tue, Dec 30, 2008 at 03:01:35PM -0500, Ronald S. Bultje wrote:
> On Tue, Dec 30, 2008 at 2:31 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > you silently remove a validity check, do you just not give a damn if the
> Sorry, I thought I had made them unsigned, that was the intention.
> When unsigned, the check makes no sense (because the value is always
> positive). I don't care if we seek (unsigned int)-1 forward. If you
> prefer signed (?), I can add the check back.
You might not care about seeking 4gb forward but a user possibly will,
any seek larger than the filesize is invalid and should be handled
as error, not as oops EOF, exit.
besides even if len is unsigned, i dont see how that would stop the
out of array write
> > besides you change a 16bit BE to a 32bit BE read, its hard to imagine how
> > both could have made sense, you should explain this assuming your code
> > does make any sense.
> You want me to say the original code is wrong? I mean come on, it
> fails to skip the index and results in garbage at the end of a file,
> it was obviously never tested. Need I say more?
> Ok, proof 1: all other level-1 IDs have a 32-bit size, it's
> incomprehensible that real would choose a 16-bit one for INDX.
> proof 2: mplayer/libmpdemux/demux_real.c:generate_index()
> i = stream_read_dword_le(demuxer->stream);
> if ((i == -256) || (i != MKTAG('I', 'N', 'D', 'X')))
> mp_msg(MSGT_DEMUX, MSGL_WARN,"Something went wrong, no index chunk
> found on given address (%d)\n",
> index_mode = -1;
> if (i == -256)
> stream_seek(demuxer->stream, origpos);
> return 0;
> //goto end;
> mp_msg(MSGT_DEMUX, MSGL_V,"Reading index table from index chunk (%d)\n",
> i = stream_read_dword(demuxer->stream);
> mp_msg(MSGT_DEMUX, MSGL_V,"size: %d bytes\n", i);
> In other words, a dword (32-bit) object size.
> Shows a 32-bit object size.
> Conclusion: the original code is wrong.
good, iam convinced
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
The misfortune of the wise is better than the prosperity of the fool.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Size: 189 bytes
Desc: Digital signature
More information about the ffmpeg-devel