Check for sk_X509_NAME_push failures.

Also tidy up the logic slightly.

Change-Id: I708254406b2df52435ec434ac9806e8eb2cbe928
Reviewed-on: https://boringssl-review.googlesource.com/11322
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>

1 files changed, 21 insertions, 20 deletions

diff --git a/ssl/ssl_file.c b/ssl/ssl_file.c
index 748d50ca..e1ebaa69 100644
--- a/ssl/ssl_file.c
+++ b/ssl/ssl_file.c
@@ -164,16 +164,17 @@ STACK_OF(X509_NAME) *SSL_load_client_CA_file(const char *file) {
       goto err;
     }
 
-    /* check for duplicates */
-    xn = X509_NAME_dup(xn);
-    if (xn == NULL) {
-      goto err;
-    }
+    /* Check for duplicates. */
     if (sk_X509_NAME_find(sk, NULL, xn)) {
+      continue;
+    }
+
+    xn = X509_NAME_dup(xn);
+    if (xn == NULL ||
+        !sk_X509_NAME_push(sk /* non-owning */, xn) ||
+        !sk_X509_NAME_push(ret /* owning */, xn)) {
       X509_NAME_free(xn);
-    } else {
-      sk_X509_NAME_push(sk, xn);
-      sk_X509_NAME_push(ret, xn);
+      goto err;
     }
   }
 
@@ -197,7 +198,7 @@ int SSL_add_file_cert_subjects_to_stack(STACK_OF(X509_NAME) *stack,
   BIO *in;
   X509 *x = NULL;
   X509_NAME *xn = NULL;
-  int ret = 1;
+  int ret = 0;
   int (*oldcmp)(const X509_NAME **a, const X509_NAME **b);
 
   oldcmp = sk_X509_NAME_set_cmp_func(stack, xname_cmp);
@@ -220,24 +221,24 @@ int SSL_add_file_cert_subjects_to_stack(STACK_OF(X509_NAME) *stack,
     if (xn == NULL) {
       goto err;
     }
-    xn = X509_NAME_dup(xn);
-    if (xn == NULL) {
-      goto err;
-    }
+
+    /* Check for duplicates. */
     if (sk_X509_NAME_find(stack, NULL, xn)) {
+      continue;
+    }
+
+    xn = X509_NAME_dup(xn);
+    if (xn == NULL ||
+        !sk_X509_NAME_push(stack, xn)) {
       X509_NAME_free(xn);
-    } else {
-      sk_X509_NAME_push(stack, xn);
+      goto err;
     }
   }
 
   ERR_clear_error();
+  ret = 1;
 
-  if (0) {
-  err:
-    ret = 0;
-  }
-
+err:
   BIO_free(in);
   X509_free(x);