[Ffmpeg-devel] Re: [PATCH] FFmpeg theora encoding

[Paul Richards]

On 21/01/07, Michael Niedermayer <michaelni at gmx.at> wrote:
> Hi
>
> On Wed, Jan 17, 2007 at 09:40:19PM +0000, Paul Richards wrote:
> [...]
>
>
> > +/*!
> > +    Concatenates an ogg_packet into the extradata.
> > +*/
> > +static void concatenate_packet(unsigned int* offset, AVCodecContext* avc_context, const ogg_packet* packet)
> > +{
> > +    avc_context->extradata_size += 2 + packet->bytes;
> > +    avc_context->extradata = av_realloc( avc_context->extradata, avc_context->extradata_size );
> > +    avc_context->extradata[ (*offset)++ ] = packet->bytes >> 8;
> > +    avc_context->extradata[ (*offset)++ ] = packet->bytes & 0xff;
> > +    memcpy( avc_context->extradata + (*offset), packet->packet, packet->bytes );
> > +    (*offset) += packet->bytes;
> > +}
>
> ive already said this function is not safe security wise ...it may be
> safe with current libtheora and the current libtheora wrapper for libavcodec
> but it is not safe in general, if the source file can somehow cause a large
> comment packet to be injected (not so unlikely as comments from the source
> should be preserved, i know they are not currently but saftey of code should
> not depend on missing features) then this could become problematic ...
>
> if this is not corrected then the patch will not be applied
>
> what must be done at minimum
> 1. check the return of av_realloc() against NULL, the first access afterwards
> is to [*offset] not to [0] ...
> 2. avc_context->extradata_size += 2 + packet->bytes; must be checked for
> overflows
>
>
> [...]
> > +    switch (result) {
> > +        case 0:
> > +            /* No packet is ready */
> > +            return 0;
> > +        case -1:
> > +            /* Encoding finished */
> > +            return 0;
>
> return 0; is duplicated
>
> case  0:/* No packet is ready */
> case -1:/* Encoding finished */
>     return 0;
>
>

These issues have been addressed.


-- 
Paul Richards
-------------- next part --------------
A non-text attachment was scrubbed...
Name: theora.patch
Type: application/octet-stream
Size: 13928 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070121/006ce431/attachment.obj>