Closed dilyanpalauzov closed 6 years ago
@ksmurchison, looks like this crash is occurring in here:
Note that icalcomponent_get_uid()
can return NULL if it didn't find such a property, so the crash occurs in the snprintf when we try to strhash(uid)
.
This function is quite different on master, but looks like it might be susceptible to a similar crash:
Note that hash_lookup(uid, ...)
will also crash out (hilariously, yet unrelatedly, also in strhash(uid)
)
What should caldav_import()
be doing when no UID is found? Throw it out? Generate one?
It looks like both versions of caldav_import
rely entirely on icalrestriction_check()
for validation. Is the failure to detect the missing UID a bug in that? (Though, even if it is, it'd be nice for Cyrus to throw back a good error instead of crashing out.)
Looks like 2.5 is not affected by this: it threw a HTTP_BAD_REQUEST error when no uid was found:
I'm looking at the current libical source on our fork, and if I'm reading it correctly, it looks like it should detect a missing uid as invalid.
@dilyanpalauzov, which version of libical do you have there?
Providing that the POST is supposed to create an URL for the object and it is common that the UID is part of the address of the object, I was expecting that the server would generate the UID.
I use the latest libical v3.0.3-52-ga579ed50.
In imap/http_caldav.c:caldav_import()
get_icalcomponent_errstr(ical)
returns NULL both before and after calling icalrestrction_check(ical)
. The latter returns 1.
In libical/src/libical/icalrestrction.c:icalrestrction_check()
method_prop is 0, so method = ICAL_METHOD_NONE
.
static const icalrestriction_property_record icalrestriction_property_record contains for ICAL_METHOD_NONE and ICAL_UID_PROPERTY:
{ICAL_METHOD_NONE, ICAL_VCALENDAR_COMPONENT, ICAL_UID_PROPERTY, ICAL_RESTRICTION_ZEROORONE, NULL},
{ICAL_METHOD_NONE, ICAL_VEVENT_COMPONENT, ICAL_UID_PROPERTY, ICAL_RESTRICTION_ZEROORONE, NULL},
{ICAL_METHOD_NONE, ICAL_VTODO_COMPONENT, ICAL_UID_PROPERTY, ICAL_RESTRICTION_ZEROORONE, NULL},
{ICAL_METHOD_NONE, ICAL_VJOURNAL_COMPONENT, ICAL_UID_PROPERTY, ICAL_RESTRICTION_ZEROORONE, NULL},
{ICAL_METHOD_NONE, ICAL_VFREEBUSY_COMPONENT, ICAL_UID_PROPERTY, ICAL_RESTRICTION_ONE, NULL},
{ICAL_METHOD_NONE, ICAL_VTIMEZONE_COMPONENT, ICAL_UID_PROPERTY, ICAL_RESTRICTION_ZERO, NULL},
{ICAL_METHOD_NONE, ICAL_XSTANDARD_COMPONENT, ICAL_UID_PROPERTY, ICAL_RESTRICTION_ZERO, NULL},
{ICAL_METHOD_NONE, ICAL_XDAYLIGHT_COMPONENT, ICAL_UID_PROPERTY, ICAL_RESTRICTION_ZERO, NULL},
{ICAL_METHOD_NONE, ICAL_XAUDIOALARM_COMPONENT, ICAL_UID_PROPERTY, ICAL_RESTRICTION_ZERO, NULL},
{ICAL_METHOD_NONE, ICAL_XDISPLAYALARM_COMPONENT, ICAL_UID_PROPERTY, ICAL_RESTRICTION_ZERO, NULL},
{ICAL_METHOD_NONE, ICAL_XEMAILALARM_COMPONENT, ICAL_UID_PROPERTY, ICAL_RESTRICTION_ZERO, NULL},
{ICAL_METHOD_NONE, ICAL_XPROCEDUREALARM_COMPONENT, ICAL_UID_PROPERTY, ICAL_RESTRICTION_ZERO, NULL},
{ICAL_METHOD_NONE, ICAL_VAVAILABILITY_COMPONENT, ICAL_UID_PROPERTY, ICAL_RESTRICTION_ONE, NULL},
{ICAL_METHOD_NONE, ICAL_XAVAILABLE_COMPONENT, ICAL_UID_PROPERTY, ICAL_RESTRICTION_ONE, NULL},
{ICAL_METHOD_NONE, ICAL_VPOLL_COMPONENT, ICAL_UID_PROPERTY, ICAL_RESTRICTION_ONE, NULL},
{ICAL_METHOD_NONE, ICAL_VPATCH_COMPONENT, ICAL_UID_PROPERTY, ICAL_RESTRICTION_ONE, NULL},
in particular ICAL_VCALENDAR_COMPONENT + ICAL_UID_PROPERTY => ICAL_RESTRICTION_ZEROORONE.
UID is s required property for a VEVENT per both RFC 5545 and 5546. The fact that icalrestriction_check() and Cyrus fail to detect that is a bug in both. Such a VEVENT should be rejected. In the case of bulk import this would result in a
Sending
causes httpd-3.0 to crash with this backtrace: