Check tag class and constructed bit in d2i_ASN1_BOOLEAN.
d2i_ASN1_BOOLEAN and i2d_ASN1_BOOLEAN don't go through the macros
because ASN1_BOOLEAN is a slightly weird type (int instead of pointer).
Their tag checks were missing a few bits.
This does not affect any other d2i functions. Those already go through
the ASN1_ITEM machinery.
Change-Id: Ic892cd2a8b8f9ceb11e43d931f8aa6df921997d3
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49866
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/asn1/a_bool.c b/crypto/asn1/a_bool.c
index 1282176..3f32e80 100644
--- a/crypto/asn1/a_bool.c
+++ b/crypto/asn1/a_bool.c
@@ -99,11 +99,16 @@
return -1;
}
- if (tag != V_ASN1_BOOLEAN) {
- OPENSSL_PUT_ERROR(ASN1, ASN1_R_EXPECTING_A_BOOLEAN);
+ if (inf & V_ASN1_CONSTRUCTED) {
+ OPENSSL_PUT_ERROR(ASN1, ASN1_R_TYPE_NOT_PRIMITIVE);
return -1;
}
+ if (tag != V_ASN1_BOOLEAN || xclass != V_ASN1_UNIVERSAL) {
+ OPENSSL_PUT_ERROR(ASN1, ASN1_R_EXPECTING_A_BOOLEAN);
+ return -1;
+ }
+
if (len != 1) {
OPENSSL_PUT_ERROR(ASN1, ASN1_R_BOOLEAN_IS_WRONG_LENGTH);
return -1;
diff --git a/crypto/asn1/asn1_test.cc b/crypto/asn1/asn1_test.cc
index 738e422..1ea7644 100644
--- a/crypto/asn1/asn1_test.cc
+++ b/crypto/asn1/asn1_test.cc
@@ -132,15 +132,49 @@
TestSerialize(obj, i2d_ASN1_OBJECT, kDER);
}
-TEST(ASN1Test, SerializeBoolean) {
+TEST(ASN1Test, Boolean) {
static const uint8_t kTrue[] = {0x01, 0x01, 0xff};
TestSerialize(0xff, i2d_ASN1_BOOLEAN, kTrue);
// Other constants are also correctly encoded as TRUE.
TestSerialize(1, i2d_ASN1_BOOLEAN, kTrue);
TestSerialize(0x100, i2d_ASN1_BOOLEAN, kTrue);
+ const uint8_t *ptr = kTrue;
+ EXPECT_EQ(0xff, d2i_ASN1_BOOLEAN(nullptr, &ptr, sizeof(kTrue)));
+ EXPECT_EQ(ptr, kTrue + sizeof(kTrue));
+
static const uint8_t kFalse[] = {0x01, 0x01, 0x00};
TestSerialize(0x00, i2d_ASN1_BOOLEAN, kFalse);
+
+ ptr = kFalse;
+ EXPECT_EQ(0, d2i_ASN1_BOOLEAN(nullptr, &ptr, sizeof(kFalse)));
+ EXPECT_EQ(ptr, kFalse + sizeof(kFalse));
+
+ const std::vector<uint8_t> kInvalidBooleans[] = {
+ // No tag header.
+ {},
+ // No length.
+ {0x01},
+ // Truncated contents.
+ {0x01, 0x01},
+ // Contents too short or too long.
+ {0x01, 0x00},
+ {0x01, 0x02, 0x00, 0x00},
+ // Wrong tag number.
+ {0x02, 0x01, 0x00},
+ // Wrong tag class.
+ {0x81, 0x01, 0x00},
+ // Element is constructed.
+ {0x21, 0x01, 0x00},
+ // TODO(https://crbug.com/boringssl/354): Reject non-DER encodings of TRUE
+ // and test this.
+ };
+ for (const auto &invalid : kInvalidBooleans) {
+ SCOPED_TRACE(Bytes(invalid));
+ ptr = invalid.data();
+ EXPECT_EQ(-1, d2i_ASN1_BOOLEAN(nullptr, &ptr, invalid.size()));
+ ERR_clear_error();
+ }
}
// The templates go through a different codepath, so test them separately.