Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

CalDav POST collection-url -> http-3.0 crash #2346

Closed
dilyanpalauzov opened this issue May 7, 2018 · 6 comments
Closed

CalDav POST collection-url -> http-3.0 crash #2346

dilyanpalauzov opened this issue May 7, 2018 · 6 comments
Assignees
@ksmurchison @elliefm
Labels
3.0 affects 3.0 3.1 affects 3.1 dev releases

Comments

@dilyanpalauzov
Copy link
Contributor

dilyanpalauzov commented May 7, 2018 •
edited
Loading

Sending

curl -u X:Y -XPOST -H"Content-Type: text/calendar" --data "
BEGIN:VCALENDAR
CALSCALE:GREGORIAN
PRODID:-//Ximian//NONSGML Test Call//EN
VERSION:2.0
BEGIN:VEVENT
DTSTAMP:20170528T094321Z
DTSTART;VALUE=DATE:20180528
SUMMARY:A BC
STATUS:CONFIRMED
TRANSP:TRANSPARENT
RECURRENCE-ID;VALUE=DATE:20180528
DTEND;VALUE=DATE:20180529
RRULE:FREQ=YEARLY
CLASS:PUBLIC
SEQUENCE:1
CREATED:20180506T183417Z
LAST-MODIFIED:20180506T183417Z
END:VEVENT
END:VCALENDAR" http://server/dav/calendars/user/X/Default/

causes httpd-3.0 to crash with this backtrace:

#0  strhash (string=string@entry=0x0) at lib/strhash.c:50
        ret_val = 0
        i = <optimized out>

#1  0x0000000000426685 in caldav_import (txn=txn@entry=0x7fffab891630, obj=obj@entry=0x1f3bc70, mailbox=0x1f3c688, 
    destdb=destdb@entry=0x1f3bd30, root=root@entry=0x1f3b650, ns=ns@entry=0x7fffab891570, flags=0)
    at imap/http_caldav.c:2923
        newical = 0x1f373e0
        tzrock = {
          old = 0x1f3bc70, 
          new = 0x1f373e0
        }
        ret = <optimized out>
        ical = 0x1f3bc70
        comp = <optimized out>
        next = 0x0
        kind = <optimized out>
        prodid = 0x1f3aa50
        version = 0x1f3a9c0
        calscale = 0x1f3ad60
        uid = 0x0
        caldavdb = <optimized out>
        resp = <optimized out>
        propstat = <optimized out>
        prop = <optimized out>
        error = <optimized out>
        post_count = 1
        len = <optimized out>

#2  0x00000000004454a7 in dav_post_import (pparams=0x48b2a0 <caldav_params>, txn=0x7fffab891630)
    at imap/http_dav.c:6494
        rights = <optimized out>
        mime = <optimized out>
        davdb = 0x1f3bd30
        root = 0x1f3b650
        ns = {0x1f379b0, 0x1f3a970, 0x0, 0x0, 0x1f3adf0, 0x0, 0x0, 0x0}
        ret = <optimized out>
        r = <optimized out>
        precond = <optimized out>
        hdr = <optimized out>
        obj = 0x1f3bc70
        outdoc = 0x1f3bbb0
        mailbox = 0x1f3c688
        qdiffs = {413, 0, 0, 0}
        ret = <optimized out>
        r = <optimized out>
        precond = <optimized out>
        rights = <optimized out>
        hdr = <optimized out>
        mime = <optimized out>
        mailbox = <optimized out>
        qdiffs = <optimized out>
        davdb = <optimized out>
        obj = <optimized out>
        outdoc = <optimized out>
        root = <optimized out>
        ns = <optimized out>
        be = <optimized out>

#3  meth_post (txn=0x7fffab891630, params=0x48b2a0 <caldav_params>) at imap/http_dav.c:6563
        hdr = <optimized out>
        action = <optimized out>
        ret = <optimized out>
        len = <optimized out>
        pparams = 0x48b2a0 <caldav_params>
        r = <optimized out>
        params = 0x48b2a0 <caldav_params>
        post_count = 0
        __s2_len = <optimized out>
        __s2 = <optimized out>
        txn = 0x7fffab891630
        r = <optimized out>
        ret = <optimized out>
        hdr = <optimized out>
        pparams = <optimized out>
        action = <optimized out>
        len = <optimized out>
        __s1_len = <optimized out>
        __result = <optimized out>
        pparams = 0x48b2a0 <caldav_params>
        action = <optimized out>
        r = <optimized out>
        ret = <optimized out>
        len = <optimized out>
        post_count = 0
        hdr = <optimized out>
        __s1_len = <optimized out>
        __s2_len = <optimized out>
        __s2 = <optimized out>
        __result = <optimized out>

#4  0x000000000045ce9d in cmdloop (conn=conn@entry=0x7fffab8957b0) at imap/httpd.c:2106
        meth_t = <optimized out>
        ret = <optimized out>
        __s1_len = <optimized out>
        req_line = 0x7fffab8925f8
        __s2_len = <optimized out>
        __s2 = <optimized out>
        __result = <optimized out>
        empty = 0
        txn = {
          conn = 0x7fffab8957b0, 
          http2 = {
            stream_id = 0, 
            num_resp_hdrs = 0, 
            resp_hdrs = {{
                name = 0x0, 
                value = 0x0, 
                namelen = 0, 
                valuelen = 0, 
                flags = 0 '\000'
              } <repeats 100 times>}
          }, 
          meth = 12, 
          flags = {
            ver = 1, 
            conn = 2, 
            upgrade = 2, 
            override = 0, 
            cors = 0, 
            mime = 0, 
            te = 0, 
            cc = 2, 
            ranges = 0, 
            vary = 10, 
            trailer = 0
          }, 
          req_line = {
            buf = "POST\000/dav/calendars/user/X/Default/\000HTTP/1.1", '\000' <repeats 7945 times>, 
            meth = 0x7fffab8925f8 "POST", 
            uri = 0x7fffab8925fd "/dav/calendars/user/X/Default/", 
            ver = 0x7fffab892627 "HTTP/1.1"
          }, 
          req_uri = 0x1f00e40, 
          req_tgt = {
            path = "/dav/calendars/user/X/Default/", '\000' <repeats 4055 times>, 
            tail = 0x7fffab894589 "", 
            namespace = 0x48b100 <namespace_calendar>, 
            userid = 0x1f37460 "X", 
            collection = 0x7fffab894581 "Default/", 
            collen = 7, 
            resource = 0x7fffab894589 "", 
            reslen = 0, 
            flags = 0, 
            allow = 2034455, 
            mbentry = 0x1f376e0, 
            mboxprefix = 0x7ffa9bb1461a "#calendars"
          }, 
          req_qparams = {
            size = 10, 
            table = 0x1f38d00, 
            pool = 0x1f374a0
          }, 
          req_hdrs = 0x1ef7990, 
          req_body = {
            flags = 40 '(', 
            framing = 2 '\002', 
            te = 0 '\000', 
            max = 2147483647, 
            len = 0, 
            payload = {
              s = 0x1f391c0 "\nBEGIN:VCALENDAR\r\nCALSCALE:GREGORIAN\r\nPRODID:-//Ximian//NONSGML Test Call//EN\r\nVERSION:2.0\r\nBEGIN:VEVENT\r\nDTSTAMP:20170528T094321Z\r\nDTSTART;VALUE=DATE:20180528
\r\nSUMMARY:A BC\r\nSTATUS:CONFIRMED\r\nTRANSP:TRANSPARENT\r\nRECURRENCE-ID;VALUE=DATE:20180528\r\nDTEND;VALUE=DATE:20180529\r\nRRULE:FREQ=YEARLY\r\nCLASS:PUBLIC\r\nSEQUENCE:1\r\nCREATED:20180506T183417Z\r\nLAS
T-MODIFIED:20180506T183417Z\r\nEND:VEVENT\r\nEND:VCALENDAR\r", 
              len = 413, 
              alloc = 512, 
              flags = 0
            }
          }, 
          auth_chal = {
            scheme = 0x48d000 <auth_schemes>, 
            param = 0x0
          }, 
          location = 0x0, 
          error = {
            desc = 0x0, 
            precond = 0, 
            node = 0x0, 
            resource = 0x0, 
            rights = 0
          }, 
          resp_body = {
            len = 0, 
            range = 0x0, 
            fname = 0x0, 
            enc = 0 '\000', 
            lang = 0x0, 
            loc = 0x0, 
            md5 = 0x0, 
            type = 0x0, 
            patch = 0x0, 
            prefs = 0, 
            link = 0x0, 
            lock = 0x0, 
            ctag = 0x0, 
            etag = 0x0, 
            lastmod = 0, 
            maxage = 0, 
            stag = 0x0, 
            cmid = 0x0, 
            iserial = 0, 
            payload = {
              s = 0x0, 
              len = 0, 
              alloc = 0, 
              flags = 0
            }
          }, 
          zbuf = {
            s = 0x0, 
            len = 0, 
            alloc = 0, 
            flags = 0
          }, 
          buf = {
            s = 0x1ef8240 "1454443908-506-143", 
            len = 0, 
            alloc = 2048, 
            flags = 0
          }
        }

#5  0x000000000045d470 in service_main (argc=<optimized out>, argv=<optimized out>, envp=envp@entry=0x7fffab897278)
    at imap/httpd.c:1116
        secprops = <optimized out>
        mechlist = 0x1ef7a00 "GSS-SPNEGO"
        mech = <optimized out>
        mechcount = -1
        mechlen = <optimized out>
        scheme = <optimized out>
        http_conn = {
          pin = 0x1ef3470, 
          pout = 0x1ef4590, 
          zstrm = 0x0, 
          brotli = 0x0, 
          http2_session = 0x0, 
          http2_options = 0x0
        }

#6  0x0000000000413c93 in main (argc=<optimized out>, argv=<optimized out>, envp=0x7fffab897278)
    at master/service.c:634
        fdflags = <optimized out>
        fd = <optimized out>
        p = <optimized out>
        service = <optimized out>
        request = <optimized out>
        opt = -1
        alt_config = <optimized out>
        call_debugger = <optimized out>
        debug_stdio = 0
        max_use = 250
        reuse_timeout = 104
        soctype = 1
        typelen = 4
        socname = {
          sa_family = 2, 
          sa_data = "\000P\177\000\000\003\000\000\000\000\000\000\000"
        }
        addrlen = 16
        id = <optimized out>
        path = "/usr/local/libexec/httpd", '\000' <repeats 3104 times>, [MANUALLY TRUNCATED FOR BREVITY]
        sbuf = {
          st_dev = 65025, 
          st_ino = 27264731, 
          st_nlink = 1, 
          st_mode = 33261, 
          st_uid = 0, 
          st_gid = 50, 
          __pad0 = 0, 
          st_rdev = 0, 
          st_size = 1591920, 
          st_blksize = 4096, 
          st_blocks = 3112, 
          st_atim = {
            tv_sec = 1525671372, 
            tv_nsec = 623150987
          }, 
          st_mtim = {
            tv_sec = 1525671253, 
            tv_nsec = 215411211
          }, 
          st_ctim = {
            tv_sec = 1525671253, 
            tv_nsec = 215411211
          }, 
          __glibc_reserved = {0, 0, 0}
        }
        start_ino = 27264731
        start_size = 1591920
        start_mtime = 1525671253
        service_argv = {
          count = 1, 
          alloc = 16, 
          data = 0x1ec9c60
        }
@elliefm
Copy link
Contributor

elliefm commented May 9, 2018

@ksmurchison, looks like this crash is occurring in here:

cyrus-imapd/imap/http_caldav.c

Lines 2920 to 2925 in 7854ec6

/* Append a unique resource name to URL and perform a PUT */
uid = icalcomponent_get_uid(comp);
txn->req_tgt.reslen =
snprintf(txn->req_tgt.resource, MAX_MAILBOX_PATH - len,
"%x-%d-%ld-%u.ics",
strhash(uid), getpid(), time(0), post_count++);

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:

cyrus-imapd/imap/http_caldav.c

Lines 3422 to 3423 in d00d029

uid = icalcomponent_get_uid(comp);
comps = hash_lookup(uid, &comp_table);

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.)

@elliefm elliefm added 3.0 affects 3.0 3.1 affects 3.1 dev releases labels May 9, 2018
@elliefm
Copy link
Contributor

elliefm commented May 9, 2018

Looks like 2.5 is not affected by this: it threw a HTTP_BAD_REQUEST error when no uid was found:

cyrus-imapd/imap/http_caldav.c

Lines 2518 to 2531 in c805d3e

meth = icalcomponent_get_method(ical);
comp = icalcomponent_get_first_real_component(ical);
if (comp) {
uid = icalcomponent_get_uid(comp);
kind = icalcomponent_isa(comp);
prop = icalcomponent_get_first_property(comp, ICAL_ORGANIZER_PROPERTY);
}
/* Check method preconditions */
if (!meth || !uid || !prop) {
txn->error.precond = CALDAV_VALID_SCHED;
ret = HTTP_BAD_REQUEST;
goto done;
}

@elliefm
Copy link
Contributor

elliefm commented May 9, 2018

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?

@dilyanpalauzov
Copy link
Contributor Author

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.

@dilyanpalauzov
Copy link
Contributor Author

dilyanpalauzov commented May 9, 2018 •
edited
Loading

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.

@ksmurchison
Copy link
Contributor

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 element containing an error.

ksmurchison added a commit that referenced this issue May 9, 2018
@ksmurchison
http_dav.c, http_caldav.c: when doing bulk import, make sure each "re…
6a36907 
…al" component has a UID (resolves issue #2346)
ksmurchison added a commit that referenced this issue May 9, 2018
@ksmurchison
http_dav.c, http_caldav.c: when doing bulk import, make sure each "re…
7c44fd5 
…al" component has a UID (resolves issue #2346)
@dilyanpalauzov dilyanpalauzov mentioned this issue May 9, 2018
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ksmurchison ksmurchison

@elliefm elliefm

Labels
3.0 affects 3.0 3.1 affects 3.1 dev releases
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@dilyanpalauzov @ksmurchison @elliefm