Fix buffer overread in mbedtls_x509_get_time()

A heap overread might happen when parsing malformed certificates.
Reported by Peng Li and Yueh-Hsun Lin.

Refactoring the parsing fixes the problem. This commit applies the
relevant part of the OpenVPN contribution applied to mbed TLS 1.3
in commit 17da9dd829.

ChangeLog

@@ -40,6 +40,8 @@ Bugfix
      cause buffer bound checks to be bypassed. Found by Eyal Itkin.
    * Fixed potential arithmetic overflow in mbedtls_base64_decode() that could
      cause buffer bound checks to be bypassed. Found by Eyal Itkin.
+   * Fixed heap overreads in mbedtls_x509_get_time(). Found by Peng
+     Li/Yueh-Hsun Lin, KNOX Security, Samsung Research America.
 
 = mbed TLS 2.4.1 branch released 2016-12-13
 

library/x509.c

@@ -480,14 +480,20 @@ int mbedtls_x509_get_name( unsigned char **p, const unsigned char *end,
     }
 }
 
-static int x509_parse_int(unsigned char **p, unsigned n, int *res){
+static int x509_parse_int( unsigned char **p, size_t n, int *res )
+{
     *res = 0;
-    for( ; n > 0; --n ){
+
-        if( ( **p < '0') || ( **p > '9' ) ) return MBEDTLS_ERR_X509_INVALID_DATE;
+    for( ; n > 0; --n )
+    {
+        if( ( **p < '0') || ( **p > '9' ) )
+            return ( MBEDTLS_ERR_X509_INVALID_DATE );
+
         *res *= 10;
         *res += ( *(*p)++ - '0' );
     }
-    return 0;
+
+    return( 0 );
 }
 
 static int x509_date_is_valid(const mbedtls_x509_time *time)
@@ -517,6 +523,70 @@ static int x509_date_is_valid(const mbedtls_x509_time *time)
     return( 0 );
 }
 
+/*
+ * Parse an ASN1_UTC_TIME (yearlen=2) or ASN1_GENERALIZED_TIME (yearlen=4)
+ * field.
+ */
+static int x509_parse_time( unsigned char **p, size_t len, size_t yearlen,
+        mbedtls_x509_time *time )
+{
+    int ret;
+
+    /*
+     * Minimum length is 10 or 12 depending on yearlen
+     */
+    if ( len < yearlen + 8 )
+        return ( MBEDTLS_ERR_X509_INVALID_DATE );
+    len -= yearlen + 8;
+
+    /*
+     * Parse year, month, day, hour, minute
+     */
+    CHECK( x509_parse_int( p, yearlen, &time->year ) );
+    if ( 2 == yearlen )
+    {
+        if ( time->year < 50 )
+            time->year += 100;
+
+        time->year += 1900;
+    }
+
+    CHECK( x509_parse_int( p, 2, &time->mon ) );
+    CHECK( x509_parse_int( p, 2, &time->day ) );
+    CHECK( x509_parse_int( p, 2, &time->hour ) );
+    CHECK( x509_parse_int( p, 2, &time->min ) );
+
+    /*
+     * Parse seconds if present
+     */
+    if ( len >= 2 )
+    {
+        CHECK( x509_parse_int( p, 2, &time->sec ) );
+        len -= 2;
+    }
+    else
+        return ( MBEDTLS_ERR_X509_INVALID_DATE );
+
+    /*
+     * Parse trailing 'Z' if present
+     */
+    if ( 1 == len && 'Z' == **p )
+    {
+        (*p)++;
+        len--;
+    }
+
+    /*
+     * We should have parsed all characters at this point
+     */
+    if ( 0 != len )
+        return ( MBEDTLS_ERR_X509_INVALID_DATE );
+
+    CHECK( x509_date_is_valid( time ) );
+
+    return ( 0 );
+}
+
 /*
  *  Time ::= CHOICE {
  *       utcTime        UTCTime,
@@ -526,7 +596,7 @@ int mbedtls_x509_get_time( unsigned char **p, const unsigned char *end,
                    mbedtls_x509_time *time )
 {
     int ret;
-    size_t len;
+    size_t len, year_len;
     unsigned char tag;
 
     if( ( end - *p ) < 1 )
@@ -536,55 +606,20 @@ int mbedtls_x509_get_time( unsigned char **p, const unsigned char *end,
     tag = **p;
 
     if( tag == MBEDTLS_ASN1_UTC_TIME )
-    {
+        year_len = 2;
-        (*p)++;
-        ret = mbedtls_asn1_get_len( p, end, &len );
-
-        if( ret != 0 )
-            return( MBEDTLS_ERR_X509_INVALID_DATE + ret );
-
-        CHECK( x509_parse_int( p, 2, &time->year ) );
-        CHECK( x509_parse_int( p, 2, &time->mon ) );
-        CHECK( x509_parse_int( p, 2, &time->day ) );
-        CHECK( x509_parse_int( p, 2, &time->hour ) );
-        CHECK( x509_parse_int( p, 2, &time->min ) );
-        if( len > 10 )
-            CHECK( x509_parse_int( p, 2, &time->sec ) );
-        if( len > 12 && *(*p)++ != 'Z' )
-            return( MBEDTLS_ERR_X509_INVALID_DATE );
-
-        time->year +=  100 * ( time->year < 50 );
-        time->year += 1900;
-
-        CHECK( x509_date_is_valid( time ) );
-
-        return( 0 );
-    }
     else if( tag == MBEDTLS_ASN1_GENERALIZED_TIME )
-    {
+        year_len = 4;
-        (*p)++;
-        ret = mbedtls_asn1_get_len( p, end, &len );
-
-        if( ret != 0 )
-            return( MBEDTLS_ERR_X509_INVALID_DATE + ret );
-
-        CHECK( x509_parse_int( p, 4, &time->year ) );
-        CHECK( x509_parse_int( p, 2, &time->mon ) );
-        CHECK( x509_parse_int( p, 2, &time->day ) );
-        CHECK( x509_parse_int( p, 2, &time->hour ) );
-        CHECK( x509_parse_int( p, 2, &time->min ) );
-        if( len > 12 )
-            CHECK( x509_parse_int( p, 2, &time->sec ) );
-        if( len > 14 && *(*p)++ != 'Z' )
-            return( MBEDTLS_ERR_X509_INVALID_DATE );
-
-        CHECK( x509_date_is_valid( time ) );
-
-        return( 0 );
-    }
     else
         return( MBEDTLS_ERR_X509_INVALID_DATE +
                 MBEDTLS_ERR_ASN1_UNEXPECTED_TAG );
+
+    (*p)++;
+    ret = mbedtls_asn1_get_len( p, end, &len );
+
+    if( ret != 0 )
+        return( MBEDTLS_ERR_X509_INVALID_DATE + ret );
+
+    return x509_parse_time( p, len, year_len, time );
 }
 
 int mbedtls_x509_get_sig( unsigned char **p, const unsigned char *end, mbedtls_x509_buf *sig )