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 778432 - Crash on state chang to NULL during mp3_type_find_at_offset
Crash on state chang to NULL during mp3_type_find_at_offset
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal normal
: 1.10.4
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-02-10 09:11 UTC by Heekyoung Seo
Modified: 2017-02-19 10:54 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
typefindfunctions: prevent unsigned int overflow (976 bytes, patch)
2017-02-10 09:20 UTC, Heekyoung Seo
none Details | Review
typefindfunctions: prevent unsigned int overflow (977 bytes, patch)
2017-02-13 00:02 UTC, Heekyoung Seo
committed Details | Review

Description Heekyoung Seo 2017-02-10 09:11:27 UTC
When changing state to NULL during mp3_type_find, assertion occurs due to unsigned int overflow. 
Variable "found" type is guint, but "found" value is changed to 4294967295 (0xffffffff) because the code that performs -1 without checking overflow. 

      if (head_data == NULL &&                                         
          gst_type_find_peek (tf, offset + start_off - 1, 1) == NULL)               
        /* Incomplete last frame - don't count it. */                               
        found--;                                                      

Therefore, probability value is calculated abnormally large and it occurs assert.

        guint probability = found * GST_TYPE_FIND_MAXIMUM *                         
            (GST_MP3_TYPEFIND_TRY_SYNC - skipped) /                                 
            GST_MP3_TYPEFIND_TRY_HEADERS / GST_MP3_TYPEFIND_TRY_SYNC; 
        ...
        g_assert (probability <= GST_TYPE_FIND_MAXIMUM);
Comment 1 Heekyoung Seo 2017-02-10 09:20:17 UTC
Created attachment 345408 [details] [review]
typefindfunctions: prevent unsigned int overflow
Comment 2 Sebastian Dröge (slomo) 2017-02-10 11:04:23 UTC
Review of attachment 345408 [details] [review]:

::: gst/typefind/gsttypefindfunctions.c
@@ +1507,3 @@
       }
       g_assert (found <= GST_MP3_TYPEFIND_TRY_HEADERS);
+      if (found > 0 && head_data == NULL &&

As you say, found is a guint... so checking for > 0 will always be TRUE
Comment 3 Heekyoung Seo 2017-02-10 12:05:42 UTC
It is false when found = 0. It happen when found == 0.
Comment 4 Heekyoung Seo 2017-02-13 00:02:47 UTC
Created attachment 345593 [details] [review]
typefindfunctions: prevent unsigned int overflow

Does it look more clear if the condition is changed as below?

-      if (found > 0 && head_data == NULL &&
+      if (found != 0 && head_data == NULL &&

Assertion causes only when found == 0.
Comment 5 Sebastian Dröge (slomo) 2017-02-14 10:22:58 UTC
Attachment 345593 [details] pushed as 0889d89 - typefindfunctions: prevent unsigned int overflow