Commit f48b83b4 authored by Emilia Kasper's avatar Emilia Kasper Committed by Matt Caswell
Browse files

Fix length checks in X509_cmp_time to avoid out-of-bounds reads. 



Also tighten X509_cmp_time to reject more than three fractional
seconds in the time; and to reject trailing garbage after the offset.

CVE-2015-1789

Reviewed-by: default avatarViktor Dukhovni <viktor@openssl.org>
Reviewed-by: default avatarRichard Levitte <levitte@openssl.org>
parent 708cf593

crypto/x509/x509_vfy.c

+47 −10
Original line number Diff line number Diff line
@@ -1808,47 +1808,84 @@ int X509_cmp_time(const ASN1_TIME *ctm, time_t *cmp_time) 
    ASN1_TIME atm;

    long offset;

    char buff1[24], buff2[24], *p;

    int i, j;

    int i, j, remaining;



    p = buff1;

    i = ctm->length;

    remaining = ctm->length;

    str = (char *)ctm->data;

    /*

     * Note that the following (historical) code allows much more slack in the

     * time format than RFC5280. In RFC5280, the representation is fixed:

     * UTCTime: YYMMDDHHMMSSZ

     * GeneralizedTime: YYYYMMDDHHMMSSZ

     */

    if (ctm->type == V_ASN1_UTCTIME) {

        if ((i < 11) || (i > 17))

        /* YYMMDDHHMM[SS]Z or YYMMDDHHMM[SS](+-)hhmm */

        int min_length = sizeof("YYMMDDHHMMZ") - 1;

        int max_length = sizeof("YYMMDDHHMMSS+hhmm") - 1;

        if (remaining < min_length || remaining > max_length)

            return 0;

        memcpy(p, str, 10);

        p += 10;

        str += 10;

        remaining -= 10;

    } else {

        if (i < 13)

        /* YYYYMMDDHHMM[SS[.fff]]Z or YYYYMMDDHHMM[SS[.f[f[f]]]](+-)hhmm */

        int min_length = sizeof("YYYYMMDDHHMMZ") - 1;

        int max_length = sizeof("YYYYMMDDHHMMSS.fff+hhmm") - 1;

        if (remaining < min_length || remaining > max_length)

            return 0;

        memcpy(p, str, 12);

        p += 12;

        str += 12;

        remaining -= 12;

    }



    if ((*str == 'Z') || (*str == '-') || (*str == '+')) {

        *(p++) = '0';

        *(p++) = '0';

    } else {

        /* SS (seconds) */

        if (remaining < 2)

            return 0;

        *(p++) = *(str++);

        *(p++) = *(str++);

        /* Skip any fractional seconds... */

        if (*str == '.') {

            str++;

            while ((*str >= '0') && (*str <= '9'))

        remaining -= 2;

        /*

         * Skip any (up to three) fractional seconds...

         * TODO(emilia): in RFC5280, fractional seconds are forbidden.

         * Can we just kill them altogether?

         */

        if (remaining && *str == '.') {

            str++;

            remaining--;

            for (i = 0; i < 3 && remaining; i++, str++, remaining--) {

                if (*str < '0' || *str > '9')

                    break;

            }

        }



    }

    *(p++) = 'Z';

    *(p++) = '\0';



    if (*str == 'Z')

    /* We now need either a terminating 'Z' or an offset. */

    if (!remaining)

        return 0;

    if (*str == 'Z') {

        if (remaining != 1)

            return 0;

        offset = 0;

    else {

    } else {

        /* (+-)HHMM */

        if ((*str != '+') && (*str != '-'))

            return 0;

        /* Historical behaviour: the (+-)hhmm offset is forbidden in RFC5280. */

        if (remaining != 5)

            return 0;

        if (str[1] < '0' || str[1] > '9' || str[2] < '0' || str[2] > '9' ||

            str[3] < '0' || str[3] > '9' || str[4] < '0' || str[4] > '9')

            return 0;

        offset = ((str[1] - '0') * 10 + (str[2] - '0')) * 60;

        offset += (str[3] - '0') * 10 + (str[4] - '0');

        if (*str == '-')