kris/boringssl
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Tidy record length check.

Browse Source 
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>
This commit is contained in:
David Benjamin 2015-04-05 02:21:33 -04:00 committed by Adam Langley
parent d81e73dcbb
commit 4a3f0732fd
3 changed files with 7 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
include/openssl/ssl3.h
View File

@ -256,11 +256,11 @@ OPENSSL_COMPILE_ASSERT(
SSL3_RT_MAX_ENCRYPTED_OVERHEAD >= SSL3_RT_SEND_MAX_ENCRYPTED_OVERHEAD, SSL3_RT_MAX_ENCRYPTED_OVERHEAD >= SSL3_RT_SEND_MAX_ENCRYPTED_OVERHEAD,
max_overheads_are_consistent); 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 \ #define SSL3_RT_MAX_ENCRYPTED_LENGTH \
(SSL3_RT_MAX_ENCRYPTED_OVERHEAD + SSL3_RT_MAX_COMPRESSED_LENGTH) (SSL3_RT_MAX_ENCRYPTED_OVERHEAD + SSL3_RT_MAX_COMPRESSED_LENGTH)
#define SSL3_RT_MAX_PACKET_SIZE \ #define SSL3_RT_MAX_PACKET_SIZE \

3
ssl/s3_both.c
View File

@ -589,8 +589,7 @@ int ssl3_setup_read_buffer(SSL *s) {
#endif #endif
if (s->s3->rbuf.buf == NULL) { if (s->s3->rbuf.buf == NULL) {
len = SSL3_RT_MAX_PLAIN_LENGTH + SSL3_RT_MAX_ENCRYPTED_OVERHEAD + len = SSL3_RT_MAX_ENCRYPTED_LENGTH + headerlen + align;
headerlen + align;
if (s->options & SSL_OP_MICROSOFT_BIG_SSLV3_BUFFER) { if (s->options & SSL_OP_MICROSOFT_BIG_SSLV3_BUFFER) {
s->s3->init_extra = 1; s->s3->init_extra = 1;
len += SSL3_RT_MAX_EXTRA; len += SSL3_RT_MAX_EXTRA;

11
ssl/s3_pkt.c
View File

@ -330,9 +330,9 @@ again:
goto err; 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; 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; goto f_err;
} }
@ -366,13 +366,6 @@ again:
/* We now have - encrypted [ MAC [ compressed [ plain ] ] ] /* We now have - encrypted [ MAC [ compressed [ plain ] ] ]
* rr->length bytes of encrypted compressed stuff. */ * 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' */ /* decrypt in place in 'rr->input' */
rr->data = rr->input; rr->data = rr->input;