From 0d90f9f74eba36b34d674d448f059500f8dd25f4 Mon Sep 17 00:00:00 2001 From: Jeremy Levartovsky <140487034+JayOfTheKeyboard@users.noreply.github.com> Date: Mon, 12 Oct 2026 21:34:38 +1100 Subject: [PATCH 1/2] Fix IPv6 SAN overflow in cert_write and cert_req mbedtls_x509_crt_parse_cn_inet_pton() requires a 16-byte buffer and writes 16 bytes for an IPv6 address, but both programs passed a 4-byte one. Use a 16-byte buffer and record the length the function returns instead of sizeof(ip), so IPv6 addresses are encoded as 16 bytes. Signed-off-by: Jeremy Levartovsky <140487034+JayOfTheKeyboard@users.noreply.github.com> --- ChangeLog.d/x509-programs-ip-san.txt | 4 ++++ programs/x509/cert_req.c | 6 +++--- programs/x509/cert_write.c | 6 +++--- 3 files changed, 10 insertions(+), 6 deletions(-) create mode 100644 ChangeLog.d/x509-programs-ip-san.txt diff --git a/ChangeLog.d/x509-programs-ip-san.txt b/ChangeLog.d/x509-programs-ip-san.txt new file mode 100644 index 0000000000..a2e40ef032 --- /dev/null +++ b/ChangeLog.d/x509-programs-ip-san.txt @@ -0,0 +1,4 @@ +Bugfix + * Fix a stack buffer overflow in the cert_write and cert_req sample + programs when an IPv6 address is given in the san parameter. IPv6 + addresses are now written correctly. diff --git a/programs/x509/cert_req.c b/programs/x509/cert_req.c index 38b7af0925..049624c9b6 100644 --- a/programs/x509/cert_req.c +++ b/programs/x509/cert_req.c @@ -59,7 +59,7 @@ int main(void) " DNS:value\n" \ " URI:value\n" \ " RFC822:value\n" \ - " IP:value (Only IPv4 is supported)\n" \ + " IP:value (IPv4 or IPv6)\n" \ " DN:list of comma separated key=value pairs\n" \ " key_usage=%%s default: (empty)\n" \ " Comma-separated-list of values:\n" \ @@ -151,7 +151,7 @@ int main(int argc, char *argv[]) const char *pers = "csr example app"; mbedtls_x509_san_list *cur, *prev; #if defined(MBEDTLS_X509_CRT_PARSE_C) - uint8_t ip[4] = { 0 }; + uint8_t ip[16] = { 0 }; #endif /* * Set to sane values @@ -270,7 +270,7 @@ usage: goto exit; } cur->node.san.unstructured_name.p = (unsigned char *) ip; - cur->node.san.unstructured_name.len = sizeof(ip); + cur->node.san.unstructured_name.len = ip_addr_len; } else if (strcmp(q, "DN") == 0) { cur->node.type = MBEDTLS_X509_SAN_DIRECTORY_NAME; /* Work around an API mismatch between string_to_names() and diff --git a/programs/x509/cert_write.c b/programs/x509/cert_write.c index fc47c6fcb0..df6b7b9c02 100644 --- a/programs/x509/cert_write.c +++ b/programs/x509/cert_write.c @@ -128,7 +128,7 @@ int main(void) " DNS:value\n" \ " URI:value\n" \ " RFC822:value\n" \ - " IP:value (Only IPv4 is supported)\n" \ + " IP:value (IPv4 or IPv6)\n" \ " DN:list of comma separated key=value pairs\n" \ " authority_identifier=%%s default: 1\n" \ " Possible values: 0, 1\n" \ @@ -316,7 +316,7 @@ int main(int argc, char *argv[]) mbedtls_ctr_drbg_context ctr_drbg; const char *pers = "crt example app"; mbedtls_x509_san_list *cur, *prev; - uint8_t ip[4] = { 0 }; + uint8_t ip[16] = { 0 }; /* * Set to sane values */ @@ -595,7 +595,7 @@ usage: goto exit; } cur->node.san.unstructured_name.p = (unsigned char *) ip; - cur->node.san.unstructured_name.len = sizeof(ip); + cur->node.san.unstructured_name.len = ip_addr_len; } else if (strcmp(q, "DN") == 0) { cur->node.type = MBEDTLS_X509_SAN_DIRECTORY_NAME; /* Work around an API mismatch between string_to_names() and From 327fca77cbd154e31efb602ce5511dba4e0b90cc Mon Sep 17 00:00:00 2001 From: Jeremy Levartovsky <140487034+JayOfTheKeyboard@users.noreply.github.com> Date: Mon, 12 Oct 2026 21:35:25 +1100 Subject: [PATCH 2/2] Keep each IP SAN entry's address in cert_write and cert_req Every IP entry in the san list pointed at the same buffer, and the list is only written once all entries are parsed, so san=IP:1.2.3.4;IP:5.6.7.8 produced two copies of 5.6.7.8. Give each IP entry its own buffer and free it with the rest of the list. Signed-off-by: Jeremy Levartovsky <140487034+JayOfTheKeyboard@users.noreply.github.com> --- ChangeLog.d/x509-programs-ip-san.txt | 2 ++ programs/x509/cert_req.c | 17 +++++++++++++---- programs/x509/cert_write.c | 15 +++++++++++++-- 3 files changed, 28 insertions(+), 6 deletions(-) diff --git a/ChangeLog.d/x509-programs-ip-san.txt b/ChangeLog.d/x509-programs-ip-san.txt index a2e40ef032..6343777718 100644 --- a/ChangeLog.d/x509-programs-ip-san.txt +++ b/ChangeLog.d/x509-programs-ip-san.txt @@ -2,3 +2,5 @@ Bugfix * Fix a stack buffer overflow in the cert_write and cert_req sample programs when an IPv6 address is given in the san parameter. IPv6 addresses are now written correctly. + * Fix the cert_write and cert_req sample programs writing the last IP + address for every IP entry when the san parameter has more than one. diff --git a/programs/x509/cert_req.c b/programs/x509/cert_req.c index 049624c9b6..2920a25f91 100644 --- a/programs/x509/cert_req.c +++ b/programs/x509/cert_req.c @@ -150,9 +150,6 @@ int main(int argc, char *argv[]) mbedtls_ctr_drbg_context ctr_drbg; const char *pers = "csr example app"; mbedtls_x509_san_list *cur, *prev; -#if defined(MBEDTLS_X509_CRT_PARSE_C) - uint8_t ip[16] = { 0 }; -#endif /* * Set to sane values */ @@ -262,14 +259,23 @@ usage: cur->node.type = MBEDTLS_X509_SAN_DNS_NAME; } else if (strcmp(q, "IP") == 0) { size_t ip_addr_len = 0; + /* Each entry needs its own buffer, because the list is + * only written once all entries have been parsed. + * mbedtls_x509_crt_parse_cn_inet_pton() needs 16 bytes. */ + unsigned char *ip = mbedtls_calloc(1, 16); + if (ip == NULL) { + mbedtls_printf("Not enough memory for subjectAltName list\n"); + goto exit; + } cur->node.type = MBEDTLS_X509_SAN_IP_ADDRESS; ip_addr_len = mbedtls_x509_crt_parse_cn_inet_pton(subtype_value, ip); if (ip_addr_len == 0) { mbedtls_printf("mbedtls_x509_crt_parse_cn_inet_pton failed to parse %s\n", subtype_value); + mbedtls_free(ip); goto exit; } - cur->node.san.unstructured_name.p = (unsigned char *) ip; + cur->node.san.unstructured_name.p = ip; cur->node.san.unstructured_name.len = ip_addr_len; } else if (strcmp(q, "DN") == 0) { cur->node.type = MBEDTLS_X509_SAN_DIRECTORY_NAME; @@ -521,6 +527,9 @@ exit: mbedtls_free(dn->val.p); mbedtls_asn1_free_named_data_list(&dn->next); } + if (cur->node.type == MBEDTLS_X509_SAN_IP_ADDRESS) { + mbedtls_free(cur->node.san.unstructured_name.p); + } mbedtls_free(cur); cur = next; } diff --git a/programs/x509/cert_write.c b/programs/x509/cert_write.c index df6b7b9c02..dc1fe6b03b 100644 --- a/programs/x509/cert_write.c +++ b/programs/x509/cert_write.c @@ -316,7 +316,6 @@ int main(int argc, char *argv[]) mbedtls_ctr_drbg_context ctr_drbg; const char *pers = "crt example app"; mbedtls_x509_san_list *cur, *prev; - uint8_t ip[16] = { 0 }; /* * Set to sane values */ @@ -587,14 +586,23 @@ usage: cur->node.type = MBEDTLS_X509_SAN_DNS_NAME; } else if (strcmp(q, "IP") == 0) { size_t ip_addr_len = 0; + /* Each entry needs its own buffer, because the list is + * only written once all entries have been parsed. + * mbedtls_x509_crt_parse_cn_inet_pton() needs 16 bytes. */ + unsigned char *ip = mbedtls_calloc(1, 16); + if (ip == NULL) { + mbedtls_printf("Not enough memory for subjectAltName list\n"); + goto exit; + } cur->node.type = MBEDTLS_X509_SAN_IP_ADDRESS; ip_addr_len = mbedtls_x509_crt_parse_cn_inet_pton(subtype_value, ip); if (ip_addr_len == 0) { mbedtls_printf("mbedtls_x509_crt_parse_cn_inet_pton failed to parse %s\n", subtype_value); + mbedtls_free(ip); goto exit; } - cur->node.san.unstructured_name.p = (unsigned char *) ip; + cur->node.san.unstructured_name.p = ip; cur->node.san.unstructured_name.len = ip_addr_len; } else if (strcmp(q, "DN") == 0) { cur->node.type = MBEDTLS_X509_SAN_DIRECTORY_NAME; @@ -1021,6 +1029,9 @@ exit: mbedtls_free(dn->val.p); mbedtls_asn1_free_named_data_list(&dn->next); } + if (cur->node.type == MBEDTLS_X509_SAN_IP_ADDRESS) { + mbedtls_free(cur->node.san.unstructured_name.p); + } mbedtls_free(cur); cur = next; }