Bug 7909 - dbus glib crashes when signals have '-' in the name
Summary: dbus glib crashes when signals have '-' in the name
Status: RESOLVED FIXED
Alias: None
Product: dbus
Classification: Unclassified
Component: GLib (show other bugs)
Version: unspecified
Hardware: x86 (IA32) Linux (All)
: medium normal
Assignee: Rob Taylor
QA Contact: John (J5) Palmieri
URL:
Whiteboard: more to do
Keywords:
Depends on: 23616
Blocks:
  Show dependency treegraph
 
Reported: 2006-08-17 14:13 UTC by Allison Lortie (desrt)
Modified: 2014-05-12 15:23 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
dbus-binding-tool: check for valid interface, member and property names (4.47 KB, patch)
2011-04-05 10:40 UTC, Simon McVittie
Details | Splinter Review

Description Allison Lortie (desrt) 2006-08-17 14:13:21 UTC
Register a signal with a '-' in the name and emit it.  The name will be flagged
as invalid by dbus and then you'll get a crash because the glib binding throws a
g_error().

The binding tool allows - to be in signal names without generating any sort of
warning and generates code which will cause dbus-glib to crash.

There should be an error at code generation time (when you run the binding tool)
or at the very least, runtime checks.
Comment 1 Rob Taylor 2006-10-25 13:23:23 UTC
There's been some discussion about allowing '-'. Need to look into this.
Comment 2 Simon McVittie 2011-04-05 10:40:06 UTC
Created attachment 45299 [details] [review]
dbus-binding-tool: check for valid interface, member and property names

Properties are currently allowed to be arbitrary UTF-8 since this matches
dbus-glib's runtime behaviour, although ideally new interfaces should
use the more restrictive member naming rules (leading to names like
MyProperty) for interop with QtDBus.

(The runtime-checks part of this bug is awaiting review as part of Bug #35766.)
Comment 3 Cosimo Alfarano 2011-08-12 09:34:44 UTC
Review of attachment 45299 [details] [review]:

dbus-gidl.c defines also arg_info_new and node_info_new which are not checked.
In my ignorance of the specs, I thought a node can be checked against g_variant_is_object_path() or being NULL and an arg and arg_type being valid utf8 or being NULL.
What does the spec say? Same observaton for parse_*

Otherwise it's OK.
Comment 4 Simon McVittie 2011-08-15 01:32:33 UTC
(In reply to comment #3)
> dbus-gidl.c defines also arg_info_new and node_info_new which are not checked.

Good catch, I'll look at those.

> In my ignorance of the specs, I thought a node can be checked against
> g_variant_is_object_path() or being NULL

Per the spec, the name attribute of the root <node> (the object being introspected) must be an object path or missing; the name attribute of a child <node> must be "relative" (which I assume means that prepending "/" gives you a valid object path).

On the other hand, dbus-binding-tool doesn't use the node name for anything, so perhaps we should just accept anything - I can see that we might break existing projects by validating the node name too strictly, although arguably they deserved to break anyway.

> and an arg and arg_type being valid utf8 or being NULL.

The name attribute of an <arg> must be UTF-8 or missing, yes. The type attribute of an <arg> must be a single complete type.
Comment 5 Simon McVittie 2014-01-14 11:23:38 UTC
(In reply to comment #3)
> Otherwise it's OK.

Merged for 0.102.
Comment 6 Dan Williams 2014-05-12 15:23:34 UTC
Merged as ee0f90d5d619ef53f30edbbeb19c7b6a5055a84b


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.