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 736490 - tsdemux: fix overflow of packet_length field of PESHeader
tsdemux: fix overflow of packet_length field of PESHeader
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
1.x
Other Linux
: Normal normal
: 1.4.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-09-11 16:57 UTC by Aurélien Zanelli
Modified: 2014-09-12 12:43 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
tsdemux: fix overflow of packet_length field of PESHeader (1.23 KB, patch)
2014-09-11 16:59 UTC, Aurélien Zanelli
committed Details | Review
MPEG-TS stream which causes the packet_length overflow (1.00 MB, video/mp2t)
2014-09-12 07:25 UTC, Aurélien Zanelli
  Details

Description Aurélien Zanelli 2014-09-11 16:57:02 UTC
If packet_length of a PES packet is greater than 65529, packet_length field of PESHeader structure, defined as a guin16, will overflow due to the fact that we add 6 bytes to it since commit d0e8427b4e7735bf5abc5cff18a03483cee1d9b5

So I will propose a patch which use a guint32 to store packet_length instead of a guint16.

You can reproduced the issue with the attached stream.
Comment 1 Aurélien Zanelli 2014-09-11 16:59:40 UTC
Created attachment 285937 [details] [review]
tsdemux: fix overflow of packet_length field of PESHeader

Patch proposal to fix the issue.
It defines packet_length as a guint32 instead of a guint16.
Comment 2 Tim-Philipp Müller 2014-09-11 17:10:38 UTC
Are you going to attach the stream as well? :)
Comment 3 Aurélien Zanelli 2014-09-12 07:25:18 UTC
Created attachment 285988 [details]
MPEG-TS stream which causes the packet_length overflow

Stream which causes the following error due to packet_length overflow:
gst-plugins-bad/gst/mpegtsdemux/tsdemux.c:1806:gst_ts_demux_parse_pes_header: invalid header and packet size combination

But in order to have the video stream, which is a Chinese AVS video, you will need patch provided in 727731 (I will rebase them agains the master branch).
Comment 4 Edward Hervey 2014-09-12 09:33:43 UTC
(should also be backported to 1.4 branch imho)


commit 6d767a09d8b85918304115255dbaacdac644a7d2
Author: Aurélien Zanelli <aurelien.zanelli@parrot.com>
Date:   Thu Sep 11 18:33:20 2014 +0200

    tsdemux: fix overflow of packet_length field of PESHeader
    
    packet_length is defined as a guint16 in the PESHeader structure. This
    definition match the specification. But since we add 6 bytes to the
    packet_length value (length of start_code + stream_id + packet_length),
    we can overflow the guint16 when the value in the PES header is greater
    than 65529.
    So use a guint32 instead of a guint16 to avoid overflow.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=736490