GNOME Bugzilla – Bug 776107
qtdemux: Crashes when parsing edit lists due to missing size checks
Last modified: 2016-12-22 12:48:39 UTC
Created attachment 341969 [details] poc of crash Did not have a lot of time to look into exactly what is going on here, but __gst_fast_read_swap32 segfaults while attempting an endianness swap during the demuxing of the video. This one gets there through mp4, but due to the nature of the files gst deals with presumably this can be reached through other paths as well. Output from asan follows and a sample file producing the crash in the latest version. ================================================================= ==21817==ERROR: AddressSanitizer: SEGV on unknown address 0x625000080000 (pc 0x7faa56155a2c bp 0x7faa55d17420 sp 0x7faa55d17420 T3) #0 0x7faa56155a2b in __gst_fast_read_swap32 /usr/include/gstreamer-1.0/gst/gstutils.h:131 #1 0x7faa5617941d in qtdemux_parse_segments /home/fuzzy/src/gst-plugins-good-1.10.2/gst/isomp4/qtdemux.c:8680 #2 0x7faa56182e22 in qtdemux_parse_trak /home/fuzzy/src/gst-plugins-good-1.10.2/gst/isomp4/qtdemux.c:10900 #3 0x7faa56188649 in qtdemux_parse_tree /home/fuzzy/src/gst-plugins-good-1.10.2/gst/isomp4/qtdemux.c:12495 #4 0x7faa56163e3a in gst_qtdemux_loop_state_header /home/fuzzy/src/gst-plugins-good-1.10.2/gst/isomp4/qtdemux.c:4191 #5 0x7faa5616c989 in gst_qtdemux_loop /home/fuzzy/src/gst-plugins-good-1.10.2/gst/isomp4/qtdemux.c:5723 #6 0x7faa5cd9eb40 (/usr/lib/libgstreamer-1.0.so.0+0xa6b40) #7 0x7faa5c805acd (/usr/lib/libglib-2.0.so.0+0x72acd) #8 0x7faa5c8050d4 (/usr/lib/libglib-2.0.so.0+0x720d4) #9 0x7faa5c279453 in start_thread (/usr/lib/libpthread.so.0+0x7453) #10 0x7faa5bfbc7de in __GI___clone (/usr/lib/libc.so.6+0xe87de) AddressSanitizer can not provide additional info. SUMMARY: AddressSanitizer: SEGV /usr/include/gstreamer-1.0/gst/gstutils.h:131 in __gst_fast_read_swap32 Thread T3 (qtdemux0:sink) created by T2 (typefind:sink) here: #0 0x7faa5d99d498 in __interceptor_pthread_create /build/gcc/src/gcc/libsanitizer/asan/asan_interceptors.cc:236 #1 0x7faa5c822b9f (/usr/lib/libglib-2.0.so.0+0x8fb9f) Thread T2 (typefind:sink) created by T1 (GstPlayer) here: #0 0x7faa5d99d498 in __interceptor_pthread_create /build/gcc/src/gcc/libsanitizer/asan/asan_interceptors.cc:236 #1 0x7faa5c822b9f (/usr/lib/libglib-2.0.so.0+0x8fb9f) Thread T1 (GstPlayer) created by T0 here: #0 0x7faa5d99d498 in __interceptor_pthread_create /build/gcc/src/gcc/libsanitizer/asan/asan_interceptors.cc:236 #1 0x7faa5c822b9f (/usr/lib/libglib-2.0.so.0+0x8fb9f) ==21817==ABORTING
Confirmed here
Created attachment 341972 [details] [review] qtdemux: Check if we have enough data available when parsing edit lists Also consume the data entry by entry to get complicated indexing out of the code.
__gst_fast_read_swap32() is used by GST_READ_UINT32_BE(). That's just reading a 32 bit integer in big endian order from some memory address, and swaps it into native endianness. The problem is not that function, but that qtdemux is calling it without checking that the memory there is actually readable.
Attachment 341972 [details] pushed as 76c007d - qtdemux: Check if we have enough data available when parsing edit lists
Thanks for reporting! I'll merge it into 1.10 soonish and it will be part of the 1.10.3 release
Nice, thanks for posting the diff that is enlightening. I also appreciate the further explanation, interesting stuff. Glad I could help :)
Sure, just ask if something is not clear and let us know if you find more