[FFmpeg-devel] [PATCH] Fix uninitialized reads on malformed ogg files.
michaelni at gmx.at
Thu Mar 8 21:52:37 CET 2012
On Thu, Mar 08, 2012 at 09:04:51PM +0100, Reimar Döffinger wrote:
> On Thu, Mar 08, 2012 at 05:51:59AM +0100, Michael Niedermayer wrote:
> > On Wed, Mar 07, 2012 at 02:26:58PM -0800, dalecurtis at chromium.org wrote:
> > > From: Dale Curtis <dalecurtis at chromium.org>
> > >
> > > The ogg decoder wasn't padding the input buffer with the appropriate
> > > FF_INPUT_BUFFER_PADDING_SIZE bytes. Which led to uninitialized reads in
> > > various pieces of parsing code when they thought they had more data than
> > > they actually did.
> > patch looks good to me
> > reimar ?
> None of the code I am aware of is supposed to require padding, so
oggparse* uses get_bits(), ive not checked if they overread and if
the patch would fix it. It just looked plausible to me ...
> I'd really like to hear what code that is. Bugs tend tocome in bunches,
> so I'd expect that code to be buggy in more ways.
> I also have some doubts that that add can never cause integer overflows.
the only code that changes the buf size is
1. init to 65307
2. *= 2
if we assume that the existing code is free of overflows then the
largest value achivable is of the form of
65307 * 2^N where N is very likely 16 but could be 48 when int is
either way theres at least 15 million bytes from these to a overflow
FF_INPUT_BUFFER_PADDING_SIZE is smaller than 15 million
if the existing code can overflow i suspect it would crash with a
null pointer dereference before + FF_INPUT_BUFFER_PADDING_SIZE can
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Opposition brings concord. Out of discord comes the fairest harmony.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Size: 198 bytes
Desc: Digital signature
More information about the ffmpeg-devel