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 747541 - gdbus segfaults with invalid --dest
gdbus segfaults with invalid --dest
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gdbus
unspecified
Other Linux
: Normal normal
: ---
Assigned To: David Zeuthen (not reading bugmail)
gtkdev
Depends on:
Blocks:
 
 
Reported: 2015-04-09 02:07 UTC by Matthias Clasen
Modified: 2015-05-05 08:37 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gdbus: Validate the --dest argument (1016 bytes, patch)
2015-04-09 02:08 UTC, Matthias Clasen
none Details | Review
gdbus: Validate the --dest argument (2.02 KB, patch)
2015-04-09 02:59 UTC, Matthias Clasen
committed Details | Review

Description Matthias Clasen 2015-04-09 02:07:15 UTC
Originally filed here: https://bugzilla.redhat.com/show_bug.cgi?id=1045518

$ gdbus introspect --system --dest /org/freedesktop/UPower --object-path /org/freedesktop/UPower/devices/mouse_0003o046DoC52Bx0009
(gdbus introspect:26604): GLib-GIO-CRITICAL **: g_dbus_connection_call_sync_internal: assertion 'bus_name == NULL || g_dbus_is_name (bus_name)' failed
[1]    26604 segmentation fault (core dumped)
Comment 1 Matthias Clasen 2015-04-09 02:08:33 UTC
Created attachment 301170 [details] [review]
gdbus: Validate the --dest argument

Passing an nonsense string for the --dest argument can lead
to a segfault of gdbus. Thats not nice, so use our existing
validation function for bus names here.
Comment 2 Allison Karlitskaya (desrt) 2015-04-09 02:30:28 UTC
Review of attachment 301170 [details] [review]:

You miss at least a couple of cases here:

 - the same problem affects things other than introspect

 - the problem also impacts the completion case, for example:

  gdbus introspect --session --dest ^ --object-path [tab]
Comment 3 Matthias Clasen 2015-04-09 02:59:56 UTC
Created attachment 301173 [details] [review]
gdbus: Validate the --dest argument

Passing an nonsense string for the --dest argument can lead
to a segfault of gdbus. Thats not nice, so use our existing
validation function for bus names here.
Comment 4 Allison Karlitskaya (desrt) 2015-04-09 14:16:51 UTC
Review of attachment 301173 [details] [review]:

Looks better -- please commit with the trivial whitespace changes.

::: gio/gdbus-tool.c
@@ -863,3 @@
         }
     }
-

accidentally deleted newline?

@@ +1614,3 @@
+  if (!request_completion && !g_dbus_is_name (opt_introspect_dest))
+    {
+        g_printerr (_("Error: %s is not a valid bus name\n"), opt_introspect_dest);

weird indent here
Comment 5 Matthias Clasen 2015-04-09 21:30:32 UTC
Attachment 301173 [details] pushed as 2110795 - gdbus: Validate the --dest argument
Comment 6 Georg Müller 2015-05-05 08:37:02 UTC
This commit only fixes the symptoms of the segmentation fault.

In my opinion, it should be better to fix the segfault itself.

In case of an error in g_dbus_connection_call_sync(), the error variable is not set in all cases (there are a lot of g_return_val_if_fail(..., NULL) in g_dbus_connection_call_sync_internal() )

In gdbus-tool.c, it is not checked whether error is null or not, but it is dereferenced to print error->message.