Check that multi-strings/CHOICE types don't use implicit tagging
authorMatt Caswell <matt@openssl.org>
Thu, 12 Nov 2020 11:58:12 +0000 (11:58 +0000)
committerMatt Caswell <matt@openssl.org>
Tue, 8 Dec 2020 10:17:03 +0000 (10:17 +0000)
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.

Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
crypto/asn1/asn1_err.c
crypto/asn1/tasn_dec.c
crypto/err/openssl.txt
include/openssl/asn1err.h

index d202094e2734875b68a16dd1bb623928e7eb0d3a..8957519cb26439534327f568b0bf449c420b8349 100644 (file)
@@ -21,6 +21,7 @@ static const ERR_STRING_DATA ASN1_str_reasons[] = {
     "asn1 sig parse error"},
     {ERR_PACK(ERR_LIB_ASN1, 0, ASN1_R_AUX_ERROR), "aux error"},
     {ERR_PACK(ERR_LIB_ASN1, 0, ASN1_R_BAD_OBJECT_HEADER), "bad object header"},
+    {ERR_PACK(ERR_LIB_ASN1, 0, ASN1_R_BAD_TEMPLATE), "bad template"},
     {ERR_PACK(ERR_LIB_ASN1, 0, ASN1_R_BMPSTRING_IS_WRONG_LENGTH),
     "bmpstring is wrong length"},
     {ERR_PACK(ERR_LIB_ASN1, 0, ASN1_R_BN_LIB), "bn lib"},
index 09adbf5d0747c982bf7469316c783ff5eaedf4f8..9a210e221f7d6e7be6639a128b69a7665eda37c7 100644 (file)
@@ -183,6 +183,15 @@ static int asn1_item_embed_d2i(ASN1_VALUE **pval, const unsigned char **in,
                                      tag, aclass, opt, ctx);
 
     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) {
+            ERR_raise(ERR_LIB_ASN1, ASN1_R_BAD_TEMPLATE);
+            goto err;
+        }
+
         p = *in;
         /* Just read in tag and class */
         ret = asn1_check_tlen(NULL, &otag, &oclass, NULL, NULL,
@@ -200,6 +209,7 @@ static int asn1_item_embed_d2i(ASN1_VALUE **pval, const unsigned char **in,
             ERR_raise(ERR_LIB_ASN1, ASN1_R_MSTRING_NOT_UNIVERSAL);
             goto err;
         }
+
         /* Check tag matches bit map */
         if (!(ASN1_tag2bit(otag) & it->utype)) {
             /* If OPTIONAL, assume this is OK */
@@ -216,6 +226,15 @@ static int asn1_item_embed_d2i(ASN1_VALUE **pval, const unsigned char **in,
         return ef->asn1_ex_d2i(pval, in, len, it, tag, aclass, opt, ctx);
 
     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) {
+            ERR_raise(ERR_LIB_ASN1, ASN1_R_BAD_TEMPLATE);
+            goto err;
+        }
+
         if (asn1_cb && !asn1_cb(ASN1_OP_D2I_PRE, pval, it, NULL))
             goto auxerr;
         if (*pval) {
index 88b6168214a5535b1d3e7e3476ecc78b78fdfc8a..a19ca7ceb959a492297175d069a55b392104841b 100644 (file)
@@ -1901,6 +1901,7 @@ ASN1_R_ASN1_PARSE_ERROR:203:asn1 parse error
 ASN1_R_ASN1_SIG_PARSE_ERROR:204:asn1 sig parse error
 ASN1_R_AUX_ERROR:100:aux error
 ASN1_R_BAD_OBJECT_HEADER:102:bad object header
+ASN1_R_BAD_TEMPLATE:230:bad template
 ASN1_R_BMPSTRING_IS_WRONG_LENGTH:214:bmpstring is wrong length
 ASN1_R_BN_LIB:105:bn lib
 ASN1_R_BOOLEAN_IS_WRONG_LENGTH:106:boolean is wrong length
index 08281187ef7f24bfd9339dd60153010c1458ee12..afae7c3a512eea531247b84898890a709453cbd4 100644 (file)
 # define ASN1_R_ASN1_SIG_PARSE_ERROR                      204
 # define ASN1_R_AUX_ERROR                                 100
 # define ASN1_R_BAD_OBJECT_HEADER                         102
+# define ASN1_R_BAD_TEMPLATE                              230
 # define ASN1_R_BMPSTRING_IS_WRONG_LENGTH                 214
 # define ASN1_R_BN_LIB                                    105
 # define ASN1_R_BOOLEAN_IS_WRONG_LENGTH                   106