Bug 7909

Summary:	dbus glib crashes when signals have '-' in the name
Product:	dbus	Reporter:	Allison Lortie (desrt) <desrt>
Component:	GLib	Assignee:	Rob Taylor <rob.taylor>
Status:	RESOLVED FIXED	QA Contact:	John (J5) Palmieri <johnp>
Severity:	normal
Priority:	medium	CC:	rob.taylor
Version:	unspecified
Hardware:	x86 (IA32)
OS:	Linux (All)
Whiteboard:	more to do
i915 platform:		i915 features:
Bug Depends on:	23616
Bug Blocks:
Attachments:	dbus-binding-tool: check for valid interface, member and property names

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.