[FFmpeg-devel] [PATCH] prevent buffer overflow with large a/mulaw frames

[Michael Niedermayer]

On Sat, Aug 08, 2009 at 06:23:16PM +1000, Peter Ross wrote:
[...]
> @@ -656,6 +646,11 @@
>                          - av_fifo_size(ost->fifo)/(ost->st->codec->channels * 2); //FIXME wrong
>  
>      if (ost->audio_resample) {
> +        double factor = (double)(enc->sample_rate * enc->channels) / (dec->sample_rate * dec->channels);
> +        int lenout = size * factor + 16;
> +        audio_buf = av_fast_realloc(audio_buf, &audio_buf_size, lenout);
> +        if (!audio_buf)
> +            av_exit(1);
>          buftmp = audio_buf;
>          size_out = audio_resample(ost->resample,
>                                    (short *)buftmp, (short *)buf,

that can be done with integers and no floats
but whats more important, please explain where the + 16 comes from and why
it is enough


> @@ -668,10 +663,13 @@
>  
>      if (!ost->audio_resample && dec->sample_fmt!=enc->sample_fmt) {
>          const void *ibuf[6]= {buftmp};
> -        void *obuf[6]= {audio_out2};
> +        void *obuf[6];
>          int istride[6]= {isize};
>          int ostride[6]= {osize};
>          int len= size_out/istride[0];
> +        audio_out2 = obuf[0] = av_fast_realloc(audio_out2, &audio_out2_size, len*osize);
> +        if (!audio_out2)
> +            av_exit(1);
>          if (av_audio_convert(ost->reformat_ctx, obuf, ostride, ibuf, istride, len)<0) {
>              printf("av_audio_convert() failed\n");
>              if (exit_on_error)
> @@ -692,7 +690,15 @@
>          av_fifo_generic_write(ost->fifo, buftmp, size_out, NULL);
>  
>          frame_bytes = enc->frame_size * osize * enc->channels;
> +        audio_buf = av_fast_realloc(audio_buf, &audio_buf_size, frame_bytes);
> +        if (!audio_buf)
> +            av_exit(1);
>  
> +        /* encoded data is written to audio_out; assume that 8 bytes per sample are required */
> +        audio_out = av_fast_realloc(audio_out, &audio_out_size, enc->frame_size * 8 * enc->channels);
> +        if (!audio_out)
> +            av_exit(1);
> +
>          while (av_fifo_size(ost->fifo) >= frame_bytes) {
>              AVPacket pkt;
>              av_init_packet(&pkt);
> @@ -731,6 +737,10 @@
>          if (coded_bps)
>              size_out *= coded_bps;
>  
> +        audio_out = av_fast_realloc(audio_out, &audio_out_size, size_out);
> +        if (!audio_out)
> +            av_exit(1);
> +
>          //FIXME pass ost->sync_opts as AVFrame.pts in avcodec_encode_audio()
>          ret = avcodec_encode_audio(enc, audio_out, size_out,
>                                     (short *)buftmp);

does your code consider:
"* @note The output buffer should be at least FF_MIN_BUFFER_SIZE bytes large.
 * However, for PCM audio the user will know how much space is needed
 * because it depends on the value passed in buf_size as described
 * below. In that case a lower value can be used.
"
?

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

I hate to see young programmers poisoned by the kind of thinking
Ulrich Drepper puts forward since it is simply too narrow -- Roman Shaposhnik
-------------- 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/20090808/d75cfe09/attachment.pgp>