commit cfa08c3b7740275ef3994ff07a2c7f4639c88ccb
author Adam Langley <alangley@gmail.com> Thu Nov 17 13:21:27 2016 -0800
committer Adam Langley <agl@google.com> Fri Nov 18 19:19:48 2016 +0000
tree f3d1b9ac22a86e3124af1ef63bc180525870d402
parent b5172a722ccb62e5bdd893e9d4c6741b8425c942

Enforce basic sanity of SCT lists.

According to the RFC[1], SCT lists may not be empty and nor may any SCT
itself be empty.

[1] https://tools.ietf.org/html/rfc6962#section-3.3

Change-Id: Ia1f855907588b36a4fea60872f87e25dc20782b4
Reviewed-on: https://boringssl-review.googlesource.com/12362
Reviewed-by: Adam Langley <agl@google.com>

ssl/t1_lib.c

diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 3679f8d..421232f 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -1357,6 +1357,7 @@
 
   /* TLS 1.3 SCTs are included in the Certificate extensions. */
   if (ssl3_protocol_version(ssl) >= TLS1_3_VERSION) {
+    *out_alert = SSL_AD_DECODE_ERROR;
     return 0;
   }
 
@@ -1364,7 +1365,7 @@
    * ClientHello and thus this function should never have been called. */
   assert(ssl->signed_cert_timestamps_enabled);
 
-  if (CBS_len(contents) == 0) {
+  if (!ssl_is_sct_list_valid(contents)) {
     *out_alert = SSL_AD_DECODE_ERROR;
     return 0;
   }
@@ -3469,3 +3470,26 @@
   EVP_PKEY_free(key);
   return ret;
 }
+
+int ssl_is_sct_list_valid(const CBS *contents) {
+  /* Shallow parse the SCT list for sanity. By the RFC
+   * (https://tools.ietf.org/html/rfc6962#section-3.3) neither the list nor any
+   * of the SCTs may be empty. */
+  CBS copy = *contents;
+  CBS sct_list;
+  if (!CBS_get_u16_length_prefixed(&copy, &sct_list) ||
+      CBS_len(&copy) != 0 ||
+      CBS_len(&sct_list) == 0) {
+    return 0;
+  }
+
+  while (CBS_len(&sct_list) > 0) {
+    CBS sct;
+    if (!CBS_get_u16_length_prefixed(&sct_list, &sct) ||
+        CBS_len(&sct) == 0) {
+      return 0;
+    }
+  }
+
+  return 1;
+}