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 796835 - srtpdec: remove bogus check that accesses uninitialized memory
srtpdec: remove bogus check that accesses uninitialized memory
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
unspecified
Other All
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2018-07-19 15:29 UTC by Michael Olbrich
Modified: 2018-11-03 14:28 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
srtpdec: remove bogus check that accesses uninitialized memory (1.15 KB, patch)
2018-07-19 15:29 UTC, Michael Olbrich
none Details | Review
srtpdec: remove bogus check that accesses uninitialized memory (1.16 KB, patch)
2018-07-20 06:54 UTC, Michael Olbrich
none Details | Review

Description Michael Olbrich 2018-07-19 15:29:23 UTC
I came across this while using valgrind to debug a unrelated problem. I'm
not sure what the check was meant to do but I see no reason the check
anything but the return value of gst_structure_get()
Comment 1 Michael Olbrich 2018-07-19 15:29:28 UTC
Created attachment 373092 [details] [review]
srtpdec: remove bogus check that accesses uninitialized memory

'buf' is uninitialized if gst_structure_get() returns FALSE so it makes no
sense to check it. If gst_structure_get() returns TRUE, then 'buf' point to
a valid buffer that should be used and there is no need to check 'buf'
again.
Comment 2 Sebastian Dröge (slomo) 2018-07-19 20:29:11 UTC
Review of attachment 373092 [details] [review]:

::: ext/srtp/gstsrtpdec.c
@@ +565,3 @@
   }
 
+  if (gst_structure_get (s, "srtp-key", GST_TYPE_BUFFER, &buf, NULL)) {

Someone could set a NULL buffer in here, that's probably what this wanted to guard against? The better solution would be to initialize it to NULL first.

Also is there the same pattern used for the SRTCP key elsewhere?
Comment 3 Michael Olbrich 2018-07-20 06:47:14 UTC
Hmm, I see. And while it makes no difference for setting 'stream->key', it would circumvent the error when a key is needed. I'll update the patch.

And I've not seen that pattern elsewhere.
Comment 4 Michael Olbrich 2018-07-20 06:54:03 UTC
Created attachment 373097 [details] [review]
srtpdec: remove bogus check that accesses uninitialized memory

'buf' is uninitialized if gst_structure_get() returns FALSE so it makes no
sense to check it. If gst_structure_get() returns TRUE, then 'buf' point to
a valid buffer that should be used and there is no need to check 'buf'
again.
Comment 5 GStreamer system administrator 2018-11-03 14:28:11 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/issues/754.