[evolution-data-server/gnome-2-32] ecal file backend: avoid manipulating the UID inside component_add()

[Patrick Ohly <pohly src gnome org>]

commit de9a79048a9d5d448231adb18fc6289b8bdfd151
Author: Patrick Ohly <patrick ohly intel com>
Date:   Thu Aug 4 17:55:07 2011 +0200

    ecal file backend: avoid manipulating the UID inside component_add()
    
    This commit fixes the following memory handling problem:
    ==10069== Invalid read of size 1
    ==10069==    at 0x4C25812: __GI_strlen (mc_replace_strmem.c:284)
    ==10069==    by 0x8EF011E: g_strdup (gstrfuncs.c:99)
    ==10069==    by 0xF4E08B6: e_cal_backend_file_create_object (e-cal-backend-file.c:2363)
    ==10069==    by 0x93E6061: e_cal_backend_sync_create_object (e-cal-backend-sync.c:214)
    ==10069==    by 0x93E86D3: _e_cal_backend_create_object (e-cal-backend-sync.c:630)
    ==10069==    by 0x93DD40B: e_cal_backend_create_object (e-cal-backend.c:1017)
    ==10069==    by 0x93F0C34: impl_Cal_createObject (e-data-cal.c:401)
    ==10069==    by 0x4E75383: _e_gdbus_gdbus_cclosure_marshaller_BOOLEAN__OBJECT_STRING (e-gdbus-marshallers.c:377)
    ==10069==    by 0x820999E: g_closure_invoke (gclosure.c:773)
    ==10069==    by 0x8225972: signal_emit_unlocked_R (gsignal.c:3256)
    ==10069==    by 0x82248D0: g_signal_emit_valist (gsignal.c:2997)
    ==10069==    by 0x8224DBC: g_signal_emit (gsignal.c:3044)
    ==10069==  Address 0x1499c7b0 is 0 bytes inside a block of size 39 free'd
    ==10069==    at 0x4C240FD: free (vg_replace_malloc.c:366)
    ==10069==    by 0x9DE952C: icalvalue_free (in /usr/lib/libical.so.0.44.0)
    ==10069==    by 0x9DDB796: icalproperty_set_value (in /usr/lib/libical.so.0.44.0)
    ==10069==    by 0x4E4FFA2: e_cal_component_set_uid (e-cal-component.c:1479)
    ==10069==    by 0xF4DB8F3: check_dup_uid (e-cal-backend-file.c:498)
    ==10069==    by 0xF4DBD9B: add_component (e-cal-backend-file.c:614)
    ==10069==    by 0xF4E0894: e_cal_backend_file_create_object (e-cal-backend-file.c:2356)
    ==10069==    by 0x93E6061: e_cal_backend_sync_create_object (e-cal-backend-sync.c:214)
    ==10069==    by 0x93E86D3: _e_cal_backend_create_object (e-cal-backend-sync.c:630)
    ==10069==    by 0x93DD40B: e_cal_backend_create_object (e-cal-backend.c:1017)
    ==10069==    by 0x93F0C34: impl_Cal_createObject (e-data-cal.c:401)
    ==10069==    by 0x4E75383: _e_gdbus_gdbus_cclosure_marshaller_BOOLEAN__OBJECT_STRING (e-gdbus-marshallers.c:377)
    
    This occurs when a client (incorrectly) tries to create a VEVENT with
    RECURRENCE-ID for a UID which already exists. The sequence of events is this:
    - e_cal_backend_file_create_object() calls lookup_component(),
      which returns NULL because it only checks for the parent event
      (will be fixed separately).
    - e_cal_backend_file_create_object() keeps a pointer to the UID.
    - check_dup_uid() repeats the UID check, but this time finds that it
      is already taken and replaces the existing UID in the component
      before adding it. The pointer in e_cal_backend_file_create_object()
      points to freed memory.
    
    I've seen cases where the hash ended up using the original UID as key,
    with a component inside that had the new, replaced UID. As a result,
    retrieving the event as reported by e_cal_get_object_list() (= rewritten UID)
    failed in e_cal_get_object() (= original UID).
    
    The UID should not be overwritten. I can't verify it anymore (events where it occured
    have already been deleted), but this rewriting might explain why some of my
    meeting update emails couldn't be applied to previously imported events.
    
    Therefore this patch moves check_dup_uid() out of component_add(). This check
    and rewriting only makes sense when reading the existing calendar file,
    as a safe-guard against on-disk corruption. When adding or modifying events
    via the API, the right reaction is to add a missing UID or or reject the
    operation with an error.
    
    All places where component_add() is used should have the necessary checks
    or are preceeded by a remove_component(), which removes the UID first.
    (cherry picked from commit ae4f4292b0e5ecbbdc74c90b75cc31367d0d270a)

 calendar/backends/file/e-cal-backend-file.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)
---
diff --git a/calendar/backends/file/e-cal-backend-file.c b/calendar/backends/file/e-cal-backend-file.c
index a9facd5..1c2bda6 100644
--- a/calendar/backends/file/e-cal-backend-file.c
+++ b/calendar/backends/file/e-cal-backend-file.c
@@ -569,6 +569,9 @@ remove_component_from_intervaltree (ECalBackendFile *cbfile, ECalComponent *comp
 /* Tries to add an icalcomponent to the file backend.  We only store the objects
  * of the types we support; all others just remain in the toplevel component so
  * that we don't lose them.
+ *
+ * The caller is responsible for ensuring that the component has a UID and that
+ * the UID is not in use already.
  */
 static void
 add_component (ECalBackendFile *cbfile, ECalComponent *comp, gboolean add_to_toplevel)
@@ -608,11 +611,6 @@ add_component (ECalBackendFile *cbfile, ECalComponent *comp, gboolean add_to_top
 		g_hash_table_insert (obj_data->recurrences, rid, comp);
 		obj_data->recurrences_list = g_list_append (obj_data->recurrences_list, comp);
 	} else {
-		/* Ensure that the UID is unique; some broken implementations spit
-		 * components with duplicated UIDs.
-		 */
-		check_dup_uid (cbfile, comp);
-
 		if (obj_data) {
 			if (obj_data->full_object) {
 				g_warning (G_STRLOC ": Tried to add an already existing object");
@@ -742,6 +740,8 @@ scan_vcalendar (ECalBackendFile *cbfile)
 		if (!e_cal_component_set_icalcomponent (comp, icalcomp))
 			continue;
 
+		check_dup_uid (cbfile, comp);
+
 		add_component (cbfile, comp, FALSE);
 	}
 }