commit 723faad51da1e5b0b7ce5a25be018a2ad2d972c6
author David Benjamin <davidben@google.com> Fri Oct 02 16:11:10 2020 -0400
committer Adam Langley <agl@google.com> Fri Oct 02 21:39:45 2020 +0000
tree a41aa7d677951227b51ce47433769ea81b9184b0
parent b117a3a0b7bd11fe6ebd503ec6b45d6b910b41a1

Fix some malloc error handling.

See 309e73dfe067b3b774ef6f57bf665f41373a81ca from upstream, though note
that v3_alt.c's fix was rewritten. (We don't have sk_reserve, and I
don't think their fix was quite right anyway.)

Change-Id: Ieabd19d87d4628658324b212cce2ed3ce451ad22
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43284
Reviewed-by: Adam Langley <agl@google.com>

crypto/x509v3/pcy_data.c

diff --git a/crypto/x509v3/pcy_data.c b/crypto/x509v3/pcy_data.c
index 498de4d..58584c2 100644
--- a/crypto/x509v3/pcy_data.c
+++ b/crypto/x509v3/pcy_data.c
@@ -98,13 +98,15 @@
     } else
         id = NULL;
     ret = OPENSSL_malloc(sizeof(X509_POLICY_DATA));
-    if (!ret)
+    if (!ret) {
+        OPENSSL_PUT_ERROR(X509V3, ERR_R_MALLOC_FAILURE);
+        ASN1_OBJECT_free(id);
         return NULL;
+    }
     ret->expected_policy_set = sk_ASN1_OBJECT_new_null();
     if (!ret->expected_policy_set) {
         OPENSSL_free(ret);
-        if (id)
-            ASN1_OBJECT_free(id);
+        ASN1_OBJECT_free(id);
         return NULL;
     }
 

crypto/x509v3/v3_alt.c

diff --git a/crypto/x509v3/v3_alt.c b/crypto/x509v3/v3_alt.c
index 0e79b45..7a6e3e0 100644
--- a/crypto/x509v3/v3_alt.c
+++ b/crypto/x509v3/v3_alt.c
@@ -288,40 +288,40 @@
 
 static int copy_issuer(X509V3_CTX *ctx, GENERAL_NAMES *gens)
 {
-    GENERAL_NAMES *ialt;
-    GENERAL_NAME *gen;
-    X509_EXTENSION *ext;
-    int i;
-    size_t j;
     if (ctx && (ctx->flags == CTX_TEST))
         return 1;
     if (!ctx || !ctx->issuer_cert) {
         OPENSSL_PUT_ERROR(X509V3, X509V3_R_NO_ISSUER_DETAILS);
-        goto err;
+        return 0;
     }
-    i = X509_get_ext_by_NID(ctx->issuer_cert, NID_subject_alt_name, -1);
+    int i = X509_get_ext_by_NID(ctx->issuer_cert, NID_subject_alt_name, -1);
     if (i < 0)
         return 1;
+
+    int ret = 0;
+    GENERAL_NAMES *ialt = NULL;
+    X509_EXTENSION *ext;
     if (!(ext = X509_get_ext(ctx->issuer_cert, i)) ||
         !(ialt = X509V3_EXT_d2i(ext))) {
         OPENSSL_PUT_ERROR(X509V3, X509V3_R_ISSUER_DECODE_ERROR);
         goto err;
     }
 
-    for (j = 0; j < sk_GENERAL_NAME_num(ialt); j++) {
-        gen = sk_GENERAL_NAME_value(ialt, j);
+    for (size_t j = 0; j < sk_GENERAL_NAME_num(ialt); j++) {
+        GENERAL_NAME *gen = sk_GENERAL_NAME_value(ialt, j);
         if (!sk_GENERAL_NAME_push(gens, gen)) {
             OPENSSL_PUT_ERROR(X509V3, ERR_R_MALLOC_FAILURE);
             goto err;
         }
+        /* Ownership of |gen| has moved from |ialt| to |gens|. */
+        sk_GENERAL_NAME_set(ialt, j, NULL);
     }
-    sk_GENERAL_NAME_free(ialt);
 
-    return 1;
+    ret = 1;
 
- err:
-    return 0;
-
+err:
+    GENERAL_NAMES_free(ialt);
+    return ret;
 }
 
 static GENERAL_NAMES *v2i_subject_alt(X509V3_EXT_METHOD *method,