[FFmpeg-devel] [PATCH] Fix uninitialized reads on malformed ogg files.

Dale Curtis dalecurtis at chromium.org
Thu Mar 8 21:27:35 CET 2012


On Thu, Mar 8, 2012 at 12:04 PM, Reimar Döffinger
<Reimar.Doeffinger at gmx.de>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
> I'd really like to hear what code that is. Bugs tend to come 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 adds are so I could do the memset w/o using
FFMIN(FF_INPUT_BUFFER_PADDING_SIZE, os->bufsize - os->bufpos). If you don't
think the buffers need padding, I'm happy to switch the patch around.

Specifically the code we found issue with was downstream in
oggparsetheora.c. If there aren't enough bits left for header parsing, the
values will load uninitialized data:

http://git.libav.org/?p=libav.git;a=blob;f=libavformat/oggparsetheora.c;h=d1559f4632da8ada7972b1a96ebedba5a26297ca;hb=HEAD#l56

- dale


More information about the ffmpeg-devel mailing list