Don't set choice selector on parse failure.

Don't set choice selector on parse failure: this can pass unexpected values to the choice callback. Instead free up partial structure directly. CVE-2016-7053 Thanks to Tyler Nighswander of ForAllSecure for reporting this issue. Reviewed-by: Richard Levitte <levitte@openssl.org>
openssl · Nov 10, 2016 · 610b662 · 610b662
1 parent 99d9784
commit 610b662
Showing 1 changed file with 7 additions and 7 deletions.
diff --git a/crypto/asn1/tasn_dec.c b/crypto/asn1/tasn_dec.c
@@ -225,16 +225,14 @@ static int asn1_item_embed_d2i(ASN1_VALUE **pval, const unsigned char **in,
             /* If field not present, try the next one */
             if (ret == -1)
                 continue;
-            /*
-             * Set the choice selector here to ensure that the value is
-             * correctly freed upon error. It may be partially initialized
-             * even if parsing failed.
-             */
-            asn1_set_choice_selector(pval, i, it);
             /* If positive return, read OK, break loop */
             if (ret > 0)
                 break;
-            /* Otherwise must be an ASN1 parsing error */
+            /*
+             * Must be an ASN1 parsing error.
+             * Free up any partial choice value
+             */
+            asn1_template_free(pchptr, tt);
             errtt = tt;
             ASN1err(ASN1_F_ASN1_ITEM_EMBED_D2I, ERR_R_NESTED_ASN1_ERROR);
             goto err;
@@ -252,6 +250,8 @@ static int asn1_item_embed_d2i(ASN1_VALUE **pval, const unsigned char **in,
             goto err;
         }
 
+        asn1_set_choice_selector(pval, i, it);
+
         if (asn1_cb && !asn1_cb(ASN1_OP_D2I_POST, pval, it, NULL))
             goto auxerr;
         *in = p;