Free structs in mbedtls_x509_get_name() on error
mbedtls_x509_get_name() allocates a linked list of mbedtls_x509_name
structs but does not free these when there is an error, leaving the
caller to free them itself. Change this to cleanup these objects within
the function in case of an error.
Signed-off-by: David Horstmann <david.horstmann@arm.com>
diff --git a/library/x509.c b/library/x509.c
index 3997ebd..5d83384 100644
--- a/library/x509.c
+++ b/library/x509.c
@@ -424,6 +424,11 @@
* 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 allocates 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 )
@@ -431,6 +436,8 @@
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 )
@@ -440,14 +447,17 @@
*/
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;
@@ -458,7 +468,10 @@
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;
}
@@ -472,10 +485,35 @@
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:
+ prev = NULL;
+
+ /* Skip the first element as we did not allocate it */
+ allocated = head->next;
+
+ /* Make sure we cannot be followed along this list */
+ head->next = NULL;
+
+ 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 )