From 47d550e862006e784525d6cd63eae9741dcef18c Mon Sep 17 00:00:00 2001 From: Patrick Wildt Date: Thu, 19 Mar 2015 15:48:00 +0100 Subject: [PATCH] OpenBSD 5.6 errata 20, March 19, 2015 Fix several crash causing defects from OpenSSL. These include: CVE-2015-0209 - Use After Free following d2i_ECPrivatekey error CVE-2015-0286 - Segmentation fault in ASN1_TYPE_cmp CVE-2015-0287 - ASN.1 structure reuse memory corruption CVE-2015-0288 - X509_to_X509_REQ NULL pointer deref CVE-2015-0289 - PKCS7 NULL pointer dereferences Several other issues did not apply or were already fixed. Refer to https://www.openssl.org/news/secadv_20150319.txt cherry-pick patrick@ ok pedro@ --- lib/libssl/src/crypto/asn1/a_int.c | 4 +- lib/libssl/src/crypto/asn1/a_set.c | 2 +- lib/libssl/src/crypto/asn1/a_type.c | 4 +- lib/libssl/src/crypto/asn1/d2i_pr.c | 2 +- lib/libssl/src/crypto/asn1/d2i_pu.c | 2 +- lib/libssl/src/crypto/asn1/n_pkey.c | 8 +-- lib/libssl/src/crypto/asn1/tasn_dec.c | 25 ++++++- lib/libssl/src/crypto/asn1/x_x509.c | 14 ++-- lib/libssl/src/crypto/ec/ec_asn1.c | 39 +++++------ lib/libssl/src/crypto/pkcs7/pk7_doit.c | 96 ++++++++++++++++++++++---- lib/libssl/src/crypto/pkcs7/pk7_lib.c | 2 + lib/libssl/src/crypto/x509/x509_req.c | 4 +- lib/libssl/src/ssl/d1_lib.c | 3 + 13 files changed, 153 insertions(+), 52 deletions(-) diff --git a/lib/libssl/src/crypto/asn1/a_int.c b/lib/libssl/src/crypto/asn1/a_int.c index ba2895db6b0..3bf120cc98b 100644 --- a/lib/libssl/src/crypto/asn1/a_int.c +++ b/lib/libssl/src/crypto/asn1/a_int.c @@ -268,7 +268,7 @@ c2i_ASN1_INTEGER(ASN1_INTEGER **a, const unsigned char **pp, long len) err: ASN1err(ASN1_F_C2I_ASN1_INTEGER, i); - if ((ret != NULL) && ((a == NULL) || (*a != ret))) + if (a == NULL || *a != ret) M_ASN1_INTEGER_free(ret); return (NULL); } @@ -335,7 +335,7 @@ d2i_ASN1_UINTEGER(ASN1_INTEGER **a, const unsigned char **pp, long length) err: ASN1err(ASN1_F_D2I_ASN1_UINTEGER, i); - if ((ret != NULL) && ((a == NULL) || (*a != ret))) + if (a == NULL || *a != ret) M_ASN1_INTEGER_free(ret); return (NULL); } diff --git a/lib/libssl/src/crypto/asn1/a_set.c b/lib/libssl/src/crypto/asn1/a_set.c index ba4f28be34d..4a14d3fd098 100644 --- a/lib/libssl/src/crypto/asn1/a_set.c +++ b/lib/libssl/src/crypto/asn1/a_set.c @@ -225,7 +225,7 @@ d2i_ASN1_SET(STACK_OF(OPENSSL_BLOCK) **a, const unsigned char **pp, long length, return ret; err: - if (ret != NULL && (a == NULL || *a != ret)) { + if (a == NULL || *a != ret) { if (free_func != NULL) sk_OPENSSL_BLOCK_pop_free(ret, free_func); else diff --git a/lib/libssl/src/crypto/asn1/a_type.c b/lib/libssl/src/crypto/asn1/a_type.c index add5ff12534..348dac93ef6 100644 --- a/lib/libssl/src/crypto/asn1/a_type.c +++ b/lib/libssl/src/crypto/asn1/a_type.c @@ -122,7 +122,9 @@ ASN1_TYPE_cmp(ASN1_TYPE *a, ASN1_TYPE *b) case V_ASN1_OBJECT: result = OBJ_cmp(a->value.object, b->value.object); break; - + case V_ASN1_BOOLEAN: + result = a->value.boolean - b->value.boolean; + break; case V_ASN1_NULL: result = 0; /* They do not have content. */ break; diff --git a/lib/libssl/src/crypto/asn1/d2i_pr.c b/lib/libssl/src/crypto/asn1/d2i_pr.c index 2deec613ed9..84ec9e74da8 100644 --- a/lib/libssl/src/crypto/asn1/d2i_pr.c +++ b/lib/libssl/src/crypto/asn1/d2i_pr.c @@ -117,7 +117,7 @@ d2i_PrivateKey(int type, EVP_PKEY **a, const unsigned char **pp, long length) return (ret); err: - if ((ret != NULL) && ((a == NULL) || (*a != ret))) + if (a == NULL || *a != ret) EVP_PKEY_free(ret); return (NULL); } diff --git a/lib/libssl/src/crypto/asn1/d2i_pu.c b/lib/libssl/src/crypto/asn1/d2i_pu.c index df6fea4af52..b1f69bf5b6f 100644 --- a/lib/libssl/src/crypto/asn1/d2i_pu.c +++ b/lib/libssl/src/crypto/asn1/d2i_pu.c @@ -130,7 +130,7 @@ d2i_PublicKey(int type, EVP_PKEY **a, const unsigned char **pp, long length) return (ret); err: - if ((ret != NULL) && ((a == NULL) || (*a != ret))) + if (a == NULL || *a != ret) EVP_PKEY_free(ret); return (NULL); } diff --git a/lib/libssl/src/crypto/asn1/n_pkey.c b/lib/libssl/src/crypto/asn1/n_pkey.c index 42431b6e24b..331e6e73233 100644 --- a/lib/libssl/src/crypto/asn1/n_pkey.c +++ b/lib/libssl/src/crypto/asn1/n_pkey.c @@ -250,11 +250,11 @@ d2i_RSA_NET(RSA **a, const unsigned char **pp, long length, return NULL; } - if ((enckey->os->length != 11) || (strncmp("private-key", - (char *)enckey->os->data, 11) != 0)) { + /* XXX 11 == strlen("private-key") */ + if (enckey->os->length != 11 || + memcmp("private-key", enckey->os->data, 11) != 0) { ASN1err(ASN1_F_D2I_RSA_NET, ASN1_R_PRIVATE_KEY_HEADER_MISSING); - NETSCAPE_ENCRYPTED_PKEY_free(enckey); - return NULL; + goto err; } if (OBJ_obj2nid(enckey->enckey->algor->algorithm) != NID_rc4) { ASN1err(ASN1_F_D2I_RSA_NET, diff --git a/lib/libssl/src/crypto/asn1/tasn_dec.c b/lib/libssl/src/crypto/asn1/tasn_dec.c index f633d03e9c3..8e2efc20ace 100644 --- a/lib/libssl/src/crypto/asn1/tasn_dec.c +++ b/lib/libssl/src/crypto/asn1/tasn_dec.c @@ -304,8 +304,16 @@ ASN1_item_ex_d2i(ASN1_VALUE **pval, const unsigned char **in, long len, if (asn1_cb && !asn1_cb(ASN1_OP_D2I_PRE, pval, it, NULL)) goto auxerr; - /* Allocate structure */ - if (!*pval && !ASN1_item_ex_new(pval, it)) { + if (*pval) { + /* Free up and zero CHOICE value if initialised */ + i = asn1_get_choice_selector(pval, it); + if ((i >= 0) && (i < it->tcount)) { + tt = it->templates + i; + pchptr = asn1_get_field_ptr(pval, tt); + ASN1_template_free(pchptr, tt); + asn1_set_choice_selector(pval, -1, it); + } + } else if (!ASN1_item_ex_new(pval, it)) { ASN1err(ASN1_F_ASN1_ITEM_EX_D2I, ERR_R_NESTED_ASN1_ERROR); goto err; @@ -391,6 +399,19 @@ ASN1_item_ex_d2i(ASN1_VALUE **pval, const unsigned char **in, long len, if (asn1_cb && !asn1_cb(ASN1_OP_D2I_PRE, pval, it, NULL)) goto auxerr; + /* Free up and zero any ADB found */ + for (i = 0, tt = it->templates; i < it->tcount; i++, tt++) { + if (tt->flags & ASN1_TFLG_ADB_MASK) { + const ASN1_TEMPLATE *seqtt; + ASN1_VALUE **pseqval; + seqtt = asn1_do_adb(pval, tt, 1); + if (!seqtt) + goto err; + pseqval = asn1_get_field_ptr(pval, seqtt); + ASN1_template_free(pseqval, seqtt); + } + } + /* Get each field entry */ for (i = 0, tt = it->templates; i < it->tcount; i++, tt++) { const ASN1_TEMPLATE *seqtt; diff --git a/lib/libssl/src/crypto/asn1/x_x509.c b/lib/libssl/src/crypto/asn1/x_x509.c index 0236a0faa27..c61b58e9f10 100644 --- a/lib/libssl/src/crypto/asn1/x_x509.c +++ b/lib/libssl/src/crypto/asn1/x_x509.c @@ -177,16 +177,20 @@ d2i_X509_AUX(X509 **a, const unsigned char **pp, long length) /* Save start position */ q = *pp; - ret = d2i_X509(a, pp, length); + ret = d2i_X509(NULL, pp, length); /* If certificate unreadable then forget it */ if (!ret) return NULL; /* update length */ length -= *pp - q; - if (!length) - return ret; - if (!d2i_X509_CERT_AUX(&ret->aux, pp, length)) - goto err; + if (length > 0) { + if (!d2i_X509_CERT_AUX(&ret->aux, pp, length)) + goto err; + } + if (a != NULL) { + X509_free(*a); + *a = ret; + } return ret; err: diff --git a/lib/libssl/src/crypto/ec/ec_asn1.c b/lib/libssl/src/crypto/ec/ec_asn1.c index c54f6593d59..439d7798efd 100644 --- a/lib/libssl/src/crypto/ec/ec_asn1.c +++ b/lib/libssl/src/crypto/ec/ec_asn1.c @@ -918,19 +918,19 @@ d2i_ECPKParameters(EC_GROUP ** a, const unsigned char **in, long len) if ((params = d2i_ECPKPARAMETERS(NULL, in, len)) == NULL) { ECerr(EC_F_D2I_ECPKPARAMETERS, EC_R_D2I_ECPKPARAMETERS_FAILURE); - ECPKPARAMETERS_free(params); - return NULL; + goto err; } if ((group = ec_asn1_pkparameters2group(params)) == NULL) { ECerr(EC_F_D2I_ECPKPARAMETERS, EC_R_PKPARAMETERS2GROUP_FAILURE); - ECPKPARAMETERS_free(params); - return NULL; + goto err; } - if (a && *a) + + if (a != NULL) { EC_GROUP_clear_free(*a); - if (a) *a = group; + } +err: ECPKPARAMETERS_free(params); return (group); } @@ -958,7 +958,6 @@ i2d_ECPKParameters(const EC_GROUP * a, unsigned char **out) EC_KEY * d2i_ECPrivateKey(EC_KEY ** a, const unsigned char **in, long len) { - int ok = 0; EC_KEY *ret = NULL; EC_PRIVATEKEY *priv_key = NULL; @@ -973,12 +972,9 @@ d2i_ECPrivateKey(EC_KEY ** a, const unsigned char **in, long len) } if (a == NULL || *a == NULL) { if ((ret = EC_KEY_new()) == NULL) { - ECerr(EC_F_D2I_ECPRIVATEKEY, - ERR_R_MALLOC_FAILURE); + ECerr(EC_F_D2I_ECPRIVATEKEY, ERR_R_MALLOC_FAILURE); goto err; } - if (a) - *a = ret; } else ret = *a; @@ -1028,17 +1024,19 @@ d2i_ECPrivateKey(EC_KEY ** a, const unsigned char **in, long len) goto err; } } - ok = 1; + + EC_PRIVATEKEY_free(priv_key); + if (a != NULL) + *a = ret; + return (ret); + err: - if (!ok) { - if (ret) - EC_KEY_free(ret); - ret = NULL; - } + if (a == NULL || *a != ret) + EC_KEY_free(ret); if (priv_key) EC_PRIVATEKEY_free(priv_key); - return (ret); + return (NULL); } int @@ -1151,8 +1149,6 @@ d2i_ECParameters(EC_KEY ** a, const unsigned char **in, long len) ECerr(EC_F_D2I_ECPARAMETERS, ERR_R_MALLOC_FAILURE); return NULL; } - if (a) - *a = ret; } else ret = *a; @@ -1160,6 +1156,9 @@ d2i_ECParameters(EC_KEY ** a, const unsigned char **in, long len) ECerr(EC_F_D2I_ECPARAMETERS, ERR_R_EC_LIB); return NULL; } + + if (a != NULL) + *a = ret; return ret; } diff --git a/lib/libssl/src/crypto/pkcs7/pk7_doit.c b/lib/libssl/src/crypto/pkcs7/pk7_doit.c index 8f1e3936356..13e12ace5aa 100644 --- a/lib/libssl/src/crypto/pkcs7/pk7_doit.c +++ b/lib/libssl/src/crypto/pkcs7/pk7_doit.c @@ -261,6 +261,28 @@ PKCS7_dataInit(PKCS7 *p7, BIO *bio) PKCS7_RECIP_INFO *ri = NULL; ASN1_OCTET_STRING *os = NULL; + if (p7 == NULL) { + PKCS7err(PKCS7_F_PKCS7_DATAINIT, PKCS7_R_INVALID_NULL_POINTER); + return NULL; + } + + /* + * The content field in the PKCS7 ContentInfo is optional, + * but that really only applies to inner content (precisely, + * detached signatures). + * + * When reading content, missing outer content is therefore + * treated as an error. + * + * When creating content, PKCS7_content_new() must be called + * before calling this method, so a NULL p7->d is always + * an error. + */ + if (p7->d.ptr == NULL) { + PKCS7err(PKCS7_F_PKCS7_DATAINIT, PKCS7_R_NO_CONTENT); + return NULL; + } + i = OBJ_obj2nid(p7->type); p7->state = PKCS7_S_HEADER; @@ -418,6 +440,17 @@ PKCS7_dataDecode(PKCS7 *p7, EVP_PKEY *pkey, BIO *in_bio, X509 *pcert) unsigned char *ek = NULL, *tkey = NULL; int eklen = 0, tkeylen = 0; + if (p7 == NULL) { + PKCS7err(PKCS7_F_PKCS7_DATADECODE, + PKCS7_R_INVALID_NULL_POINTER); + return NULL; + } + + if (p7->d.ptr == NULL) { + PKCS7err(PKCS7_F_PKCS7_DATADECODE, PKCS7_R_NO_CONTENT); + return NULL; + } + i = OBJ_obj2nid(p7->type); p7->state = PKCS7_S_HEADER; @@ -713,6 +746,17 @@ PKCS7_dataFinal(PKCS7 *p7, BIO *bio) STACK_OF(PKCS7_SIGNER_INFO) *si_sk = NULL; ASN1_OCTET_STRING *os = NULL; + if (p7 == NULL) { + PKCS7err(PKCS7_F_PKCS7_DATAFINAL, + PKCS7_R_INVALID_NULL_POINTER); + return 0; + } + + if (p7->d.ptr == NULL) { + PKCS7err(PKCS7_F_PKCS7_DATAFINAL, PKCS7_R_NO_CONTENT); + return 0; + } + EVP_MD_CTX_init(&ctx_tmp); i = OBJ_obj2nid(p7->type); p7->state = PKCS7_S_HEADER; @@ -758,6 +802,7 @@ PKCS7_dataFinal(PKCS7 *p7, BIO *bio) /* If detached data then the content is excluded */ if (PKCS7_type_is_data(p7->d.sign->contents) && p7->detached) { M_ASN1_OCTET_STRING_free(os); + os = NULL; p7->d.sign->contents->d.data = NULL; } break; @@ -772,6 +817,7 @@ PKCS7_dataFinal(PKCS7 *p7, BIO *bio) if (PKCS7_type_is_data(p7->d.digest->contents) && p7->detached) { M_ASN1_OCTET_STRING_free(os); + os = NULL; p7->d.digest->contents->d.data = NULL; } break; @@ -837,22 +883,32 @@ PKCS7_dataFinal(PKCS7 *p7, BIO *bio) M_ASN1_OCTET_STRING_set(p7->d.digest->digest, md_data, md_len); } - if (!PKCS7_is_detached(p7) && !(os->flags & ASN1_STRING_FLAG_NDEF)) { - char *cont; - long contlen; - btmp = BIO_find_type(bio, BIO_TYPE_MEM); - if (btmp == NULL) { - PKCS7err(PKCS7_F_PKCS7_DATAFINAL, - PKCS7_R_UNABLE_TO_FIND_MEM_BIO); + if (!PKCS7_is_detached(p7)) { + /* + * NOTE: only reach os == NULL here because detached + * digested data support is broken? + */ + if (os == NULL) goto err; + if (!(os->flags & ASN1_STRING_FLAG_NDEF)) { + char *cont; + long contlen; + + btmp = BIO_find_type(bio, BIO_TYPE_MEM); + if (btmp == NULL) { + PKCS7err(PKCS7_F_PKCS7_DATAFINAL, + PKCS7_R_UNABLE_TO_FIND_MEM_BIO); + goto err; + } + contlen = BIO_get_mem_data(btmp, &cont); + /* + * Mark the BIO read only then we can use its copy + * of the data instead of making an extra copy. + */ + BIO_set_flags(btmp, BIO_FLAGS_MEM_RDONLY); + BIO_set_mem_eof_return(btmp, 0); + ASN1_STRING_set0(os, (unsigned char *)cont, contlen); } - contlen = BIO_get_mem_data(btmp, &cont); - /* Mark the BIO read only then we can use its copy of the data - * instead of making an extra copy. - */ - BIO_set_flags(btmp, BIO_FLAGS_MEM_RDONLY); - BIO_set_mem_eof_return(btmp, 0); - ASN1_STRING_set0(os, (unsigned char *)cont, contlen); } ret = 1; err: @@ -927,6 +983,17 @@ PKCS7_dataVerify(X509_STORE *cert_store, X509_STORE_CTX *ctx, BIO *bio, STACK_OF(X509) *cert; X509 *x509; + if (p7 == NULL) { + PKCS7err(PKCS7_F_PKCS7_DATAVERIFY, + PKCS7_R_INVALID_NULL_POINTER); + return 0; + } + + if (p7->d.ptr == NULL) { + PKCS7err(PKCS7_F_PKCS7_DATAVERIFY, PKCS7_R_NO_CONTENT); + return 0; + } + if (PKCS7_type_is_signed(p7)) { cert = p7->d.sign->cert; } else if (PKCS7_type_is_signedAndEnveloped(p7)) { @@ -963,6 +1030,7 @@ PKCS7_dataVerify(X509_STORE *cert_store, X509_STORE_CTX *ctx, BIO *bio, return PKCS7_signatureVerify(bio, p7, si, x509); err: + return ret; } diff --git a/lib/libssl/src/crypto/pkcs7/pk7_lib.c b/lib/libssl/src/crypto/pkcs7/pk7_lib.c index 27370800c97..b614e3b2ddf 100644 --- a/lib/libssl/src/crypto/pkcs7/pk7_lib.c +++ b/lib/libssl/src/crypto/pkcs7/pk7_lib.c @@ -460,6 +460,8 @@ PKCS7_set_digest(PKCS7 *p7, const EVP_MD *md) STACK_OF(PKCS7_SIGNER_INFO) * PKCS7_get_signer_info(PKCS7 *p7) { + if (p7 == NULL || p7->d.ptr == NULL) + return (NULL); if (PKCS7_type_is_signed(p7)) { return (p7->d.sign->signer_info); } else if (PKCS7_type_is_signedAndEnveloped(p7)) { diff --git a/lib/libssl/src/crypto/x509/x509_req.c b/lib/libssl/src/crypto/x509/x509_req.c index 22d2124614e..7b5b3f22a85 100644 --- a/lib/libssl/src/crypto/x509/x509_req.c +++ b/lib/libssl/src/crypto/x509/x509_req.c @@ -95,7 +95,9 @@ X509_to_X509_REQ(X509 *x, EVP_PKEY *pkey, const EVP_MD *md) if (!X509_REQ_set_subject_name(ret, X509_get_subject_name(x))) goto err; - pktmp = X509_get_pubkey(x); + if ((pktmp = X509_get_pubkey(x)) == NULL) + goto err; + i = X509_REQ_set_pubkey(ret, pktmp); EVP_PKEY_free(pktmp); if (!i) diff --git a/lib/libssl/src/ssl/d1_lib.c b/lib/libssl/src/ssl/d1_lib.c index ff78d0cf3ae..3abe8132d3e 100644 --- a/lib/libssl/src/ssl/d1_lib.c +++ b/lib/libssl/src/ssl/d1_lib.c @@ -449,6 +449,9 @@ dtls1_listen(SSL *s, struct sockaddr *client) { int ret; + /* Ensure there is no state left over from a previous invocation */ + SSL_clear(s); + SSL_set_options(s, SSL_OP_COOKIE_EXCHANGE); s->d1->listen = 1;