Fix up some integer types in crypto/asn1
tag and utype are always accessed as int, so make the structs match.
Boolean ASN1_ITEMs put an ASN1_BOOLEAN in it->size, so add a cast. Also
fix the time set_string functions to call the underlying CBS parser
directly, so they don't need to put a strlen into an int.
Bug: 516
Change-Id: Ie10e7eaf58ec0b0dec59813a0ddcb0197fce1fd1
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55449
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
diff --git a/crypto/asn1/a_gentm.c b/crypto/asn1/a_gentm.c
index 283ff7d..0104af0 100644
--- a/crypto/asn1/a_gentm.c
+++ b/crypto/asn1/a_gentm.c
@@ -81,22 +81,16 @@
}
int ASN1_GENERALIZEDTIME_set_string(ASN1_GENERALIZEDTIME *s, const char *str) {
- ASN1_GENERALIZEDTIME t;
-
- t.type = V_ASN1_GENERALIZEDTIME;
- t.length = strlen(str);
- t.data = (unsigned char *)str;
- if (ASN1_GENERALIZEDTIME_check(&t)) {
- if (s != NULL) {
- if (!ASN1_STRING_set((ASN1_STRING *)s, (unsigned char *)str, t.length)) {
- return 0;
- }
- s->type = V_ASN1_GENERALIZEDTIME;
- }
- return 1;
- } else {
+ size_t len = strlen(str);
+ CBS cbs;
+ CBS_init(&cbs, (const uint8_t *)str, len);
+ if (!CBS_parse_generalized_time(&cbs, /*out_tm=*/NULL,
+ /*allow_timezone_offset=*/0) ||
+ !ASN1_STRING_set(s, str, len)) {
return 0;
}
+ s->type = V_ASN1_GENERALIZEDTIME;
+ return 1;
}
ASN1_GENERALIZEDTIME *ASN1_GENERALIZEDTIME_set(ASN1_GENERALIZEDTIME *s,
diff --git a/crypto/asn1/a_object.c b/crypto/asn1/a_object.c
index c854fb8..8c420c6 100644
--- a/crypto/asn1/a_object.c
+++ b/crypto/asn1/a_object.c
@@ -107,8 +107,12 @@
}
static int write_str(BIO *bp, const char *str) {
- int len = strlen(str);
- return BIO_write(bp, str, len) == len ? len : -1;
+ size_t len = strlen(str);
+ if (len > INT_MAX) {
+ OPENSSL_PUT_ERROR(ASN1, ERR_R_OVERFLOW);
+ return -1;
+ }
+ return BIO_write(bp, str, (int)len) == (int)len ? (int)len : -1;
}
int i2a_ASN1_OBJECT(BIO *bp, const ASN1_OBJECT *a) {
diff --git a/crypto/asn1/a_strex.c b/crypto/asn1/a_strex.c
index c996acc..229de94 100644
--- a/crypto/asn1/a_strex.c
+++ b/crypto/asn1/a_strex.c
@@ -121,7 +121,7 @@
return maybe_write(out, &u8, 1) ? 1 : -1;
}
- int len = strlen(buf);
+ int len = (int)strlen(buf); // |buf| is guaranteed to be short.
return maybe_write(out, buf, len) ? len : -1;
}
diff --git a/crypto/asn1/a_time.c b/crypto/asn1/a_time.c
index 39b54a6..643584b 100644
--- a/crypto/asn1/a_time.c
+++ b/crypto/asn1/a_time.c
@@ -164,28 +164,9 @@
return NULL;
}
-
int ASN1_TIME_set_string(ASN1_TIME *s, const char *str) {
- ASN1_TIME t;
-
- t.length = strlen(str);
- t.data = (unsigned char *)str;
- t.flags = 0;
-
- t.type = V_ASN1_UTCTIME;
-
- if (!ASN1_TIME_check(&t)) {
- t.type = V_ASN1_GENERALIZEDTIME;
- if (!ASN1_TIME_check(&t)) {
- return 0;
- }
- }
-
- if (s && !ASN1_STRING_copy((ASN1_STRING *)s, (ASN1_STRING *)&t)) {
- return 0;
- }
-
- return 1;
+ return ASN1_UTCTIME_set_string(s, str) ||
+ ASN1_GENERALIZEDTIME_set_string(s, str);
}
static int asn1_time_to_tm(struct tm *tm, const ASN1_TIME *t,
diff --git a/crypto/asn1/a_utctm.c b/crypto/asn1/a_utctm.c
index 201c654..bf09c90 100644
--- a/crypto/asn1/a_utctm.c
+++ b/crypto/asn1/a_utctm.c
@@ -82,22 +82,16 @@
}
int ASN1_UTCTIME_set_string(ASN1_UTCTIME *s, const char *str) {
- ASN1_UTCTIME t;
-
- t.type = V_ASN1_UTCTIME;
- t.length = strlen(str);
- t.data = (unsigned char *)str;
- if (ASN1_UTCTIME_check(&t)) {
- if (s != NULL) {
- if (!ASN1_STRING_set((ASN1_STRING *)s, (unsigned char *)str, t.length)) {
- return 0;
- }
- s->type = V_ASN1_UTCTIME;
- }
- return 1;
- } else {
+ size_t len = strlen(str);
+ CBS cbs;
+ CBS_init(&cbs, (const uint8_t *)str, len);
+ if (!CBS_parse_utc_time(&cbs, /*out_tm=*/NULL,
+ /*allow_timezone_offset=*/1) ||
+ !ASN1_STRING_set(s, str, len)) {
return 0;
}
+ s->type = V_ASN1_UTCTIME;
+ return 1;
}
ASN1_UTCTIME *ASN1_UTCTIME_set(ASN1_UTCTIME *s, time_t t) {
diff --git a/crypto/asn1/asn1_test.cc b/crypto/asn1/asn1_test.cc
index 3182a85..fc17f40 100644
--- a/crypto/asn1/asn1_test.cc
+++ b/crypto/asn1/asn1_test.cc
@@ -1048,6 +1048,34 @@
}
}
+TEST(ASN1Test, TimeSetString) {
+ bssl::UniquePtr<ASN1_STRING> s(ASN1_STRING_new());
+ ASSERT_TRUE(s);
+
+ ASSERT_TRUE(ASN1_UTCTIME_set_string(s.get(), "700101000000Z"));
+ EXPECT_EQ(V_ASN1_UTCTIME, ASN1_STRING_type(s.get()));
+ EXPECT_EQ("700101000000Z", ASN1StringToStdString(s.get()));
+
+ ASSERT_TRUE(ASN1_GENERALIZEDTIME_set_string(s.get(), "19700101000000Z"));
+ EXPECT_EQ(V_ASN1_GENERALIZEDTIME, ASN1_STRING_type(s.get()));
+ EXPECT_EQ("19700101000000Z", ASN1StringToStdString(s.get()));
+
+ // |ASN1_TIME_set_string| accepts either format. It relies on there being no
+ // overlap between the two.
+ ASSERT_TRUE(ASN1_TIME_set_string(s.get(), "700101000000Z"));
+ EXPECT_EQ(V_ASN1_UTCTIME, ASN1_STRING_type(s.get()));
+ EXPECT_EQ("700101000000Z", ASN1StringToStdString(s.get()));
+
+ ASSERT_TRUE(ASN1_TIME_set_string(s.get(), "19700101000000Z"));
+ EXPECT_EQ(V_ASN1_GENERALIZEDTIME, ASN1_STRING_type(s.get()));
+ EXPECT_EQ("19700101000000Z", ASN1StringToStdString(s.get()));
+
+ // Invalid inputs are rejected.
+ EXPECT_FALSE(ASN1_UTCTIME_set_string(s.get(), "nope"));
+ EXPECT_FALSE(ASN1_GENERALIZEDTIME_set_string(s.get(), "nope"));
+ EXPECT_FALSE(ASN1_TIME_set_string(s.get(), "nope"));
+}
+
TEST(ASN1Test, AdjTime) {
struct tm tm1, tm2;
int days, secs;
diff --git a/crypto/asn1/tasn_fre.c b/crypto/asn1/tasn_fre.c
index 3da1fa6..82c0a0c 100644
--- a/crypto/asn1/tasn_fre.c
+++ b/crypto/asn1/tasn_fre.c
@@ -218,7 +218,7 @@
case V_ASN1_BOOLEAN:
if (it) {
- *(ASN1_BOOLEAN *)pval = it->size;
+ *(ASN1_BOOLEAN *)pval = (ASN1_BOOLEAN)it->size;
} else {
*(ASN1_BOOLEAN *)pval = -1;
}
diff --git a/crypto/asn1/tasn_new.c b/crypto/asn1/tasn_new.c
index 97411c5..563674d 100644
--- a/crypto/asn1/tasn_new.c
+++ b/crypto/asn1/tasn_new.c
@@ -273,9 +273,6 @@
// all the old functions.
static int ASN1_primitive_new(ASN1_VALUE **pval, const ASN1_ITEM *it) {
- ASN1_TYPE *typ;
- int utype;
-
if (!it) {
return 0;
}
@@ -284,6 +281,7 @@
// |ASN1_PRIMITIVE_FUNCS| table of calbacks.
assert(it->funcs == NULL);
+ int utype;
if (it->itype == ASN1_ITYPE_MSTRING) {
utype = -1;
} else {
@@ -295,15 +293,15 @@
return 1;
case V_ASN1_BOOLEAN:
- *(ASN1_BOOLEAN *)pval = it->size;
+ *(ASN1_BOOLEAN *)pval = (ASN1_BOOLEAN)it->size;
return 1;
case V_ASN1_NULL:
*pval = (ASN1_VALUE *)1;
return 1;
- case V_ASN1_ANY:
- typ = OPENSSL_malloc(sizeof(ASN1_TYPE));
+ case V_ASN1_ANY: {
+ ASN1_TYPE *typ = OPENSSL_malloc(sizeof(ASN1_TYPE));
if (!typ) {
return 0;
}
@@ -311,6 +309,7 @@
typ->type = -1;
*pval = (ASN1_VALUE *)typ;
break;
+ }
default:
*pval = (ASN1_VALUE *)ASN1_STRING_type_new(utype);
@@ -333,7 +332,7 @@
utype = it->utype;
}
if (utype == V_ASN1_BOOLEAN) {
- *(ASN1_BOOLEAN *)pval = it->size;
+ *(ASN1_BOOLEAN *)pval = (ASN1_BOOLEAN)it->size;
} else {
*pval = NULL;
}
diff --git a/include/openssl/asn1t.h b/include/openssl/asn1t.h
index b32fe34..324eb57 100644
--- a/include/openssl/asn1t.h
+++ b/include/openssl/asn1t.h
@@ -349,7 +349,7 @@
struct ASN1_TEMPLATE_st {
uint32_t flags; /* Various flags */
-long tag; /* tag, not used if no tagging */
+int tag; /* tag, not used if no tagging */
unsigned long offset; /* Offset of this field in structure */
const char *field_name; /* Field name */
ASN1_ITEM_EXP *item; /* Relevant ASN1_ITEM or ASN1_ADB */
@@ -455,7 +455,7 @@
struct ASN1_ITEM_st {
char itype; /* The item type, primitive, SEQUENCE, CHOICE or extern */
-long utype; /* underlying type */
+int utype; /* underlying type */
const ASN1_TEMPLATE *templates; /* If SEQUENCE or CHOICE this contains the contents */
long tcount; /* Number of templates if SEQUENCE or CHOICE */
const void *funcs; /* functions that handle this type */
