After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 726788 - Crash on malformed vCard
Crash on malformed vCard
Status: RESOLVED FIXED
Product: evolution-data-server
Classification: Platform
Component: Contacts
unspecified
Other Linux
: Normal normal
: ---
Assigned To: evolution-addressbook-maintainers
Evolution QA team
Depends on:
Blocks:
 
 
Reported: 2014-03-20 18:49 UTC by Xavier Claessens
Modified: 2014-03-24 14:53 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
Test case (266 bytes, text/x-csrc)
2014-03-20 18:49 UTC, Xavier Claessens 		  Details
EVCard: Fix possible crash when parsing invalid vcard (1.10 KB, patch)
2014-03-20 18:59 UTC, Xavier Claessens 		accepted-commit_after_freeze Details | Review
EVCard: Fix possible crash when parsing invalid vcard (1.11 KB, patch)
2014-03-20 20:30 UTC, Xavier Claessens 		committed Details | Review

Description Xavier Claessens 2014-03-20 18:49:58 UTC 
Created attachment 272512 [details]
Test case

Here is a really simple test case that lead to a crash. I know that's not a valid vcard, but that's not a reason to crash.
Comment 1 Xavier Claessens 2014-03-20 18:52:18 UTC 
Note that you need G_SLICE=always-malloc to make it crash. Running it with valgrind shows that it's reading freed memory.
Comment 2 Xavier Claessens 2014-03-20 18:59:51 UTC 
Created attachment 272514 [details] [review]
EVCard: Fix possible crash when parsing invalid vcard

If the vcard is "END:VCARD" then the attribute is added to
the list, then freed. So next time we iterate on attributes it
will read freed memory and crash.
Comment 3 Matthew Barnes 2014-03-20 20:14:17 UTC 
I'd prefer it if strcmp()-style calls explicitly check the return value for equality or inequality with 0.  It makes the code more readable.  Otherwise the patch looks okay to me.  Can commit after the code freeze ends next week.
Comment 4 Xavier Claessens 2014-03-20 20:30:01 UTC 
Right, I hate if(strcmp()) as well. Was unsure because that file often doesn't comparent to 0.
Comment 5 Xavier Claessens 2014-03-20 20:30:24 UTC 
Created attachment 272518 [details] [review]
EVCard: Fix possible crash when parsing invalid vcard

If the vcard is "END:VCARD" then the attribute is added to
the list, then freed. So next time we iterate on attributes it
will read freed memory and crash.
Comment 6 Milan Crha 2014-03-24 14:53:39 UTC 
Created commit 7d6f05a in eds master (3.13.1+) [1]
Created commit c79455a in eds evolution-data-server-3-12 (3.12.1+)

[1] https://git.gnome.org/browse/evolution-data-server/commit/?id=c79455a