Complain if we are attempting to encode with an invalid ASN.1 template

It never makes sense for multi-string or CHOICE types to have implicit tagging. If we have a template that uses the in this way then we should immediately fail. Thanks to David Benjamin from Google for reporting this issue. Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
openssl · Dec 8, 2020 · 41d6263 · 41d6263
1 parent 1ecc76f
commit 41d6263
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 5 deletions.
diff --git a/crypto/asn1/asn1_err.c b/crypto/asn1/asn1_err.c
@@ -1,6 +1,6 @@
 /*
  * Generated by util/mkerr.pl DO NOT EDIT
- * Copyright 1995-2018 The OpenSSL Project Authors. All Rights Reserved.
+ * Copyright 1995-2020 The OpenSSL Project Authors. All Rights Reserved.
  *
  * Licensed under the OpenSSL license (the "License").  You may not use
  * this file except in compliance with the License.  You can obtain a copy
@@ -49,6 +49,7 @@ static const ERR_STRING_DATA ASN1_str_functs[] = {
      "asn1_item_embed_d2i"},
     {ERR_PACK(ERR_LIB_ASN1, ASN1_F_ASN1_ITEM_EMBED_NEW, 0),
      "asn1_item_embed_new"},
+    {ERR_PACK(ERR_LIB_ASN1, ASN1_F_ASN1_ITEM_EX_I2D, 0), "ASN1_item_ex_i2d"},
     {ERR_PACK(ERR_LIB_ASN1, ASN1_F_ASN1_ITEM_FLAGS_I2D, 0),
      "asn1_item_flags_i2d"},
     {ERR_PACK(ERR_LIB_ASN1, ASN1_F_ASN1_ITEM_I2D_BIO, 0), "ASN1_item_i2d_bio"},

diff --git a/crypto/asn1/tasn_enc.c b/crypto/asn1/tasn_enc.c
@@ -103,9 +103,25 @@ int ASN1_item_ex_i2d(ASN1_VALUE **pval, unsigned char **out,
         return asn1_i2d_ex_primitive(pval, out, it, tag, aclass);
 
     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_I2D, ASN1_R_BAD_TEMPLATE);
+            return -1;
+        }
         return asn1_i2d_ex_primitive(pval, out, it, -1, aclass);
 
     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_I2D, ASN1_R_BAD_TEMPLATE);
+            return -1;
+        }
         if (asn1_cb && !asn1_cb(ASN1_OP_I2D_PRE, pval, it, NULL))
             return 0;
         i = asn1_get_choice_selector(pval, it);

diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt
@@ -36,6 +36,7 @@ ASN1_F_ASN1_ITEM_D2I_FP:206:ASN1_item_d2i_fp
 ASN1_F_ASN1_ITEM_DUP:191:ASN1_item_dup
 ASN1_F_ASN1_ITEM_EMBED_D2I:120:asn1_item_embed_d2i
 ASN1_F_ASN1_ITEM_EMBED_NEW:121:asn1_item_embed_new
+ASN1_F_ASN1_ITEM_EX_I2D:144:ASN1_item_ex_i2d
 ASN1_F_ASN1_ITEM_FLAGS_I2D:118:asn1_item_flags_i2d
 ASN1_F_ASN1_ITEM_I2D_BIO:192:ASN1_item_i2d_bio
 ASN1_F_ASN1_ITEM_I2D_FP:193:ASN1_item_i2d_fp

diff --git a/include/openssl/asn1err.h b/include/openssl/asn1err.h
@@ -1,6 +1,6 @@
 /*
  * Generated by util/mkerr.pl DO NOT EDIT
- * Copyright 1995-2019 The OpenSSL Project Authors. All Rights Reserved.
+ * Copyright 1995-2020 The OpenSSL Project Authors. All Rights Reserved.
  *
  * Licensed under the OpenSSL license (the "License").  You may not use
  * this file except in compliance with the License.  You can obtain a copy
@@ -11,9 +11,7 @@
 #ifndef HEADER_ASN1ERR_H
 # define HEADER_ASN1ERR_H
 
-# ifndef HEADER_SYMHACKS_H
-#  include <openssl/symhacks.h>
-# endif
+# include <openssl/symhacks.h>
 
 # ifdef  __cplusplus
 extern "C"
@@ -53,6 +51,7 @@ int ERR_load_ASN1_strings(void);
 # define ASN1_F_ASN1_ITEM_DUP                             191
 # define ASN1_F_ASN1_ITEM_EMBED_D2I                       120
 # define ASN1_F_ASN1_ITEM_EMBED_NEW                       121
+# define ASN1_F_ASN1_ITEM_EX_I2D                          144
 # define ASN1_F_ASN1_ITEM_FLAGS_I2D                       118
 # define ASN1_F_ASN1_ITEM_I2D_BIO                         192
 # define ASN1_F_ASN1_ITEM_I2D_FP                          193