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

[Howard Chu]

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.)
-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/