Fix i2v_GENERAL_NAME to not assume NUL terminated strings

ASN.1 strings may not be NUL terminated. Don't assume they are.

CVE-2021-3712

Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>

diff --git a/crypto/x509v3/v3_alt.c b/crypto/x509v3/v3_alt.c
index 4dce0041012e44b755df4cb141cf85f348f35b85..6e5f9f8b0eac676c23e3c42aedc46dc55a1e62bb 100644 (file)
--- a/crypto/x509v3/v3_alt.c
+++ b/crypto/x509v3/v3_alt.c
@@ -9,6 +9,7 @@
 
 #include <stdio.h>
 #include "internal/cryptlib.h"
 
 #include <stdio.h>
 #include "internal/cryptlib.h"

+#include "crypto/x509.h"

 #include <openssl/conf.h>
 #include <openssl/x509v3.h>
 #include "ext_dat.h"
 #include <openssl/conf.h>
 #include <openssl/x509v3.h>
 #include "ext_dat.h"


@@ -99,17 +100,20 @@ STACK_OF(CONF_VALUE) *i2v_GENERAL_NAME(X509V3_EXT_METHOD *method,
         break;
 
     case GEN_EMAIL:
         break;
 
     case GEN_EMAIL:

-        if (!X509V3_add_value_uchar("email", gen->d.ia5->data, &ret))
+        if (!x509v3_add_len_value_uchar("email", gen->d.ia5->data,
+                                        gen->d.ia5->length, &ret))

             return NULL;
         break;
 
     case GEN_DNS:
             return NULL;
         break;
 
     case GEN_DNS:

-        if (!X509V3_add_value_uchar("DNS", gen->d.ia5->data, &ret))
+        if (!x509v3_add_len_value_uchar("DNS", gen->d.ia5->data,
+                                        gen->d.ia5->length, &ret))

             return NULL;
         break;
 
     case GEN_URI:
             return NULL;
         break;
 
     case GEN_URI:

-        if (!X509V3_add_value_uchar("URI", gen->d.ia5->data, &ret))
+        if (!x509v3_add_len_value_uchar("URI", gen->d.ia5->data,
+                                        gen->d.ia5->length, &ret))

             return NULL;
         break;
 
             return NULL;
         break;
 

diff --git a/crypto/x509v3/v3_utl.c b/crypto/x509v3/v3_utl.c
index 7281a7b917a8cc127ef019cd8339ffb5eb6e1e51..004ef55df96b84d3b7ba0a6b58309173d4da64c2 100644 (file)
--- a/crypto/x509v3/v3_utl.c
+++ b/crypto/x509v3/v3_utl.c
@@ -12,6 +12,7 @@
 #include "e_os.h"
 #include "internal/cryptlib.h"
 #include <stdio.h>
 #include "e_os.h"
 #include "internal/cryptlib.h"
 #include <stdio.h>

+#include <string.h>

 #include "crypto/ctype.h"
 #include <openssl/conf.h>
 #include <openssl/crypto.h>
 #include "crypto/ctype.h"
 #include <openssl/conf.h>
 #include <openssl/crypto.h>


@@ -34,17 +35,26 @@ static int ipv6_hex(unsigned char *out, const char *in, int inlen);
 
 /* Add a CONF_VALUE name value pair to stack */
 
 
 /* Add a CONF_VALUE name value pair to stack */
 

-int X509V3_add_value(const char *name, const char *value,
-                     STACK_OF(CONF_VALUE) **extlist)
+static int x509v3_add_len_value(const char *name, const char *value,
+                                size_t vallen, STACK_OF(CONF_VALUE) **extlist)

 {
     CONF_VALUE *vtmp = NULL;
     char *tname = NULL, *tvalue = NULL;
     int sk_allocated = (*extlist == NULL);
 
 {
     CONF_VALUE *vtmp = NULL;
     char *tname = NULL, *tvalue = NULL;
     int sk_allocated = (*extlist == NULL);
 

-    if (name && (tname = OPENSSL_strdup(name)) == NULL)
-        goto err;
-    if (value && (tvalue = OPENSSL_strdup(value)) == NULL)
+    if (name != NULL && (tname = OPENSSL_strdup(name)) == NULL)

         goto err;
         goto err;

+    if (value != NULL && vallen > 0) {
+        /*
+         * We tolerate a single trailing NUL character, but otherwise no
+         * embedded NULs
+         */
+        if (memchr(value, 0, vallen - 1) != NULL)
+            goto err;
+        tvalue = OPENSSL_strndup(value, vallen);
+        if (tvalue == NULL)
+            goto err;
+    }

     if ((vtmp = OPENSSL_malloc(sizeof(*vtmp))) == NULL)
         goto err;
     if (sk_allocated && (*extlist = sk_CONF_VALUE_new_null()) == NULL)
     if ((vtmp = OPENSSL_malloc(sizeof(*vtmp))) == NULL)
         goto err;
     if (sk_allocated && (*extlist = sk_CONF_VALUE_new_null()) == NULL)


@@ -67,10 +77,26 @@ int X509V3_add_value(const char *name, const char *value,
     return 0;
 }
 
     return 0;
 }
 

+int X509V3_add_value(const char *name, const char *value,
+                     STACK_OF(CONF_VALUE) **extlist)
+{
+    return x509v3_add_len_value(name, value,
+                                value != NULL ? strlen((const char *)value) : 0,
+                                extlist);
+}
+

 int X509V3_add_value_uchar(const char *name, const unsigned char *value,
                            STACK_OF(CONF_VALUE) **extlist)
 {
 int X509V3_add_value_uchar(const char *name, const unsigned char *value,
                            STACK_OF(CONF_VALUE) **extlist)
 {

-    return X509V3_add_value(name, (const char *)value, extlist);
+    return x509v3_add_len_value(name, (const char *)value,
+                                value != NULL ? strlen((const char *)value) : 0,
+                                extlist);
+}
+
+int x509v3_add_len_value_uchar(const char *name, const unsigned char *value,
+                               size_t vallen, STACK_OF(CONF_VALUE) **extlist)
+{
+    return x509v3_add_len_value(name, (const char *)value, vallen, extlist);

 }
 
 /* Free function for STACK_OF(CONF_VALUE) */
 }
 
 /* Free function for STACK_OF(CONF_VALUE) */

diff --git a/include/crypto/x509.h b/include/crypto/x509.h
index b53c2b03c39ec4625f0439a8c2e61c51edcca724..7ffb8abfe71b4cd1ed5780fd843c3013d2108f0a 100644 (file)
--- a/include/crypto/x509.h
+++ b/include/crypto/x509.h
@@ -8,6 +8,8 @@
  */
 
 #include "internal/refcount.h"
  */
 
 #include "internal/refcount.h"

+#include <openssl/x509.h>
+#include <openssl/conf.h>

 
 /* Internal X509 structures and functions: not for application use */
 
 
 /* Internal X509 structures and functions: not for application use */
 


@@ -284,3 +286,6 @@ int a2i_ipadd(unsigned char *ipout, const char *ipasc);
 int x509_set1_time(ASN1_TIME **ptm, const ASN1_TIME *tm);
 
 void x509_init_sig_info(X509 *x);
 int x509_set1_time(ASN1_TIME **ptm, const ASN1_TIME *tm);
 
 void x509_init_sig_info(X509 *x);

+
+int x509v3_add_len_value_uchar(const char *name, const unsigned char *value,
+                               size_t vallen, STACK_OF(CONF_VALUE) **extlist);