X509: add more error codes on malloc or sk_TYP_push failure

Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
(Merged from https://github.com/openssl/openssl/pull/5837)

9 files changed, 104 insertions, 55 deletions

diff --git a/crypto/x509/by_dir.c b/crypto/x509/by_dir.c
index f64cf38..f213eec 100644
--- a/crypto/x509/by_dir.c
+++ b/crypto/x509/by_dir.c
@@ -48,7 +48,7 @@ static int get_cert_by_subject(X509_LOOKUP *xl, X509_LOOKUP_TYPE type,
                                X509_NAME *name, X509_OBJECT *ret);
 static X509_LOOKUP_METHOD x509_dir_lookup = {
     "Load certs from files in a directory",
-    new_dir,                    /* new */
+    new_dir,                    /* new_item */
     free_dir,                   /* free */
     NULL,                       /* init */
     NULL,                       /* shutdown */
@@ -68,15 +68,13 @@ static int dir_ctrl(X509_LOOKUP *ctx, int cmd, const char *argp, long argl,
                     char **retp)
 {
     int ret = 0;
-    BY_DIR *ld;
-    char *dir = NULL;
-
-    ld = (BY_DIR *)ctx->method_data;
+    BY_DIR *ld = (BY_DIR *)ctx->method_data;
 
     switch (cmd) {
     case X509_L_ADD_DIR:
         if (argl == X509_FILETYPE_DEFAULT) {
-            dir = (char *)getenv(X509_get_default_cert_dir_env());
+            const char *dir = getenv(X509_get_default_cert_dir_env());
+
             if (dir)
                 ret = add_cert_dir(ld, dir, X509_FILETYPE_PEM);
             else
@@ -94,23 +92,30 @@ static int dir_ctrl(X509_LOOKUP *ctx, int cmd, const char *argp, long argl,
 
 static int new_dir(X509_LOOKUP *lu)
 {
-    BY_DIR *a;
+    BY_DIR *a = OPENSSL_malloc(sizeof(*a));
 
-    if ((a = OPENSSL_malloc(sizeof(*a))) == NULL)
+    if (a == NULL) {
+        X509err(X509_F_NEW_DIR, ERR_R_MALLOC_FAILURE);
         return 0;
+    }
+
     if ((a->buffer = BUF_MEM_new()) == NULL) {
-        OPENSSL_free(a);
-        return 0;
+        X509err(X509_F_NEW_DIR, ERR_R_MALLOC_FAILURE);
+        goto err;
     }
     a->dirs = NULL;
     a->lock = CRYPTO_THREAD_lock_new();
     if (a->lock == NULL) {
         BUF_MEM_free(a->buffer);
-        OPENSSL_free(a);
-        return 0;
+        X509err(X509_F_NEW_DIR, ERR_R_MALLOC_FAILURE);
+        goto err;
     }
     lu->method_data = (char *)a;
     return 1;
+
+ err:
+    OPENSSL_free(a);
+    return 0;
 }
 
 static void by_dir_hash_free(BY_DIR_HASH *hash)
@@ -137,9 +142,8 @@ static void by_dir_entry_free(BY_DIR_ENTRY *ent)
 
 static void free_dir(X509_LOOKUP *lu)
 {
-    BY_DIR *a;
+    BY_DIR *a = (BY_DIR *)lu->method_data;
 
-    a = (BY_DIR *)lu->method_data;
     sk_BY_DIR_ENTRY_pop_free(a->dirs, by_dir_entry_free);
     BUF_MEM_free(a->buffer);
     CRYPTO_THREAD_lock_free(a->lock);
@@ -162,6 +166,7 @@ static int add_cert_dir(BY_DIR *ctx, const char *dir, int type)
     do {
         if ((*p == LIST_SEPARATOR_CHAR) || (*p == '\0')) {
             BY_DIR_ENTRY *ent;
+
             ss = s;
             s = p + 1;
             len = p - ss;
@@ -182,8 +187,10 @@ static int add_cert_dir(BY_DIR *ctx, const char *dir, int type)
                 }
             }
             ent = OPENSSL_malloc(sizeof(*ent));
-            if (ent == NULL)
+            if (ent == NULL) {
+                X509err(X509_F_ADD_CERT_DIR, ERR_R_MALLOC_FAILURE);
                 return 0;
+            }
             ent->dir_type = type;
             ent->hashes = sk_BY_DIR_HASH_new(by_dir_hash_cmp);
             ent->dir = OPENSSL_strndup(ss, len);
@@ -193,6 +200,7 @@ static int add_cert_dir(BY_DIR *ctx, const char *dir, int type)
             }
             if (!sk_BY_DIR_ENTRY_push(ctx->dirs, ent)) {
                 by_dir_entry_free(ent);
+                X509err(X509_F_ADD_CERT_DIR, ERR_R_MALLOC_FAILURE);
                 return 0;
             }
         }
@@ -244,6 +252,7 @@ static int get_cert_by_subject(X509_LOOKUP *xl, X509_LOOKUP_TYPE type,
         BY_DIR_ENTRY *ent;
         int idx;
         BY_DIR_HASH htmp, *hent;
+
         ent = sk_BY_DIR_ENTRY_value(ctx->dirs, i);
         j = strlen(ent->dir) + 1 + 8 + 6 + 1 + 1;
         if (!BUF_MEM_grow(b, j)) {
@@ -340,7 +349,7 @@ static int get_cert_by_subject(X509_LOOKUP *xl, X509_LOOKUP_TYPE type,
                 if (idx >= 0)
                     hent = sk_BY_DIR_HASH_value(ent->hashes, idx);
             }
-            if (!hent) {
+            if (hent == NULL) {
                 hent = OPENSSL_malloc(sizeof(*hent));
                 if (hent == NULL) {
                     CRYPTO_THREAD_unlock(ctx->lock);
@@ -353,6 +362,7 @@ static int get_cert_by_subject(X509_LOOKUP *xl, X509_LOOKUP_TYPE type,
                 if (!sk_BY_DIR_HASH_push(ent->hashes, hent)) {
                     CRYPTO_THREAD_unlock(ctx->lock);
                     OPENSSL_free(hent);
+                    X509err(X509_F_GET_CERT_BY_SUBJECT, ERR_R_MALLOC_FAILURE);
                     ok = 0;
                     goto finish;
                 }
diff --git a/crypto/x509/by_file.c b/crypto/x509/by_file.c
index 52955b6..bd39ecb 100644
--- a/crypto/x509/by_file.c
+++ b/crypto/x509/by_file.c
@@ -21,7 +21,7 @@ static int by_file_ctrl(X509_LOOKUP *ctx, int cmd, const char *argc,
                         long argl, char **ret);
 static X509_LOOKUP_METHOD x509_file_lookup = {
     "Load file into cache",
-    NULL,                       /* new */
+    NULL,                       /* new_item */
     NULL,                       /* free */
     NULL,                       /* init */
     NULL,                       /* shutdown */
diff --git a/crypto/x509/x509_err.c b/crypto/x509/x509_err.c
index 84c726f..7b9f505 100644
--- a/crypto/x509/x509_err.c
+++ b/crypto/x509/x509_err.c
@@ -1,6 +1,6 @@
 /*
  * Generated by util/mkerr.pl DO NOT EDIT
- * Copyright 1995-2017 The OpenSSL Project Authors. All Rights Reserved.
+ * Copyright 1995-2018 The OpenSSL Project Authors. All Rights Reserved.
  *
  * Licensed under the OpenSSL license (the "License").  You may not use
  * this file except in compliance with the License.  You can obtain a copy
@@ -24,10 +24,13 @@ static const ERR_STRING_DATA X509_str_functs[] = {
     {ERR_PACK(ERR_LIB_X509, X509_F_DIR_CTRL, 0), "dir_ctrl"},
     {ERR_PACK(ERR_LIB_X509, X509_F_GET_CERT_BY_SUBJECT, 0),
      "get_cert_by_subject"},
+    {ERR_PACK(ERR_LIB_X509, X509_F_I2D_X509_AUX, 0), "i2d_X509_AUX"},
+    {ERR_PACK(ERR_LIB_X509, X509_F_LOOKUP_CERTS_SK, 0), "lookup_certs_sk"},
     {ERR_PACK(ERR_LIB_X509, X509_F_NETSCAPE_SPKI_B64_DECODE, 0),
      "NETSCAPE_SPKI_b64_decode"},
     {ERR_PACK(ERR_LIB_X509, X509_F_NETSCAPE_SPKI_B64_ENCODE, 0),
      "NETSCAPE_SPKI_b64_encode"},
+    {ERR_PACK(ERR_LIB_X509, X509_F_NEW_DIR, 0), "new_dir"},
     {ERR_PACK(ERR_LIB_X509, X509_F_X509AT_ADD1_ATTR, 0), "X509at_add1_attr"},
     {ERR_PACK(ERR_LIB_X509, X509_F_X509V3_ADD_EXT, 0), "X509v3_add_ext"},
     {ERR_PACK(ERR_LIB_X509, X509_F_X509_ATTRIBUTE_CREATE_BY_NID, 0),
@@ -43,6 +46,8 @@ static const ERR_STRING_DATA X509_str_functs[] = {
     {ERR_PACK(ERR_LIB_X509, X509_F_X509_CHECK_PRIVATE_KEY, 0),
      "X509_check_private_key"},
     {ERR_PACK(ERR_LIB_X509, X509_F_X509_CRL_DIFF, 0), "X509_CRL_diff"},
+    {ERR_PACK(ERR_LIB_X509, X509_F_X509_CRL_METHOD_NEW, 0),
+     "X509_CRL_METHOD_new"},
     {ERR_PACK(ERR_LIB_X509, X509_F_X509_CRL_PRINT_FP, 0), "X509_CRL_print_fp"},
     {ERR_PACK(ERR_LIB_X509, X509_F_X509_EXTENSION_CREATE_BY_NID, 0),
      "X509_EXTENSION_create_by_NID"},
@@ -56,8 +61,10 @@ static const ERR_STRING_DATA X509_str_functs[] = {
      "X509_load_cert_file"},
     {ERR_PACK(ERR_LIB_X509, X509_F_X509_LOAD_CRL_FILE, 0),
      "X509_load_crl_file"},
+    {ERR_PACK(ERR_LIB_X509, X509_F_X509_LOOKUP_NEW, 0), "X509_LOOKUP_new"},
     {ERR_PACK(ERR_LIB_X509, X509_F_X509_NAME_ADD_ENTRY, 0),
      "X509_NAME_add_entry"},
+    {ERR_PACK(ERR_LIB_X509, X509_F_X509_NAME_CANON, 0), "x509_name_canon"},
     {ERR_PACK(ERR_LIB_X509, X509_F_X509_NAME_ENTRY_CREATE_BY_NID, 0),
      "X509_NAME_ENTRY_create_by_NID"},
     {ERR_PACK(ERR_LIB_X509, X509_F_X509_NAME_ENTRY_CREATE_BY_TXT, 0),
@@ -81,6 +88,8 @@ static const ERR_STRING_DATA X509_str_functs[] = {
      "X509_STORE_add_cert"},
     {ERR_PACK(ERR_LIB_X509, X509_F_X509_STORE_ADD_CRL, 0),
      "X509_STORE_add_crl"},
+    {ERR_PACK(ERR_LIB_X509, X509_F_X509_STORE_ADD_LOOKUP, 0),
+     "X509_STORE_add_lookup"},
     {ERR_PACK(ERR_LIB_X509, X509_F_X509_STORE_CTX_GET1_ISSUER, 0),
      "X509_STORE_CTX_get1_issuer"},
     {ERR_PACK(ERR_LIB_X509, X509_F_X509_STORE_CTX_INIT, 0),
@@ -89,6 +98,7 @@ static const ERR_STRING_DATA X509_str_functs[] = {
      "X509_STORE_CTX_new"},
     {ERR_PACK(ERR_LIB_X509, X509_F_X509_STORE_CTX_PURPOSE_INHERIT, 0),
      "X509_STORE_CTX_purpose_inherit"},
+    {ERR_PACK(ERR_LIB_X509, X509_F_X509_STORE_NEW, 0), "X509_STORE_new"},
     {ERR_PACK(ERR_LIB_X509, X509_F_X509_TO_X509_REQ, 0), "X509_to_X509_REQ"},
     {ERR_PACK(ERR_LIB_X509, X509_F_X509_TRUST_ADD, 0), "X509_TRUST_add"},
     {ERR_PACK(ERR_LIB_X509, X509_F_X509_TRUST_SET, 0), "X509_TRUST_set"},
diff --git a/crypto/x509/x509_lu.c b/crypto/x509/x509_lu.c
index 7407005..fbeed01 100644
--- a/crypto/x509/x509_lu.c
+++ b/crypto/x509/x509_lu.c
@@ -17,14 +17,15 @@
 
 X509_LOOKUP *X509_LOOKUP_new(X509_LOOKUP_METHOD *method)
 {
-    X509_LOOKUP *ret;
+    X509_LOOKUP *ret = OPENSSL_zalloc(sizeof(*ret));
 
-    ret = OPENSSL_zalloc(sizeof(*ret));
-    if (ret == NULL)
+    if (ret == NULL) {
+        X509err(X509_F_X509_LOOKUP_NEW, ERR_R_MALLOC_FAILURE);
         return NULL;
+    }
 
     ret->method = method;
-    if ((method->new_item != NULL) && !method->new_item(ret)) {
+    if (method->new_item != NULL && method->new_item(ret) == 0) {
         OPENSSL_free(ret);
         return NULL;
     }
@@ -141,25 +142,36 @@ static int x509_object_cmp(const X509_OBJECT *const *a,
 
 X509_STORE *X509_STORE_new(void)
 {
-    X509_STORE *ret;
+    X509_STORE *ret = OPENSSL_zalloc(sizeof(*ret));
 
-    if ((ret = OPENSSL_zalloc(sizeof(*ret))) == NULL)
+    if (ret == NULL) {
+        X509err(X509_F_X509_STORE_NEW, ERR_R_MALLOC_FAILURE);
         return NULL;
-    if ((ret->objs = sk_X509_OBJECT_new(x509_object_cmp)) == NULL)
+    }
+    if ((ret->objs = sk_X509_OBJECT_new(x509_object_cmp)) == NULL) {
+        X509err(X509_F_X509_STORE_NEW, ERR_R_MALLOC_FAILURE);
         goto err;
+    }
     ret->cache = 1;
-    if ((ret->get_cert_methods = sk_X509_LOOKUP_new_null()) == NULL)
+    if ((ret->get_cert_methods = sk_X509_LOOKUP_new_null()) == NULL) {
+        X509err(X509_F_X509_STORE_NEW, ERR_R_MALLOC_FAILURE);
         goto err;
+    }
 
-    if ((ret->param = X509_VERIFY_PARAM_new()) == NULL)
+    if ((ret->param = X509_VERIFY_PARAM_new()) == NULL) {
+        X509err(X509_F_X509_STORE_NEW, ERR_R_MALLOC_FAILURE);
         goto err;
-
-    if (!CRYPTO_new_ex_data(CRYPTO_EX_INDEX_X509_STORE, ret, &ret->ex_data))
+    }
+    if (!CRYPTO_new_ex_data(CRYPTO_EX_INDEX_X509_STORE, ret, &ret->ex_data)) {
+        X509err(X509_F_X509_STORE_NEW, ERR_R_MALLOC_FAILURE);
         goto err;
+    }
 
     ret->lock = CRYPTO_THREAD_lock_new();
-    if (ret->lock == NULL)
+    if (ret->lock == NULL) {
+        X509err(X509_F_X509_STORE_NEW, ERR_R_MALLOC_FAILURE);
         goto err;
+    }
 
     ret->references = 1;
     return ret;
@@ -228,17 +240,18 @@ X509_LOOKUP *X509_STORE_add_lookup(X509_STORE *v, X509_LOOKUP_METHOD *m)
     }
     /* a new one */
     lu = X509_LOOKUP_new(m);
-    if (lu == NULL)
+    if (lu == NULL) {
+        X509err(X509_F_X509_STORE_ADD_LOOKUP, ERR_R_MALLOC_FAILURE);
         return NULL;
-    else {
-        lu->store_ctx = v;
-        if (sk_X509_LOOKUP_push(v->get_cert_methods, lu))
-            return lu;
-        else {
-            X509_LOOKUP_free(lu);
-            return NULL;
-        }
     }
+
+    lu->store_ctx = v;
+    if (sk_X509_LOOKUP_push(v->get_cert_methods, lu))
+        return lu;
+    /* malloc failed */
+    X509err(X509_F_X509_STORE_ADD_LOOKUP, ERR_R_MALLOC_FAILURE);
+    X509_LOOKUP_free(lu);
+    return NULL;
 }
 
 X509_OBJECT *X509_STORE_CTX_get_obj_by_subject(X509_STORE_CTX *vs,
diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c
index 9ebda1b..fd8ecfa 100644
--- a/crypto/x509/x509_vfy.c
+++ b/crypto/x509/x509_vfy.c
@@ -366,6 +366,7 @@ static STACK_OF(X509) *lookup_certs_sk(X509_STORE_CTX *ctx, X509_NAME *nm)
     STACK_OF(X509) *sk = NULL;
     X509 *x;
     int i;
+
     for (i = 0; i < sk_X509_num(ctx->other_ctx); i++) {
         x = sk_X509_value(ctx->other_ctx, i);
         if (X509_NAME_cmp(nm, X509_get_subject_name(x)) == 0) {
@@ -373,6 +374,8 @@ static STACK_OF(X509) *lookup_certs_sk(X509_STORE_CTX *ctx, X509_NAME *nm)
                 sk = sk_X509_new_null();
             if (sk == NULL || sk_X509_push(sk, x) == 0) {
                 sk_X509_pop_free(sk, X509_free);
+                X509err(X509_F_LOOKUP_CERTS_SK, ERR_R_MALLOC_FAILURE);
+                ctx->error = X509_V_ERR_OUT_OF_MEM;
                 return NULL;
             }
             X509_up_ref(x);
diff --git a/crypto/x509/x_crl.c b/crypto/x509/x_crl.c
index dbed850..5098b5f 100644
--- a/crypto/x509/x_crl.c
+++ b/crypto/x509/x_crl.c
@@ -309,6 +309,7 @@ static int X509_REVOKED_cmp(const X509_REVOKED *const *a,
 int X509_CRL_add0_revoked(X509_CRL *crl, X509_REVOKED *rev)
 {
     X509_CRL_INFO *inf;
+
     inf = &crl->crl;
     if (inf->revoked == NULL)
         inf->revoked = sk_X509_REVOKED_new(X509_REVOKED_cmp);
@@ -429,10 +430,12 @@ X509_CRL_METHOD *X509_CRL_METHOD_new(int (*crl_init) (X509_CRL *crl),
                                      int (*crl_verify) (X509_CRL *crl,
                                                         EVP_PKEY *pk))
 {
-    X509_CRL_METHOD *m;
-    m = OPENSSL_malloc(sizeof(*m));
-    if (m == NULL)
+    X509_CRL_METHOD *m = OPENSSL_malloc(sizeof(*m));
+
+    if (m == NULL) {
+        X509err(X509_F_X509_CRL_METHOD_NEW, ERR_R_MALLOC_FAILURE);
         return NULL;
+    }
     m->crl_init = crl_init;
     m->crl_free = crl_free;
     m->crl_lookup = crl_lookup;
diff --git a/crypto/x509/x_name.c b/crypto/x509/x_name.c
index d36a9d3..a30b5e1 100644
--- a/crypto/x509/x_name.c
+++ b/crypto/x509/x_name.c
@@ -300,7 +300,7 @@ static int x509_name_ex_print(BIO *out, ASN1_VALUE **pval,
 static int x509_name_canon(X509_NAME *a)
 {
     unsigned char *p;
-    STACK_OF(STACK_OF_X509_NAME_ENTRY) *intname = NULL;
+    STACK_OF(STACK_OF_X509_NAME_ENTRY) *intname;
     STACK_OF(X509_NAME_ENTRY) *entries = NULL;
     X509_NAME_ENTRY *entry, *tmpentry = NULL;
     int i, set = -1, ret = 0, len;
@@ -313,44 +313,53 @@ static int x509_name_canon(X509_NAME *a)
         return 1;
     }
     intname = sk_STACK_OF_X509_NAME_ENTRY_new_null();
-    if (!intname)
+    if (intname == NULL) {
+        X509err(X509_F_X509_NAME_CANON, ERR_R_MALLOC_FAILURE);
         goto err;
+    }
     for (i = 0; i < sk_X509_NAME_ENTRY_num(a->entries); i++) {
         entry = sk_X509_NAME_ENTRY_value(a->entries, i);
         if (entry->set != set) {
             entries = sk_X509_NAME_ENTRY_new_null();
-            if (!entries)
+            if (entries == NULL)
                 goto err;
             if (!sk_STACK_OF_X509_NAME_ENTRY_push(intname, entries)) {
                 sk_X509_NAME_ENTRY_free(entries);
+                X509err(X509_F_X509_NAME_CANON, ERR_R_MALLOC_FAILURE);
                 goto err;
             }
             set = entry->set;
         }
         tmpentry = X509_NAME_ENTRY_new();
-        if (tmpentry == NULL)
+        if (tmpentry == NULL) {
+            X509err(X509_F_X509_NAME_CANON, ERR_R_MALLOC_FAILURE);
             goto err;
+        }
         tmpentry->object = OBJ_dup(entry->object);
-        if (tmpentry->object == NULL)
+        if (tmpentry->object == NULL) {
+            X509err(X509_F_X509_NAME_CANON, ERR_R_MALLOC_FAILURE);
             goto err;
+        }
         if (!asn1_string_canon(tmpentry->value, entry->value))
             goto err;
-        if (!sk_X509_NAME_ENTRY_push(entries, tmpentry))
+        if (!sk_X509_NAME_ENTRY_push(entries, tmpentry)) {
+            X509err(X509_F_X509_NAME_CANON, ERR_R_MALLOC_FAILURE);
             goto err;
+        }
         tmpentry = NULL;
     }
 
     /* Finally generate encoding */
-
     len = i2d_name_canon(intname, NULL);
     if (len < 0)
         goto err;
     a->canon_enclen = len;
 
     p = OPENSSL_malloc(a->canon_enclen);
-
-    if (p == NULL)
+    if (p == NULL) {
+        X509err(X509_F_X509_NAME_CANON, ERR_R_MALLOC_FAILURE);
         goto err;
+    }
 
     a->canon_enc = p;
 
@@ -359,7 +368,6 @@ static int x509_name_canon(X509_NAME *a)
     ret = 1;
 
  err:
-
     X509_NAME_ENTRY_free(tmpentry);
     sk_STACK_OF_X509_NAME_ENTRY_pop_free(intname,
                                          local_sk_X509_NAME_ENTRY_pop_free);
diff --git a/crypto/x509/x_pubkey.c b/crypto/x509/x_pubkey.c
index 73977cd..8616297 100644
--- a/crypto/x509/x_pubkey.c
+++ b/crypto/x509/x_pubkey.c
@@ -101,7 +101,7 @@ int X509_PUBKEY_set(X509_PUBKEY **x, EVP_PKEY *pkey)
 
 
 static int x509_pubkey_decode(EVP_PKEY **ppkey, X509_PUBKEY *key)
-    {
+{
     EVP_PKEY *pkey = EVP_PKEY_new();
 
     if (pkey == NULL) {
diff --git a/crypto/x509/x_x509.c b/crypto/x509/x_x509.c
index c28fdda..cf71180 100644
--- a/crypto/x509/x_x509.c
+++ b/crypto/x509/x_x509.c
@@ -189,8 +189,10 @@ int i2d_X509_AUX(X509 *a, unsigned char **pp)
 
     /* Allocate requisite combined storage */
     *pp = tmp = OPENSSL_malloc(length);
-    if (tmp == NULL)
-        return -1; /* Push error onto error stack? */
+    if (tmp == NULL) {
+        X509err(X509_F_I2D_X509_AUX, ERR_R_MALLOC_FAILURE);
+        return -1;
+    }
 
     /* Encode, but keep *pp at the originally malloced pointer */
     length = i2d_x509_aux_internal(a, &tmp);