Skip to content

Commit

Permalink
Fix i2v_GENERAL_NAME to not assume NUL terminated strings
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
mattcaswell committed Aug 24, 2021
1 parent 4f850d7 commit 174ba80
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 9 deletions.
10 changes: 7 additions & 3 deletions crypto/x509v3/v3_alt.c
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#include <stdio.h>
#include "internal/cryptlib.h"
#include "crypto/x509.h"
#include <openssl/conf.h>
#include <openssl/x509v3.h>
#include "ext_dat.h"
Expand Down Expand Up @@ -99,17 +100,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;

Expand Down
38 changes: 32 additions & 6 deletions crypto/x509v3/v3_utl.c
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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 */

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 && 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)
Expand All @@ -67,10 +77,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) */
Expand Down
5 changes: 5 additions & 0 deletions include/crypto/x509.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
*/

#include "internal/refcount.h"
#include <openssl/x509.h>
#include <openssl/conf.h>

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

Expand Down Expand Up @@ -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 x509v3_add_len_value_uchar(const char *name, const unsigned char *value,
size_t vallen, STACK_OF(CONF_VALUE) **extlist);

0 comments on commit 174ba80

Please sign in to comment.