Commit fa57f74a authored by Emilia Kasper's avatar Emilia Kasper Committed by Dr. Stephen Henson
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 92f9a8bf
Loading
Loading
Loading
Loading
+47 −10
Original line number Diff line number Diff line
@@ -1007,47 +1007,84 @@ int X509_cmp_time(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 == '-')