Skip to content

Commit 43a1e73

Browse files
committed
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>
1 parent 1782587 commit 43a1e73

File tree

3 files changed

+26
-3
lines changed

3 files changed

+26
-3
lines changed
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
Security
2+
* Fix possible use-after-free or double-free in code calling
3+
mbedtls_x509_string_to_names(). This was caused by the function calling
4+
mbedtls_asn1_free_named_data_list() on its head argument, while the
5+
documentation did no suggest it did, making it likely for callers relying
6+
on the documented behaviour to still hold pointers to memory blocks after
7+
they were free()d, resulting in high risk of use-after-free or double-free,
8+
with consequences ranging up to arbitrary code execution.
9+
In particular, the two sample programs x509/cert_write and x509/cert_req
10+
were affected (use-after-free if the san string contains more than one DN).
11+
Code that does not call mbedtls_string_to_names() directly is not affected.
12+
Found by Linh Le and Ngan Nguyen from Calif.
13+
14+
Changes
15+
* The function mbedtls_x509_string_to_names() now requires its head argument
16+
to point to NULL on entry. This make it likely that existing risky uses of
17+
this function (see the entry in the Security section) will be detected and
18+
fixed.

include/mbedtls/x509.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,8 @@ int mbedtls_x509_dn_gets(char *buf, size_t size, const mbedtls_x509_name *dn);
332332
* call to mbedtls_asn1_free_named_data_list().
333333
*
334334
* \param[out] head Address in which to store the pointer to the head of the
335-
* allocated list of mbedtls_x509_name
335+
* allocated list of mbedtls_x509_name. Must point to NULL on
336+
* entry.
336337
* \param[in] name The string representation of a DN to convert
337338
*
338339
* \return 0 on success, or a negative error code.

library/x509_create.c

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -467,8 +467,12 @@ int mbedtls_x509_string_to_names(mbedtls_asn1_named_data **head, const char *nam
467467
unsigned char data[MBEDTLS_X509_MAX_DN_NAME_SIZE];
468468
size_t data_len = 0;
469469

470-
/* Clear existing chain if present */
471-
mbedtls_asn1_free_named_data_list(head);
470+
/* Ensure the output parameter is not already populated.
471+
* (If it were, overwriting it would likely cause a memory leak.)
472+
*/
473+
if (*head != NULL) {
474+
return MBEDTLS_ERR_X509_BAD_INPUT_DATA;
475+
}
472476

473477
while (c <= end) {
474478
if (in_attr_type && *c == '=') {

0 commit comments

Comments
 (0)