GNOME Bugzilla – Bug 727460
mpegts: add atsc terrestrial virtual channel table
Last modified: 2014-04-10 07:10:26 UTC
add atsc terrestrial virtual channel table
Created attachment 273431 [details] [review] add terrestrial virtual channel table
Review of attachment 273431 [details] [review]: ::: gst-libs/gst/mpegts/gst-atsc-section.c @@ +3,3 @@ + * + * Authors: + * Edward Hervey <edward@collabora.com> I think that copyright notice here is wrong :) @@ +96,3 @@ + + /* Skip already parsed data */ + data += 8; You should check sizes here imho @@ +109,3 @@ + for (guint i = 0; i < source_nb; i++) { + GstMpegTsAtscTVCTSource *source; + And inside the loop, or in front of the loop for the complete loop too
(In reply to comment #2) > Review of attachment 273431 [details] [review]: > > ::: gst-libs/gst/mpegts/gst-atsc-section.c > @@ +3,3 @@ > + * > + * Authors: > + * Edward Hervey <edward@collabora.com> > > I think that copyright notice here is wrong :) you right, I have overseen. > > @@ +96,3 @@ > + > + /* Skip already parsed data */ > + data += 8; > > You should check sizes here imho I don't know how many bytes it has. I can only check a minimum. > > @@ +109,3 @@ > + for (guint i = 0; i < source_nb; i++) { > + GstMpegTsAtscTVCTSource *source; > + > > And inside the loop, or in front of the loop for the complete loop too The same like above. But we have a number of entries (source_nb).
What I mean is that you should check if you have the data available before you try to read it :) So check that you always have at least as you expect next available.
> > > > @@ +96,3 @@ > > + > > + /* Skip already parsed data */ > > + data += 8; > > > > You should check sizes here imho > I don't know how many bytes it has. I can only check a minimum. 2 bytes (protocol_version + num_channels_in_section) before and 2 (descriptor-loop-length) + 4 (crc) after source entry loop is the minimum at this point.
Created attachment 273585 [details] [review] add atsc terrestial virtual channel table
Review of attachment 273585 [details] [review]: ::: gst-libs/gst/mpegts/gst-atsc-section.c @@ +155,3 @@ + data += 2; + + descriptors_loop_length = GST_READ_UINT16_BE (data) & 0x03FF; You need to check that you actually have that much data available
Review of attachment 273585 [details] [review]: Doesn't do it gst_mpegts_parse_descriptors?
(In reply to comment #8) > Review of attachment 273585 [details] [review]: > > Doesn't do it gst_mpegts_parse_descriptors? Yes, with the length you pass to it which comes from the data and which you don't check against the amount of data you really have available.
Created attachment 273862 [details] [review] add atsc terrestial virtual channel table
commit f566d172025f785bcc8b4ad4110399140f2512bd Author: Stefan Ringel <linuxtv@stefanringel.de> Date: Wed Apr 9 10:04:46 2014 +0200 mpegts: add atsc terrestrial virtual channel table https://bugzilla.gnome.org/show_bug.cgi?id=727460