From 5989da22a9d32cd314411f3f79df4ae580d7d285 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 21 May 2025 14:35:42 +0200 Subject: [PATCH 1/6] Add tests for bug in mbedtls_x509_string_to_names() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The commented out tests cause crashes (in different ways) until the bug is fixed; the first two test are passing already and are here mostly to provide a reference point. The bug report was using programs/x509/cert_write, but string_to_names() is what it was really targetting, which is better for automated tests. The strings used are a minor adapation of those from the report. Signed-off-by: Manuel Pégourié-Gonnard --- tests/suites/test_suite_x509write.data | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/suites/test_suite_x509write.data b/tests/suites/test_suite_x509write.data index e4e08dafc0..e5224218c5 100644 --- a/tests/suites/test_suite_x509write.data +++ b/tests/suites/test_suite_x509write.data @@ -254,6 +254,27 @@ mbedtls_x509_string_to_names:"C=NL, O=Of\\CCspark, OU=PolarSSL":"C=NL, O=Of\\CCs X509 String to Names #20 (Reject empty AttributeValue) mbedtls_x509_string_to_names:"C=NL, O=, OU=PolarSSL":"":MBEDTLS_ERR_X509_INVALID_NAME:0 +# Note: the behaviour is incorrect, output from string->names->string should be +# the same as the input, rather than just the last component, see +# https://github.com/Mbed-TLS/mbedtls/issues/10189 +# Still including tests for the current incorrect behaviour because of the +# variants below where we want to ensure at least that no memory corruption +# happens (which would be a lot worse than just a functional bug). +X509 String to Names (repeated OID) +mbedtls_x509_string_to_names:"CN=ab,CN=cd,CN=ef":"CN=ef":0:0 + +# Note: when a value starts with a # sign, it's treated as the hex encoding of +# the DER encoding of the value. Here, 0400 is a zero-length OCTET STRING. +# The tag actually doesn't matter for our purposes, only the length. +X509 String to Names (repeated OID, 1st is zero-length) +mbedtls_x509_string_to_names:"CN=#0400,CN=cd,CN=ef":"CN=ef":0:0 + +#X509 String to Names (repeated OID, middle is zero-length) +#mbedtls_x509_string_to_names:"CN=ab,CN=#0400,CN=ef":"CN=ef":0:0 + +#X509 String to Names (repeated OID, last is zero-length) +#mbedtls_x509_string_to_names:"CN=ab,CN=cd,CN=#0400":"CN=ef":0:0 + X509 Round trip test (Escaped characters) mbedtls_x509_string_to_names:"CN=Lu\\C4\\8Di\\C4\\87, O=Offspark, OU=PolarSSL":"CN=Lu\\C4\\8Di\\C4\\87, O=Offspark, OU=PolarSSL":0:0 From d1090d70ffd084b8750b64334a32b8b6d473ee19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 28 May 2025 13:06:27 +0200 Subject: [PATCH 2/6] Update crypto submodule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- tf-psa-crypto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tf-psa-crypto b/tf-psa-crypto index 35ae18cf89..9af7c0e7ba 160000 --- a/tf-psa-crypto +++ b/tf-psa-crypto @@ -1 +1 @@ -Subproject commit 35ae18cf891d3675584da41f7e830f1de5f87f07 +Subproject commit 9af7c0e7ba4d6bf2a9c3e56a3e3f04b4b053ce47 From d2262f23049356528e7a7849dcd18928f484255e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 28 May 2025 13:07:42 +0200 Subject: [PATCH 3/6] Uncomment tests now that crypto is fixed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- tests/suites/test_suite_x509write.data | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/suites/test_suite_x509write.data b/tests/suites/test_suite_x509write.data index e5224218c5..96311f3b56 100644 --- a/tests/suites/test_suite_x509write.data +++ b/tests/suites/test_suite_x509write.data @@ -269,11 +269,11 @@ mbedtls_x509_string_to_names:"CN=ab,CN=cd,CN=ef":"CN=ef":0:0 X509 String to Names (repeated OID, 1st is zero-length) mbedtls_x509_string_to_names:"CN=#0400,CN=cd,CN=ef":"CN=ef":0:0 -#X509 String to Names (repeated OID, middle is zero-length) -#mbedtls_x509_string_to_names:"CN=ab,CN=#0400,CN=ef":"CN=ef":0:0 +X509 String to Names (repeated OID, middle is zero-length) +mbedtls_x509_string_to_names:"CN=ab,CN=#0400,CN=ef":"CN=ef":0:0 -#X509 String to Names (repeated OID, last is zero-length) -#mbedtls_x509_string_to_names:"CN=ab,CN=cd,CN=#0400":"CN=ef":0:0 +X509 String to Names (repeated OID, last is zero-length) +mbedtls_x509_string_to_names:"CN=ab,CN=cd,CN=#0400":"CN=ef":0:0 X509 Round trip test (Escaped characters) mbedtls_x509_string_to_names:"CN=Lu\\C4\\8Di\\C4\\87, O=Offspark, OU=PolarSSL":"CN=Lu\\C4\\8Di\\C4\\87, O=Offspark, OU=PolarSSL":0:0 From 5f6310b65f6ad3cf2faa62b9c8a2109ecf0bedb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 26 May 2025 12:38:52 +0200 Subject: [PATCH 4/6] Add ChangeLog entry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- ChangeLog.d/fix-string-to-names-store-named-data.txt | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 ChangeLog.d/fix-string-to-names-store-named-data.txt diff --git a/ChangeLog.d/fix-string-to-names-store-named-data.txt b/ChangeLog.d/fix-string-to-names-store-named-data.txt new file mode 100644 index 0000000000..422ce07f85 --- /dev/null +++ b/ChangeLog.d/fix-string-to-names-store-named-data.txt @@ -0,0 +1,12 @@ +Security + * Fix a bug in mbedtls_asn1_store_named_data() where it would sometimes leave + an item in the output list in an inconsistent state with val.p == NULL but + val.len > 0. This impacts applications that call this function directly, + or indirectly via mbedtls_x509_string_to_names() or one of the + mbedtls_x509write_{crt,csr}_set_{subject,issuer}_name() functions. The + inconsistent state of the output could then cause a NULL dereference either + inside the same call to mbedtls_x509_string_to_names(), or in subsequent + users of the output structure, such as mbedtls_x509_write_names(). This + only affects applications that create (as opposed to consume) X.509 + certificates, CSRs or CRLS, or that call mbedtls_asn1_store_named_data() + directly. Found by Linh Le and Ngan Nguyen from Calif. From dc82fa67c5cfab62010d4d642015c267b0739307 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 28 May 2025 13:10:44 +0200 Subject: [PATCH 5/6] Keep only the X.509 part from the Changelog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- .../fix-string-to-names-store-named-data.txt | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/ChangeLog.d/fix-string-to-names-store-named-data.txt b/ChangeLog.d/fix-string-to-names-store-named-data.txt index 422ce07f85..e517cbb72a 100644 --- a/ChangeLog.d/fix-string-to-names-store-named-data.txt +++ b/ChangeLog.d/fix-string-to-names-store-named-data.txt @@ -1,12 +1,8 @@ Security - * Fix a bug in mbedtls_asn1_store_named_data() where it would sometimes leave - an item in the output list in an inconsistent state with val.p == NULL but - val.len > 0. This impacts applications that call this function directly, - or indirectly via mbedtls_x509_string_to_names() or one of the - mbedtls_x509write_{crt,csr}_set_{subject,issuer}_name() functions. The - inconsistent state of the output could then cause a NULL dereference either - inside the same call to mbedtls_x509_string_to_names(), or in subsequent + * Fix a bug in mbedtls_x509_string_to_names() and the + mbedtls_x509write_{crt,csr}_set_{subject,issuer}_name() functions, + where some inputs would cause an inconsistent state to be reached, causing + a NULL dereference either in the function itself, or in subsequent users of the output structure, such as mbedtls_x509_write_names(). This only affects applications that create (as opposed to consume) X.509 - certificates, CSRs or CRLS, or that call mbedtls_asn1_store_named_data() - directly. Found by Linh Le and Ngan Nguyen from Calif. + certificates, CSRs or CRLs. Found by Linh Le and Ngan Nguyen from Calif. From f5a63d1456f109c369500d89f605ea308ea14f1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 10 Jun 2025 09:56:40 +0200 Subject: [PATCH 6/6] Fix invalid test data by aligning with 3.6 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- tests/suites/test_suite_x509write.data | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/suites/test_suite_x509write.data b/tests/suites/test_suite_x509write.data index 96311f3b56..4dcd967226 100644 --- a/tests/suites/test_suite_x509write.data +++ b/tests/suites/test_suite_x509write.data @@ -273,7 +273,7 @@ X509 String to Names (repeated OID, middle is zero-length) mbedtls_x509_string_to_names:"CN=ab,CN=#0400,CN=ef":"CN=ef":0:0 X509 String to Names (repeated OID, last is zero-length) -mbedtls_x509_string_to_names:"CN=ab,CN=cd,CN=#0400":"CN=ef":0:0 +mbedtls_x509_string_to_names:"CN=ab,CN=cd,CN=#0400":"CN=#0000":0:MAY_FAIL_GET_NAME X509 Round trip test (Escaped characters) mbedtls_x509_string_to_names:"CN=Lu\\C4\\8Di\\C4\\87, O=Offspark, OU=PolarSSL":"CN=Lu\\C4\\8Di\\C4\\87, O=Offspark, OU=PolarSSL":0:0