[FFmpeg-devel] [PATCH] pnm parser crashes on corrupted frames

[Michael Niedermayer]

On Sat, Mar 27, 2010 at 02:45:11PM -0700, Howard Chu wrote:
> Reimar D?ffinger wrote:
>> On Sat, Mar 27, 2010 at 02:16:13PM -0700, Howard Chu wrote:
>>> While using gource http://code.google.com/p/gource/ to generate a
>>> movie it looks like occasionally some corrupted frames get spit out,
>>> and these can cause ffmpeg to SEGV. This simple patch avoids one of
>>> the crash situations.
>
>>> Index: libavcodec/pnm.c
>>> ===================================================================
>>> --- libavcodec/pnm.c	(revision 22688)
>>> +++ libavcodec/pnm.c	(working copy)
>>> @@ -134,6 +134,8 @@
>>>           return -1;
>>>       pnm_get(s, buf1, sizeof(buf1));
>>>       avctx->height = atoi(buf1);
>>> +    if (avctx->height<= 0)
>>> +        return -1;
>>>       if(avcodec_check_dimensions(avctx, avctx->width, avctx->height))
>>>           return -1;
>>
>> Having another dimension check in addition to avcodec_check_dimensions
>> definitely is not acceptable!
>> What exactly is the issue?
>
> Sorry, I've had that patch in my source tree since January and had 
> forgotten about it until I was regenerating the rtmp patches. So the 
> details are no longer clear to me.
>
> However, your statement is quite inconsistent; there is already a check for 
> avctx->width on the preceding lines. I was simply adding the equally 
> necessary check on height:
>
> >>>>
>     pnm_get(s, buf1, sizeof(buf1));
>     avctx->width = atoi(buf1);
>     if (avctx->width <= 0)
>         return -1;
>     pnm_get(s, buf1, sizeof(buf1));
>     avctx->height = atoi(buf1);
>     if (avctx->height <= 0)
>         return -1;
>     if(avcodec_check_dimensions(avctx, avctx->width, avctx->height))
>         return -1;
> <<<<
>
> Looking at the code in avcodec_check_dimensions, it may be that there's no 
> actual crash, this patch just avoids the log message that 
> avcodec_check_dimensions would spit out. Again, from a consistency 
> perspective, if the width can be ignored silently, then so should the 
> height. (And as I think about it, I was probably getting a ton of those log 
> messages before.)

i guess the width check is there to prevent the pnm_get()
this does not apply to height as avcodec_check_dimensions checks
it completely alraedy

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

It is dangerous to be right in matters on which the established authorities
are wrong. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100328/838816ff/attachment.pgp>