[gstreamer-bugs] [Bug 508291] [id3demux] must check if language code in id3v2 COMM frames is UTF-8

[GStreamer (bugzilla.gnome.org)]

If you have any questions why you received this email, please see the text at
the end of this email. Replies to this email are NOT read, please see the text
at the end of this email. You can add comments to this bug at:
  http://bugzilla.gnome.org/show_bug.cgi?id=508291

  GStreamer | gst-plugins-good | Ver: HEAD CVS

Tim-Philipp Müller changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #102460|none                        |reviewed
               Flag|                            |
 Attachment #102461|none                        |reviewed
               Flag|                            |
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED
            Summary|id3demux should check       |[id3demux] must check if
                   |language code in id3v2      |language code in id3v2 COMM
                   |comment tag for non-utf8    |frames is UTF-8
                   |strings                     |
   Target Milestone|HEAD                        |0.10.7




------- Comment #4 from Tim-Philipp Müller  2008-01-09 15:20 UTC -------
Nice catch! Indeed, we should check that. I've solved it a bit differently
though:

  2008-01-09  Tim-Philipp Müller  <tim at centricular dot net>

        Based on patch by: Tommi Myöhänen <ext-tommi.myohanen nokia com>

        * gst/id3demux/id3v2frames.c: (parse_comment_frame):
          Make sure the ISO 639-X language code in ID3v2 COMM frames
          is actually valid UTF-8 (or rather: ASCII), so we don't end
          up with non-UTF8 strings in tags if there's garbage in the
          language field. Also make sure the language code is always
          lower case. Fixes: #508291.



> Also, the warning comes from gst_structure_id_set_value() where non-utf8
> strings are skipped, but only if gst-debug is enabled. To avoid confusion I
> think enabling gst-debug should only add the debug prints and not change the
> behaviour of the code.

I agree it shouldn't change behaviour of the code, but allowing non-UTF8
strings to end up in structures/taglists is just a very bad idea, since it can
easily lead to random crashes or invalid memory accesses when apps pass those
strings to g_markup_escape() or g_markup_printf_escaped(), like they would if
they wanted to construct fancy pango markup.

Therefore, I've changed gststructure.c to always check strings for UTF-8
conformance and to always return if the string is non-UTF-8. The performance
impact should be negligible, and it might reveal bugs in proprietary plugins
that are only tested in environments where gst-debug is disabled:

  2008-01-09  Tim-Philipp Müller  <tim at centricular dot net>

        * gst/gststructure.c: (gst_structure_id_set_value):
          Always check UTF-8 conformance of structure strings and not only
          if the debugging system is enabled; reasoning: the behaviour of
          the actual code shouldn't really change depending on whether the
          debugging system is enabled or not (#508291).


-- 
See http://bugzilla.gnome.org/page.cgi?id=email.html for more info about why you received
this email, why you can't respond via email, how to stop receiving
emails (or reduce the number you receive), and how to contact someone
if you are having problems with the system.

You can add comments to this bug at http://bugzilla.gnome.org/show_bug.cgi?id=508291.