Commit f3ad4106b8 for openssl.org
commit f3ad4106b8b81a18616131fe09f8332b38e8bfd3
Author: Bob Beck <beck@openssl.org>
Date: Wed Aug 5 10:48:04 2026 -0600
Make ASN1_STRING_to_UTF8() return a NUL terminated buffer
While it's really pretty foolish to do so (since it can
contain legitimate 0 bytes in the output) Some callers were
treating the returned buffer as a C string, in spite of the fact
that the documentation does not say that it is and a length
is returned.
Unfortunately, this probably stems from the bad habit in
OpenSSL of trying to make things C strings anyway, - The conversion
path already allocated the extra byte and terminated it as if it
is a usable C string. The same-format path stored exact-length data
from an ASN1_STRING with no NUL, so termination depended on the input
type and how it was constructed.
So be consistent, and NUL terminate the returned buffer in both cases.
We then document that the retured buffer *IS* nul terminated, add
unit tests because of the Beyonce rule, and document in the
warnings that in spite of the output buffer being nul terminated
it is potentially a bad idea to use C string functions on it as
it can contain internal NUL bytes.
Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Andrew Dinh <andrewd@openssl.org>
Reviewed-by: Nikola Pajkovsky <nikolap@openssl.org>
Merge-date: Thu Aug 27 13:52:42 2026
Merged-from: https://github.com/openssl/openssl/pull/32178
diff --git a/crypto/asn1/a_mbstr.c b/crypto/asn1/a_mbstr.c
index 7f43b60b87..f4ec953f01 100644
--- a/crypto/asn1/a_mbstr.c
+++ b/crypto/asn1/a_mbstr.c
@@ -171,14 +171,18 @@ int ASN1_mbstring_ncopy(ASN1_STRING **out, const unsigned char *in, int len,
}
/* If both the same type just copy across */
if (inform == outform) {
- if (!ASN1_STRING_set1_data(dest, in, len)) {
+ if ((p = OPENSSL_malloc((size_t)len + 1)) == NULL) {
if (free_out) {
ASN1_STRING_free(dest);
*out = NULL;
}
- ERR_raise(ERR_LIB_ASN1, ERR_R_ASN1_LIB);
return -1;
}
+ if (len > 0)
+ memcpy(p, in, (size_t)len);
+ p[len] = '\0';
+ dest->data = p;
+ dest->length = len;
return str_type;
}
diff --git a/crypto/asn1/a_strex.c b/crypto/asn1/a_strex.c
index e6e58988d4..a6b022ad50 100644
--- a/crypto/asn1/a_strex.c
+++ b/crypto/asn1/a_strex.c
@@ -647,6 +647,7 @@ int ASN1_STRING_to_UTF8(unsigned char **out, const ASN1_STRING *in)
B_ASN1_UTF8STRING);
if (ret < 0)
return ret;
+ /* ASN1_mbstring_copy() guarantees the data it produced is NUL terminated */
*out = stmp.data;
return stmp.length;
}
diff --git a/doc/man3/ASN1_STRING_length.pod b/doc/man3/ASN1_STRING_length.pod
index d7dc04578a..bc630fd6e1 100644
--- a/doc/man3/ASN1_STRING_length.pod
+++ b/doc/man3/ASN1_STRING_length.pod
@@ -74,9 +74,11 @@ ASN1_STRING_type() returns the type of I<x>, using standard constants
such as B<V_ASN1_OCTET_STRING>.
ASN1_STRING_to_UTF8() converts the string I<in> to UTF8 format, the
-converted data is allocated in a buffer in I<*out>. The length of
+converted data is allocated in a buffer in I<*out>. The buffer is NUL
+terminated, so it is a valid C string, but the returned length does not
+include the terminating NUL. The length of
I<out> is returned or a negative error code. The buffer I<*out>
-should be freed using OPENSSL_free().
+should be freed using OPENSSL_free(). See L</WARNINGS> below.
=head1 NOTES
@@ -102,6 +104,16 @@ Similar care should be taken to ensure the data is in the correct
format when calling ASN1_STRING_set(), ASN1_STRING_set1_data(), or
ASN1_STRING_set1_string().
+=head1 WARNINGS
+
+Although the buffer returned by ASN1_STRING_to_UTF8() is NUL terminated and
+is therefore a valid C string, the converted data may itself contain embedded
+NUL bytes. Using C string functions such as strlen() or strcmp() on
+the output is therefore inherently dangerous: they will stop at the first
+embedded NUL and silently process only part of the string, which can lead to
+incorrect comparisons or truncated data. Callers should use the returned
+length and not rely on NUL termination.
+
=head1 RETURN VALUES
ASN1_STRING_length() returns the length of the content of I<x>.
diff --git a/test/asn1_internal_test.c b/test/asn1_internal_test.c
index 9ede04cc02..ac2dd9ec8d 100644
--- a/test/asn1_internal_test.c
+++ b/test/asn1_internal_test.c
@@ -591,6 +591,72 @@ static int test_ossl_uni2utf8(void)
return ok;
}
+static int test_asn1_string_to_utf8(void)
+{
+ static const unsigned char bmp[] = { 0x00, 'A', 0x00, 'B' };
+ ASN1_STRING in;
+ unsigned char *out = NULL;
+ int len, ok = 0;
+
+ in.flags = 0;
+
+ /* UTF8String in: same-format path of ASN1_mbstring_copy() */
+ in.type = V_ASN1_UTF8STRING;
+ in.data = (unsigned char *)"ABC";
+ in.length = 3;
+ len = ASN1_STRING_to_UTF8(&out, &in);
+ if (!TEST_int_eq(len, 3)
+ || !TEST_ptr(out)
+ || !TEST_mem_eq(out, len, "ABC", 3)
+ || !TEST_true(out[len] == '\0'))
+ goto err;
+ OPENSSL_free(out);
+ out = NULL;
+
+ /* BMPString in: converting path */
+ in.type = V_ASN1_BMPSTRING;
+ in.data = (unsigned char *)bmp;
+ in.length = (int)sizeof(bmp);
+ len = ASN1_STRING_to_UTF8(&out, &in);
+ if (!TEST_int_eq(len, 2)
+ || !TEST_ptr(out)
+ || !TEST_mem_eq(out, len, "AB", 2)
+ || !TEST_true(out[len] == '\0'))
+ goto err;
+ OPENSSL_free(out);
+ out = NULL;
+
+ /* Empty input still yields a NUL terminated buffer */
+ in.type = V_ASN1_UTF8STRING;
+ in.data = (unsigned char *)"";
+ in.length = 0;
+ len = ASN1_STRING_to_UTF8(&out, &in);
+ if (!TEST_int_eq(len, 0)
+ || !TEST_ptr(out)
+ || !TEST_true(out[0] == '\0'))
+ goto err;
+ OPENSSL_free(out);
+ out = NULL;
+
+ /*
+ * The decoder represents an empty string as a NULL data pointer with a
+ * zero length, not as a pointer to zero bytes, so cover that separately.
+ */
+ in.type = V_ASN1_UTF8STRING;
+ in.data = NULL;
+ in.length = 0;
+ len = ASN1_STRING_to_UTF8(&out, &in);
+ if (!TEST_int_eq(len, 0)
+ || !TEST_ptr(out)
+ || !TEST_true(out[0] == '\0'))
+ goto err;
+
+ ok = 1;
+err:
+ OPENSSL_free(out);
+ return ok;
+}
+
int setup_tests(void)
{
ADD_TEST(test_tbl_standard);
@@ -604,5 +670,6 @@ int setup_tests(void)
ADD_TEST(test_asn1_time_tm_conversions);
ADD_TEST(test_mbstring_ncopy);
ADD_TEST(test_ossl_uni2utf8);
+ ADD_TEST(test_asn1_string_to_utf8);
return 1;
}