Backport of:

From c521851116486d0cb351c46506d309dce0ae4c56 Mon Sep 17 00:00:00 2001
From: Matt Caswell <matt@openssl.org>
Date: Thu, 12 Nov 2020 11:58:12 +0000
Subject: [PATCH] Check that multi-strings/CHOICE types don't use implicit
 tagging

It never makes sense for multi-string or CHOICE types to use implicit
tagging since the content would be ambiguous. It is an error in the
template if this ever happens. If we detect it we should stop parsing.

Thanks to David Benjamin from Google for reporting this issue.
---
 crypto/asn1/asn1_err.c    |  1 +
 crypto/asn1/tasn_dec.c    | 19 +++++++++++++++++++
 crypto/err/openssl.txt    |  1 +
 include/openssl/asn1err.h |  1 +
 4 files changed, 22 insertions(+)

--- a/crypto/asn1/asn1_err.c
+++ b/crypto/asn1/asn1_err.c
@@ -202,6 +202,7 @@ static ERR_STRING_DATA ASN1_str_reasons[
     {ERR_REASON(ASN1_R_AUX_ERROR), "aux error"},
     {ERR_REASON(ASN1_R_BAD_CLASS), "bad class"},
     {ERR_REASON(ASN1_R_BAD_OBJECT_HEADER), "bad object header"},
+    {ERR_REASON(ASN1_R_BAD_TEMPLATE), "bad template"},
     {ERR_REASON(ASN1_R_BAD_PASSWORD_READ), "bad password read"},
     {ERR_REASON(ASN1_R_BAD_TAG), "bad tag"},
     {ERR_REASON(ASN1_R_BMPSTRING_IS_WRONG_LENGTH),
--- a/crypto/asn1/tasn_dec.c
+++ b/crypto/asn1/tasn_dec.c
@@ -223,6 +223,15 @@ static int asn1_item_ex_d2i(ASN1_VALUE *
         break;
 
     case ASN1_ITYPE_MSTRING:
+        /*
+         * It never makes sense for multi-strings to have implicit tagging, so
+         * if tag != -1, then this looks like an error in the template.
+         */
+        if (tag != -1) {
+            ASN1err(ASN1_F_ASN1_ITEM_EX_D2I, ASN1_R_BAD_TEMPLATE);
+            goto err;
+        }
+
         p = *in;
         /* Just read in tag and class */
         ret = asn1_check_tlen(NULL, &otag, &oclass, NULL, NULL,
@@ -240,6 +249,7 @@ static int asn1_item_ex_d2i(ASN1_VALUE *
             ASN1err(ASN1_F_ASN1_ITEM_EX_D2I, ASN1_R_MSTRING_NOT_UNIVERSAL);
             goto err;
         }
+
         /* Check tag matches bit map */
         if (!(ASN1_tag2bit(otag) & it->utype)) {
             /* If OPTIONAL, assume this is OK */
@@ -316,6 +326,15 @@ static int asn1_item_ex_d2i(ASN1_VALUE *
         goto err;
 
     case ASN1_ITYPE_CHOICE:
+        /*
+         * It never makes sense for CHOICE types to have implicit tagging, so
+         * if tag != -1, then this looks like an error in the template.
+         */
+        if (tag != -1) {
+            ASN1err(ASN1_F_ASN1_ITEM_EX_D2I, ASN1_R_BAD_TEMPLATE);
+            goto err;
+        }
+
         if (asn1_cb && !asn1_cb(ASN1_OP_D2I_PRE, pval, it, NULL))
             goto auxerr;
         if (*pval) {
--- a/crypto/asn1/asn1.h
+++ b/crypto/asn1/asn1.h
@@ -1306,6 +1306,7 @@ void ERR_load_ASN1_strings(void);
 # define ASN1_R_AUX_ERROR                                 100
 # define ASN1_R_BAD_CLASS                                 101
 # define ASN1_R_BAD_OBJECT_HEADER                         102
+# define ASN1_R_BAD_TEMPLATE                              230
 # define ASN1_R_BAD_PASSWORD_READ                         103
 # define ASN1_R_BAD_TAG                                   104
 # define ASN1_R_BMPSTRING_IS_WRONG_LENGTH                 214