Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#13238 closed patch (fixed)

vvconfig: Use compare function that will not crash on NULL string

Reported by: haakon Owned by:
Milestone: 2.7.10 Component: plugins
Version: 2.7.9 Keywords: vvconfig crash strcmp
Cc:

Description

At vvconfig plugin in get_plugin_frame(), string comparison function that accepts NULL pointer must be used for g_list_find_custom() call, as the list is terminated with an item containing NULL pointer. Pidgin crashes whenever device name is not found in the list.

Attachments (2)

0001-vvconfig-use-string-compare-accepting-NULL.patch (997 bytes) - added by haakon 8 years ago.
0001-vvconfig-remove-terminating-NULL-value-from-device-l.patch (2.5 KB) - added by haakon 8 years ago.
Newer version of patch, the prior one should be ignored.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 8 years ago by Robby

  • Milestone set to Patches Needing Review

comment:2 Changed 8 years ago by darkrain42

  • Milestone changed from Patches Needing Review to Patches Needing Improvement

purple_strequal is not actually a GCompareFunc. It returns TRUE or FALSE, whereas strcmp-like functions return -1, 0, or 1 (where 0, a.k.a. FALSE, means the two strings are 'equal').

Something like g_strcmp0 would do exactly what you're looking for, but neither rekkanoryo nor I recalled (off the top of our heads) if our V/V code requires glib 2.16 or not (I vaguely think it's actually glib 2.14)

comment:3 Changed 8 years ago by darkrain42

Relevant snippet from d@cpi:

(18:41:12) darkrain: Are we requiring 2.16 for V/V support, by chance?
(18:41:16) darkrain: *glib 2.16
(18:41:35) rekkanoryo: I think we were, but I don't recall for sure
(18:41:58) rekkanoryo: maiku would know

comment:4 Changed 8 years ago by darkrain42

Why is the linked list being terminated with an element whose value is NULL?

To clarify, that shouldn't be necessary at all. I'd suggest fixing the code in get_element_devices() not to do that unless it's really necessary.

Changed 8 years ago by haakon

Newer version of patch, the prior one should be ignored.

comment:5 Changed 8 years ago by haakon

From the code I can't see any reason for those terminating NULLs, I removed them and it seems to work correctly, no more crashes on strcmp now. Maybe original developer can give explanation if this was some relict code from previous implementation.

Please review the new patch, I also found that we should use g_list_find_custom() in device_changed_cb() function.

comment:6 Changed 8 years ago by darkrain42

  • Milestone changed from Patches Needing Improvement to 2.7.10

Looks right.

comment:7 Changed 8 years ago by jakub.adam@…

  • Resolution set to fixed
  • Status changed from new to closed

(In bbef7cf10a9629d710dbb4b4e0843c6e1edf6c68):
vvconfig: Don't crash when the stored device isn't found in the list of available devices. Fixes #13238.

comment:8 follow-up: Changed 8 years ago by ari

It seems that this bug is not entirely fixed: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=611678#44

comment:9 in reply to: ↑ 8 Changed 8 years ago by darkrain42

Replying to ari:

It seems that this bug is not entirely fixed: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=611678#44

It looks like this is a separate crash -- it's crashing on line 93, a.k.a.

klass = G_OBJECT_GET_CLAS (element);

(and I don't immediately see what's going on there)

Note: See TracTickets for help on using tickets.
All information, including names and email addresses, entered onto this website or sent to mailing lists affiliated with this website will be public. Do not post confidential information, especially passwords!