Bug 38060

Summary:	message-mixin: check we have >= 2 parts before accessing the second one
Product:	Telepathy	Reporter:	Danielle Madeley <danielle>
Component:	tp-glib	Assignee:	Telepathy bugs list <telepathy-bugs>
Status:	RESOLVED FIXED	QA Contact:	Telepathy bugs list <telepathy-bugs>
Severity:	normal
Priority:	medium	CC:	pochu27
Version:	git master	Keywords:	patch
Hardware:	Other
OS:	All
URL:	http://cgit.collabora.com/git/user/danni/telepathy-glib.git/log/?h=crash-38060
Whiteboard:
i915 platform:		i915 features:

Description Danielle Madeley 2011-06-07 20:36:57 UTC

tp-glib doesn't check there is actually a second message-part before accessing it (telepathy-glib/message-mixin.c:654), if your CM is broken this will cause a crash. It would be much better to generate a critical-warning that the CM is broken and do something to recover.

Comment 1 Danielle Madeley 2011-06-07 21:14:47 UTC

Actually now I've stared at this properly, I'm pretty sure it's actually an off-by-one error, we should be requesting part 0 (the header).

http://cgit.collabora.com/git/user/danni/telepathy-glib.git/log/?h=crash-38060

Comment 2 Danielle Madeley 2011-06-07 21:38:56 UTC

There is a shocking lack of test coverage here too :(

Comment 3 Emilio Pozuelo Monfort 2011-06-08 01:03:27 UTC

Not a tp-glib reviewer, but:

Looks good to me.

-      if (echo != NULL)
+      if (echo != NULL && echo->len >= 1)

I guess echo->len should always be > 0, right? If so perhaps a g_warning or a DEBUG could be useful in order to notice the problem and fix the CMs.

Comment 4 Will Thompson 2011-06-08 09:10:23 UTC

Good catch.

I echo comment 2, and comment 3. It's an error to include an echo with no parts. It's hard to do, since tp_cm_message_new() requires initial_parts to be ≥1. So a warning wouldn't hurt, but I don't think it's absolutely essential, because the CM developer would have to go out of their way to get into this situation.

Comment 5 Danielle Madeley 2011-06-08 17:20:01 UTC

Updated to include a warning.

Comment 6 Will Thompson 2011-07-25 10:33:39 UTC

Merged to 0.14 and to master, thanks.

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.