[FFmpeg-devel] More ALS buffer overflows

[Reimar Döffinger]

On Thu, Feb 18, 2010 at 11:30:37PM +0100, Thilo Borgmann wrote:
> Am 18.02.10 23:19, schrieb M?ns Rullg?rd:
> > ==30999== Memcheck, a memory error detector
> > ==30999== Copyright (C) 2002-2009, and GNU GPL'd, by Julian Seward et al.
> > ==30999== Using Valgrind-3.5.0 and LibVEX; rerun with -h for copyright info
> > ==30999== Command: ./ffmpeg_g -i /misc/samples/mphq/fate-suite/lossless-audio/als_02_2ch48k16b.mp4 -f crc -
> > ==30999== 
> > FFmpeg version git-svn-r21885, Copyright (c) 2000-2010 the FFmpeg developers
> >   built on Feb 18 2010 21:42:57 with gcc 3.4.6 (Gentoo 3.4.6-r2)
> >   configuration: --cc=gcc-3.4.6
> >   libavutil     50. 9. 0 / 50. 9. 0
> >   libavcodec    52.54. 0 / 52.54. 0
> >   libavformat   52.52. 0 / 52.52. 0
> >   libavdevice   52. 2. 0 / 52. 2. 0
> >   libswscale     0. 9. 0 /  0. 9. 0
> > Input #0, mov,mp4,m4a,3gp,3g2,mj2, from '/misc/samples/mphq/fate-suite/lossless-audio/als_02_2ch48k16b.mp4':
> >   Metadata:
> >     major_brand     : mp42
> >     minor_version   : 0
> >     compatible_brands: mp42isom
> >   Duration: 00:00:14.81, start: 0.000000, bitrate: 437 kb/s
> >     Stream #0.0(und): Audio: als, 48000 Hz, 2 channels, s16, 437 kb/s
> > Output #0, crc, to 'pipe:':
> >   Metadata:
> >     encoder         : Lavf52.52.0
> >     Stream #0.0(und): Audio: pcm_s16le, 48000 Hz, 2 channels, s16, 1536 kb/s
> > Stream mapping:
> >   Stream #0.0 -> #0.0
> > Press [q] to stop encoding
> > Multiple frames in a packet from stream 0
> > ==30999== Invalid read of size 4
> > ==30999==    at 0x4AD3F3: decode_rice (bswap.h:40)
> > ==30999==    by 0x4AE0CC: read_var_block_data (alsdec.c:806)
> > ==30999==    by 0x4AE9BE: read_decode_block (alsdec.c:933)
> > ==30999==    by 0x4AF167: decode_frame (alsdec.c:1023)
> > ==30999==    by 0x49482C: avcodec_decode_audio3 (utils.c:631)
> > ==30999==    by 0x406849: output_packet (ffmpeg.c:1340)
> > ==30999==    by 0x40D9F4: main (ffmpeg.c:2324)
> > ==30999==  Address 0x622960d is 809,549 bytes inside a block of size 809,551 alloc'd
> > ==30999==    at 0x4C228A0: memalign (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> > ==30999==    by 0x4C2295A: posix_memalign (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> > ==30999==    by 0x7A5E44: av_malloc (mem.c:83)
> > ==30999==    by 0x48D6C9: av_new_packet (avpacket.c:52)
> > ==30999==    by 0x413D5A: av_get_packet (utils.c:292)
> > ==30999==    by 0x435B79: mov_read_packet (mov.c:2225)
> > ==30999==    by 0x414206: av_read_packet (utils.c:598)
> > ==30999==    by 0x4158F7: av_read_frame_internal (utils.c:1021)
> > ==30999==    by 0x41766B: av_find_stream_info (utils.c:2151)
> > ==30999==    by 0x408CE7: opt_input_file (ffmpeg.c:2917)
> > ==30999==    by 0x40E076: parse_options (cmdutils.c:179)
> > ==30999==    by 0x40B77F: main (ffmpeg.c:4007)
> > ==30999== 
> > ==30999== Invalid read of size 1
> > ==30999==    at 0x4AD3B6: decode_rice (get_bits.h:401)
> > ==30999==    by 0x4AE0CC: read_var_block_data (alsdec.c:806)
> > ==30999==    by 0x4AE9BE: read_decode_block (alsdec.c:933)
> > ==30999==    by 0x4AF167: decode_frame (alsdec.c:1023)
> > ==30999==    by 0x49482C: avcodec_decode_audio3 (utils.c:631)
> > ==30999==    by 0x406849: output_packet (ffmpeg.c:1340)
> > ==30999==    by 0x40D9F4: main (ffmpeg.c:2324)
> > ==30999==  Address 0x6229610 is 1 bytes after a block of size 809,551 alloc'd
> > ==30999==    at 0x4C228A0: memalign (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> > ==30999==    by 0x4C2295A: posix_memalign (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> > ==30999==    by 0x7A5E44: av_malloc (mem.c:83)
> > ==30999==    by 0x48D6C9: av_new_packet (avpacket.c:52)
> > ==30999==    by 0x413D5A: av_get_packet (utils.c:292)
> > ==30999==    by 0x435B79: mov_read_packet (mov.c:2225)
> > ==30999==    by 0x414206: av_read_packet (utils.c:598)
> > ==30999==    by 0x4158F7: av_read_frame_internal (utils.c:1021)
> > ==30999==    by 0x41766B: av_find_stream_info (utils.c:2151)
> > ==30999==    by 0x408CE7: opt_input_file (ffmpeg.c:2917)
> > ==30999==    by 0x40E076: parse_options (cmdutils.c:179)
> > ==30999==    by 0x40B77F: main (ffmpeg.c:4007)
> > ==30999== 
> > CRC=0xadfe5448
> > size=       0kB time=15.28 bitrate=   0.0kbits/s    
> > video:0kB audio:2865kB global headers:0kB muxing overhead -99.999489%
> > ==30999== 
> > ==30999== HEAP SUMMARY:
> > ==30999==     in use at exit: 0 bytes in 0 blocks
> > ==30999==   total heap usage: 821 allocs, 821 frees, 6,067,795 bytes allocated
> > ==30999== 
> > ==30999== All heap blocks were freed -- no leaks are possible
> > ==30999== 
> > ==30999== For counts of detected and suppressed errors, rerun with: -v
> > ==30999== ERROR SUMMARY: 406 errors from 2 contexts (suppressed: 6 from 6)
> > 
> 
> 
> This seems to be the same issue we are discussing on log at revision
> 21799. The latest patch I posted there you might want to check
> (als_fixltp.patch). If helpful, I'm going to commit it soon with nicer
> indention or any comment given...

That patch is not at all a solution for this issue, at best it will hide it,
while leaving the possibly exploitable code in.
The issue here is that the following code snipped has either wrong or missing
boundary checks:
        current_res = bd->raw_samples + start;

        for (sb = 0; sb < sub_blocks; sb++, start = 0)
            for (; start < sb_length; start++)
                *current_res++ = decode_rice(gb, s[sb]);