commit af07365b498b4cf183493a86dcfd768b3a5e8eaa
author David Benjamin <davidben@chromium.org> Thu Nov 12 23:09:30 2015 -0800
committer Adam Langley <agl@google.com> Mon Nov 16 23:17:42 2015 +0000
tree 7fa249d562d2a8538c80597b2d67e9b53eed0a4e
parent 780cd92b98eface541ac700cffcfde80c2ef3eb9

Check for overflow when parsing a CBS with d2i_*.

Until we've done away with the d2i_* stack completely, boundaries need
to be mindful of the type mismatch. d2i_* takes a long, not a size_t.

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

crypto/pkcs8/pkcs8.c

diff --git a/crypto/pkcs8/pkcs8.c b/crypto/pkcs8/pkcs8.c
index c097881..31a34a7 100644
--- a/crypto/pkcs8/pkcs8.c
+++ b/crypto/pkcs8/pkcs8.c
@@ -773,13 +773,14 @@
       goto err;
     }
 
-    if (OBJ_cbs2nid(&contents_type) != NID_pkcs7_data) {
+    if (OBJ_cbs2nid(&contents_type) != NID_pkcs7_data ||
+        CBS_len(&ai) > LONG_MAX) {
       OPENSSL_PUT_ERROR(PKCS8, PKCS8_R_BAD_PKCS12_DATA);
       goto err;
     }
 
     inp = CBS_data(&ai);
-    algor = d2i_X509_ALGOR(NULL, &inp, CBS_len(&ai));
+    algor = d2i_X509_ALGOR(NULL, &inp, (long)CBS_len(&ai));
     if (algor == NULL) {
       goto err;
     }
@@ -822,9 +823,14 @@
       goto err;
     }
 
+    if (CBS_len(&wrapped_contents) > LONG_MAX) {
+      OPENSSL_PUT_ERROR(PKCS8, PKCS8_R_BAD_PKCS12_DATA);
+      goto err;
+    }
+
     /* encrypted isn't actually an X.509 signature, but it has the same
      * structure as one and so |X509_SIG| is reused to store it. */
-    encrypted = d2i_X509_SIG(NULL, &inp, CBS_len(&wrapped_contents));
+    encrypted = d2i_X509_SIG(NULL, &inp, (long)CBS_len(&wrapped_contents));
     if (encrypted == NULL) {
       OPENSSL_PUT_ERROR(PKCS8, PKCS8_R_BAD_PKCS12_DATA);
       goto err;
@@ -861,8 +867,12 @@
     }
 
     if (OBJ_cbs2nid(&cert_type) == NID_x509Certificate) {
+      if (CBS_len(&cert) > LONG_MAX) {
+        OPENSSL_PUT_ERROR(PKCS8, PKCS8_R_BAD_PKCS12_DATA);
+        goto err;
+      }
       const uint8_t *inp = CBS_data(&cert);
-      X509 *x509 = d2i_X509(NULL, &inp, CBS_len(&cert));
+      X509 *x509 = d2i_X509(NULL, &inp, (long)CBS_len(&cert));
       if (!x509) {
         OPENSSL_PUT_ERROR(PKCS8, PKCS8_R_BAD_PKCS12_DATA);
         goto err;

crypto/x509/pkcs7.c

diff --git a/crypto/x509/pkcs7.c b/crypto/x509/pkcs7.c
index 2087f94..9e6a52f 100644
--- a/crypto/x509/pkcs7.c
+++ b/crypto/x509/pkcs7.c
@@ -15,6 +15,7 @@
 #include <openssl/x509.h>
 
 #include <assert.h>
+#include <limits.h>
 
 #include <openssl/bytestring.h>
 #include <openssl/err.h>
@@ -114,8 +115,11 @@
       goto err;
     }
 
+    if (CBS_len(&cert) > LONG_MAX) {
+      goto err;
+    }
     inp = CBS_data(&cert);
-    x509 = d2i_X509(NULL, &inp, CBS_len(&cert));
+    x509 = d2i_X509(NULL, &inp, (long)CBS_len(&cert));
     if (!x509) {
       goto err;
     }
@@ -181,8 +185,11 @@
       goto err;
     }
 
+    if (CBS_len(&crl_data) > LONG_MAX) {
+      goto err;
+    }
     inp = CBS_data(&crl_data);
-    crl = d2i_X509_CRL(NULL, &inp, CBS_len(&crl_data));
+    crl = d2i_X509_CRL(NULL, &inp, (long)CBS_len(&crl_data));
     if (!crl) {
       goto err;
     }

ssl/s3_clnt.c

diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c
index eb96780..352dae3 100644
--- a/ssl/s3_clnt.c
+++ b/ssl/s3_clnt.c
@@ -1003,8 +1003,9 @@
       OPENSSL_PUT_ERROR(SSL, SSL_R_CERT_LENGTH_MISMATCH);
       goto f_err;
     }
+    /* A u24 length cannot overflow a long. */
     data = CBS_data(&certificate);
-    x = d2i_X509(NULL, &data, CBS_len(&certificate));
+    x = d2i_X509(NULL, &data, (long)CBS_len(&certificate));
     if (x == NULL) {
       al = SSL_AD_BAD_CERTIFICATE;
       OPENSSL_PUT_ERROR(SSL, ERR_R_ASN1_LIB);
@@ -1410,7 +1411,8 @@
 
     data = CBS_data(&distinguished_name);
 
-    xn = d2i_X509_NAME(NULL, &data, CBS_len(&distinguished_name));
+    /* A u16 length cannot overflow a long. */
+    xn = d2i_X509_NAME(NULL, &data, (long)CBS_len(&distinguished_name));
     if (xn == NULL) {
       ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
       OPENSSL_PUT_ERROR(SSL, ERR_R_ASN1_LIB);

ssl/s3_srvr.c

diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c
index 8cfa0e6..1616b22 100644
--- a/ssl/s3_srvr.c
+++ b/ssl/s3_srvr.c
@@ -2193,8 +2193,9 @@
     }
     is_first_certificate = 0;
 
+    /* A u24 length cannot overflow a long. */
     data = CBS_data(&certificate);
-    x = d2i_X509(NULL, &data, CBS_len(&certificate));
+    x = d2i_X509(NULL, &data, (long)CBS_len(&certificate));
     if (x == NULL) {
       al = SSL_AD_BAD_CERTIFICATE;
       OPENSSL_PUT_ERROR(SSL, ERR_R_ASN1_LIB);

ssl/ssl_asn1.c

diff --git a/ssl/ssl_asn1.c b/ssl/ssl_asn1.c
index 0ad4a11..5ec33eb 100644
--- a/ssl/ssl_asn1.c
+++ b/ssl/ssl_asn1.c
@@ -492,8 +492,12 @@
 }
 
 static X509 *parse_x509(CBS *cbs) {
+  if (CBS_len(cbs) > LONG_MAX) {
+    OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_SSL_SESSION);
+    return NULL;
+  }
   const uint8_t *ptr = CBS_data(cbs);
-  X509 *ret = d2i_X509(NULL, &ptr, CBS_len(cbs));
+  X509 *ret = d2i_X509(NULL, &ptr, (long)CBS_len(cbs));
   if (ret == NULL) {
     return NULL;
   }