On Wed Jul 20 19:46:37 2016, d...@inky.com wrote:
> OS: Mac OS X 11.11.5
> Version: OpenSSL 1.1-pre6 (head code as of yesterday)
> When the server fails under some circumstances, this line reads a bad
> address:
> /* write the header */
>
> *(outbuf[j]++) = type & 0xff;
>
> Because outbuf is 3. This is because prior to the alignment code,
> outbuf is
> NULL.
> outbuf is set to s->rlayer->wbuf[0].buf, which at that point has been
> set to
> NULL by the code guarded by
> #if !defined(OPENSSL_NO_MULTIBLOCK) && EVP_CIPH_FLAG_TLS1_1_MULTIBLOCK
>
> in ssl3_write_bytes.
> I'm sorry I can't give you a simple reproducer; I was able to
> reproduce it by
> mailing very large files with our mail app. Eventually the Exchange
> server
> fails and downstream code resets the write buffer and the multiblock
> code sets
> s->rlayer->wbuf[0].buf to NULL.
> The workaround is to compile with -DOPENSSL_NO_MULTIBLOCK -- I've
> verified
> that this eliminates the crash in practice.
> Feel free to email me if you want me to put in to some test code and
> reproduce
> it.
> Dave
> Sent with [inky](http://inky.com?kme=signature)
Hi Dave
Please could you try the attached patch and see if that resolves the issue?
Thanks
Matt
--
Ticket here: http://rt.openssl.org/Ticket/Display.html?id=4618
Please log in as guest with password guest if prompted
>From 32f6b811837e0279e8cbc13426ae700aff3414eb Mon Sep 17 00:00:00 2001
From: Matt Caswell <m...@openssl.org>
Date: Mon, 25 Jul 2016 10:36:57 +0100
Subject: [PATCH] Fix crash as a result of MULTIBLOCK
The MULTIBLOCK code uses a "jumbo" sized write buffer which it allocates
and then frees later. Pipelining however introduced multiple pipelines. It
keeps track of how many pipelines are initialised using numwpipes.
Unfortunately the MULTIBLOCK code was not updating this when in deallocated
its buffers, leading to a buffer being marked as initialised but set to
NULL.
---
ssl/record/rec_layer_s3.c | 26 ++++++++++++--------------
ssl/record/record_locl.h | 2 +-
ssl/record/ssl3_buffer.c | 31 ++++++++++++++++---------------
3 files changed, 29 insertions(+), 30 deletions(-)
diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c
index b562913..2d0fca2 100644
--- a/ssl/record/rec_layer_s3.c
+++ b/ssl/record/rec_layer_s3.c
@@ -423,23 +423,21 @@ int ssl3_write_bytes(SSL *s, int type, const void *buf_, int len)
else
packlen *= 4;
- wb->buf = OPENSSL_malloc(packlen);
- if (wb->buf == NULL) {
+ if (!ssl3_setup_write_buffer(s, 1, packlen)) {
SSLerr(SSL_F_SSL3_WRITE_BYTES, ERR_R_MALLOC_FAILURE);
return -1;
}
- wb->len = packlen;
} else if (tot == len) { /* done? */
- OPENSSL_free(wb->buf); /* free jumbo buffer */
- wb->buf = NULL;
+ /* free jumbo buffer */
+ ssl3_release_write_buffer(s);
return tot;
}
n = (len - tot);
for (;;) {
if (n < 4 * max_send_fragment) {
- OPENSSL_free(wb->buf); /* free jumbo buffer */
- wb->buf = NULL;
+ /* free jumbo buffer */
+ ssl3_release_write_buffer(s);
break;
}
@@ -471,8 +469,8 @@ int ssl3_write_bytes(SSL *s, int type, const void *buf_, int len)
sizeof(mb_param), &mb_param);
if (packlen <= 0 || packlen > (int)wb->len) { /* never happens */
- OPENSSL_free(wb->buf); /* free jumbo buffer */
- wb->buf = NULL;
+ /* free jumbo buffer */
+ ssl3_release_write_buffer(s);
break;
}
@@ -502,15 +500,15 @@ int ssl3_write_bytes(SSL *s, int type, const void *buf_, int len)
i = ssl3_write_pending(s, type, &buf[tot], nw);
if (i <= 0) {
if (i < 0 && (!s->wbio || !BIO_should_retry(s->wbio))) {
- OPENSSL_free(wb->buf);
- wb->buf = NULL;
+ /* free jumbo buffer */
+ ssl3_release_write_buffer(s);
}
s->rlayer.wnum = tot;
return i;
}
if (i == (int)n) {
- OPENSSL_free(wb->buf); /* free jumbo buffer */
- wb->buf = NULL;
+ /* free jumbo buffer */
+ ssl3_release_write_buffer(s);
return tot + i;
}
n -= i;
@@ -650,7 +648,7 @@ int do_ssl3_write(SSL *s, int type, const unsigned char *buf,
}
if (s->rlayer.numwpipes < numpipes)
- if (!ssl3_setup_write_buffer(s, numpipes))
+ if (!ssl3_setup_write_buffer(s, numpipes, 0))
return -1;
if (totlen == 0 && !create_empty_fragment)
diff --git a/ssl/record/record_locl.h b/ssl/record/record_locl.h
index ff1eb32..435e92a 100644
--- a/ssl/record/record_locl.h
+++ b/ssl/record/record_locl.h
@@ -69,7 +69,7 @@ void SSL3_BUFFER_clear(SSL3_BUFFER *b);
void SSL3_BUFFER_set_data(SSL3_BUFFER *b, const unsigned char *d, int n);
void SSL3_BUFFER_release(SSL3_BUFFER *b);
__owur int ssl3_setup_read_buffer(SSL *s);
-__owur int ssl3_setup_write_buffer(SSL *s, unsigned int numwpipes);
+__owur int ssl3_setup_write_buffer(SSL *s, unsigned int numwpipes, size_t len);
int ssl3_release_read_buffer(SSL *s);
int ssl3_release_write_buffer(SSL *s);
diff --git a/ssl/record/ssl3_buffer.c b/ssl/record/ssl3_buffer.c
index 940b73e..9638002 100644
--- a/ssl/record/ssl3_buffer.c
+++ b/ssl/record/ssl3_buffer.c
@@ -74,33 +74,34 @@ int ssl3_setup_read_buffer(SSL *s)
return 0;
}
-int ssl3_setup_write_buffer(SSL *s, unsigned int numwpipes)
+int ssl3_setup_write_buffer(SSL *s, unsigned int numwpipes, size_t len)
{
unsigned char *p;
- size_t len, align = 0, headerlen;
+ size_t align = 0, headerlen;
SSL3_BUFFER *wb;
unsigned int currpipe;
s->rlayer.numwpipes = numwpipes;
-
- if (SSL_IS_DTLS(s))
- headerlen = DTLS1_RT_HEADER_LENGTH + 1;
- else
- headerlen = SSL3_RT_HEADER_LENGTH;
+ if (len == 0) {
+ if (SSL_IS_DTLS(s))
+ headerlen = DTLS1_RT_HEADER_LENGTH + 1;
+ else
+ headerlen = SSL3_RT_HEADER_LENGTH;
#if defined(SSL3_ALIGN_PAYLOAD) && SSL3_ALIGN_PAYLOAD!=0
- align = (-SSL3_RT_HEADER_LENGTH) & (SSL3_ALIGN_PAYLOAD - 1);
+ align = (-SSL3_RT_HEADER_LENGTH) & (SSL3_ALIGN_PAYLOAD - 1);
#endif
- len = s->max_send_fragment
- + SSL3_RT_SEND_MAX_ENCRYPTED_OVERHEAD + headerlen + align;
+ len = s->max_send_fragment
+ + SSL3_RT_SEND_MAX_ENCRYPTED_OVERHEAD + headerlen + align;
#ifndef OPENSSL_NO_COMP
- if (ssl_allow_compression(s))
- len += SSL3_RT_MAX_COMPRESSED_OVERHEAD;
+ if (ssl_allow_compression(s))
+ len += SSL3_RT_MAX_COMPRESSED_OVERHEAD;
#endif
- if (!(s->options & SSL_OP_DONT_INSERT_EMPTY_FRAGMENTS))
- len += headerlen + align + SSL3_RT_SEND_MAX_ENCRYPTED_OVERHEAD;
+ if (!(s->options & SSL_OP_DONT_INSERT_EMPTY_FRAGMENTS))
+ len += headerlen + align + SSL3_RT_SEND_MAX_ENCRYPTED_OVERHEAD;
+ }
wb = RECORD_LAYER_get_wbuf(&s->rlayer);
for (currpipe = 0; currpipe < numwpipes; currpipe++) {
@@ -125,7 +126,7 @@ int ssl3_setup_buffers(SSL *s)
{
if (!ssl3_setup_read_buffer(s))
return 0;
- if (!ssl3_setup_write_buffer(s, 1))
+ if (!ssl3_setup_write_buffer(s, 1, 0))
return 0;
return 1;
}
--
2.7.4
--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
