[FFmpeg-devel] lavc/alsdec: allow for predictor orders higher than block length which is valid in ALS.

Submitted by Thilo Borgmann on Nov. 9, 2016, 7:02 p.m.

Details

Message ID 582372D1.6060501@mail.de
State New
Headers show

Commit Message

Thilo Borgmann Nov. 9, 2016, 7:02 p.m.
Hi,

fixes ticket #5297 reverting an old commit.

-Thilo
From 50f62f88696d1401d93c552d52fe1b9c396f8a00 Mon Sep 17 00:00:00 2001
From: Thilo Borgmann <thilo.borgmann@mail.de>
Date: Wed, 9 Nov 2016 20:00:02 +0100
Subject: [PATCH] lavc/alsdec: allow for predictor orders higher than block
 length which is valid in ALS.

Reverts: 18f94df8af04f2c02a25a7dec512289feff6517f
Fixes ticket #5297
---
 libavcodec/alsdec.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

Comments

Michael Niedermayer Nov. 10, 2016, 11:39 a.m.
On Wed, Nov 09, 2016 at 08:02:41PM +0100, Thilo Borgmann wrote:
> Hi,
> 
> fixes ticket #5297 reverting an old commit.
> 
> -Thilo

>  alsdec.c |    6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 82dc6f263e8a3f35e5321f355a61b2f3304f531d  0001-lavc-alsdec-allow-for-predictor-orders-higher-than-b.patch
> From 50f62f88696d1401d93c552d52fe1b9c396f8a00 Mon Sep 17 00:00:00 2001
> From: Thilo Borgmann <thilo.borgmann@mail.de>
> Date: Wed, 9 Nov 2016 20:00:02 +0100
> Subject: [PATCH] lavc/alsdec: allow for predictor orders higher than block
>  length which is valid in ALS.
> 
> Reverts: 18f94df8af04f2c02a25a7dec512289feff6517f
> Fixes ticket #5297
> ---
>  libavcodec/alsdec.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)

this causes segfautlts:

valgrind ./ffmpeg_g -i abd3c041acbcb816be113455d138166b-asan_heap-oob_b11634_3707_cov_1707137151_als_05_2ch48k16b.mp4 -f null -
==15702== Memcheck, a memory error detector
==15702== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al.
==15702== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info
==15702== Command: ./ffmpeg_g -i abd3c041acbcb816be113455d138166b-asan_heap-oob_b11634_3707_cov_1707137151_als_05_2ch48k16b.mp4 -f null -
==15702==
[...]

==15702== Invalid read of size 4
==15702==    at 0x792EC8: decode_var_block_data (alsdec.c:931)
==15702==    by 0x7932EE: decode_block (alsdec.c:1029)
==15702==    by 0x7933AC: read_decode_block (alsdec.c:1053)
==15702==    by 0x79356D: decode_blocks_ind (alsdec.c:1100)
==15702==    by 0x795611: read_frame_data (alsdec.c:1640)
==15702==    by 0x795EF6: decode_frame (alsdec.c:1782)
==15702==    by 0xB213E1: avcodec_decode_audio4 (utils.c:2362)
==15702==    by 0xB22E01: do_decode (utils.c:2793)
==15702==    by 0xB231AB: avcodec_send_packet (utils.c:2877)
==15702==    by 0x430FD5: decode (ffmpeg.c:2049)
==15702==    by 0x431135: decode_audio (ffmpeg.c:2079)
==15702==    by 0x432F4D: process_input_packet (ffmpeg.c:2493)
==15702==  Address 0x1067ed10 is 0 bytes after a block of size 6,224 alloc'd
==15702==    at 0x4C2A6C5: memalign (vg_replace_malloc.c:727)
==15702==    by 0x4C2A760: posix_memalign (vg_replace_malloc.c:876)
==15702==    by 0x1063B9F: av_malloc (mem.c:97)
==15702==    by 0x1063E47: av_mallocz (mem.c:254)
==15702==    by 0x790931: av_mallocz_array (mem.h:230)
==15702==    by 0x796EE0: decode_init (alsdec.c:2061)
==15702==    by 0xB1EE96: avcodec_open2 (utils.c:1603)
==15702==    by 0x433E04: init_input_stream (ffmpeg.c:2755)
==15702==    by 0x436A38: transcode_init (ffmpeg.c:3509)
==15702==    by 0x43A3A2: transcode (ffmpeg.c:4395)
==15702==    by 0x43AC2F: main (ffmpeg.c:4629)

[...]
Michael Niedermayer Nov. 10, 2016, 11:47 a.m.
On Thu, Nov 10, 2016 at 12:39:24PM +0100, Michael Niedermayer wrote:
> On Wed, Nov 09, 2016 at 08:02:41PM +0100, Thilo Borgmann wrote:
> > Hi,
> > 
> > fixes ticket #5297 reverting an old commit.
> > 
> > -Thilo
> 
> >  alsdec.c |    6 +-----
> >  1 file changed, 1 insertion(+), 5 deletions(-)
> > 82dc6f263e8a3f35e5321f355a61b2f3304f531d  0001-lavc-alsdec-allow-for-predictor-orders-higher-than-b.patch
> > From 50f62f88696d1401d93c552d52fe1b9c396f8a00 Mon Sep 17 00:00:00 2001
> > From: Thilo Borgmann <thilo.borgmann@mail.de>
> > Date: Wed, 9 Nov 2016 20:00:02 +0100
> > Subject: [PATCH] lavc/alsdec: allow for predictor orders higher than block
> >  length which is valid in ALS.
> > 
> > Reverts: 18f94df8af04f2c02a25a7dec512289feff6517f
> > Fixes ticket #5297
> > ---
> >  libavcodec/alsdec.c | 6 +-----
> >  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> this causes segfautlts:
> 
> valgrind ./ffmpeg_g -i abd3c041acbcb816be113455d138166b-asan_heap-oob_b11634_3707_cov_1707137151_als_05_2ch48k16b.mp4 -f null -
> ==15702== Memcheck, a memory error detector
> ==15702== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al.
> ==15702== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info
> ==15702== Command: ./ffmpeg_g -i abd3c041acbcb816be113455d138166b-asan_heap-oob_b11634_3707_cov_1707137151_als_05_2ch48k16b.mp4 -f null -
> ==15702==
> [...]
> 
> ==15702== Invalid read of size 4
> ==15702==    at 0x792EC8: decode_var_block_data (alsdec.c:931)
> ==15702==    by 0x7932EE: decode_block (alsdec.c:1029)
> ==15702==    by 0x7933AC: read_decode_block (alsdec.c:1053)
> ==15702==    by 0x79356D: decode_blocks_ind (alsdec.c:1100)
> ==15702==    by 0x795611: read_frame_data (alsdec.c:1640)
> ==15702==    by 0x795EF6: decode_frame (alsdec.c:1782)
> ==15702==    by 0xB213E1: avcodec_decode_audio4 (utils.c:2362)
> ==15702==    by 0xB22E01: do_decode (utils.c:2793)
> ==15702==    by 0xB231AB: avcodec_send_packet (utils.c:2877)
> ==15702==    by 0x430FD5: decode (ffmpeg.c:2049)
> ==15702==    by 0x431135: decode_audio (ffmpeg.c:2079)
> ==15702==    by 0x432F4D: process_input_packet (ffmpeg.c:2493)
> ==15702==  Address 0x1067ed10 is 0 bytes after a block of size 6,224 alloc'd
> ==15702==    at 0x4C2A6C5: memalign (vg_replace_malloc.c:727)
> ==15702==    by 0x4C2A760: posix_memalign (vg_replace_malloc.c:876)
> ==15702==    by 0x1063B9F: av_malloc (mem.c:97)
> ==15702==    by 0x1063E47: av_mallocz (mem.c:254)
> ==15702==    by 0x790931: av_mallocz_array (mem.h:230)
> ==15702==    by 0x796EE0: decode_init (alsdec.c:2061)
> ==15702==    by 0xB1EE96: avcodec_open2 (utils.c:1603)
> ==15702==    by 0x433E04: init_input_stream (ffmpeg.c:2755)
> ==15702==    by 0x436A38: transcode_init (ffmpeg.c:3509)
> ==15702==    by 0x43A3A2: transcode (ffmpeg.c:4395)
> ==15702==    by 0x43AC2F: main (ffmpeg.c:4629)

=================================================================
==31617==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x62300000dd50 at pc 0x000000c1c462 bp 0x7ffd3a768600 sp 0x7ffd3a7685f8
READ of size 4 at 0x62300000dd50 thread T0
    #0 0xc1c461 in decode_var_block_data libavcodec/alsdec.c:931:28
    #1 0xc17c80 in decode_block libavcodec/alsdec.c:1029:15
    #2 0xc197d0 in read_decode_block libavcodec/alsdec.c:1053:12
    #3 0xc14f9a in decode_blocks_ind libavcodec/alsdec.c:1100:20
    #4 0xc1318d in read_frame_data libavcodec/alsdec.c:1640:23
    #5 0xc0fdd7 in decode_frame libavcodec/alsdec.c:1782:26
    #6 0x13f3cc8 in avcodec_decode_audio4 libavcodec/utils.c:2362:19
    #7 0x13f7d06 in do_decode libavcodec/utils.c:2793:15
    #8 0x13f79c9 in avcodec_send_packet libavcodec/utils.c:2877:12
    #9 0x532efb in decode ffmpeg.c:2049:15
    #10 0x52db05 in decode_audio ffmpeg.c:2079:11
    #11 0x5186ad in process_input_packet ffmpeg.c:2493:19
    #12 0x52483d in process_input ffmpeg.c:4282:5
    #13 0x516639 in transcode_step ffmpeg.c:4370:11
    #14 0x50fd99 in transcode ffmpeg.c:4424:15
    #15 0x50ecfe in main ffmpeg.c:4629:9
    #16 0x7f844cc367ec in __libc_start_main /build/eglibc-oqps9y/eglibc-2.15/csu/libc-start.c:226
    #17 0x424258 in _start (ffmpeg_g+0x424258)

0x62300000dd50 is located 0 bytes to the right of 6224-byte region [0x62300000c500,0x62300000dd50)
allocated by thread T0 here:
    #0 0x4b4dc5 in __interceptor_posix_memalign (ffmpeg_g+0x4b4dc5)
    #1 0x1f419a8 in av_malloc libavutil/mem.c:97:9
    #2 0x1f41f2e in av_mallocz libavutil/mem.c:254:17
    #3 0xc0ee66 in decode_init libavcodec/alsdec.c:2061:29
    #4 0x13eda4f in avcodec_open2 libavcodec/utils.c:1603:15
    #5 0x51b0e3 in init_input_stream ffmpeg.c:2755:20
    #6 0x51362e in transcode_init ffmpeg.c:3509:20
    #7 0x50fcec in transcode ffmpeg.c:4395:11
    #8 0x50ecfe in main ffmpeg.c:4629:9
    #9 0x7f844cc367ec in __libc_start_main /build/eglibc-oqps9y/eglibc-2.15/csu/libc-start.c:226

SUMMARY: AddressSanitizer: heap-buffer-overflow libavcodec/alsdec.c:931:28 in decode_var_block_data
Shadow bytes around the buggy address:
  0x0c467fff9b50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c467fff9b60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c467fff9b70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c467fff9b80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c467fff9b90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c467fff9ba0: 00 00 00 00 00 00 00 00 00 00[fa]fa fa fa fa fa
  0x0c467fff9bb0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c467fff9bc0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c467fff9bd0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c467fff9be0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c467fff9bf0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==31617==ABORTING


[...]
Thilo Borgmann Nov. 10, 2016, 6:51 p.m.
Am 10.11.16 um 12:47 schrieb Michael Niedermayer:
> On Thu, Nov 10, 2016 at 12:39:24PM +0100, Michael Niedermayer wrote:
>> On Wed, Nov 09, 2016 at 08:02:41PM +0100, Thilo Borgmann wrote:
>>> Hi,
>>>
>>> fixes ticket #5297 reverting an old commit.
>>>
>>> -Thilo
>>
>>>  alsdec.c |    6 +-----
>>>  1 file changed, 1 insertion(+), 5 deletions(-)
>>> 82dc6f263e8a3f35e5321f355a61b2f3304f531d  0001-lavc-alsdec-allow-for-predictor-orders-higher-than-b.patch
>>> From 50f62f88696d1401d93c552d52fe1b9c396f8a00 Mon Sep 17 00:00:00 2001
>>> From: Thilo Borgmann <thilo.borgmann@mail.de>
>>> Date: Wed, 9 Nov 2016 20:00:02 +0100
>>> Subject: [PATCH] lavc/alsdec: allow for predictor orders higher than block
>>>  length which is valid in ALS.
>>>
>>> Reverts: 18f94df8af04f2c02a25a7dec512289feff6517f
>>> Fixes ticket #5297
>>> ---
>>>  libavcodec/alsdec.c | 6 +-----
>>>  1 file changed, 1 insertion(+), 5 deletions(-)
>>
>> this causes segfautlts:

Thanks will have another look!

-Thilo

Patch hide | download patch | download mbox

diff --git a/libavcodec/alsdec.c b/libavcodec/alsdec.c
index 3986347..2b2273e 100644
--- a/libavcodec/alsdec.c
+++ b/libavcodec/alsdec.c
@@ -705,11 +705,7 @@  static int read_var_block_data(ALSDecContext *ctx, ALSBlockData *bd)
         } else {
             *bd->opt_order = sconf->max_order;
         }
-        if (*bd->opt_order > bd->block_length) {
-            *bd->opt_order = bd->block_length;
-            av_log(avctx, AV_LOG_ERROR, "Predictor order too large.\n");
-            return AVERROR_INVALIDDATA;
-        }
+
         opt_order = *bd->opt_order;
 
         if (opt_order) {