wbrawner/openssl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

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

Browse source 
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: Viktor Dukhovni <viktor@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
This commit is contained in:
Emilia Kasper 2015-04-08 16:56:43 +02:00 committed by Matt Caswell
parent dd90a91d87
commit 370ac32030
1 changed files with 47 additions and 10 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

57
crypto/x509/x509_vfy.c
View file

@ -1637,47 +1637,84 @@ int X509_cmp_time(const ASN1_TIME *ctm, time_t *cmp_time)
ASN1_TIME atm; ASN1_TIME atm;
long offset; long offset;
char buff1[24], buff2[24], *p; char buff1[24], buff2[24], *p;
int i, j; int i, j, remaining;
p = buff1; p = buff1;
i = ctm->length; remaining = ctm->length;
str = (char *)ctm->data; 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 (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; return 0;
memcpy(p, str, 10); memcpy(p, str, 10);
p += 10; p += 10;
str += 10; str += 10;
remaining -= 10;
} else { } 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; return 0;
memcpy(p, str, 12); memcpy(p, str, 12);
p += 12; p += 12;
str += 12; str += 12;
remaining -= 12;
} }
if ((*str == 'Z') || (*str == '-') || (*str == '+')) { if ((*str == 'Z') || (*str == '-') || (*str == '+')) {
*(p++) = '0'; *(p++) = '0';
*(p++) = '0'; *(p++) = '0';
} else { } else {
/* SS (seconds) */
if (remaining < 2)
return 0;
*(p++) = *(str++); *(p++) = *(str++);
*(p++) = *(str++); *(p++) = *(str++);
/* Skip any fractional seconds... */ remaining -= 2;
if (*str == '.') { /*
* 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++; str++;
while ((*str >= '0') && (*str <= '9')) remaining--;
str++; for (i = 0; i < 3 && remaining; i++, str++, remaining--) {
if (*str < '0' || *str > '9')
break;
}
} }
} }
*(p++) = 'Z'; *(p++) = 'Z';
*(p++) = '\0'; *(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; offset = 0;
else { } else {
/* (+-)HHMM */
if ((*str != '+') && (*str != '-')) if ((*str != '+') && (*str != '-'))
return 0; 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[1] - '0') * 10 + (str[2] - '0')) * 60;
offset += (str[3] - '0') * 10 + (str[4] - '0'); offset += (str[3] - '0') * 10 + (str[4] - '0');
if (*str == '-') if (*str == '-')