From 3b97a07eb5cd22998727005d0621a0be3c0a125d Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Thu, 11 May 2023 17:08:07 +0100 Subject: [PATCH] Always free SANs on error In mbedtls_x509_get_subject_alt_name(), clean up the mbedtls_x509_sequence on error. This fixes previous behaviour where the sequence was sometimes freed and sometimes not. In practice, any sensible freeing code handles both cases fine, but this is useful in case there is no freeing. Signed-off-by: David Horstmann --- library/x509.c | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/library/x509.c b/library/x509.c index 5f6715aa25..8fef80b236 100644 --- a/library/x509.c +++ b/library/x509.c @@ -1223,7 +1223,8 @@ int mbedtls_x509_get_subject_alt_name_ext(unsigned char **p, (*p)++; if ((ret = mbedtls_asn1_get_len(p, end, &tag_len)) != 0) { - return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_X509_INVALID_EXTENSIONS, ret); + ret = MBEDTLS_ERROR_ADD(MBEDTLS_ERR_X509_INVALID_EXTENSIONS, ret); + goto error; } tmp_san_buf.p = *p; @@ -1231,8 +1232,9 @@ int mbedtls_x509_get_subject_alt_name_ext(unsigned char **p, if ((tmp_san_buf.tag & MBEDTLS_ASN1_TAG_CLASS_MASK) != MBEDTLS_ASN1_CONTEXT_SPECIFIC) { - return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_X509_INVALID_EXTENSIONS, - MBEDTLS_ERR_ASN1_UNEXPECTED_TAG); + ret = MBEDTLS_ERROR_ADD(MBEDTLS_ERR_X509_INVALID_EXTENSIONS, + MBEDTLS_ERR_ASN1_UNEXPECTED_TAG); + goto error; } /* @@ -1244,23 +1246,26 @@ int mbedtls_x509_get_subject_alt_name_ext(unsigned char **p, * and clear the allocated sequences. */ if (ret != 0 && ret != MBEDTLS_ERR_X509_FEATURE_UNAVAILABLE) { - mbedtls_asn1_sequence_free(subject_alt_name->next); - subject_alt_name->next = NULL; - return ret; + goto error; } mbedtls_x509_free_subject_alt_name(&dummy_san_buf); /* Allocate and assign next pointer */ if (cur->buf.p != NULL) { if (cur->next != NULL) { + /* + * The mbedtls_x509_sequence already contains something, + * leave it alone and return an error. + */ return MBEDTLS_ERR_X509_INVALID_EXTENSIONS; } cur->next = mbedtls_calloc(1, sizeof(mbedtls_asn1_sequence)); if (cur->next == NULL) { - return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_X509_INVALID_EXTENSIONS, - MBEDTLS_ERR_ASN1_ALLOC_FAILED); + ret = MBEDTLS_ERROR_ADD(MBEDTLS_ERR_X509_INVALID_EXTENSIONS, + MBEDTLS_ERR_ASN1_ALLOC_FAILED); + goto error; } cur = cur->next; @@ -1274,11 +1279,17 @@ int mbedtls_x509_get_subject_alt_name_ext(unsigned char **p, cur->next = NULL; if (*p != end) { - return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_X509_INVALID_EXTENSIONS, - MBEDTLS_ERR_ASN1_LENGTH_MISMATCH); + ret = MBEDTLS_ERROR_ADD(MBEDTLS_ERR_X509_INVALID_EXTENSIONS, + MBEDTLS_ERR_ASN1_LENGTH_MISMATCH); + goto error; } return 0; + +error: + mbedtls_asn1_sequence_free(subject_alt_name->next); + subject_alt_name->next = NULL; + return ret; } /*