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.

RT#4618

Reviewed-by: Rich Salz <rsalz@openssl.org>

diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c
index b56291359bed3f92e652a3bd46dbc23808bbdf03..2d0fca2672aafc39d660c92cc66b9fbba272c044 100644 (file)
--- 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 ff1eb32191eabc26718b380acc47163996dcb7cb..435e92a11e439bc695587b429535a5d384b02c2f 100644 (file)
--- 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 940b73e5833d74a2dcaf8f746de8b43099869d28..963800238b1a7cbde2d9f18a2d795983ed1d2728 100644 (file)
--- 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;
 }