[Evolution-hackers] Memory corruption bug in timezone handling

[Robie Basak <robie basak canonical com>]

Hi,

I've been investigating a memory corruption issue in evolution which
causes a crash on my system. I think the problem crosses an API boundary
and resolving it is non-trivial, so I'd like to better understand what
is supposed to happen. Any insight into this would be appreciated.

The problem seems to be that
icaltimezone.c:icaltimezone_get_builtin_timezone calls icalarray_append,
which moves the entire array to grow it. But an ECalShellView is
maintaining a pointer inside that array (via a very long chain of
indirection) which becomes invalid as the array is moved. This causes
later corruption, invalid reads from freed memory, and eventually
segfaults from both the corruption (which appear quite random).

For example: after the corruption, I see:

#5 0x00007f2f479925a6 in e_calendar_item_draw_day_numbers (cells_y=45,
    cells_x=7, start_weekday=3, month=2, year=2012, col=0, row=0,
    cr=0x7f2f456ec9e0, calitem=0x7f2f4b154cd0, width=<optimized out>,
    height=<optimized out>) at e-calendar-item.c:1485
1485 today_tm = (*calitem->time_callback) (calitem, calitem->time_callback_data);
(gdb) p ((GnomeCalendar *)(((ECalShellView *)calitem->time_callback_data)->priv->cal_shell_content->priv->calendar))->priv->model->priv->zone->builtin_timezone
$49 = (icaltimezone *) 0x2000000020

This causes a segfault.

This appears to happen because builtin_timezone is pointing to an area
that has been freed and malloc has reallocated that memory to something
else.

So my questions are:

1) Are builtin_timezone pointers provided by libical expected to be
stable? Should libical be adding to its internal array and thus be
moving this objects and invalidating old pointers?

2) If libical can do this, how should evolution be keeping references to
these objects in a way that they will not be invalidated?

valgrind log, which I think captures the first invalid pointer read and
also shows when the pointer became invalid:

==4519== Invalid read of size 8
==4519==    at 0xE6FEB46: icaltimezone_get_utc_offset_of_utc_time (icaltimezone.c:981)
==4519==    by 0xE6FE652: icaltimezone_convert_time (icaltimezone.c:794)
==4519==    by 0xE6F9EE0: icaltime_from_timet_with_zone (icaltime.c:224)
==4519==    by 0x18810169: tag_calendar_cb (tag-calendar.c:120)
==4519==    by 0x932B1E7: process_instances (e-cal-client.c:1961)
==4519==    by 0x932B314: generate_instances_for_object_got_objects_cb (e-cal-client.c:1992)
==4519==    by 0x932A799: got_objects_for_uid_cb (e-cal-client.c:1711)
==4519==    by 0x626CC16: g_simple_async_result_complete (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.3000.0)
==4519==    by 0x5536C5B: finish_async_op (e-client.c:2281)
==4519==    by 0x5536F55: async_result_ready_cb (e-client.c:2318)
==4519==    by 0x626CC16: g_simple_async_result_complete (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.3000.0)
==4519==    by 0x626CD28: ??? (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.3000.0)
==4519==  Address 0x1c11c8d8 is 29,928 bytes inside a block of size 29,952 free'd
==4519==    at 0x4C282E0: free (vg_replace_malloc.c:366)
==4519==    by 0xE6E8E5E: icalarray_expand (icalarray.c:159)
==4519==    by 0xE6E8BE8: icalarray_append (icalarray.c:89)
==4519==    by 0xE6FF54A: icaltimezone_get_builtin_timezone (icaltimezone.c:1414)
==4519==    by 0xE6FF8A6: icaltimezone_get_builtin_timezone_from_tzid (icaltimezone.c:1525)
==4519==    by 0xE6EC18F: icalcomponent_get_datetime (icalcomponent.c:1566)
==4519==    by 0xE6EC28A: icalcomponent_get_dtstart (icalcomponent.c:1594)
==4519==    by 0x187FB7EA: ensure_dates_are_in_default_zone (gnome-cal.c:744)
==4519==    by 0x187FBA21: dn_client_view_objects_added_cb (gnome-cal.c:773)
==4519==    by 0x65560A3: g_closure_invoke (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.3000.0)
==4519==    by 0x6568029: ??? (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.3000.0)
==4519==    by 0x65716B0: g_signal_emit_valist (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.3000.0)

I think this is difficult to reproduce since it depends on the size of
the array at the time it needs to be appended to, and I'm not sure
exactly what triggers this condition. I am currently in a situation
where the bug triggers a crash every time, but I understand that this is
probably due to a coincidence of a number of different factors related
to the exact data I have in my calendar. I am using Ubuntu's evolution
3.2.2-0ubuntu0.1 and libical 0.44-3, but I've looked at your current git
head and I think the same problem exists.

The bug I've filed in Ubuntu is here:
	https://bugs.launchpad.net/ubuntu/+source/evolution/+bug/956843

If someone could please confirm my findings, I would be happy to file a
GNOME Bugzilla bug as well. I also think this may be the same issue as
Bugzilla bug 672518.

Thanks,

Robie