use-after-free in matroska demuxer

[Evgeniy Stepanov]

I've got this AddressSanitizer report twice, both times when seeking back in mplayer in an .mkv file. I don't have a reliable reproducer. "Heap-buffer-overflow" in the first line is really a use-after-free, that's an issue with the tool.

Looks like matroska_parse_cluster_incremental() deletes the current cluster when it encounters a new one, but the old one can still be used for seeking?

ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7f9b247b1480 at pc 0xa2639c bp 0x7ffff0c41cb0 sp 0x7ffff0c41ca8
READ of size 8 at 0x7f9b247b1480 thread T0
    #0 0xa2639b in matroska_read_seek /build/mplayer/ffmpeg/libavformat/matroskadec.c:2410
    #1 0xb76c34 in seek_frame_internal /build/mplayer/ffmpeg/libavformat/utils.c:1952
    #2 0x8360a4 in demux_seek_lavf /build/mplayer/libmpdemux/demux_lavf.c:690
    #3 0x6941b9 in demux_seek /build/mplayer/libmpdemux/demuxer.c:1372
    #4 0x4a0f30 in seek /build/mplayer/mplayer.c:2710
    #5 0x49db59 in main /build/mplayer/mplayer.c:3964
    #6 0x7f9b374c0efe in __libc_start_main /build/buildd/eglibc-2.13/csu/libc-start.c:226
0x7f9b247b1480 is located 1088 bytes inside of 13340-byte region [0x7f9b247b1040,0x7f9b247b445c)
freed by thread T0 here:
    #0 0x1a2c710 in free ??:0
    #1 0x19edaca in av_free /build/mplayer/ffmpeg/libavutil/mem.c:190
    #2 0xa29e69 in ebml_free /build/mplayer/ffmpeg/libavformat/matroskadec.c:969
    #3 0xa29f17 in ebml_free /build/mplayer/ffmpeg/libavformat/matroskadec.c:975
    #4 0xa26b00 in matroska_parse_cluster_incremental /build/mplayer/ffmpeg/libavformat/matroskadec.c:2281
    #5 0xa249e5 in matroska_read_packet /build/mplayer/ffmpeg/libavformat/matroskadec.c:2361
    #6 0xb6ffb9 in ff_read_packet /build/mplayer/ffmpeg/libavformat/utils.c:710
    #7 0xb72d63 in read_frame_internal /build/mplayer/ffmpeg/libavformat/utils.c:1284
    #8 0xb71fa6 in av_read_frame /build/mplayer/ffmpeg/libavformat/utils.c:1436
    #9 0x833c6f in demux_lavf_fill_buffer /build/mplayer/libmpdemux/demux_lavf.c:612
    #10 0x690af5 in demux_fill_buffer /build/mplayer/libmpdemux/demuxer.c:633
    #11 0x691a29 in ds_get_packet /build/mplayer/libmpdemux/demuxer.c:832
    #12 0x8245f5 in decode_audio /build/mplayer/libmpcodecs/ad_ffmpeg.c:271
    #13 0x5e1fdb in filter_n_bytes /build/mplayer/libmpcodecs/dec_audio.c:393
    #14 0x49ad47 in fill_audio_out_buffers /build/mplayer/mplayer.c:2173
previously allocated by thread T0 here:
    #0 0x1a2ca4a in posix_memalign ??:0
    #1 0x19edbad in av_malloc /build/mplayer/ffmpeg/libavutil/mem.c:97
    #2 0x14729a6 in ff_fast_malloc /build/mplayer/ffmpeg/libavcodec/utils.c:82
    #3 0xa2b69a in ebml_read_binary /build/mplayer/ffmpeg/libavformat/matroskadec.c:765
    #4 0xa2b81f in ebml_parse_elem /build/mplayer/ffmpeg/libavformat/matroskadec.c:947
    #5 0xa26984 in ebml_parse /build/mplayer/ffmpeg/libavformat/matroskadec.c:865
    #6 0xa249e5 in matroska_read_packet /build/mplayer/ffmpeg/libavformat/matroskadec.c:2361
    #7 0xb6ffb9 in ff_read_packet /build/mplayer/ffmpeg/libavformat/utils.c:710
    #8 0xb72d63 in read_frame_internal /build/mplayer/ffmpeg/libavformat/utils.c:1284
    #9 0xb71fa6 in av_read_frame /build/mplayer/ffmpeg/libavformat/utils.c:1436

[Carl Eugen Hoyos]

Could you add the FFmpeg version you are testing?
(A crash in mkv was fixed tonight.)

[Evgeniy Stepanov]

This was observed with git synced at:

commit e56b0984103b981ec25fe8a22ef9c4905b9751dd
Author: Clément Bœsch <​ubitux@gmail.com>
Date: Thu Oct 25 00:27:10 2012 +0200

lavf/srtenc: ignore invalid timed packets instead of failing.

This way we don't abort in the middle of remuxing, just warn about an
event ignored. The index increment is moved to make sure the output
numbers still make sense.

The fix for the crash you mentioned seems unrelated (it's in RM audio code, mine was h264/ac3).

[Evgeniy Stepanov]

I think I got this.
First of all, the report is a bit off. This is indeed a heap-buffer-overflow, but the original allocation stack is lost because it is waaay off to the right of the actual allocation.

This is what I believe is going on.

At matroskadev.c:2414 index_sub value is obtained as an index into the index table of the subtitle track. Then, in line 2417 it is used as an index into whatever track we are seeking in: st->index_entries[index_sub].pos. It seems like sizes of index tables for different tracks do not have to be connected in any way, right?

[Carl Eugen Hoyos]

Please test if the patch attached to ​http://thread.gmane.org/gmane.comp.video.ffmpeg.devel/164093 fixes the problem.

[Carl Eugen Hoyos]

Maybe fixed by Michael, please consider testing current git head!