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>
Reviewed-by: David Benjamin <davidben@google.com>
This commit is contained in:
Matt Caswell 2021-08-18 12:24:22 +01:00
parent ed3f51ec7d
commit ad6ac17489
3 changed files with 53 additions and 16 deletions

View File

@ -9,6 +9,7 @@
#include <stdio.h>
#include "internal/cryptlib.h"
#include "crypto/x509.h"
#include <openssl/conf.h>
#include <openssl/x509v3.h>
#include <openssl/bio.h>
@ -87,36 +88,41 @@ STACK_OF(CONF_VALUE) *i2v_GENERAL_NAME(X509V3_EXT_METHOD *method,
switch (OBJ_obj2nid(gen->d.otherName->type_id)) {
case NID_id_on_SmtpUTF8Mailbox:
if (gen->d.otherName->value->type != V_ASN1_UTF8STRING
|| !X509V3_add_value_uchar("othername: SmtpUTF8Mailbox:",
|| !x509v3_add_len_value_uchar("othername: SmtpUTF8Mailbox:",
gen->d.otherName->value->value.utf8string->data,
gen->d.otherName->value->value.utf8string->length,
&ret))
return NULL;
break;
case NID_XmppAddr:
if (gen->d.otherName->value->type != V_ASN1_UTF8STRING
|| !X509V3_add_value_uchar("othername: XmppAddr:",
|| !x509v3_add_len_value_uchar("othername: XmppAddr:",
gen->d.otherName->value->value.utf8string->data,
gen->d.otherName->value->value.utf8string->length,
&ret))
return NULL;
break;
case NID_SRVName:
if (gen->d.otherName->value->type != V_ASN1_IA5STRING
|| !X509V3_add_value_uchar("othername: SRVName:",
|| !x509v3_add_len_value_uchar("othername: SRVName:",
gen->d.otherName->value->value.ia5string->data,
gen->d.otherName->value->value.ia5string->length,
&ret))
return NULL;
break;
case NID_ms_upn:
if (gen->d.otherName->value->type != V_ASN1_UTF8STRING
|| !X509V3_add_value_uchar("othername: UPN:",
|| !x509v3_add_len_value_uchar("othername: UPN:",
gen->d.otherName->value->value.utf8string->data,
gen->d.otherName->value->value.utf8string->length,
&ret))
return NULL;
break;
case NID_NAIRealm:
if (gen->d.otherName->value->type != V_ASN1_UTF8STRING
|| !X509V3_add_value_uchar("othername: NAIRealm:",
|| !x509v3_add_len_value_uchar("othername: NAIRealm:",
gen->d.otherName->value->value.utf8string->data,
gen->d.otherName->value->value.utf8string->length,
&ret))
return NULL;
break;
@ -129,14 +135,16 @@ STACK_OF(CONF_VALUE) *i2v_GENERAL_NAME(X509V3_EXT_METHOD *method,
/* check if the value is something printable */
if (gen->d.otherName->value->type == V_ASN1_IA5STRING) {
if (X509V3_add_value_uchar(othername,
if (x509v3_add_len_value_uchar(othername,
gen->d.otherName->value->value.ia5string->data,
gen->d.otherName->value->value.ia5string->length,
&ret))
return ret;
}
if (gen->d.otherName->value->type == V_ASN1_UTF8STRING) {
if (X509V3_add_value_uchar(othername,
if (x509v3_add_len_value_uchar(othername,
gen->d.otherName->value->value.utf8string->data,
gen->d.otherName->value->value.utf8string->length,
&ret))
return ret;
}
@ -157,17 +165,20 @@ STACK_OF(CONF_VALUE) *i2v_GENERAL_NAME(X509V3_EXT_METHOD *method,
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:
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:
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;

View File

@ -12,6 +12,7 @@
#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>
@ -36,17 +37,23 @@ static int ipv6_hex(unsigned char *out, const char *in, int inlen);
/* 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);
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;
if (value != NULL) {
/* We don't allow embeded NUL characters */
if (memchr(value, 0, vallen) != 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)
@ -69,10 +76,26 @@ int X509V3_add_value(const char *name, const char *value,
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)
{
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) */

View File

@ -361,3 +361,6 @@ int ossl_i2d_X448_PUBKEY(const ECX_KEY *a, unsigned char **pp);
EVP_PKEY *ossl_d2i_PUBKEY_legacy(EVP_PKEY **a, const unsigned char **pp,
long length);
#endif
int x509v3_add_len_value_uchar(const char *name, const unsigned char *value,
size_t vallen, STACK_OF(CONF_VALUE) **extlist);