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 788838 - GstQueueArray: add function guards to avoid NULL queue pointer
GstQueueArray: add function guards to avoid NULL queue pointer
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
1.12.3
Other All
: Normal minor
: 1.13.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
: 788928 788929 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2017-10-11 16:37 UTC by Ashish Kumar
Modified: 2017-10-16 11:26 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
[PATCH]Possible crash due to dereferencing a null pointer in gst_queue_array_is_empty (704 bytes, patch)
2017-10-11 16:38 UTC, Ashish Kumar
none Details | Review
[PATCH]Possible crash due to dereferencing a null pointer in gst_queue_array_is_empty (708 bytes, patch)
2017-10-12 04:39 UTC, Ashish Kumar
none Details | Review
[PATCH] Gstreamer: Fix for possible crash due to null pointer (874 bytes, patch)
2017-10-13 07:25 UTC, Ashish Kumar
none Details | Review
Gstreamer: Fix for possible crashes due to null pointer dereferencing in public GstQueueArray APIs (3.84 KB, patch)
2017-10-13 11:31 UTC, Ashish Kumar
none Details | Review
[PATCH] Gstreamer: Fix for possible crashes due to null pointer dereferencing in public GstQueueArray APIs (3.56 KB, patch)
2017-10-16 10:43 UTC, Ashish Kumar
committed Details | Review

Description Ashish Kumar 2017-10-11 16:37:18 UTC
In 
File : gstqueuearray.c
Function : gboolean gst_queue_array_is_empty (GstQueueArray * array)
Line No. 320

There is a possibility of crash if array is null, due to dereferencing of null pointer.

 gboolean
gst_queue_array_is_empty (GstQueueArray * array)
{
  return (array->length == 0);
}

----------------
Submitting patch with null pointer check.
Comment 1 Ashish Kumar 2017-10-11 16:38:44 UTC
Created attachment 361342 [details] [review]
[PATCH]Possible crash due to dereferencing a null pointer in gst_queue_array_is_empty

Please review and share the feedback.
Comment 2 Sebastian Dröge (slomo) 2017-10-11 16:48:03 UTC
Review of attachment 361342 [details] [review]:

::: gstreamer-1.12.3_org/libs/gst/base/gstqueuearray.c
@@ +318,3 @@
 gst_queue_array_is_empty (GstQueueArray * array)
 {
+  return (!array || array->length == 0);

It's a programming error to pass NULL here.

Use "g_return_val_if_fail (array != NULL, FALSE);"
Comment 3 Ashish Kumar 2017-10-12 04:39:39 UTC
Created attachment 361396 [details] [review]
[PATCH]Possible crash due to dereferencing a null pointer in gst_queue_array_is_empty

Patch updated as per review comments.
Comment 4 Sebastian Dröge (slomo) 2017-10-12 07:27:59 UTC
Comment on attachment 361396 [details] [review]
[PATCH]Possible crash due to dereferencing a null pointer in gst_queue_array_is_empty

Looks good now but please submit the patch with a proper commit message, and please also check if similar guards are missing from other public GstQueueArray API.

See https://gstreamer.freedesktop.org/documentation/contribute/#how-to-submit-patches for details about how the commit message should look like.
Comment 5 Ashish Kumar 2017-10-13 07:25:31 UTC
Created attachment 361484 [details] [review]
[PATCH] Gstreamer: Fix for possible crash due to null pointer

Commit message improved in the patch.
Comment 6 Tim-Philipp Müller 2017-10-13 10:57:50 UTC
*** Bug 788928 has been marked as a duplicate of this bug. ***
Comment 7 Tim-Philipp Müller 2017-10-13 11:05:44 UTC
*** Bug 788929 has been marked as a duplicate of this bug. ***
Comment 8 Ashish Kumar 2017-10-13 11:31:16 UTC
Created attachment 361505 [details] [review]
Gstreamer: Fix for possible crashes due to null pointer dereferencing in public GstQueueArray APIs

Attached the consolidated patch for all the fixes.
Please review and share your feedback.
Comment 9 Sebastian Dröge (slomo) 2017-10-13 15:31:10 UTC
Review of attachment 361505 [details] [review]:

::: gstreamer-1.12.3/libs/gst/base/gstqueuearray.c
@@ +220,3 @@
 gst_queue_array_do_expand (GstQueueArray * array)
 {
+  g_return_if_fail (array != NULL);

This is an internal function. Only add these guards to public functions
Comment 10 Ashish Kumar 2017-10-16 10:43:00 UTC
Created attachment 361658 [details] [review]
[PATCH] Gstreamer: Fix for possible crashes due to null pointer dereferencing in public GstQueueArray APIs

Review comments implemented.
Comment 11 Sebastian Dröge (slomo) 2017-10-16 11:25:38 UTC
commit 250d3e728444f468c8f8fb2344d053865ce7e95d (HEAD -> master)
Author: Ashish Kumar <kr.ashish@samsung.com>
Date:   Mon Oct 16 16:06:37 2017 +0530

    queuearray: Fix for possible crashes due to null pointer dereferencing
    
    https://bugzilla.gnome.org/show_bug.cgi?id=788838