[openssl] master update
Matt Caswell
matt at openssl.org
Mon Nov 4 12:58:01 UTC 2019
The branch master has been updated
via aec9667bd19a8ca9bdd519db3a231a95b9e92674 (commit)
via 45b244620a74248b46ebe1c85e86437b9641447a (commit)
from dcea51afe9e3aec83a0c53f435beec9bc0faa07b (commit)
- Log -----------------------------------------------------------------
commit aec9667bd19a8ca9bdd519db3a231a95b9e92674
Author: Matt Caswell <matt at openssl.org>
Date: Wed Oct 30 13:23:18 2019 +0000
Don't assume the type we read was the type we expected
i2v_GENERAL_NAME and GENERAL_NAME_print were assuming that the type of
of a GENERAL_NAME (OTHERNAME) that we read in was the type we expected
it to be. If its something else then this can cause unexpected
behaviour. In the added fuzz test case an OOB read was occurring.
This issue was recently added by commit 4baee2d.
Credit to OSSFuzz for finding this issue.
Reviewed-by: Dmitry Belyavskiy <beldmit at gmail.com>
Reviewed-by: Viktor Dukhovni <viktor at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/10300)
commit 45b244620a74248b46ebe1c85e86437b9641447a
Author: Matt Caswell <matt at openssl.org>
Date: Wed Oct 30 13:20:33 2019 +0000
Don't leak memory in the event of a failure in i2v_GENERAL_NAMES
i2v_GENERAL_NAMES call i2v_GENERAL_NAME repeatedly as required. Each
time i2v_GENERAL_NAME gets called it allocates adds data to the passed in
stack and then returns a pointer to the stack, or NULL on failure. If
the passed in stack is itself NULL then it allocates one.
i2v_GENERAL_NAMES was not correctly handling the case where a NULL gets
returned from i2v_GENERAL_NAME. If a stack had already been allocated then
it just leaked it.
Reviewed-by: Dmitry Belyavskiy <beldmit at gmail.com>
Reviewed-by: Viktor Dukhovni <viktor at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/10300)
-----------------------------------------------------------------------
Summary of changes:
crypto/x509/v3_alt.c | 64 +++++++++++++++++----
.../x509/9901a721c7fe85b8208198cc5e77ac719f592577 | Bin 0 -> 1329 bytes
2 files changed, 52 insertions(+), 12 deletions(-)
create mode 100644 fuzz/corpora/x509/9901a721c7fe85b8208198cc5e77ac719f592577
diff --git a/crypto/x509/v3_alt.c b/crypto/x509/v3_alt.c
index 5d1ece71cb..f31b884db1 100644
--- a/crypto/x509/v3_alt.c
+++ b/crypto/x509/v3_alt.c
@@ -52,11 +52,24 @@ STACK_OF(CONF_VALUE) *i2v_GENERAL_NAMES(X509V3_EXT_METHOD *method,
{
int i;
GENERAL_NAME *gen;
+ STACK_OF(CONF_VALUE) *tmpret = NULL, *origret = ret;
+
for (i = 0; i < sk_GENERAL_NAME_num(gens); i++) {
gen = sk_GENERAL_NAME_value(gens, i);
- ret = i2v_GENERAL_NAME(method, gen, ret);
+ /*
+ * i2v_GENERAL_NAME allocates ret if it is NULL. If something goes
+ * wrong we need to free the stack - but only if it was empty when we
+ * originally entered this function.
+ */
+ tmpret = i2v_GENERAL_NAME(method, gen, ret);
+ if (tmpret == NULL) {
+ if (origret == NULL)
+ sk_CONF_VALUE_pop_free(ret, X509V3_conf_free);
+ return NULL;
+ }
+ ret = tmpret;
}
- if (!ret)
+ if (ret == NULL)
return sk_CONF_VALUE_new_null();
return ret;
}
@@ -73,19 +86,31 @@ STACK_OF(CONF_VALUE) *i2v_GENERAL_NAME(X509V3_EXT_METHOD *method,
case GEN_OTHERNAME:
switch (OBJ_obj2nid(gen->d.otherName->type_id)) {
case NID_id_on_SmtpUTF8Mailbox:
- if (!X509V3_add_value_uchar("othername: SmtpUTF8Mailbox:", gen->d.otherName->value->value.utf8string->data, &ret))
+ if (gen->d.otherName->value->type != V_ASN1_UTF8STRING
+ || !X509V3_add_value_uchar("othername: SmtpUTF8Mailbox:",
+ gen->d.otherName->value->value.utf8string->data,
+ &ret))
return NULL;
break;
case NID_XmppAddr:
- if (!X509V3_add_value_uchar("othername: XmppAddr:", gen->d.otherName->value->value.utf8string->data, &ret))
+ if (gen->d.otherName->value->type != V_ASN1_UTF8STRING
+ || !X509V3_add_value_uchar("othername: XmppAddr:",
+ gen->d.otherName->value->value.utf8string->data,
+ &ret))
return NULL;
break;
case NID_SRVName:
- if (!X509V3_add_value_uchar("othername: SRVName:", gen->d.otherName->value->value.ia5string->data, &ret))
+ if (gen->d.otherName->value->type != V_ASN1_IA5STRING
+ || !X509V3_add_value_uchar("othername: SRVName:",
+ gen->d.otherName->value->value.ia5string->data,
+ &ret))
return NULL;
break;
case NID_ms_upn:
- if (!X509V3_add_value_uchar("othername: UPN:", gen->d.otherName->value->value.utf8string->data, &ret))
+ if (gen->d.otherName->value->type != V_ASN1_UTF8STRING
+ || !X509V3_add_value_uchar("othername: UPN:",
+ gen->d.otherName->value->value.utf8string->data,
+ &ret))
return NULL;
break;
default:
@@ -161,21 +186,36 @@ STACK_OF(CONF_VALUE) *i2v_GENERAL_NAME(X509V3_EXT_METHOD *method,
int GENERAL_NAME_print(BIO *out, GENERAL_NAME *gen)
{
unsigned char *p;
- int i;
+ int i, nid;
+
switch (gen->type) {
case GEN_OTHERNAME:
- switch (OBJ_obj2nid(gen->d.otherName->type_id)) {
+ nid = OBJ_obj2nid(gen->d.otherName->type_id);
+ /* Validate the types are as we expect before we use them */
+ if ((nid == NID_SRVName
+ && gen->d.otherName->value->type != V_ASN1_IA5STRING)
+ || (nid != NID_SRVName
+ && gen->d.otherName->value->type != V_ASN1_UTF8STRING)) {
+ BIO_printf(out, "othername:<unsupported>");
+ break;
+ }
+
+ switch (nid) {
case NID_id_on_SmtpUTF8Mailbox:
- BIO_printf(out, "othername:SmtpUTF8Mailbox:%s", gen->d.otherName->value->value.utf8string->data);
+ BIO_printf(out, "othername:SmtpUTF8Mailbox:%s",
+ gen->d.otherName->value->value.utf8string->data);
break;
case NID_XmppAddr:
- BIO_printf(out, "othername:XmppAddr:%s", gen->d.otherName->value->value.utf8string->data);
+ BIO_printf(out, "othername:XmppAddr:%s",
+ gen->d.otherName->value->value.utf8string->data);
break;
case NID_SRVName:
- BIO_printf(out, "othername:SRVName:%s", gen->d.otherName->value->value.ia5string->data);
+ BIO_printf(out, "othername:SRVName:%s",
+ gen->d.otherName->value->value.ia5string->data);
break;
case NID_ms_upn:
- BIO_printf(out, "othername:UPN:%s", gen->d.otherName->value->value.utf8string->data);
+ BIO_printf(out, "othername:UPN:%s",
+ gen->d.otherName->value->value.utf8string->data);
break;
default:
BIO_printf(out, "othername:<unsupported>");
diff --git a/fuzz/corpora/x509/9901a721c7fe85b8208198cc5e77ac719f592577 b/fuzz/corpora/x509/9901a721c7fe85b8208198cc5e77ac719f592577
new file mode 100644
index 0000000000..40369cd294
Binary files /dev/null and b/fuzz/corpora/x509/9901a721c7fe85b8208198cc5e77ac719f592577 differ
More information about the openssl-commits
mailing list