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