commit c4d5925ca6cf0e8e2617e457d0af4b78514e6f37
author David Benjamin <davidben@google.com> Thu Feb 09 19:41:18 2017 -0500
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Fri Feb 17 01:11:21 2017 +0000
tree 9ae61b422f3ebd964dde287520d2885523188ce1
parent ec1d9637e19634a8b0c65511e91b4ac01776014c

Fix various malloc failure checks.

asn1_template_noexp_d2i call ASN1_item_ex_free(&skfield,...) on error.

Reworked error handling in asn1_item_ex_combine_new:
- call ASN1_item_ex_free and return the correct error code if
  ASN1_template_new failed.
- dont call ASN1_item_ex_free if ASN1_OP_NEW_PRE failed.

Reworked error handing in x509_name_ex_d2i and x509_name_encode.

(Imported from upstream's 748cb9a17f4f2b77aad816cf658cd4025dc847ee.)

I believe the tasn1_new.c change is a no-op since we have no
ASN1_OP_NEW_PRE hooks anymore. I'm not sure what the commit message is
referring to with ASN1_template_new. It also seems odd as
ASN1_item_ex_free should probably be able to survive *pval being NULL.
Whatever.

We'd previously tried to fix x509_name_ex_d2i, but I think ours wasn't
quite right. (This thing is a mess...) I've aligned that function with
upstream.

Change-Id: Ie71521cd8a1ec357876caadd13be1ce247110f76
Reviewed-on: https://boringssl-review.googlesource.com/13831
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>

crypto/asn1/tasn_dec.c

diff --git a/crypto/asn1/tasn_dec.c b/crypto/asn1/tasn_dec.c
index 40778a8..404f969 100644
--- a/crypto/asn1/tasn_dec.c
+++ b/crypto/asn1/tasn_dec.c
@@ -667,6 +667,7 @@
             }
             len -= p - q;
             if (!sk_ASN1_VALUE_push((STACK_OF(ASN1_VALUE) *)*val, skfield)) {
+                ASN1_item_ex_free(&skfield, ASN1_ITEM_ptr(tt->item));
                 OPENSSL_PUT_ERROR(ASN1, ERR_R_MALLOC_FAILURE);
                 goto err;
             }

crypto/asn1/tasn_new.c

diff --git a/crypto/asn1/tasn_new.c b/crypto/asn1/tasn_new.c
index 053b732..10cf954 100644
--- a/crypto/asn1/tasn_new.c
+++ b/crypto/asn1/tasn_new.c
@@ -160,7 +160,7 @@
         }
         asn1_set_choice_selector(pval, -1, it);
         if (asn1_cb && !asn1_cb(ASN1_OP_NEW_POST, pval, it, NULL))
-            goto auxerr;
+            goto auxerr2;
         break;
 
     case ASN1_ITYPE_NDEF_SEQUENCE:
@@ -188,10 +188,10 @@
         for (i = 0, tt = it->templates; i < it->tcount; tt++, i++) {
             pseqval = asn1_get_field_ptr(pval, tt);
             if (!ASN1_template_new(pseqval, tt))
-                goto memerr;
+                goto memerr2;
         }
         if (asn1_cb && !asn1_cb(ASN1_OP_NEW_POST, pval, it, NULL))
-            goto auxerr;
+            goto auxerr2;
         break;
     }
 #ifdef CRYPTO_MDEBUG
@@ -200,18 +200,20 @@
 #endif
     return 1;
 
+ memerr2:
+    ASN1_item_ex_free(pval, it);
  memerr:
     OPENSSL_PUT_ERROR(ASN1, ERR_R_MALLOC_FAILURE);
-    ASN1_item_ex_free(pval, it);
 #ifdef CRYPTO_MDEBUG
     if (it->sname)
         CRYPTO_pop_info();
 #endif
     return 0;
 
+ auxerr2:
+    ASN1_item_ex_free(pval, it);
  auxerr:
     OPENSSL_PUT_ERROR(ASN1, ASN1_R_AUX_ERROR);
-    ASN1_item_ex_free(pval, it);
 #ifdef CRYPTO_MDEBUG
     if (it->sname)
         CRYPTO_pop_info();