Add checks on sk_TYPE_push() returned result

Reviewed-by: NRich Salz <rsalz@openssl.org>
Reviewed-by: NMatt Caswell <matt@openssl.org>

crypto/engine/eng_dyn.c

@@ -349,11 +349,15 @@ static int dynamic_ctrl(ENGINE *e, int cmd, long i, void *p, void (*f) (void))
         }
         {
             char *tmp_str = OPENSSL_strdup(p);
-            if (!tmp_str) {
+            if (tmp_str == NULL) {
+                ENGINEerr(ENGINE_F_DYNAMIC_CTRL, ERR_R_MALLOC_FAILURE);
+                return 0;
+            }
+            if (!sk_OPENSSL_STRING_push(ctx->dirs, tmp_str)) {
+                OPENSSL_free(tmp_str);
                 ENGINEerr(ENGINE_F_DYNAMIC_CTRL, ERR_R_MALLOC_FAILURE);
                 return 0;
             }
-            sk_OPENSSL_STRING_insert(ctx->dirs, tmp_str, -1);
         }
         return 1;
     default:

include/openssl/ssl.h

@@ -2113,6 +2113,7 @@ void ERR_load_SSL_strings(void);
 # define SSL_F_SSL_DANE_ENABLE                            395
 # define SSL_F_SSL_DO_CONFIG                              391
 # define SSL_F_SSL_DO_HANDSHAKE                           180
+# define SSL_F_SSL_DUP_CA_LIST                            408
 # define SSL_F_SSL_ENABLE_CT                              402
 # define SSL_F_SSL_GET_NEW_SESSION                        181
 # define SSL_F_SSL_GET_PREV_SESSION                       217

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)

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:

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);

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);