[FFmpeg-devel] [PATCH] rawdec: allocate a buffer in the appropriate size in the copy case.

[Michael Niedermayer]

On Sun, Jun 16, 2013 at 08:28:33AM +0200, Hendrik Leppkes wrote:
> On Sun, Jun 16, 2013 at 12:41 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > On Sat, Jun 15, 2013 at 03:47:42PM +0200, Hendrik Leppkes wrote:
> >> Otherwise the created buffer can be smaller than buf_size, which results
> >> in buffer overreads if the original image has extra padding on every line.
> >> ---
> >>  libavcodec/rawdec.c | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/libavcodec/rawdec.c b/libavcodec/rawdec.c
> >> index 4699242..c9b6802 100644
> >> --- a/libavcodec/rawdec.c
> >> +++ b/libavcodec/rawdec.c
> >> @@ -190,7 +190,7 @@ static int raw_decode(AVCodecContext *avctx, void *data, int *got_frame,
> >>          return res;
> >>
> >>      if (need_copy)
> >> -        frame->buf[0] = av_buffer_alloc(context->frame_size);
> >> +        frame->buf[0] = av_buffer_alloc(buf_size);
> >>      else
> >>          frame->buf[0] = av_buffer_ref(avpkt->buf);
> >>      if (!frame->buf[0])
> >
> > this doesnt look safe, i think the code can write more than buf_size
> > into this
> >
> 
> Looking at the 2/4bpp case, i think it should be FFMAX(buf_size,
> context->frame_size) instead, because the 2/4bpp case is the only case
> where the input data isn't copied offer without modification.
> Does this sound right?

yes, patch applied

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Concerning the gods, I have no means of knowing whether they exist or not
or of what sort they may be, because of the obscurity of the subject, and
the brevity of human life -- Protagoras
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130616/165022fc/attachment.asc>