Add checks on sk_TYPE_push() returned value

Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>

8 files changed, 70 insertions, 26 deletions

diff --git a/crypto/asn1/asn_mime.c b/crypto/asn1/asn_mime.c
index a6b38935a2..a4527a160a 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_log.c b/crypto/ct/ct_log.c
index 6fc21b7269..7298f1bfd4 100644
--- a/crypto/ct/ct_log.c
+++ b/crypto/ct/ct_log.c
@@ -182,7 +182,10 @@ static int ctlog_store_load_log(const char *log_name, int log_name_len,
         return 1;
     }
 
-    sk_CTLOG_push(load_ctx->log_store->logs, ct_log);
+    if (!sk_CTLOG_push(load_ctx->log_store->logs, ct_log)) {
+        CTLOG_free(ct_log);
+        return -1;
+    }
     return 1;
 }
 
diff --git a/crypto/evp/evp_pbe.c b/crypto/evp/evp_pbe.c
index 623f447387..ce7aa2cfa1 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 c655a908dd..ed98df8c2f 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 195d87c8be..33a30bdf1c 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 5f7167048e..a96f41e1b9 100644
--- a/crypto/pkcs7/pk7_attr.c
+++ b/crypto/pkcs7/pk7_attr.c
@@ -74,7 +74,11 @@ int PKCS7_simple_smimecap(STACK_OF(X509_ALGOR) *sk, int nid, int arg)
         alg->parameter->value.integer = nbit;
         alg->parameter->type = V_ASN1_INTEGER;
     }
-    sk_X509_ALGOR_push(sk, alg);
+    if (!sk_X509_ALGOR_push(sk, alg)) {
+        PKCS7err(PKCS7_F_PKCS7_SIMPLE_SMIMECAP, ERR_R_MALLOC_FAILURE);
+        X509_ALGOR_free(alg);
+        return 0;
+    }
     return 1;
 }
 
diff --git a/crypto/ui/ui_lib.c b/crypto/ui/ui_lib.c
index 2940b2fd4a..8992ae77c2 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 0b5b5b9ed7..843f3519dc 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;
 }