Add checks on sk_TYPE_push() returned result

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

4 files changed, 40 insertions, 18 deletions

diff --git a/ssl/d1_srtp.c b/ssl/d1_srtp.c
index 91d373fdf4..94c0127312 100644
--- a/ssl/d1_srtp.c
+++ b/ssl/d1_srtp.c
@@ -81,16 +81,18 @@ static int ssl_ctx_make_profiles(const char *profiles_string,
             if (sk_SRTP_PROTECTION_PROFILE_find(profiles, p) >= 0) {
                 SSLerr(SSL_F_SSL_CTX_MAKE_PROFILES,
                        SSL_R_BAD_SRTP_PROTECTION_PROFILE_LIST);
-                sk_SRTP_PROTECTION_PROFILE_free(profiles);
-                return 1;
+                goto err;
             }
 
-            sk_SRTP_PROTECTION_PROFILE_push(profiles, p);
+            if (!sk_SRTP_PROTECTION_PROFILE_push(profiles, p)) {
+                SSLerr(SSL_F_SSL_CTX_MAKE_PROFILES,
+                       SSL_R_SRTP_COULD_NOT_ALLOCATE_PROFILES);
+                goto err;
+            }
         } else {
             SSLerr(SSL_F_SSL_CTX_MAKE_PROFILES,
                    SSL_R_SRTP_UNKNOWN_PROTECTION_PROFILE);
-            sk_SRTP_PROTECTION_PROFILE_free(profiles);
-            return 1;
+            goto err;
         }
 
         if (col)
@@ -102,6 +104,9 @@ static int ssl_ctx_make_profiles(const char *profiles_string,
     *out = profiles;
 
     return 0;
+err:
+    sk_SRTP_PROTECTION_PROFILE_free(profiles);
+    return 1;
 }
 
 int SSL_CTX_set_tlsext_use_srtp(SSL_CTX *ctx, const char *profiles)
diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c
index 44dac24c8c..bd831bc48d 100644
--- a/ssl/s3_lib.c
+++ b/ssl/s3_lib.c
@@ -3410,10 +3410,15 @@ long ssl3_ctx_ctrl(SSL_CTX *ctx, int cmd, long larg, void *parg)
         /* A Thawte special :-) */
     case SSL_CTRL_EXTRA_CHAIN_CERT:
         if (ctx->extra_certs == NULL) {
-            if ((ctx->extra_certs = sk_X509_new_null()) == NULL)
-                return (0);
+            if ((ctx->extra_certs = sk_X509_new_null()) == NULL) {
+                SSLerr(SSL_F_SSL3_CTX_CTRL, ERR_R_MALLOC_FAILURE);
+                return 0;
+            }
+        }
+        if (!sk_X509_push(ctx->extra_certs, (X509 *)parg)) {
+            SSLerr(SSL_F_SSL3_CTX_CTRL, ERR_R_MALLOC_FAILURE);
+            return 0;
         }
-        sk_X509_push(ctx->extra_certs, (X509 *)parg);
         break;
 
     case SSL_CTRL_GET_EXTRA_CHAIN_CERTS:
diff --git a/ssl/ssl_cert.c b/ssl/ssl_cert.c
index d668afafe7..c6e2d09eb7 100644
--- a/ssl/ssl_cert.c
+++ b/ssl/ssl_cert.c
@@ -470,11 +470,16 @@ STACK_OF(X509_NAME) *SSL_dup_CA_list(STACK_OF(X509_NAME) *sk)
     X509_NAME *name;
 
     ret = sk_X509_NAME_new_null();
+    if (ret == NULL) {
+        SSLerr(SSL_F_SSL_DUP_CA_LIST, ERR_R_MALLOC_FAILURE);
+        return NULL;
+    }
     for (i = 0; i < sk_X509_NAME_num(sk); i++) {
         name = X509_NAME_dup(sk_X509_NAME_value(sk, i));
-        if ((name == NULL) || !sk_X509_NAME_push(ret, name)) {
+        if (name == NULL || !sk_X509_NAME_push(ret, name)) {
             sk_X509_NAME_pop_free(ret, X509_NAME_free);
-            return (NULL);
+            X509_NAME_free(name);
+            return NULL;
         }
     }
     return (ret);
@@ -598,14 +603,18 @@ STACK_OF(X509_NAME) *SSL_load_client_CA_file(const char *file)
         if (lh_X509_NAME_retrieve(name_hash, xn) != NULL) {
             /* Duplicate. */
             X509_NAME_free(xn);
+            xn = NULL;
         } else {
-            lh_X509_NAME_insert(name_hash, xn);
-            sk_X509_NAME_push(ret, xn);
+            if (!lh_X509_NAME_insert(name_hash, xn))
+                goto err;
+            if (!sk_X509_NAME_push(ret, xn))
+                goto err;
         }
     }
     goto done;
 
  err:
+    X509_NAME_free(xn);
     sk_X509_NAME_pop_free(ret, X509_NAME_free);
     ret = NULL;
  done:
@@ -656,17 +665,20 @@ int SSL_add_file_cert_subjects_to_stack(STACK_OF(X509_NAME) *stack,
         xn = X509_NAME_dup(xn);
         if (xn == NULL)
             goto err;
-        if (sk_X509_NAME_find(stack, xn) >= 0)
+        if (sk_X509_NAME_find(stack, xn) >= 0) {
+            /* Duplicate. */
             X509_NAME_free(xn);
-        else
-            sk_X509_NAME_push(stack, xn);
+        } else if (!sk_X509_NAME_push(stack, xn)) {
+            X509_NAME_free(xn);
+            goto err;
+        }
     }
 
     ERR_clear_error();
     goto done;
 
  err:
-        ret = 0;
+    ret = 0;
  done:
     BIO_free(in);
     X509_free(x);
diff --git a/ssl/ssl_ciph.c b/ssl/ssl_ciph.c
index 5dbe1d0154..2fc4309a91 100644
--- a/ssl/ssl_ciph.c
+++ b/ssl/ssl_ciph.c
@@ -1855,8 +1855,8 @@ int SSL_COMP_add_compression_method(int id, COMP_METHOD *cm)
                SSL_R_DUPLICATE_COMPRESSION_ID);
         return (1);
     }
-    if ((ssl_comp_methods == NULL)
-               || !sk_SSL_COMP_push(ssl_comp_methods, comp)) {
+    if (ssl_comp_methods == NULL
+        || !sk_SSL_COMP_push(ssl_comp_methods, comp)) {
         OPENSSL_free(comp);
         CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_ENABLE);
         SSLerr(SSL_F_SSL_COMP_ADD_COMPRESSION_METHOD, ERR_R_MALLOC_FAILURE);