commit 4a3f0732fd15ce3f38cb1f0879d3f8e64db888dd
author David Benjamin <davidben@chromium.org> Sun Apr 05 02:21:33 2015 -0400
committer Adam Langley <agl@google.com> Mon Apr 06 20:50:45 2015 +0000
tree 59ff629b519d3b69310f23c81eb7b867651a419c
parent d81e73dcbb497e0b3a2830881c09163af6fe7e21

Tidy record length check.

Compression is gone, so don't allow for compression overhead. With that fixed,
the second rr->length check in ssl3_get_record matches the length computation
which sizes the read buffer. The first is wrong and doesn't account for the
alignment padding. Move the second to the first.

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

include/openssl/ssl3.h

diff --git a/include/openssl/ssl3.h b/include/openssl/ssl3.h
index ab31a62..09705e1 100644
--- a/include/openssl/ssl3.h
+++ b/include/openssl/ssl3.h
@@ -256,11 +256,11 @@
     SSL3_RT_MAX_ENCRYPTED_OVERHEAD >= SSL3_RT_SEND_MAX_ENCRYPTED_OVERHEAD,
     max_overheads_are_consistent);
 
+/* SSL3_RT_MAX_COMPRESSED_LENGTH is an alias for
+ * |SSL3_RT_MAX_PLAIN_LENGTH|. Compression is gone, so don't include the
+ * compression overhead. */
+#define SSL3_RT_MAX_COMPRESSED_LENGTH SSL3_RT_MAX_PLAIN_LENGTH
 
-/* If compression isn't used don't include the compression overhead */
-
-#define SSL3_RT_MAX_COMPRESSED_LENGTH \
-  (SSL3_RT_MAX_PLAIN_LENGTH + SSL3_RT_MAX_COMPRESSED_OVERHEAD)
 #define SSL3_RT_MAX_ENCRYPTED_LENGTH \
   (SSL3_RT_MAX_ENCRYPTED_OVERHEAD + SSL3_RT_MAX_COMPRESSED_LENGTH)
 #define SSL3_RT_MAX_PACKET_SIZE \

ssl/s3_both.c

diff --git a/ssl/s3_both.c b/ssl/s3_both.c
index 1126c11..3cb2152 100644
--- a/ssl/s3_both.c
+++ b/ssl/s3_both.c
@@ -589,8 +589,7 @@
 #endif
 
   if (s->s3->rbuf.buf == NULL) {
-    len = SSL3_RT_MAX_PLAIN_LENGTH + SSL3_RT_MAX_ENCRYPTED_OVERHEAD +
-          headerlen + align;
+    len = SSL3_RT_MAX_ENCRYPTED_LENGTH + headerlen + align;
     if (s->options & SSL_OP_MICROSOFT_BIG_SSLV3_BUFFER) {
       s->s3->init_extra = 1;
       len += SSL3_RT_MAX_EXTRA;

ssl/s3_pkt.c

diff --git a/ssl/s3_pkt.c b/ssl/s3_pkt.c
index 2d888e2..e4613f0 100644
--- a/ssl/s3_pkt.c
+++ b/ssl/s3_pkt.c
@@ -330,9 +330,9 @@
       goto err;
     }
 
-    if (rr->length > s->s3->rbuf.len - SSL3_RT_HEADER_LENGTH) {
+    if (rr->length > SSL3_RT_MAX_ENCRYPTED_LENGTH + extra) {
       al = SSL_AD_RECORD_OVERFLOW;
-      OPENSSL_PUT_ERROR(SSL, ssl3_get_record, SSL_R_PACKET_LENGTH_TOO_LONG);
+      OPENSSL_PUT_ERROR(SSL, ssl3_get_record, SSL_R_ENCRYPTED_LENGTH_TOO_LONG);
       goto f_err;
     }
 
@@ -366,13 +366,6 @@
   /* We now have - encrypted [ MAC [ compressed [ plain ] ] ]
    * rr->length bytes of encrypted compressed stuff. */
 
-  /* check is not needed I believe */
-  if (rr->length > SSL3_RT_MAX_ENCRYPTED_LENGTH + extra) {
-    al = SSL_AD_RECORD_OVERFLOW;
-    OPENSSL_PUT_ERROR(SSL, ssl3_get_record, SSL_R_ENCRYPTED_LENGTH_TOO_LONG);
-    goto f_err;
-  }
-
   /* decrypt in place in 'rr->input' */
   rr->data = rr->input;