Merge pull request #1363 from gilles-peskine-arm/3.6-restricted-merge-20250606
Merge mbedtls-3.6 into mbedtls-3.6-restricted
diff --git a/ChangeLog.d/fix-asn1write-raw-buffer.txt b/ChangeLog.d/fix-asn1write-raw-buffer.txt
new file mode 100644
index 0000000..292631a
--- /dev/null
+++ b/ChangeLog.d/fix-asn1write-raw-buffer.txt
@@ -0,0 +1,5 @@
+Bugfix
+ * When calling mbedtls_asn1_write_raw_buffer() with NULL, 0 as the last two
+ arguments, undefined behaviour would be triggered, in the form of a call to
+ memcpy(..., NULL, 0). This was harmless in practice, but could trigger
+ complains from sanitizers or static analyzers.
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 0000000..422ce07
--- /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.
diff --git a/library/asn1write.c b/library/asn1write.c
index 775a9ef..97f9db0 100644
--- a/library/asn1write.c
+++ b/library/asn1write.c
@@ -90,7 +90,9 @@
len = size;
(*p) -= len;
- memcpy(*p, buf, len);
+ if (len != 0) {
+ memcpy(*p, buf, len);
+ }
return (int) len;
}
@@ -412,6 +414,7 @@
} else if (val_len == 0) {
mbedtls_free(cur->val.p);
cur->val.p = NULL;
+ cur->val.len = 0;
} else if (cur->val.len != val_len) {
/*
* Enlarge existing value buffer if needed
diff --git a/tests/suites/test_suite_asn1write.function b/tests/suites/test_suite_asn1write.function
index f5fc025..b1e66ed 100644
--- a/tests/suites/test_suite_asn1write.function
+++ b/tests/suites/test_suite_asn1write.function
@@ -550,6 +550,9 @@
}
if (new_len == 0) {
TEST_ASSERT(found->val.p == NULL);
+ /* If new_len != 0, then new_val != NULL and the length has been checked
+ * above by TEST_MEMORY_COMPARE(). But not here, so we need to check. */
+ TEST_EQUAL(found->val.len, 0);
} else if (new_len == old_len) {
TEST_ASSERT(found->val.p == old_val);
} else {
@@ -583,8 +586,10 @@
TEST_MEMORY_COMPARE(found->oid.p, found->oid.len, oid, oid_len);
if (new_len == 0) {
TEST_ASSERT(found->val.p == NULL);
+ TEST_EQUAL(found->val.len, 0);
} else if (new_val == NULL) {
TEST_ASSERT(found->val.p != NULL);
+ TEST_EQUAL(found->val.len, new_len);
} else {
TEST_ASSERT(found->val.p != new_val);
TEST_MEMORY_COMPARE(found->val.p, found->val.len,
diff --git a/tests/suites/test_suite_x509write.data b/tests/suites/test_suite_x509write.data
index 0cbad4b..56af0ce 100644
--- a/tests/suites/test_suite_x509write.data
+++ b/tests/suites/test_suite_x509write.data
@@ -254,6 +254,27 @@
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=#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