Fix undocumented free() in x509_string_to_names()
Now programs/x509/cert_write san="DN:CN=#0000;DN:CN=#0000" is no longer
crashing with use-after-free, instead it's now failing cleanly:
failed
! mbedtls_x509_string_to_names returned -0x2800 - X509 - Input invalid
That's better of course but still not great, will be fixed by future
commits.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
diff --git a/ChangeLog.d/fix-string-to-names-memory-management.txt b/ChangeLog.d/fix-string-to-names-memory-management.txt
new file mode 100644
index 0000000..1b21982
--- /dev/null
+++ b/ChangeLog.d/fix-string-to-names-memory-management.txt
@@ -0,0 +1,18 @@
+Security
+ * Fix possible use-after-free or double-free in code calling
+ mbedtls_x509_string_to_names(). This was caused by the function calling
+ mbedtls_asn1_free_named_data_list() on its head argument, while the
+ documentation did no suggest it did, making it likely for callers relying
+ on the documented behaviour to still hold pointers to memory blocks after
+ they were free()d, resulting in high risk of use-after-free or double-free,
+ with consequences ranging up to arbitrary code execution.
+ In particular, the two sample programs x509/cert_write and x509/cert_req
+ were affected (use-after-free if the san string contains more than one DN).
+ Code that does not call mbedtls_string_to_names() directly is not affected.
+ Found by Linh Le and Ngan Nguyen from Calif.
+
+Changes
+ * The function mbedtls_x509_string_to_names() now requires its head argument
+ to point to NULL on entry. This make it likely that existing risky uses of
+ this function (see the entry in the Security section) will be detected and
+ fixed.
diff --git a/include/mbedtls/x509.h b/include/mbedtls/x509.h
index 453f598..6b10461 100644
--- a/include/mbedtls/x509.h
+++ b/include/mbedtls/x509.h
@@ -332,7 +332,8 @@
* call to mbedtls_asn1_free_named_data_list().
*
* \param[out] head Address in which to store the pointer to the head of the
- * allocated list of mbedtls_x509_name
+ * allocated list of mbedtls_x509_name. Must point to NULL on
+ * entry.
* \param[in] name The string representation of a DN to convert
*
* \return 0 on success, or a negative error code.
diff --git a/library/x509_create.c b/library/x509_create.c
index 839b5df..420e36b 100644
--- a/library/x509_create.c
+++ b/library/x509_create.c
@@ -292,8 +292,12 @@
unsigned char data[MBEDTLS_X509_MAX_DN_NAME_SIZE];
size_t data_len = 0;
- /* Clear existing chain if present */
- mbedtls_asn1_free_named_data_list(head);
+ /* Ensure the output parameter is not already populated.
+ * (If it were, overwriting it would likely cause a memory leak.)
+ */
+ if (*head != NULL) {
+ return MBEDTLS_ERR_X509_BAD_INPUT_DATA;
+ }
while (c <= end) {
if (in_attr_type && *c == '=') {