[openssl-commits] [openssl] master update

[Matt Caswell]

The branch master has been updated
       via  6b41b3f5eacc6b1bb851c9dce22d6e893f32ea7d (commit)
      from  3a752c85ee38a92d7777b8fe1cce2e54bf619529 (commit)


- Log -----------------------------------------------------------------
commit 6b41b3f5eacc6b1bb851c9dce22d6e893f32ea7d
Author: Matt Caswell <matt at openssl.org>
Date:   Thu May 21 14:06:52 2015 +0100

    Fix a memory leak in compression
    
    The function RECORD_LAYER_clear() is supposed to clear the contents of the
    RECORD_LAYER structure, but retain certain data such as buffers that are
    allocated. Unfortunately one buffer (for compression) got missed and was
    inadvertently being wiped, thus causing a memory leak.
    
    In part this is due to the fact that RECORD_LAYER_clear() was reaching
    inside SSL3_BUFFERs and SSL3_RECORDs, which it really shouldn't. So, I've
    rewritten it to only clear the data it knows about, and to defer clearing
    of SSL3_RECORD and SSL3_BUFFER structures to SSL_RECORD_clear() and the
    new function SSL3_BUFFER_clear().
    
    Reviewed-by: Tim Hudson <tjh at openssl.org>
    Reviewed-by: Rich Salz <rsalz at openssl.org>

-----------------------------------------------------------------------

Summary of changes:
 ssl/record/rec_layer_s3.c | 49 +++++++++++++++++++++++------------------------
 ssl/record/record_locl.h  |  1 +
 ssl/record/ssl3_buffer.c  | 13 +++++++++++++
 ssl/record/ssl3_record.c  |  8 +++++++-
 4 files changed, 45 insertions(+), 26 deletions(-)

diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c
index 456fac4..47a021d 100644
--- a/ssl/record/rec_layer_s3.c
+++ b/ssl/record/rec_layer_s3.c
@@ -142,35 +142,34 @@ void RECORD_LAYER_init(RECORD_LAYER *rl, SSL *s)
 
 void RECORD_LAYER_clear(RECORD_LAYER *rl)
 {
-    unsigned char *rp, *wp;
-    size_t rlen, wlen;
-    int read_ahead;
-    SSL *s;
-    DTLS_RECORD_LAYER *d;
-
-    s = rl->s;
-    d = rl->d;
-    read_ahead = rl->read_ahead;
-    rp = SSL3_BUFFER_get_buf(&rl->rbuf);
-    rlen = SSL3_BUFFER_get_len(&rl->rbuf);
-    wp = SSL3_BUFFER_get_buf(&rl->wbuf);
-    wlen = SSL3_BUFFER_get_len(&rl->wbuf);
-    memset(rl, 0, sizeof(*rl));
-    SSL3_BUFFER_set_buf(&rl->rbuf, rp);
-    SSL3_BUFFER_set_len(&rl->rbuf, rlen);
-    SSL3_BUFFER_set_buf(&rl->wbuf, wp);
-    SSL3_BUFFER_set_len(&rl->wbuf, wlen);
-
-    /* Do I need to do this? As far as I can tell read_ahead did not
+    rl->rstate = SSL_ST_READ_HEADER;
+
+    /* Do I need to clear read_ahead? As far as I can tell read_ahead did not
      * previously get reset by SSL_clear...so I'll keep it that way..but is
      * that right?
      */
-    rl->read_ahead = read_ahead;
-    rl->rstate = SSL_ST_READ_HEADER;
-    rl->s = s;
-    rl->d = d;
+
+    rl->packet = NULL;
+    rl->packet_length = 0;
+    rl->wnum = 0;
+    memset(rl->alert_fragment, 0, sizeof(rl->alert_fragment));
+    rl->alert_fragment_len = 0;
+    memset(rl->handshake_fragment, 0, sizeof(rl->handshake_fragment));
+    rl->handshake_fragment_len = 0;
+    rl->wpend_tot = 0;
+    rl->wpend_type = 0;
+    rl->wpend_ret = 0;
+    rl->wpend_buf = NULL;
+
+    SSL3_BUFFER_clear(&rl->rbuf);
+    SSL3_BUFFER_clear(&rl->wbuf);
+    SSL3_RECORD_clear(&rl->rrec);
+    SSL3_RECORD_clear(&rl->wrec);
+
+    memset(rl->read_sequence, 0, sizeof(rl->read_sequence));
+    memset(rl->write_sequence, 0, sizeof(rl->write_sequence));
     
-    if (d)
+    if (rl->d)
         DTLS_RECORD_LAYER_clear(rl);
 }
 
diff --git a/ssl/record/record_locl.h b/ssl/record/record_locl.h
index b2222d7..f92e89d 100644
--- a/ssl/record/record_locl.h
+++ b/ssl/record/record_locl.h
@@ -162,6 +162,7 @@ void dtls1_record_bitmap_update(SSL *s, DTLS1_BITMAP *bitmap);
 #define SSL3_BUFFER_add_offset(b, o)        ((b)->offset += (o))
 #define SSL3_BUFFER_is_initialised(b)       ((b)->buf != NULL)
 
+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);
diff --git a/ssl/record/ssl3_buffer.c b/ssl/record/ssl3_buffer.c
index 5a8d34c..66fb721 100644
--- a/ssl/record/ssl3_buffer.c
+++ b/ssl/record/ssl3_buffer.c
@@ -120,6 +120,19 @@ void SSL3_BUFFER_set_data(SSL3_BUFFER *b, const unsigned char *d, int n)
     b->offset = 0;
 }
 
+/*
+ * Clear the contents of an SSL3_BUFFER but retain any memory allocated
+ */
+void SSL3_BUFFER_clear(SSL3_BUFFER *b)
+{
+    unsigned char *buf = b->buf;
+    size_t len = b->len;
+
+    memset(b, 0, sizeof(*b));
+    b->buf = buf;
+    b->len = len;
+}
+
 void SSL3_BUFFER_release(SSL3_BUFFER *b)
 {
     OPENSSL_free(b->buf);
diff --git a/ssl/record/ssl3_record.c b/ssl/record/ssl3_record.c
index b0eb7cc..5070bc3 100644
--- a/ssl/record/ssl3_record.c
+++ b/ssl/record/ssl3_record.c
@@ -132,9 +132,15 @@ static const unsigned char ssl3_pad_2[48] = {
     0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c
 };
 
+/*
+ * Clear the contents of an SSL3_RECORD but retain any memory allocated
+ */
 void SSL3_RECORD_clear(SSL3_RECORD *r)
 {
-    memset(r->seq_num, 0, sizeof(r->seq_num));
+    unsigned char *comp = r->comp;
+
+    memset(r, 0, sizeof(*r));
+    r->comp = comp;
 }
 
 void SSL3_RECORD_release(SSL3_RECORD *r)