GNOME Bugzilla – Bug 384989
misparsing of Real Media mux leads to buffer overrun
Last modified: 2006-12-12 13:47:32 UTC
Please describe the problem: Sample file: http://samples.mplayerhq.hu/real/AC-sipr/autahi-vox.rm This file contains a single audio stream; Sipro encoding and stream version = 4. When parsing the Media Properties (MDPR) header in the mux file, the rm demuxer assumes that the codec specific opaque data is always 16 bytes long. In the sample file, it is in fact zero bytes long. No bounds checking appears to be performed. The result is that later, when the codec specific data is actually read, Valgrind reports invalid memory accesses. Steps to reproduce: 1. Download the above file. 2. Run, under Valgrind, any app that will use GStreamer to play the file. (Note: it won't play since GStreamer doesn't yet support the Sipro codec). 3. Examine the Valgrind log (see below). Actual results: *** Mis-parsing of MDPR audio stream version 4. ==20955== Invalid read of size 1 ==20955== at 0x1B901750: memcpy (in /usr/lib/valgrind/vgpreload_memcheck.so) ==20955== by 0x1C36D96E: gst_rmdemux_parse_mdpr (rmdemux.c:1392) ==20955== by 0x1C36E57D: gst_rmdemux_chain (rmdemux.c:1036) ==20955== by 0x1C36F9D1: gst_rmdemux_loop (rmdemux.c:827) ==20955== by 0x1B953C7E: gst_task_func (gsttask.c:192) ==20955== by 0x1BBA7A54: (within /opt/gnome/lib/libglib-2.0.so.0.800.1) ==20955== by 0x1BBA5B60: (within /opt/gnome/lib/libglib-2.0.so.0.800.1) ==20955== by 0x1B9EA296: start_thread (in /lib/tls/libpthread-2.3.5.so) ==20955== by 0x1BCA637D: clone (in /lib/tls/libc-2.3.5.so) ==20955== by 0x1C573BAF: ??? ==20955== Address 0x1C2ABF0A is 13 bytes after a block of size 141 alloc'd ==20955== at 0x1B8FF8A6: malloc (in /usr/lib/valgrind/vgpreload_memcheck.so) ==20955== by 0x1BB8E45A: g_malloc (in /opt/gnome/lib/libglib-2.0.so.0.800.1) ==20955== by 0x1B922BF1: gst_buffer_new_and_alloc (gstbuffer.c:289) ==20955== by 0x1C25384C: gst_file_src_create (gstfilesrc.c:784) ==20955== by 0x1BF7160A: gst_base_src_get_range (gstbasesrc.c:1355) ==20955== by 0x1BF71EF5: gst_base_src_pad_get_range (gstbasesrc.c:1425) ==20955== by 0x1B941F30: gst_pad_get_range (gstpad.c:3732) ==20955== by 0x1B9422FA: gst_pad_pull_range (gstpad.c:3826) ==20955== by 0x1B932B0E: gst_proxy_pad_do_getrange (gstghostpad.c:201) ==20955== by 0x1B941F30: gst_pad_get_range (gstpad.c:3732) ==20955== by 0x1B9422FA: gst_pad_pull_range (gstpad.c:3826) ==20955== by 0x1C25DEE8: gst_type_find_element_getrange (gsttypefindelement.c:661) ==20955== by 0x1B941F30: gst_pad_get_range (gstpad.c:3732) ==20955== by 0x1B9422FA: gst_pad_pull_range (gstpad.c:3826) ==20955== by 0x1C36F7CA: gst_rmdemux_loop (rmdemux.c:805) ==20955== by 0x1B953C7E: gst_task_func (gsttask.c:192) ==20955== by 0x1BBA7A54: (within /opt/gnome/lib/libglib-2.0.so.0.800.1) ==20955== by 0x1BBA5B60: (within /opt/gnome/lib/libglib-2.0.so.0.800.1) ==20955== by 0x1B9EA296: start_thread (in /lib/tls/libpthread-2.3.5.so) ==20955== by 0x1BCA637D: clone (in /lib/tls/libc-2.3.5.so) + many more along the same lines. Expected results: Does this happen every time? Yes. Other information: Hex dump of the MDPR header for the stream: 0000040: XXXX XXXX 4d44 5052 0000 0097 0000 0000 MDPR........ 0000050: 0000 3e80 0000 3e80 0000 0140 0000 0140 ..>...>....@...@ 0000060: 0000 0000 0000 03c0 0001 f91c 0c41 7564 .............Aud 0000070: 696f 2053 7472 6561 6d14 6175 6469 6f2f io Stream.audio/ 0000080: 782d 706e 2d72 6561 6c61 7564 696f 0000 x-pn-realaudio.. 0000090: 0049 2e72 61fd 0004 0000 2e72 6134 0000 .I.ra......ra4.. 00000a0: 0004 0004 0000 0039 0003 0000 0140 0003 .......9.....@.. 00000b0: f480 0001 d4c0 0001 d4c0 0006 0140 0000 .............@.. 00000c0: 0000 3e80 0000 0010 0001 0473 6970 7204 ..>........sipr. 00000d0: 7369 7072 0107 0000 0000 00XX XXXX XXXX sipr....... |-- 16 bytes ---> The arrow above shows the codec specific data according to the current code. We can see that it runs off the end of the header. Annotated version: 0000040: XXXX XXXX 4d44 5052 0000 0097 0000 0000 MDPR........ 0000050: 0000 3e80 0000 3e80 0000 0140 0000 0140 ..>...>....@...@ 0000060: 0000 0000 0000 03c0 0001 f91c 0c41 7564 .............Aud 0000070: 696f 2053 7472 6561 6d14 6175 6469 6f2f io Stream.audio/ 0000080: 782d 706e 2d72 6561 6c61 7564 696f 0000 x-pn-realaudio.. 0000090: 0049 2e72 61fd 0004 0000 2e72 6134 0000 .I.ra......ra4.. ^^^^ ^^^^ Stream type FOURCC ^^^^ ^^^^ Stream version = 4 00000a0: 0004 0004 0000 0039 0003 0000 0140 0003 .......9.....@.. 00000b0: f480 0001 d4c0 0001 d4c0 0006 0140 0000 .............@.. 00000c0: 0000 3e80 0000 0010 0001 0473 6970 7204 ..>........sipr. Codec FOURCC prefixed by 1 byte ^^^^ ^^^^ ^^^^ length field (x two) 00000d0: 7369 7072 0107 0000 0000 00XX XXXX XXXX sipr....... ^^^^^^^^^ ^^^^ ^^ Three unknown bytes ^^ ^^^^ ^^ Length field for codec specific data.
Created attachment 78189 [details] [review] Corrective patch This patch causes the length as reported in the mux file to be read, rather than assuming 16 bytes. Note: My reference for this interpretation of the mux file format is the equivalent mplayer/xine demuxers.
Note: While this patch is, hopefully, an improvement. The mux file is an untrusted source of information. Thus, the length field might be incorrect and this ought to be guarded against. For example, with the above patch applied, if I use a hex editor to change the length field from 0x00000000 to 0xffffffff this is the result: $> ./xcfile crash.rm out.mp3 GLib-ERROR **: gmem.c:141: failed to allocate 4294967295 bytes aborting... Aborted (core dumped)
I've created a new report for this second issue since it happens with CVS HEAD for version 5 streams (see #384996).
See bug #384996 comment 2 for a patch with bounds checking.
Should be fixed in CVS now, thanks for the patch! 2006-12-12 Tim-Philipp Müller <tim at centricular dot net> Based on patch by: Roland Kay <roland.kay at ox compsoc net> * gst/realmedia/rmdemux.c: (gst_rmdemux_parse_mdpr): For version 4 streams, read the extra codec data size from the header instead of assuming it is always 16 (also read it from the right position) (#384989). For version 4 and 5 streams, check that the specified extra codec data size doesn't make us read beyond the chunk boundary (#384996).
No problem. Thanks for the quick response!