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 7deb706653..145c3e274b 100644 --- a/programs/x509/cert_req.c +++ b/programs/x509/cert_req.c @@ -152,9 +152,6 @@ int main(int argc, char *argv[]) char *p, *q, *r; mbedtls_x509write_csr req; mbedtls_x509_san_list *cur, *prev; -#if defined(MBEDTLS_X509_CRT_PARSE_C) - uint8_t ip[16] = { 0 }; -#endif /* * Set to sane values */ @@ -260,14 +257,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; @@ -565,6 +571,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 95dc7d902d..40d70f5633 100644 --- a/programs/x509/cert_write.c +++ b/programs/x509/cert_write.c @@ -316,7 +316,6 @@ int main(int argc, char *argv[]) size_t serial_len; mbedtls_asn1_sequence *ext_key_usage; mbedtls_x509_san_list *cur, *prev; - uint8_t ip[16] = { 0 }; /* * Set to sane values */ @@ -642,14 +641,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; @@ -1065,6 +1073,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; }