Re: [FFmpeg-devel] [FFmpeg-cvslog] avcodec/mlpdec: relax channels checking

Re: [FFmpeg-devel] [FFmpeg-cvslog] avcodec/mlpdec: relax channels checking

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 4836 bytes --]

On Sun, Sep 18, 2022 at 01:21:23PM +0000, Paul B Mahol wrote:
> ffmpeg | branch: master | Paul B Mahol <onemda@gmail.com> | Sat Sep 17 14:08:58 2022 +0200| [adaa06581c5444c94eef72d61b8166f096e2687a] | committer: Paul B Mahol
> 
> avcodec/mlpdec: relax channels checking
> 
> Internal TrueHD decoder channel rematrix can mix 2 stereo substreams
> into single mono stream.
> 
> Fixes #1726
> 
> > http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=adaa06581c5444c94eef72d61b8166f096e2687a
> ---
> 
>  libavcodec/mlpdec.c | 23 +----------------------
>  1 file changed, 1 insertion(+), 22 deletions(-)

This produces out of array accesses

Also this was never posted to the mailing list. Simply removing checks is not
a solution, how can that even be a solution ?!

please make sure you post all patches especially ones thats have not been tested
very well to the mailing list

thx


==764==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x62700000365c at pc 0x000000571644 bp 0x7ffe8bba5f60 sp 0x7ffe8bba5f58
READ of size 4 at 0x62700000365c thread T0
SCARINESS: 27 (4-byte-read-heap-buffer-overflow-far-from-bounds)
    #0 0x571643 in ff_mlp_pack_output /src/ffmpeg/libavcodec/mlpdsp.c:116:30
    #1 0x56f95f in output_data /src/ffmpeg/libavcodec/mlpdec.c:1126:30
    #2 0x56bee8 in read_access_unit /src/ffmpeg/libavcodec/mlpdec.c:1354:16
    #3 0x51f46a in decode_simple_internal /src/ffmpeg/libavcodec/decode.c:307:15
    #4 0x51f46a in decode_simple_receive_frame /src/ffmpeg/libavcodec/decode.c:563:15
    #5 0x51f46a in decode_receive_frame_internal /src/ffmpeg/libavcodec/decode.c:584:15
    #6 0x51ea5d in avcodec_send_packet /src/ffmpeg/libavcodec/decode.c:665:15
    #7 0x5054ab in LLVMFuzzerTestOneInput /src/ffmpeg/tools/target_dec_fuzzer.c:510:25
    #8 0xd9ec9a in ExecuteFilesOnyByOne aflplusplus/utils/aflpp_driver/aflpp_driver.c:234:7
    #9 0xd9ea6c in main aflplusplus/utils/aflpp_driver/aflpp_driver.c:318:12
    #10 0x7f5a5cfd90b2 in __libc_start_main /build/glibc-eX1tMB/glibc-2.31/csu/libc-start.c:308:16
    #11 0x442f0d in _start
0x62700000365c is located 28 bytes to the right of 13632-byte region [0x627000000100,0x627000003640)
allocated by thread T0 here:
    #0 0x4c4e97 in posix_memalign /src/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
    #1 0xcebcb1 in av_malloc /src/ffmpeg/libavutil/mem.c:105:9
    #2 0xcebcb1 in av_mallocz /src/ffmpeg/libavutil/mem.c:266:17
    #3 0x57270b in init_context_defaults /src/ffmpeg/libavcodec/options.c:129:24
    #4 0x57270b in avcodec_alloc_context3 /src/ffmpeg/libavcodec/options.c:156:9
    #5 0x502963 in LLVMFuzzerTestOneInput /src/ffmpeg/tools/target_dec_fuzzer.c:311:27
    #6 0xd9ec9a in ExecuteFilesOnyByOne aflplusplus/utils/aflpp_driver/aflpp_driver.c:234:7
SUMMARY: AddressSanitizer: heap-buffer-overflow (/mnt/scratch0/clusterfuzz/bot/builds/clusterfuzz-builds-afl_ffmpeg_bbf927d7e4cde0b71897048111a2d684e48dfab7/revisions/ffmpeg_AV_CODEC_ID_TRUEHD_fuzzer+0x571643)
Shadow bytes around the buggy address:
  0x0c4e7fff8670: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4e7fff8680: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4e7fff8690: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4e7fff86a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4e7fff86b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c4e7fff86c0: 00 00 00 00 00 00 00 00 fa fa fa[fa]fa fa fa fa
  0x0c4e7fff86d0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4e7fff86e0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4e7fff86f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4e7fff8700: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4e7fff8710: 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
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  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
==764==ABORTING


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

Awnsering whenever a program halts or runs forever is
On a turing machine, in general impossible (turings halting problem).
On any real computer, always possible as a real computer has a finite number
of states N, and will either halt in less than N cycles or never halt.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [FFmpeg-cvslog] avcodec/mlpdec: relax channels checking

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1359 bytes --]

On Thu, Sep 29, 2022 at 11:59:18PM +0200, Michael Niedermayer wrote:
> On Sun, Sep 18, 2022 at 01:21:23PM +0000, Paul B Mahol wrote:
> > ffmpeg | branch: master | Paul B Mahol <onemda@gmail.com> | Sat Sep 17 14:08:58 2022 +0200| [adaa06581c5444c94eef72d61b8166f096e2687a] | committer: Paul B Mahol
> > 
> > avcodec/mlpdec: relax channels checking
> > 
> > Internal TrueHD decoder channel rematrix can mix 2 stereo substreams
> > into single mono stream.
> > 
> > Fixes #1726
> > 
> > > http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=adaa06581c5444c94eef72d61b8166f096e2687a
> > ---
> > 
> >  libavcodec/mlpdec.c | 23 +----------------------
> >  1 file changed, 1 insertion(+), 22 deletions(-)
> 
> This produces out of array accesses
> 
> Also this was never posted to the mailing list. Simply removing checks is not
> a solution, how can that even be a solution ?!
> 
> please make sure you post all patches especially ones thats have not been tested
> very well to the mailing list

I see it was posted to the ML, iam sorry it seems i must have missed it somehow
its still buggy though


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

"Nothing to hide" only works if the folks in power share the values of
you and everyone you know entirely and always will -- Tom Scott


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [FFmpeg-cvslog] avcodec/mlpdec: relax channels checking

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1620 bytes --]

On Fri, Sep 30, 2022 at 12:03:25AM +0200, Michael Niedermayer wrote:
> On Thu, Sep 29, 2022 at 11:59:18PM +0200, Michael Niedermayer wrote:
> > On Sun, Sep 18, 2022 at 01:21:23PM +0000, Paul B Mahol wrote:
> > > ffmpeg | branch: master | Paul B Mahol <onemda@gmail.com> | Sat Sep 17 14:08:58 2022 +0200| [adaa06581c5444c94eef72d61b8166f096e2687a] | committer: Paul B Mahol
> > > 
> > > avcodec/mlpdec: relax channels checking
> > > 
> > > Internal TrueHD decoder channel rematrix can mix 2 stereo substreams
> > > into single mono stream.
> > > 
> > > Fixes #1726
> > > 
> > > > http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=adaa06581c5444c94eef72d61b8166f096e2687a
> > > ---
> > > 
> > >  libavcodec/mlpdec.c | 23 +----------------------
> > >  1 file changed, 1 insertion(+), 22 deletions(-)
> > 
> > This produces out of array accesses
> > 
> > Also this was never posted to the mailing list. Simply removing checks is not
> > a solution, how can that even be a solution ?!
> > 
> > please make sure you post all patches especially ones thats have not been tested
> > very well to the mailing list
> 
> I see it was posted to the ML, iam sorry it seems i must have missed it somehow
> its still buggy though

patch which seems to fix this is attached. Didnt had as much time to write this
ATM as i would have liked ...
Please review.

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".