GNOME Bugzilla – Bug 684175
Check email value in e_destination_set_contact()
Last modified: 2013-01-23 15:01:18 UTC
Evolution 3.4.3-1 crashes when accessing a group contact. Program terminated with signal 11, Segmentation fault. #0 __strlen_sse2 () at ../sysdeps/x86_64/multiarch/../strlen.S:43 43 ../sysdeps/x86_64/multiarch/../strlen.S: Datei oder Verzeichnis nicht gefunden. A group contact was created and could be selected just fine when writing an email. Wanting to change an address in the contact group by going to the contacts and clicking on this group contact, Evolution crashes. This is reproducible and some address seems to be out of bounds.
+ Trace 230857
Thread 1 (Thread 0x7fbf5e0d1980 (LWP 7909))
The corresponding code is in the following. if (type == CONTACT) { CamelInternetAddress *addr; const gchar *name, *email; gchar *raw; raw = e_vcard_attribute_get_value (attr->data); addr = camel_internet_address_new (); camel_address_unformat (CAMEL_ADDRESS (addr), raw); camel_internet_address_get (addr, 0, &name, &email); e_destination_set_name (s_dest, name); → e_destination_set_email (s_dest, email); dest->priv->list_alldests = g_list_append (dest->priv->list_alldests, s_dest); g_object_unref (addr); g_free (raw); } else { […] So `camel_internet_address_get()` some lines above seems to do something incorrectly and the result should be checked somehow. -- System Information: Debian Release: wheezy/sid APT prefers unstable APT policy: (500, 'unstable') Architecture: amd64 (x86_64) Kernel: Linux 3.2.0-3-amd64 (SMP w/2 CPU cores) Locale: LANG=de_DE.UTF-8, LC_CTYPE=de_DE.UTF-8 (charmap=UTF-8) Shell: /bin/sh linked to /bin/dash Versions of packages evolution-data-server depends on: ii evolution-data-server-common 3.4.3-1 ii gconf-service 3.2.5-1+build1 ii libatk1.0-0 2.4.0-2 ii libc6 2.13-35 ii libcairo-gobject2 1.12.2-2 ii libcairo2 1.12.2-2 ii libcamel-1.2-33 3.4.3-1 ii libcomerr2 1.42.5-1 ii libdb5.1 5.1.29-5 ii libdbus-glib-1-2 0.100-1 ii libebackend-1.2-2 3.4.3-1 ii libebook-1.2-13 3.4.3-1 ii libecal-1.2-11 3.4.3-1 ii libedata-book-1.2-13 3.4.3-1 ii libedata-cal-1.2-15 3.4.3-1 ii libedataserver-1.2-16 3.4.3-1 ii libgconf-2-4 3.2.5-1+build1 ii libgdata13 0.12.0-1 ii libgdk-pixbuf2.0-0 2.26.1-1 ii libglib2.0-0 2.33.12+really2.32.3-1 ii libgoa-1.0-0 3.4.2-1 ii libgssapi-krb5-2 1.10.1+dfsg-2 ii libgtk-3-0 3.4.2-3 ii libgweather-3-0 3.4.1-1+build1 ii libical0 0.48-2 ii libk5crypto3 1.10.1+dfsg-2 ii libkrb5-3 1.10.1+dfsg-2 ii libldap-2.4-2 2.4.31-1 ii libnspr4 2:4.9.2-1 ii libnspr4-0d 2:4.9.2-1 ii libnss3 2:3.13.6-1 ii libnss3-1d 2:3.13.6-1 ii liboauth0 0.9.4-3+b1 ii libpango1.0-0 1.30.0-1 ii libsoup2.4-1 2.38.1-2 ii libsqlite3-0 3.7.13-1 ii libxml2 2.8.0+dfsg1-5 ii zlib1g 1:1.2.7.dfsg-13 evolution-data-server recommends no packages. Versions of packages evolution-data-server suggests: ii evolution 3.4.3-1 ii evolution-data-server-dbg 3.4.3-1 -- no debconf information
lindi- in #debian-gnome on irc.oftc.net advised me to get the following information to figure out if GDB is not able to see that address. (gdb) info register rax 0xfffffffffffffff0 -16 rbx 0xffffffffffffffff -1 rcx 0x3f 63 rdx 0x7fbf60866290 140459934900880 rsi 0xffffffffffffffff -1 rdi 0xffffffffffffffff -1 rbp 0x7fbf6083f3c0 0x7fbf6083f3c0 rsp 0x7fff4585ed48 0x7fff4585ed48 r8 0x7fbf6083f3a0 140459934741408 r9 0x2c0 704 r10 0x0 0 r11 0x1 1 r12 0xffffffffffffffff -1 r13 0x7fbf6083f550 140459934741840 r14 0x7fbf60892220 140459935080992 r15 0x0 0 rip 0x7fbf5b86cc41 0x7fbf5b86cc41 <__strlen_sse2+49> eflags 0x10286 [ PF SF IF RF ] cs 0x33 51 ss 0x2b 43 ds 0x0 0 es 0x0 0 fs 0x0 0 gs 0x0 0 (gdb) x/4i $rip => 0x7fbf5b86cc41 <__strlen_sse2+49>: pcmpeqb (%rax),%xmm0 0x7fbf5b86cc45 <__strlen_sse2+53>: mov $0xffffffff,%esi 0x7fbf5b86cc4a <__strlen_sse2+58>: sub %rax,%rcx 0x7fbf5b86cc4d <__strlen_sse2+61>: shl %cl,%esi
The message written to the Linux kernel ring buffer is the following. [19830.065929] evolution[7909]: segfault at fffffffffffffff0 ip 00007fbf5b86cc41 sp 00007fff4585ed48 error 4 in libc-2.13.so[7fbf5b7ed000+17d000]
Looking for the offending addressbook entry, I found a log file which apparently was created when the crash happened. $ ls -lh ~/.local/share/evolution/addressbook/system/log.0000000001 -rw-r----- 1 joeyh joeyh 10M Sep 16 17:23 evolution/addressbook/system/log.0000000001 `less` says it is a binary file, but I can see several VCARD entries in there.
Created attachment 224720 [details] [review] [PATCH] Bug #684175 – Check for corrupt addressbook For whatever reasons, addressbooks can be corrupt. Clicking on such an entry in the Contacts overview crashes Evolution with a segmentation fault. In the overview the first line of the excerpt is empty. Therefore the return values have to be checked, which is also good programming practice. In case of #684175 [1] in camel_address_unformat (CAMEL_ADDRESS (addr), raw); according to GDB `raw` is `\000` and returns `FALSE`. In `camel/camel-internet-address.c` /** * camel_internet_address_get: * @addr: a #CamelInternetAddress object * @index: address's array index * @namep: holder for the returned name, or %NULL, if not required. * @addressp: holder for the returned address, or %NULL, if not required. * * Get the address at @index. * * Returns: %TRUE if such an address exists, or %FALSE otherwise **/ gboolean camel_internet_address_get (CamelInternetAddress *addr, gint index, const gchar **namep, const gchar **addressp) { struct _address *a; g_assert (CAMEL_IS_INTERNET_ADDRESS (addr)); if (index < 0 || index >= ((CamelAddress *) addr)->addresses->len) return FALSE; […] the check fails here, because `index = 0` and also `((CamelAddress *) addr)->addresses->len = 0` that is why `FALSE` is returned. Therefore check the return address and throw an error instead of crashing with a segmentation fault. [1] https://bugzilla.gnome.org/show_bug.cgi?id=684175
This is the value of `raw` GDB shows. $19 = 0 '\000'
On the list, Dan Vrátil reviewed the patch [1] and suggested some long term solution [4]. [1] https://mail.gnome.org/archives/evolution-hackers/2012-September/msg00015.html [2] https://mail.gnome.org/archives/evolution-hackers/2012-September/msg00019.html
Created attachment 225002 [details] [review] Updated patch just using `g_return_if_fail()` (In reply to comment #6) > On the list, Dan Vrátil reviewed the patch [1] and suggested some long term > solution [2]. > [1] https://mail.gnome.org/archives/evolution-hackers/2012-September/msg00015.html > [2] https://mail.gnome.org/archives/evolution-hackers/2012-September/msg00019.html
*** Bug 672726 has been marked as a duplicate of this bug. ***
Downstream bug report about the same from 3.4.4: https://bugzilla.redhat.com/show_bug.cgi?id=862234
Thanks for a bug report and patch. I do not think it is the best option to use g_return_if_fail, because you are working with a user input, which can be basically of any form, and throwing an error on the console is not needed here, because these critical warnings are for programming errors, not for user input errors. I would go the way: if (camel_address_unformat (...) > 0 && camel_internet_address_get (...)) { e_destination_set_name (...); ... } which will gracefully cope with user input errors.
(also the commit message doesn't need to be that long, just for a few liner patch)
(In reply to comment #10) > Thanks for a bug report and patch. I do not think it is the best option to use > g_return_if_fail, because you are working with a user input, which can be > basically of any form, and throwing an error on the console is not needed here, > because these critical warnings are for programming errors, not for user input > errors. I do not see, how this is only a user input error. As described in the commit message the addressbook is corrupt (for whatever reason). > I would go the way: > if (camel_address_unformat (...) > 0 && > camel_internet_address_get (...)) { > e_destination_set_name (...); > ... > } > > which will gracefully cope with user input errors. I have to look into this again.
(In reply to comment #11) > (also the commit message doesn't need to be that long, just for a few liner > patch) I guess we can agree that we disagree on that issue. ;-)
Created attachment 226811 [details] vcard.vcf I can reproduce this when I import this vCard into my local addressbook and add it into a contact list, which I'm creating. I guess this can happen when importing data, that the EMAIL will be left empty.
Created attachment 229902 [details] [review] [PATCH] Bug #684175 – Check for corrupt address book I updated the patch according to Milan’s suggestion. 1. There are no unit tests for Evolution, right? Otherwise a test case should be written for this issue too. 2. As noted in the TODO in the commit message, in my opinion the user should be notified in some way. Dan’s proposal using GError all the way(?) would probably solve that.
Thanks for the update. Patch looks good, I'm committing it to sources. About the test case, no evolution doesn't have test suits, but evolution-data-server does. The changes are in eds, thus it makes sense to have test rather there. Created commit 2df3cc4 in eds master (3.7.3+) Created commit ac7de1e in eds gnome-3-6 (3.6.3+)
*** Bug 691657 has been marked as a duplicate of this bug. ***