[openssl-commits] [openssl] master update
Matt Caswell
matt at openssl.org
Tue Jul 5 16:49:03 UTC 2016
The branch master has been updated
via b77a86535e96d8b256cb26c27d43d22904aad718 (commit)
via 4aed8756d86e2b934e83d916e57bee91c83c4b28 (commit)
via e57036f2bf810e807700c80d8ff4f7d100890100 (commit)
via 68efafc513788863339c199d22048ef275832094 (commit)
from c2d551c01930df54bce6517cfecd214db6e98e80 (commit)
- Log -----------------------------------------------------------------
commit b77a86535e96d8b256cb26c27d43d22904aad718
Author: FdaSilvaYY <fdasilvayy at gmail.com>
Date: Sun Jun 26 21:20:52 2016 +0200
Fix mem error handling in PKCS7_simple_smimecap
Reviewed-by: Rich Salz <rsalz at openssl.org>
Reviewed-by: Matt Caswell <matt at openssl.org>
commit 4aed8756d86e2b934e83d916e57bee91c83c4b28
Author: FdaSilvaYY <fdasilvayy at gmail.com>
Date: Mon Jun 27 22:42:25 2016 +0200
Improve some error management code in CT
Separate invalid input case from any internal (malloc) failure
Reviewed-by: Rich Salz <rsalz at openssl.org>
Reviewed-by: Matt Caswell <matt at openssl.org>
commit e57036f2bf810e807700c80d8ff4f7d100890100
Author: FdaSilvaYY <fdasilvayy at gmail.com>
Date: Mon Jun 27 21:58:32 2016 +0200
Fix some memory error handling in CT
Reviewed-by: Rich Salz <rsalz at openssl.org>
Reviewed-by: Matt Caswell <matt at openssl.org>
commit 68efafc513788863339c199d22048ef275832094
Author: FdaSilvaYY <fdasilvayy at gmail.com>
Date: Mon Jun 27 21:57:58 2016 +0200
Add checks on sk_TYPE_push() returned value
Reviewed-by: Rich Salz <rsalz at openssl.org>
Reviewed-by: Matt Caswell <matt at openssl.org>
-----------------------------------------------------------------------
Summary of changes:
crypto/asn1/asn_mime.c | 24 +++++++++++++----
crypto/ct/ct_b64.c | 25 +++++++++++------
crypto/ct/ct_log.c | 70 +++++++++++++++++++++++++++---------------------
crypto/evp/evp_pbe.c | 5 +++-
crypto/objects/o_names.c | 12 ++++++---
crypto/ocsp/ocsp_cl.c | 2 +-
crypto/pkcs7/pk7_attr.c | 21 +++++++++------
crypto/ui/ui_lib.c | 8 ++++--
crypto/x509/x509_lu.c | 34 ++++++++++++++---------
include/openssl/ct.h | 7 +++--
10 files changed, 135 insertions(+), 73 deletions(-)
diff --git a/crypto/asn1/asn_mime.c b/crypto/asn1/asn_mime.c
index a6b3893..a4527a1 100644
--- a/crypto/asn1/asn_mime.c
+++ b/crypto/asn1/asn_mime.c
@@ -625,7 +625,7 @@ static STACK_OF(MIME_HEADER) *mime_parse_hdr(BIO *bio)
char *p, *q, c;
char *ntmp;
char linebuf[MAX_SMLEN];
- MIME_HEADER *mhdr = NULL;
+ MIME_HEADER *mhdr = NULL, *new_hdr = NULL;
STACK_OF(MIME_HEADER) *headers;
int len, state, save_state = 0;
@@ -662,8 +662,13 @@ static STACK_OF(MIME_HEADER) *mime_parse_hdr(BIO *bio)
if (c == ';') {
mime_debug("Found End Value\n");
*p = 0;
- mhdr = mime_hdr_new(ntmp, strip_ends(q));
- sk_MIME_HEADER_push(headers, mhdr);
+ new_hdr = mime_hdr_new(ntmp, strip_ends(q));
+ if (new_hdr == NULL)
+ goto err;
+ if (!sk_MIME_HEADER_push(headers, new_hdr))
+ goto err;
+ mhdr = new_hdr;
+ new_hdr = NULL;
ntmp = NULL;
q = p + 1;
state = MIME_NAME;
@@ -714,8 +719,13 @@ static STACK_OF(MIME_HEADER) *mime_parse_hdr(BIO *bio)
}
if (state == MIME_TYPE) {
- mhdr = mime_hdr_new(ntmp, strip_ends(q));
- sk_MIME_HEADER_push(headers, mhdr);
+ new_hdr = mime_hdr_new(ntmp, strip_ends(q));
+ if (new_hdr == NULL)
+ goto err;
+ if (!sk_MIME_HEADER_push(headers, new_hdr))
+ goto err;
+ mhdr = new_hdr;
+ new_hdr = NULL;
} else if (state == MIME_VALUE)
mime_hdr_addparam(mhdr, ntmp, strip_ends(q));
if (p == linebuf)
@@ -724,6 +734,10 @@ static STACK_OF(MIME_HEADER) *mime_parse_hdr(BIO *bio)
return headers;
+err:
+ mime_hdr_free(new_hdr);
+ sk_MIME_HEADER_pop_free(headers, mime_hdr_free);
+ return NULL;
}
static char *strip_ends(char *name)
diff --git a/crypto/ct/ct_b64.c b/crypto/ct/ct_b64.c
index d6279d2..9cf7c51 100644
--- a/crypto/ct/ct_b64.c
+++ b/crypto/ct/ct_b64.c
@@ -115,17 +115,26 @@ SCT *SCT_new_from_base64(unsigned char version, const char *logid_base64,
return NULL;
}
-CTLOG *CTLOG_new_from_base64(const char *pkey_base64, const char *name)
+/*
+ * This methods returns: 1 on Success,
+ * 0 on decoding failure,
+ * -1 on internal (malloc) failure, or invalid parameter if any.
+ */
+int CTLOG_new_from_base64(CTLOG **ct_log, const char *pkey_base64, const char *name)
{
unsigned char *pkey_der = NULL;
int pkey_der_len = ct_base64_decode(pkey_base64, &pkey_der);
const unsigned char *p;
EVP_PKEY *pkey = NULL;
- CTLOG *log = NULL;
+
+ if (ct_log == NULL) {
+ CTerr(CT_F_CTLOG_NEW_FROM_BASE64, ERR_R_PASSED_INVALID_ARGUMENT);
+ return 0;
+ }
if (pkey_der_len <= 0) {
CTerr(CT_F_CTLOG_NEW_FROM_BASE64, CT_R_LOG_CONF_INVALID_KEY);
- return NULL;
+ return 0;
}
p = pkey_der;
@@ -133,14 +142,14 @@ CTLOG *CTLOG_new_from_base64(const char *pkey_base64, const char *name)
OPENSSL_free(pkey_der);
if (pkey == NULL) {
CTerr(CT_F_CTLOG_NEW_FROM_BASE64, CT_R_LOG_CONF_INVALID_KEY);
- return NULL;
+ return 0;
}
- log = CTLOG_new(pkey, name);
- if (log == NULL) {
+ *ct_log = CTLOG_new(pkey, name);
+ if (*ct_log == NULL) {
EVP_PKEY_free(pkey);
- return NULL;
+ return -1;
}
- return log;
+ return 1;
}
diff --git a/crypto/ct/ct_log.c b/crypto/ct/ct_log.c
index 6fc21b7..1874d91 100644
--- a/crypto/ct/ct_log.c
+++ b/crypto/ct/ct_log.c
@@ -58,15 +58,10 @@ static CTLOG_STORE_LOAD_CTX *ctlog_store_load_ctx_new()
{
CTLOG_STORE_LOAD_CTX *ctx = OPENSSL_zalloc(sizeof(*ctx));
- if (ctx == NULL) {
+ if (ctx == NULL)
CTerr(CT_F_CTLOG_STORE_LOAD_CTX_NEW, ERR_R_MALLOC_FAILURE);
- goto err;
- }
return ctx;
-err:
- ctlog_store_load_ctx_free(ctx);
- return NULL;
}
static void ctlog_store_load_ctx_free(CTLOG_STORE_LOAD_CTX* ctx)
@@ -98,8 +93,10 @@ CTLOG_STORE *CTLOG_STORE_new(void)
{
CTLOG_STORE *ret = OPENSSL_zalloc(sizeof(*ret));
- if (ret == NULL)
- goto err;
+ if (ret == NULL) {
+ CTerr(CT_F_CTLOG_STORE_NEW, ERR_R_MALLOC_FAILURE);
+ return NULL;
+ }
ret->logs = sk_CTLOG_new_null();
if (ret->logs == NULL)
@@ -107,7 +104,7 @@ CTLOG_STORE *CTLOG_STORE_new(void)
return ret;
err:
- CTLOG_STORE_free(ret);
+ OPENSSL_free(ret);
return NULL;
}
@@ -119,31 +116,23 @@ void CTLOG_STORE_free(CTLOG_STORE *store)
}
}
-static CTLOG *ctlog_new_from_conf(const CONF *conf, const char *section)
+static int ctlog_new_from_conf(CTLOG **ct_log, const CONF *conf, const char *section)
{
- CTLOG *ret = NULL;
- char *description = NCONF_get_string(conf, section, "description");
+ const char *description = NCONF_get_string(conf, section, "description");
char *pkey_base64;
if (description == NULL) {
CTerr(CT_F_CTLOG_NEW_FROM_CONF, CT_R_LOG_CONF_MISSING_DESCRIPTION);
- goto end;
+ return 0;
}
pkey_base64 = NCONF_get_string(conf, section, "key");
if (pkey_base64 == NULL) {
CTerr(CT_F_CTLOG_NEW_FROM_CONF, CT_R_LOG_CONF_MISSING_KEY);
- goto end;
- }
-
- ret = CTLOG_new_from_base64(pkey_base64, description);
- if (ret == NULL) {
- CTerr(CT_F_CTLOG_NEW_FROM_CONF, CT_R_LOG_CONF_INVALID);
- goto end;
+ return 0;
}
-end:
- return ret;
+ return CTLOG_new_from_base64(ct_log, pkey_base64, description);
}
int CTLOG_STORE_load_default_file(CTLOG_STORE *store)
@@ -157,33 +146,50 @@ int CTLOG_STORE_load_default_file(CTLOG_STORE *store)
}
/*
- * Called by CONF_parse_list, which stops if this returns <= 0, so don't unless
- * something very bad happens. Otherwise, one bad log entry would stop loading
- * of any of the following log entries.
+ * Called by CONF_parse_list, which stops if this returns <= 0,
+ * Otherwise, one bad log entry would stop loading of any of
+ * the following log entries.
+ * It may stop parsing and returns -1 on any internal (malloc) error.
*/
static int ctlog_store_load_log(const char *log_name, int log_name_len,
void *arg)
{
CTLOG_STORE_LOAD_CTX *load_ctx = arg;
- CTLOG *ct_log;
+ CTLOG *ct_log = NULL;
/* log_name may not be null-terminated, so fix that before using it */
char *tmp;
+ int ret = 0;
/* log_name will be NULL for empty list entries */
if (log_name == NULL)
return 1;
tmp = OPENSSL_strndup(log_name, log_name_len);
- ct_log = ctlog_new_from_conf(load_ctx->conf, tmp);
+ if (tmp == NULL)
+ goto mem_err;
+
+ ret = ctlog_new_from_conf(&ct_log, load_ctx->conf, tmp);
OPENSSL_free(tmp);
- if (ct_log == NULL) {
+
+ if (ret < 0) {
+ /* Propagate any internal error */
+ return ret;
+ }
+ if (ret == 0) {
/* If we can't load this log, record that fact and skip it */
++load_ctx->invalid_log_entries;
return 1;
}
- sk_CTLOG_push(load_ctx->log_store->logs, ct_log);
+ if (!sk_CTLOG_push(load_ctx->log_store->logs, ct_log)) {
+ goto mem_err;
+ }
return 1;
+
+mem_err:
+ CTLOG_free(ct_log);
+ CTerr(CT_F_CTLOG_STORE_LOAD_LOG, ERR_R_MALLOC_FAILURE);
+ return -1;
}
int CTLOG_STORE_load_file(CTLOG_STORE *store, const char *file)
@@ -231,11 +237,13 @@ CTLOG *CTLOG_new(EVP_PKEY *public_key, const char *name)
CTLOG *ret = CTLOG_new_null();
if (ret == NULL)
- goto err;
+ return NULL;
ret->name = OPENSSL_strdup(name);
- if (ret->name == NULL)
+ if (ret->name == NULL) {
+ CTerr(CT_F_CTLOG_NEW, ERR_R_MALLOC_FAILURE);
goto err;
+ }
ret->public_key = public_key;
if (ct_v1_log_id_from_pkey(public_key, ret->log_id) != 1)
diff --git a/crypto/evp/evp_pbe.c b/crypto/evp/evp_pbe.c
index 623f447..ce7aa2c 100644
--- a/crypto/evp/evp_pbe.c
+++ b/crypto/evp/evp_pbe.c
@@ -173,7 +173,10 @@ int EVP_PBE_alg_add_type(int pbe_type, int pbe_nid, int cipher_nid,
pbe_tmp->md_nid = md_nid;
pbe_tmp->keygen = keygen;
- sk_EVP_PBE_CTL_push(pbe_algs, pbe_tmp);
+ if (!sk_EVP_PBE_CTL_push(pbe_algs, pbe_tmp)) {
+ OPENSSL_free(pbe_tmp);
+ goto err;
+ }
return 1;
err:
diff --git a/crypto/objects/o_names.c b/crypto/objects/o_names.c
index c655a90..ed98df8 100644
--- a/crypto/objects/o_names.c
+++ b/crypto/objects/o_names.c
@@ -76,8 +76,7 @@ int OBJ_NAME_new_index(unsigned long (*hash_func) (const char *),
int (*cmp_func) (const char *, const char *),
void (*free_func) (const char *, int, const char *))
{
- int ret;
- int i;
+ int ret, i, push;
NAME_FUNCS *name_funcs;
if (name_funcs_stack == NULL) {
@@ -102,8 +101,15 @@ int OBJ_NAME_new_index(unsigned long (*hash_func) (const char *),
name_funcs->hash_func = OPENSSL_LH_strhash;
name_funcs->cmp_func = obj_strcmp;
CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_DISABLE);
- sk_NAME_FUNCS_push(name_funcs_stack, name_funcs);
+
+ push = sk_NAME_FUNCS_push(name_funcs_stack, name_funcs);
CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_ENABLE);
+
+ if (!push) {
+ OBJerr(OBJ_F_OBJ_NAME_NEW_INDEX, ERR_R_MALLOC_FAILURE);
+ OPENSSL_free(name_funcs);
+ return 0;
+ }
}
name_funcs = sk_NAME_FUNCS_value(name_funcs_stack, ret);
if (hash_func != NULL)
diff --git a/crypto/ocsp/ocsp_cl.c b/crypto/ocsp/ocsp_cl.c
index 195d87c..33a30bd 100644
--- a/crypto/ocsp/ocsp_cl.c
+++ b/crypto/ocsp/ocsp_cl.c
@@ -32,7 +32,7 @@ OCSP_ONEREQ *OCSP_request_add0_id(OCSP_REQUEST *req, OCSP_CERTID *cid)
OCSP_ONEREQ *one = NULL;
if ((one = OCSP_ONEREQ_new()) == NULL)
- goto err;
+ return NULL;
OCSP_CERTID_free(one->reqCert);
one->reqCert = cid;
if (req && !sk_OCSP_ONEREQ_push(req->tbsRequest.requestList, one))
diff --git a/crypto/pkcs7/pk7_attr.c b/crypto/pkcs7/pk7_attr.c
index 5f71670..e90bf03 100644
--- a/crypto/pkcs7/pk7_attr.c
+++ b/crypto/pkcs7/pk7_attr.c
@@ -49,6 +49,7 @@ STACK_OF(X509_ALGOR) *PKCS7_get_smimecap(PKCS7_SIGNER_INFO *si)
/* Basic smime-capabilities OID and optional integer arg */
int PKCS7_simple_smimecap(STACK_OF(X509_ALGOR) *sk, int nid, int arg)
{
+ ASN1_INTEGER *nbit = NULL;
X509_ALGOR *alg;
if ((alg = X509_ALGOR_new()) == NULL) {
@@ -58,24 +59,28 @@ int PKCS7_simple_smimecap(STACK_OF(X509_ALGOR) *sk, int nid, int arg)
ASN1_OBJECT_free(alg->algorithm);
alg->algorithm = OBJ_nid2obj(nid);
if (arg > 0) {
- ASN1_INTEGER *nbit;
if ((alg->parameter = ASN1_TYPE_new()) == NULL) {
- PKCS7err(PKCS7_F_PKCS7_SIMPLE_SMIMECAP, ERR_R_MALLOC_FAILURE);
- return 0;
+ goto err;
}
if ((nbit = ASN1_INTEGER_new()) == NULL) {
- PKCS7err(PKCS7_F_PKCS7_SIMPLE_SMIMECAP, ERR_R_MALLOC_FAILURE);
- return 0;
+ goto err;
}
if (!ASN1_INTEGER_set(nbit, arg)) {
- PKCS7err(PKCS7_F_PKCS7_SIMPLE_SMIMECAP, ERR_R_MALLOC_FAILURE);
- return 0;
+ goto err;
}
alg->parameter->value.integer = nbit;
alg->parameter->type = V_ASN1_INTEGER;
+ nbit = NULL;
+ }
+ if (!sk_X509_ALGOR_push(sk, alg)) {
+ goto err;
}
- sk_X509_ALGOR_push(sk, alg);
return 1;
+err:
+ PKCS7err(PKCS7_F_PKCS7_SIMPLE_SMIMECAP, ERR_R_MALLOC_FAILURE);
+ ASN1_INTEGER_free(nbit);
+ X509_ALGOR_free(alg);
+ return 0;
}
int PKCS7_add_attrib_content_type(PKCS7_SIGNER_INFO *si, ASN1_OBJECT *coid)
diff --git a/crypto/ui/ui_lib.c b/crypto/ui/ui_lib.c
index 2940b2f..8992ae7 100644
--- a/crypto/ui/ui_lib.c
+++ b/crypto/ui/ui_lib.c
@@ -127,8 +127,10 @@ static int general_allocate_string(UI *ui, const char *prompt,
s->_.string_data.test_buf = test_buf;
ret = sk_UI_STRING_push(ui->strings, s);
/* sk_push() returns 0 on error. Let's adapt that */
- if (ret <= 0)
+ if (ret <= 0) {
ret--;
+ free_string(s);
+ }
} else
free_string(s);
}
@@ -172,8 +174,10 @@ static int general_allocate_boolean(UI *ui,
/*
* sk_push() returns 0 on error. Let's adapt that
*/
- if (ret <= 0)
+ if (ret <= 0) {
ret--;
+ free_string(s);
+ }
} else
free_string(s);
}
diff --git a/crypto/x509/x509_lu.c b/crypto/x509/x509_lu.c
index 0b5b5b9..843f351 100644
--- a/crypto/x509/x509_lu.c
+++ b/crypto/x509/x509_lu.c
@@ -301,7 +301,7 @@ int X509_STORE_CTX_get_by_subject(X509_STORE_CTX *vs, X509_LOOKUP_TYPE type,
int X509_STORE_add_cert(X509_STORE *ctx, X509 *x)
{
X509_OBJECT *obj;
- int ret = 1;
+ int ret = 1, added = 1;
if (x == NULL)
return 0;
@@ -310,28 +310,33 @@ int X509_STORE_add_cert(X509_STORE *ctx, X509 *x)
return 0;
obj->type = X509_LU_X509;
obj->data.x509 = x;
+ X509_OBJECT_up_ref_count(obj);
CRYPTO_THREAD_write_lock(ctx->lock);
- X509_OBJECT_up_ref_count(obj);
-
if (X509_OBJECT_retrieve_match(ctx->objs, obj)) {
- X509_OBJECT_free(obj);
X509err(X509_F_X509_STORE_ADD_CERT,
X509_R_CERT_ALREADY_IN_HASH_TABLE);
ret = 0;
- } else
- sk_X509_OBJECT_push(ctx->objs, obj);
+ } else {
+ added = sk_X509_OBJECT_push(ctx->objs, obj);
+ ret = added != 0;
+ }
CRYPTO_THREAD_unlock(ctx->lock);
+ if (!ret) /* obj not pushed */
+ X509_OBJECT_free(obj);
+ if (!added) /* on push failure */
+ X509err(X509_F_X509_STORE_ADD_CERT, ERR_R_MALLOC_FAILURE);
+
return ret;
}
int X509_STORE_add_crl(X509_STORE *ctx, X509_CRL *x)
{
X509_OBJECT *obj;
- int ret = 1;
+ int ret = 1, added = 1;
if (x == NULL)
return 0;
@@ -340,20 +345,25 @@ int X509_STORE_add_crl(X509_STORE *ctx, X509_CRL *x)
return 0;
obj->type = X509_LU_CRL;
obj->data.crl = x;
+ X509_OBJECT_up_ref_count(obj);
CRYPTO_THREAD_write_lock(ctx->lock);
- X509_OBJECT_up_ref_count(obj);
-
if (X509_OBJECT_retrieve_match(ctx->objs, obj)) {
- X509_OBJECT_free(obj);
X509err(X509_F_X509_STORE_ADD_CRL, X509_R_CERT_ALREADY_IN_HASH_TABLE);
ret = 0;
- } else
- sk_X509_OBJECT_push(ctx->objs, obj);
+ } else {
+ added = sk_X509_OBJECT_push(ctx->objs, obj);
+ ret = added != 0;
+ }
CRYPTO_THREAD_unlock(ctx->lock);
+ if (!ret) /* obj not pushed */
+ X509_OBJECT_free(obj);
+ if (!added) /* on push failure */
+ X509err(X509_F_X509_STORE_ADD_CRL, ERR_R_MALLOC_FAILURE);
+
return ret;
}
diff --git a/include/openssl/ct.h b/include/openssl/ct.h
index f9586dc..be7a953 100644
--- a/include/openssl/ct.h
+++ b/include/openssl/ct.h
@@ -419,10 +419,11 @@ CTLOG *CTLOG_new(EVP_PKEY *public_key, const char *name);
CTLOG *CTLOG_new_null(void);
/*
- * Creates a new CT log instance with the given base64 public_key and |name|.
+ * Creates a new CT |ct_log| instance with the given base64 public_key and |name|.
* Should be deleted by the caller using CTLOG_free when no longer needed.
*/
-CTLOG *CTLOG_new_from_base64(const char *pkey_base64, const char *name);
+int CTLOG_new_from_base64(CTLOG ** ct_log,
+ const char *pkey_base64, const char *name);
/*
* Deletes a CT log instance and its fields.
@@ -491,6 +492,8 @@ void ERR_load_CT_strings(void);
# define CT_F_CTLOG_NEW_NULL 120
# define CT_F_CTLOG_STORE_LOAD_CTX_NEW 122
# define CT_F_CTLOG_STORE_LOAD_FILE 123
+# define CT_F_CTLOG_STORE_LOAD_LOG 130
+# define CT_F_CTLOG_STORE_NEW 131
# define CT_F_CT_BASE64_DECODE 124
# define CT_F_CT_POLICY_EVAL_CTX_NEW 133
# define CT_F_CT_V1_LOG_ID_FROM_PKEY 125
More information about the openssl-commits
mailing list