From 5d8618539f8e186c1b2c1b5a548d6f85936fe41f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= <mpg@elzevir.fr>
Date: Fri, 17 Oct 2014 12:41:41 +0200
Subject: [PATCH] Fix memory leak while parsing some X.509 certs

---
 ChangeLog                              |  5 +++
 library/x509.c                         | 47 ++++++++++----------------
 tests/suites/test_suite_x509parse.data |  2 +-
 3 files changed, 24 insertions(+), 30 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 8e74c0b6e..84420b9e6 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -4,6 +4,9 @@ PolarSSL ChangeLog (Sorted per branch, date)
 Security
    * Lowest common hash was selected from signature_algorithms extension in
      TLS 1.2 (found by Darren Bane) (introduced in 1.3.8).
+   * Remotely-triggerable memory leak when parsing some X.509 certificates
+     (server is not affected if it doesn't ask for a client certificate).
+     (Found using Codenomicon Defensics.)
 
 Bugfix
    * Support escaping of commas in x509_string_to_names()
@@ -36,6 +39,8 @@ Changes
    * POLARSSL_MPI_MAX_SIZE now defaults to 1024 in order to allow 8192 bits
      RSA keys.
    * Accept spaces at end of line or end of buffer in base64_decode().
+   * X.509 certificates with more than one AttributeTypeAndValue per
+     RelativeDistinguishedName are not accepted any more.
 
 = PolarSSL 1.3.8 released 2014-07-11
 Security
diff --git a/library/x509.c b/library/x509.c
index 49f7672f1..941472c9d 100644
--- a/library/x509.c
+++ b/library/x509.c
@@ -409,58 +409,47 @@ static int x509_get_attr_type_value( unsigned char **p,
  *  AttributeType ::= OBJECT IDENTIFIER
  *
  *  AttributeValue ::= ANY DEFINED BY AttributeType
+ *
+ *  We restrict RelativeDistinguishedName to be a set of 1 element. This is
+ *  the most common case, and our x509_name structure currently can't handle
+ *  more than that.
  */
 int x509_get_name( unsigned char **p, const unsigned char *end,
                    x509_name *cur )
 {
     int ret;
-    size_t len;
-    const unsigned char *end2;
-    x509_name *use;
+    size_t set_len;
+    const unsigned char *end_set;
 
-    if( ( ret = asn1_get_tag( p, end, &len,
+    /*
+     * parse first SET, restricted to 1 element
+     */
+    if( ( ret = asn1_get_tag( p, end, &set_len,
             ASN1_CONSTRUCTED | ASN1_SET ) ) != 0 )
         return( POLARSSL_ERR_X509_INVALID_NAME + ret );
 
-    end2 = end;
-    end  = *p + len;
-    use = cur;
+    end_set  = *p + set_len;
 
-    do
-    {
-        if( ( ret = x509_get_attr_type_value( p, end, use ) ) != 0 )
-            return( ret );
+    if( ( ret = x509_get_attr_type_value( p, end_set, cur ) ) != 0 )
+        return( ret );
 
-        if( *p != end )
-        {
-            use->next = (x509_name *) polarssl_malloc(
-                    sizeof( x509_name ) );
-
-            if( use->next == NULL )
-                return( POLARSSL_ERR_X509_MALLOC_FAILED );
-
-            memset( use->next, 0, sizeof( x509_name ) );
-
-            use = use->next;
-        }
-    }
-    while( *p != end );
+    if( *p != end_set )
+        return( POLARSSL_ERR_X509_FEATURE_UNAVAILABLE );
 
     /*
      * recurse until end of SEQUENCE is reached
      */
-    if( *p == end2 )
+    if( *p == end )
         return( 0 );
 
-    cur->next = (x509_name *) polarssl_malloc(
-         sizeof( x509_name ) );
+    cur->next = (x509_name *) polarssl_malloc( sizeof( x509_name ) );
 
     if( cur->next == NULL )
         return( POLARSSL_ERR_X509_MALLOC_FAILED );
 
     memset( cur->next, 0, sizeof( x509_name ) );
 
-    return( x509_get_name( p, end2, cur->next ) );
+    return( x509_get_name( p, end, cur->next ) );
 }
 
 /*
diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data
index 4cec07051..c5c4af761 100644
--- a/tests/suites/test_suite_x509parse.data
+++ b/tests/suites/test_suite_x509parse.data
@@ -750,7 +750,7 @@ X509 Certificate ASN1 (TBSCertificate, issuer, no string data)
 x509parse_crt:"30253023a0030201028204deadbeef300d06092a864886f70d0101020500300731053003060013":"":POLARSSL_ERR_X509_INVALID_NAME + POLARSSL_ERR_ASN1_OUT_OF_DATA
 
 X509 Certificate ASN1 (TBSCertificate, issuer, no full following string)
-x509parse_crt:"302b3029a0030201028204deadbeef300d06092a864886f70d0101020500300d310b3009060013045465737400":"":POLARSSL_ERR_X509_INVALID_NAME + POLARSSL_ERR_ASN1_UNEXPECTED_TAG
+x509parse_crt:"302b3029a0030201028204deadbeef300d06092a864886f70d0101020500300d310b3009060013045465737400":"":POLARSSL_ERR_X509_FEATURE_UNAVAILABLE
 
 X509 Certificate ASN1 (TBSCertificate, valid issuer, no validity)
 x509parse_crt:"302a3028a0030201028204deadbeef300d06092a864886f70d0101020500300c310a30080600130454657374":"":POLARSSL_ERR_X509_INVALID_DATE + POLARSSL_ERR_ASN1_OUT_OF_DATA