Check some ASN1_STRING types in crypto/x509 This adds runtime checks that types which are aliases of ASN1_STRING are in fact the expected ASN.1 type. Not comprehensive -- I got the obvious ones from x509.h. These checks are not generally covered by unit tests, except for one which was easy to test as a sanity-check. Bug: 445 Change-Id: I8cd689b6b1e6121fce62c7f0ab25fee7e2a0b2ff Update-Note: Various X.509 functions will now fail given the wrong ASN1_STRING subtype. Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54286 Reviewed-by: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com>

diff --git a/crypto/x509/a_sign.c b/crypto/x509/a_sign.c
index ed9e79bb..3711a00 100644
--- a/crypto/x509/a_sign.c
+++ b/crypto/x509/a_sign.c

@@ -67,6 +67,10 @@
 int ASN1_item_sign(const ASN1_ITEM *it, X509_ALGOR *algor1, X509_ALGOR *algor2,
                    ASN1_BIT_STRING *signature, void *asn, EVP_PKEY *pkey,
                    const EVP_MD *type) {
+  if (signature->type != V_ASN1_BIT_STRING) {
+    OPENSSL_PUT_ERROR(ASN1, ASN1_R_WRONG_TYPE);
+    return 0;
+  }
   EVP_MD_CTX ctx;
   EVP_MD_CTX_init(&ctx);
   if (!EVP_DigestSignInit(&ctx, NULL, type, NULL, pkey)) {
@@ -83,6 +87,11 @@
   unsigned char *buf_in = NULL, *buf_out = NULL;
   size_t inl = 0, outl = 0;
 
+  if (signature->type != V_ASN1_BIT_STRING) {
+    OPENSSL_PUT_ERROR(ASN1, ASN1_R_WRONG_TYPE);
+    goto err;
+  }
+
   pkey = EVP_PKEY_CTX_get0_pkey(ctx->pctx);
 
   // Write out the requested copies of the AlgorithmIdentifier.

diff --git a/crypto/x509/x509_cmp.c b/crypto/x509/x509_cmp.c
index 9105b2f..a85c7c1 100644
--- a/crypto/x509/x509_cmp.c
+++ b/crypto/x509/x509_cmp.c

@@ -232,6 +232,10 @@
     return NULL;
   }
 
+  if (serial->type != V_ASN1_INTEGER) {
+    return NULL;
+  }
+
   x.cert_info = &cinf;
   cinf.serialNumber = serial;
   cinf.issuer = name;

diff --git a/crypto/x509/x509_lu.c b/crypto/x509/x509_lu.c
index 8165d24..ea32e7b 100644
--- a/crypto/x509/x509_lu.c
+++ b/crypto/x509/x509_lu.c

@@ -146,6 +146,9 @@
   if ((ctx->method == NULL) || (ctx->method->get_by_issuer_serial == NULL)) {
     return 0;
   }
+  if (serial->type != V_ASN1_INTEGER) {
+    return 0;
+  }
   return ctx->method->get_by_issuer_serial(ctx, type, name, serial, ret) > 0;
 }
 

diff --git a/crypto/x509/x509_set.c b/crypto/x509/x509_set.c
index 0f0c5d0..8c920be 100644
--- a/crypto/x509/x509_set.c
+++ b/crypto/x509/x509_set.c

@@ -98,8 +98,12 @@
 }
 
 int X509_set_serialNumber(X509 *x, const ASN1_INTEGER *serial) {
-  ASN1_INTEGER *in;
+  if (serial->type != V_ASN1_INTEGER) {
+    OPENSSL_PUT_ERROR(ASN1, ASN1_R_WRONG_TYPE);
+    return 0;
+  }
 
+  ASN1_INTEGER *in;
   if (x == NULL) {
     return 0;
   }

diff --git a/crypto/x509/x509_test.cc b/crypto/x509/x509_test.cc
index 1bcc569..c639ef0 100644
--- a/crypto/x509/x509_test.cc
+++ b/crypto/x509/x509_test.cc

@@ -4806,3 +4806,16 @@
   X509_NAME_ENTRY_free(X509_NAME_delete_entry(name.get(), 1));
   check_name("CN=Name,O=Org");
 }
+
+// Tests that non-integer types are rejected when passed as an argument to
+// X509_set_serialNumber().
+TEST(X509Test, SetSerialNumberChecksASN1StringType) {
+  bssl::UniquePtr<X509> root = CertFromPEM(kRootCAPEM);
+  ASSERT_TRUE(root);
+
+  // Passing an IA5String to X509_set_serialNumber() should fail.
+  bssl::UniquePtr<ASN1_IA5STRING> str(ASN1_IA5STRING_new());
+  ASSERT_TRUE(str);
+  int r = X509_set_serialNumber(root.get(), str.get());
+  ASSERT_EQ(r, 0);
+}

diff --git a/crypto/x509/x509cset.c b/crypto/x509/x509cset.c
index 9558128..408723f 100644
--- a/crypto/x509/x509cset.c
+++ b/crypto/x509/x509cset.c

@@ -216,6 +216,11 @@
                                   const ASN1_INTEGER *serial) {
   ASN1_INTEGER *in;
 
+  if (serial->type != V_ASN1_INTEGER) {
+    OPENSSL_PUT_ERROR(ASN1, ASN1_R_WRONG_TYPE);
+    return 0;
+  }
+
   if (revoked == NULL) {
     return 0;
   }

diff --git a/crypto/x509/x_crl.c b/crypto/x509/x_crl.c
index a8e6c74..573c115 100644
--- a/crypto/x509/x_crl.c
+++ b/crypto/x509/x_crl.c

@@ -430,6 +430,9 @@
 
 static int crl_lookup(X509_CRL *crl, X509_REVOKED **ret, ASN1_INTEGER *serial,
                       X509_NAME *issuer) {
+  if (serial->type != V_ASN1_INTEGER) {
+    return 0;
+  }
   X509_REVOKED rtmp, *rev;
   size_t idx;
   rtmp.serialNumber = serial;