Merge pull request #6391 from davidhorstmann-arm/fix-x509-get-name-cleanup
The Open CI ran successfully thus I think we can ignore the internal CI.
This commit is contained in:
commit
4f7feca0dc
4 changed files with 116 additions and 4 deletions
4
ChangeLog.d/fix_x509_get_name_mem_leak.txt
Normal file
4
ChangeLog.d/fix_x509_get_name_mem_leak.txt
Normal file
|
@ -0,0 +1,4 @@
|
|||
Bugfix
|
||||
* Fix memory leak in ssl_parse_certificate_request() caused by
|
||||
mbedtls_x509_get_name() not freeing allocated objects in case of error.
|
||||
Change mbedtls_x509_get_name() to clean up allocated objects on error.
|
|
@ -459,6 +459,11 @@ static int x509_get_attr_type_value( unsigned char **p,
|
|||
* For the general case we still use a flat list, but we mark elements of the
|
||||
* same set so that they are "merged" together in the functions that consume
|
||||
* this list, eg mbedtls_x509_dn_gets().
|
||||
*
|
||||
* On success, this function may allocate a linked list starting at cur->next
|
||||
* that must later be free'd by the caller using mbedtls_free(). In error
|
||||
* cases, this function frees all allocated memory internally and the caller
|
||||
* has no freeing responsibilities.
|
||||
*/
|
||||
int mbedtls_x509_get_name( unsigned char **p, const unsigned char *end,
|
||||
mbedtls_x509_name *cur )
|
||||
|
@ -466,6 +471,8 @@ int mbedtls_x509_get_name( unsigned char **p, const unsigned char *end,
|
|||
int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
|
||||
size_t set_len;
|
||||
const unsigned char *end_set;
|
||||
mbedtls_x509_name *head = cur;
|
||||
mbedtls_x509_name *prev, *allocated;
|
||||
|
||||
/* don't use recursion, we'd risk stack overflow if not optimized */
|
||||
while( 1 )
|
||||
|
@ -475,14 +482,17 @@ int mbedtls_x509_get_name( unsigned char **p, const unsigned char *end,
|
|||
*/
|
||||
if( ( ret = mbedtls_asn1_get_tag( p, end, &set_len,
|
||||
MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SET ) ) != 0 )
|
||||
return( MBEDTLS_ERROR_ADD( MBEDTLS_ERR_X509_INVALID_NAME, ret ) );
|
||||
{
|
||||
ret = MBEDTLS_ERROR_ADD( MBEDTLS_ERR_X509_INVALID_NAME, ret );
|
||||
goto error;
|
||||
}
|
||||
|
||||
end_set = *p + set_len;
|
||||
|
||||
while( 1 )
|
||||
{
|
||||
if( ( ret = x509_get_attr_type_value( p, end_set, cur ) ) != 0 )
|
||||
return( ret );
|
||||
goto error;
|
||||
|
||||
if( *p == end_set )
|
||||
break;
|
||||
|
@ -493,7 +503,10 @@ int mbedtls_x509_get_name( unsigned char **p, const unsigned char *end,
|
|||
cur->next = mbedtls_calloc( 1, sizeof( mbedtls_x509_name ) );
|
||||
|
||||
if( cur->next == NULL )
|
||||
return( MBEDTLS_ERR_X509_ALLOC_FAILED );
|
||||
{
|
||||
ret = MBEDTLS_ERR_X509_ALLOC_FAILED;
|
||||
goto error;
|
||||
}
|
||||
|
||||
cur = cur->next;
|
||||
}
|
||||
|
@ -507,10 +520,30 @@ int mbedtls_x509_get_name( unsigned char **p, const unsigned char *end,
|
|||
cur->next = mbedtls_calloc( 1, sizeof( mbedtls_x509_name ) );
|
||||
|
||||
if( cur->next == NULL )
|
||||
return( MBEDTLS_ERR_X509_ALLOC_FAILED );
|
||||
{
|
||||
ret = MBEDTLS_ERR_X509_ALLOC_FAILED;
|
||||
goto error;
|
||||
}
|
||||
|
||||
cur = cur->next;
|
||||
}
|
||||
|
||||
error:
|
||||
/* Skip the first element as we did not allocate it */
|
||||
allocated = head->next;
|
||||
|
||||
while( allocated != NULL )
|
||||
{
|
||||
prev = allocated;
|
||||
allocated = allocated->next;
|
||||
|
||||
mbedtls_platform_zeroize( prev, sizeof( *prev ) );
|
||||
mbedtls_free( prev );
|
||||
}
|
||||
|
||||
mbedtls_platform_zeroize( head, sizeof( *head ) );
|
||||
|
||||
return( ret );
|
||||
}
|
||||
|
||||
static int x509_parse_int( unsigned char **p, size_t n, int *res )
|
||||
|
|
|
@ -415,6 +415,46 @@ mbedtls_x509_dn_get_next:"C=NL, O=PolarSSL, CN=PolarSSL Server 1":0x03:"C":1:"C=
|
|||
X509 Get Next DN #4 Consecutive Multivalue RDNs
|
||||
mbedtls_x509_dn_get_next:"C=NL, O=PolarSSL, title=Example, CN=PolarSSL Server 1":0x05:"C title":2:"C=NL + O=PolarSSL, title=Example + CN=PolarSSL Server 1"
|
||||
|
||||
# Parse the following valid DN:
|
||||
#
|
||||
# 31 0B <- Set of
|
||||
# 30 09 <- Sequence of
|
||||
# 06 03 55 04 06 <- OID 2.5.4.6 countryName (C)
|
||||
# 13 02 4E 4C <- PrintableString "NL"
|
||||
# 31 11 <- Set of
|
||||
# 30 0F <- Sequence of
|
||||
# 06 03 55 04 0A <- OID 2.5.4.10 organizationName (O)
|
||||
# 0C 08 50 6F 6C 61 72 53 53 4C <- UTF8String "PolarSSL"
|
||||
# 31 19 <- Set of
|
||||
# 30 17 <- Sequence of
|
||||
# 06 03 55 04 03 <- OID 2.5.4.3 commonName (CN)
|
||||
# 0C 10 50 6F 6C 61 72 53 53 4C 20 54 65 73 74 20 43 41 <- UTF8String "PolarSSL Test CA"
|
||||
#
|
||||
X509 Get Name Valid DN
|
||||
mbedtls_x509_get_name:"310B3009060355040613024E4C3111300F060355040A0C08506F6C617253534C3119301706035504030C10506F6C617253534C2054657374204341":0
|
||||
|
||||
# Parse the following corrupted DN:
|
||||
#
|
||||
# 31 0B <- Set of
|
||||
# 30 09 <- Sequence of
|
||||
# 06 03 55 04 06 <- OID 2.5.4.6 countryName (C)
|
||||
# 13 02 4E 4C <- PrintableString "NL"
|
||||
# 31 11 <- Set of
|
||||
# 30 0F <- Sequence of
|
||||
# 06 03 55 04 0A <- OID 2.5.4.10 organizationName (O)
|
||||
# 0C 08 50 6F 6C 61 72 53 53 4C <- UTF8String "PolarSSL"
|
||||
# 30 19 <- Sequence of (corrupted)
|
||||
# 30 17 <- Sequence of
|
||||
# 06 03 55 04 03 <- OID 2.5.4.3 commonName (CN)
|
||||
# 0C 10 50 6F 6C 61 72 53 53 4C 20 54 65 73 74 20 43 41 <- UTF8String "PolarSSL Test CA"
|
||||
#
|
||||
# The third 'Set of' is corrupted to instead be a 'Sequence of', causing an
|
||||
# error and forcing mbedtls_x509_get_name() to clean up the names it has
|
||||
# already allocated.
|
||||
#
|
||||
X509 Get Name Corrupted DN Mem Leak
|
||||
mbedtls_x509_get_name:"310B3009060355040613024E4C3111300F060355040A0C08506F6C617253534C3019301706035504030C10506F6C617253534C2054657374204341":MBEDTLS_ERR_X509_INVALID_NAME + MBEDTLS_ERR_ASN1_UNEXPECTED_TAG
|
||||
|
||||
X509 Time Expired #1
|
||||
depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_RSA_C:MBEDTLS_HAVE_TIME_DATE:MBEDTLS_HAS_ALG_SHA_1_VIA_MD_OR_PSA_BASED_ON_USE_PSA
|
||||
mbedtls_x509_time_is_past:"data_files/server1.crt":"valid_from":1
|
||||
|
|
|
@ -818,6 +818,41 @@ exit:
|
|||
}
|
||||
/* END_CASE */
|
||||
|
||||
/* BEGIN_CASE depends_on:MBEDTLS_X509_CRT_PARSE_C */
|
||||
void mbedtls_x509_get_name( char * rdn_sequence, int exp_ret )
|
||||
{
|
||||
unsigned char *name;
|
||||
unsigned char *p;
|
||||
size_t name_len;
|
||||
mbedtls_x509_name head;
|
||||
mbedtls_x509_name *allocated, *prev;
|
||||
int ret;
|
||||
|
||||
memset( &head, 0, sizeof( head ) );
|
||||
|
||||
name = mbedtls_test_unhexify_alloc( rdn_sequence, &name_len );
|
||||
p = name;
|
||||
|
||||
ret = mbedtls_x509_get_name( &p, ( name + name_len ), &head );
|
||||
if( ret == 0 )
|
||||
{
|
||||
allocated = head.next;
|
||||
|
||||
while( allocated != NULL )
|
||||
{
|
||||
prev = allocated;
|
||||
allocated = allocated->next;
|
||||
|
||||
mbedtls_free( prev );
|
||||
}
|
||||
}
|
||||
|
||||
TEST_EQUAL( ret, exp_ret );
|
||||
|
||||
mbedtls_free( name );
|
||||
}
|
||||
/* END_CASE */
|
||||
|
||||
/* BEGIN_CASE depends_on:MBEDTLS_X509_CREATE_C:MBEDTLS_X509_USE_C:MBEDTLS_X509_CRT_PARSE_C:!MBEDTLS_X509_REMOVE_INFO */
|
||||
void mbedtls_x509_dn_get_next( char * name_str, int next_merged, char * expected_oids, int exp_count, char * exp_dn_gets )
|
||||
{
|
||||
|
|
Loading…
Reference in a new issue