[Ffmpeg-devel] [PATCH] THP PCM decoder (GSoC Qualification)

[Michael Niedermayer]

Hi

On Sat, Apr 07, 2007 at 11:39:58AM +0200, Marco Gerards wrote:
> Uoti Urpala <uoti.urpala at pp1.inet.fi> writes:
> 
> > On Fri, 2007-04-06 at 18:24 +0200, Marco Gerards wrote:
> >> Changing samplecnt to an unsigned int solved the problem.  I am
> >> sorry it took me this much time to notice this problem.
> >
> > I think you still missed another problem (overflow).
> 
> You are completely right.  This is fixed now.
> 
> Here is a new patch.  I fixed all issues (both playback and code)
> which were found.
[...]
>          break;
> +    case CODEC_ID_ADPCM_THP:
> +      {
> +        GetBitContext gb;
> +        int table[16][2];
> +        unsigned int samplecnt;
> +        int prev1[2], prev2[2];
> +        int ch;
> +
> +        if (buf_size < 80) {
> +            av_log(avctx, AV_LOG_ERROR, "frame too small\n");
> +            return -1;
> +        }
> +
> +        init_get_bits(&gb, src, buf_size * 8);
> +        src += buf_size;
> +
> +                    get_bits_long(&gb, 32); /* Channel size */
> +        samplecnt = get_bits_long(&gb, 32);
> +
> +        for (ch = 0; ch < 2; ch++)
> +            for (i = 0; i < 16; i++)
> +                table[i][ch] = get_sbits(&gb, 16);
> +
> +        /* Initialize the previous sample.  */
> +        for (ch = 0; ch < 2; ch++) {
> +            prev1[ch] = get_sbits(&gb, 16);
> +            prev2[ch] = get_sbits(&gb, 16);
> +        }
> +
> +        if (samples + samplecnt * (st + 1L) >= samples_end) {
> +            av_log(avctx, AV_LOG_ERROR, "allocated output buffer is too small\n");
> +            return -1;
> +        }

this check is still insufficient
samplecnt= 0xFFFFFFFF, st=0 will amongth many others still pass as long as
pointers are 32bit (yeah 32bit systems are still the majority AFIAK)
and on 64bit its a gamble if random pointer + 0xFFFFFFFF will overflow
probably it wont but thats just luck


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Good people do not need laws to tell them to act responsibly, while bad
people will find a way around the laws. -- Plato
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070407/099ec7d6/attachment.pgp>