Reject -1 types in ASN1_TYPE and MSTRINGs when encoding. See https://github.com/openssl/openssl/issues/16538 Update-Note: A default-constructed object with a required ANY or string-like CHOICE field cannot be encoded until the field is specified. Note this affects i2d_X509: notBefore and notAfter are string-like CHOICEs in OpenSSL. Bug: 429 Change-Id: I97d971fa588ab72be25a4c1eb7310ed330f16c4f Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49349 Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/crypto/asn1/asn1_test.cc b/crypto/asn1/asn1_test.cc index e1c9cbd..5cd17af 100644 --- a/crypto/asn1/asn1_test.cc +++ b/crypto/asn1/asn1_test.cc
@@ -1096,6 +1096,29 @@ EXPECT_EQ(-1, i2d_X509_ALGOR(alg.get(), nullptr)); } +// Encoding invalid |ASN1_TYPE|s should fail. |ASN1_TYPE|s are +// default-initialized to an invalid type. +TEST(ASN1Test, InvalidASN1Type) { + bssl::UniquePtr<ASN1_TYPE> obj(ASN1_TYPE_new()); + ASSERT_TRUE(obj); + EXPECT_EQ(-1, obj->type); + EXPECT_EQ(-1, i2d_ASN1_TYPE(obj.get(), nullptr)); +} + +// Encoding invalid MSTRING types should fail. An MSTRING is a CHOICE of +// string-like types. They are initialized to an invalid type. +TEST(ASN1Test, InvalidMSTRING) { + bssl::UniquePtr<ASN1_STRING> obj(ASN1_TIME_new()); + ASSERT_TRUE(obj); + EXPECT_EQ(-1, obj->type); + EXPECT_EQ(-1, i2d_ASN1_TIME(obj.get(), nullptr)); + + obj.reset(DIRECTORYSTRING_new()); + ASSERT_TRUE(obj); + EXPECT_EQ(-1, obj->type); + EXPECT_EQ(-1, i2d_DIRECTORYSTRING(obj.get(), nullptr)); +} + // The ASN.1 macros do not work on Windows shared library builds, where usage of // |OPENSSL_EXPORT| is a bit stricter. #if !defined(OPENSSL_WINDOWS) || !defined(BORINGSSL_SHARED_LIBRARY)
diff --git a/crypto/asn1/tasn_enc.c b/crypto/asn1/tasn_enc.c index 49b79d2..7e79da2 100644 --- a/crypto/asn1/tasn_enc.c +++ b/crypto/asn1/tasn_enc.c
@@ -554,6 +554,11 @@ /* If MSTRING type set the underlying type */ strtmp = (ASN1_STRING *)*pval; utype = strtmp->type; + if (utype < 0 && utype != V_ASN1_OTHER) { + /* MSTRINGs can have type -1 when default-constructed. */ + OPENSSL_PUT_ERROR(ASN1, ASN1_R_WRONG_TYPE); + return -1; + } /* Negative INTEGER and ENUMERATED values use |ASN1_STRING| type values * that do not match their corresponding utype values. INTEGERs cannot * participate in MSTRING types, but ENUMERATEDs can. @@ -574,6 +579,11 @@ ASN1_TYPE *typ; typ = (ASN1_TYPE *)*pval; utype = typ->type; + if (utype < 0 && utype != V_ASN1_OTHER) { + /* |ASN1_TYPE|s can have type -1 when default-constructed. */ + OPENSSL_PUT_ERROR(ASN1, ASN1_R_WRONG_TYPE); + return -1; + } *putype = utype; pval = &typ->value.asn1_value; } else