After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 530531 - [typefinding] bad read in mpeg_video_stream_type_find
[typefinding] bad read in mpeg_video_stream_type_find
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal normal
: 0.10.20
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2008-04-29 10:08 UTC by Andy Wingo
Modified: 2008-04-30 14:42 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
first half-meg of playtime (512.00 KB, application/octet-stream)
2008-04-30 08:24 UTC, Andy Wingo
Details

Description Andy Wingo 2008-04-29 10:08:26 UTC
==4328== Invalid read of size 1
==4328==    at 0xC81C758: mpeg_video_stream_type_find (gsttypefindfunctions.c:1691)
==4328==    by 0x5321E0E: gst_type_find_factory_call_function (gsttypefindfactory.c:243)
==4328==    by 0xC1F4124: gst_type_find_helper_get_range (gsttypefindhelper.c:270)
==4328==    by 0xBFC8482: gst_type_find_element_activate (gsttypefindelement.c:771)
==4328==    by 0x530A3C1: gst_pad_set_active (gstpad.c:658)
==4328==    by 0x52F27E5: activate_pads (gstelement.c:2509)
==4328==    by 0x52FD902: gst_iterator_fold (gstiterator.c:503)
==4328==    by 0x52F226A: iterator_activate_fold_with_resync (gstelement.c:2541)
==4328==    by 0x52F2377: gst_element_pads_activate (gstelement.c:2585)
==4328==    by 0x52F2680: gst_element_change_state_func (gstelement.c:2651)
==4328==    by 0xBFC69CC: gst_type_find_element_change_state (gsttypefindelement.c:821)
==4328==    by 0x52EEFDA: gst_element_change_state (gstelement.c:2425)
==4328==  Address 0xD48F52C is 4 bytes after a block of size 2,048 alloc'd
==4328==    at 0x4A059F6: malloc (vg_replace_malloc.c:149)
==4328==    by 0x5F29FF7: g_malloc (gmem.c:131)
==4328==    by 0x52E358F: gst_buffer_new_and_alloc (gstbuffer.c:328)
==4328==    by 0xBFBC951: gst_file_src_create (gstfilesrc.c:786)
==4328==    by 0xC1E74EF: gst_base_src_get_range (gstbasesrc.c:1818)
==4328==    by 0xC1E89FD: gst_base_src_pad_get_range (gstbasesrc.c:1936)
==4328==    by 0x5302F4D: gst_pad_get_range (gstpad.c:3857)
==4328==    by 0x5303468: gst_pad_pull_range (gstpad.c:3990)
==4328==    by 0x52F8C3E: gst_proxy_pad_do_getrange (gstghostpad.c:205)
==4328==    by 0xC1F4405: helper_find_peek (gsttypefindhelper.c:137)
==4328==    by 0x53218CF: gst_type_find_peek (gsttypefind.c:133)
==4328==    by 0xC81C6F1: mpeg_video_stream_type_find (gsttypefindfunctions.c:1673)

It seems obvious in the code (gsttypefindfunctions.c:1691) that size can go negative (indicating a read past the end of the array). However I do not know what the right fix is.

The file is a quicktime movie.

/kipple/pt-asp1.mp4: ISO Media, MPEG v4 system, version 1
Comment 1 Tim-Philipp Müller 2008-04-29 11:20:49 UTC
Any chance you could you attach the first 1M or so of the file in question? (For posterity/test suite)
Comment 2 Andy Wingo 2008-04-30 08:24:43 UTC
Created attachment 110148 [details]
first half-meg of playtime
Comment 3 Tim-Philipp Müller 2008-04-30 14:42:41 UTC
I think the minimal fix is to just remove the

        if (data[3] == 0xB3)
          continue;

where the bad read happens; it doesn't look like it's meant to be there and only a small optimisation anyway as far as I can tell.


However, I couldn't help myself and did a bit more (in preparation of fixing this function a bit more - it doesn't seem to recognise everything it should):

 2008-04-30  Tim-Philipp Müller  <tim.muller at collabora co uk>

        * gst/typefind/gsttypefindfunctions.c: (MpegVideoStreamCtx),
          (mpeg_video_stream_ctx_advance), (mpeg_video_stream_ctx_ensure_data),
          (mpeg_video_stream_type_find):
          Refactor a bit: use context structure to track parsing offset and size of
          available data and make the code a bit clearer. Fixes bad memory access
          in #356937.