commit 4f9a7ba47ef46f67d300913460977bcd0a60edfa
author David Benjamin <davidben@google.com> Tue Aug 24 10:28:13 2021 -0400
committer Boringssl LUCI CQ <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com> Tue Aug 24 21:57:37 2021 +0000
tree cbe25686a46806749e13142472ad9c71df957aff
parent 954506271422245ac264fc8f4d0598edb59ddf75

Do not rely on ASN1_STRING being NUL-terminated.

This imports part of the fix for CVE-2021-3712, commits
d9d838ddc0ed083fb4c26dd067e71aad7c65ad16,
5f54e57406ca17731b9ade3afd561d3c652e07f2,
23446958685a593d4d9434475734b99138902ed2,
and bb4d2ed4091408404e18b3326e3df67848ef63d0 from upstream. The
others will be imported in follow-up CLs.

Change-Id: Ic35aeb3895935ee94b82a295efade32782e8d1bc
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49005
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>

crypto/x509/t_x509a.c

diff --git a/crypto/x509/t_x509a.c b/crypto/x509/t_x509a.c
index 7fbb47b..4c7b212 100644
--- a/crypto/x509/t_x509a.c
+++ b/crypto/x509/t_x509a.c
@@ -102,8 +102,10 @@
         BIO_puts(out, "\n");
     } else
         BIO_printf(out, "%*sNo Rejected Uses.\n", indent, "");
-    if (aux->alias)
-        BIO_printf(out, "%*sAlias: %s\n", indent, "", aux->alias->data);
+    if (aux->alias) {
+        BIO_printf(out, "%*sAlias: %.*s\n", indent, "", aux->alias->length,
+                   aux->alias->data);
+    }
     if (aux->keyid) {
         BIO_printf(out, "%*sKey Id: ", indent, "");
         for (j = 0; j < aux->keyid->length; j++)

crypto/x509v3/v3_cpols.c

diff --git a/crypto/x509v3/v3_cpols.c b/crypto/x509v3/v3_cpols.c
index a2ed01c..9f66f47 100644
--- a/crypto/x509v3/v3_cpols.c
+++ b/crypto/x509v3/v3_cpols.c
@@ -432,8 +432,8 @@
         qualinfo = sk_POLICYQUALINFO_value(quals, i);
         switch (OBJ_obj2nid(qualinfo->pqualid)) {
         case NID_id_qt_cps:
-            BIO_printf(out, "%*sCPS: %s\n", indent, "",
-                       qualinfo->d.cpsuri->data);
+            BIO_printf(out, "%*sCPS: %.*s\n", indent, "",
+                       qualinfo->d.cpsuri->length, qualinfo->d.cpsuri->data);
             break;
 
         case NID_id_qt_unotice:
@@ -457,8 +457,8 @@
     if (notice->noticeref) {
         NOTICEREF *ref;
         ref = notice->noticeref;
-        BIO_printf(out, "%*sOrganization: %s\n", indent, "",
-                   ref->organization->data);
+        BIO_printf(out, "%*sOrganization: %.*s\n", indent, "",
+                   ref->organization->length, ref->organization->data);
         BIO_printf(out, "%*sNumber%s: ", indent, "",
                    sk_ASN1_INTEGER_num(ref->noticenos) > 1 ? "s" : "");
         for (i = 0; i < sk_ASN1_INTEGER_num(ref->noticenos); i++) {
@@ -480,8 +480,8 @@
         BIO_puts(out, "\n");
     }
     if (notice->exptext)
-        BIO_printf(out, "%*sExplicit Text: %s\n", indent, "",
-                   notice->exptext->data);
+        BIO_printf(out, "%*sExplicit Text: %.*s\n", indent, "",
+                   notice->exptext->length, notice->exptext->data);
 }
 
 void X509_POLICY_NODE_print(BIO *out, X509_POLICY_NODE *node, int indent)

crypto/x509v3/v3_pci.c

diff --git a/crypto/x509v3/v3_pci.c b/crypto/x509v3/v3_pci.c
index f9031c0..57b64ef 100644
--- a/crypto/x509v3/v3_pci.c
+++ b/crypto/x509v3/v3_pci.c
@@ -75,7 +75,8 @@
     i2a_ASN1_OBJECT(out, pci->proxyPolicy->policyLanguage);
     BIO_puts(out, "\n");
     if (pci->proxyPolicy->policy && pci->proxyPolicy->policy->data)
-        BIO_printf(out, "%*sPolicy Text: %s\n", indent, "",
+        BIO_printf(out, "%*sPolicy Text: %.*s\n", indent, "",
+                   pci->proxyPolicy->policy->length,
                    pci->proxyPolicy->policy->data);
     return 1;
 }

crypto/x509v3/v3_utl.c

diff --git a/crypto/x509v3/v3_utl.c b/crypto/x509v3/v3_utl.c
index 0b4ad80..695e224 100644
--- a/crypto/x509v3/v3_utl.c
+++ b/crypto/x509v3/v3_utl.c
@@ -631,27 +631,45 @@
 
 static int append_ia5(STACK_OF(OPENSSL_STRING) **sk, ASN1_IA5STRING *email)
 {
-    char *emtmp;
     /* First some sanity checks */
     if (email->type != V_ASN1_IA5STRING)
         return 1;
-    if (!email->data || !email->length)
+    if (email->data == NULL || email->length == 0)
         return 1;
+    /* |OPENSSL_STRING| cannot represent strings with embedded NULs. Do not
+     * report them as outputs. */
+    if (OPENSSL_memchr(email->data, 0, email->length) != NULL)
+        return 1;
+
+    char *emtmp = NULL;
     if (!*sk)
         *sk = sk_OPENSSL_STRING_new(sk_strcmp);
     if (!*sk)
-        return 0;
+        goto err;
+
+    emtmp = OPENSSL_strndup((char *)email->data, email->length);
+    if (emtmp == NULL) {
+        goto err;
+    }
+
     /* Don't add duplicates */
     sk_OPENSSL_STRING_sort(*sk);
-    if (sk_OPENSSL_STRING_find(*sk, NULL, (char *)email->data))
+    if (sk_OPENSSL_STRING_find(*sk, NULL, emtmp)) {
+        OPENSSL_free(emtmp);
         return 1;
-    emtmp = OPENSSL_strdup((char *)email->data);
-    if (!emtmp || !sk_OPENSSL_STRING_push(*sk, emtmp)) {
-        X509_email_free(*sk);
-        *sk = NULL;
-        return 0;
+    }
+    if (!sk_OPENSSL_STRING_push(*sk, emtmp)) {
+        goto err;
     }
     return 1;
+
+err:
+    /* TODO(davidben): Fix the error-handling in this file. It currently relies
+     * on |append_ia5| leaving |*sk| at NULL on error. */
+    OPENSSL_free(emtmp);
+    X509_email_free(*sk);
+    *sk = NULL;
+    return 0;
 }
 
 void X509_email_free(STACK_OF(OPENSSL_STRING) *sk)