From 98856d4b9dc1a59a576816dbb097aa9d9e6de47a Mon Sep 17 00:00:00 2001
From: Adam Langley <agl@google.com>
Date: Tue, 23 Jun 2015 16:23:41 -0700
Subject: Fix for CVE-2015-1789.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

X509_cmp_time does not properly check the length of the ASN1_TIME string
and can read a few bytes out of bounds. In addition, X509_cmp_time
accepts an arbitrary number of fractional seconds in the time string.

An attacker can use this to craft malformed certificates and CRLs of
various sizes and potentially cause a segmentation fault, resulting in a
DoS on applications that verify certificates or CRLs. TLS clients that
verify CRLs are affected. TLS clients and servers with client
authentication enabled may be affected if they use custom verification
callbacks.

This change cherry-picks the following changes from BoringSSL:

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

Change-Id: Ia7d0c5d889f61a3c4be6ea79a5ab41f67bc3c65c
---
 src/crypto/x509/x509_vfy.c | 54 ++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 47 insertions(+), 7 deletions(-)

diff --git a/src/crypto/x509/x509_vfy.c b/src/crypto/x509/x509_vfy.c
index 2ba9c84..f53f279 100644
--- a/src/crypto/x509/x509_vfy.c
+++ b/src/crypto/x509/x509_vfy.c
@@ -1829,49 +1829,89 @@ 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)) return 0;
+		/* 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) return 0;
+		/* 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 == '.')
+		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++;
-			while ((*str >= '0') && (*str <= '9')) str++;
+			remaining--;
+			for (i = 0; i < 3 && remaining; i++, str++, remaining--)
+				{
+				if (*str < '0' || *str > '9')
+					break;
+				}
 			}
 		
 		}
 	*(p++)='Z';
 	*(p++)='\0';
 
+	/* 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
 		{
+		/* (+-)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 == '-')
-- 
cgit v1.1