Commit 6b41b3f5 authored by Matt Caswell's avatar Matt Caswell
Browse files

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: default avatarTim Hudson <tjh@openssl.org>
Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
parent 3a752c85

ssl/record/rec_layer_s3.c

+24 −25
Original line number Diff line number Diff line
@@ -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;



    if (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 (rl->d)

        DTLS_RECORD_LAYER_clear(rl);

}

ssl/record/record_locl.h

+1 −0
Original line number Diff line number Diff line
@@ -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);

ssl/record/ssl3_buffer.c

+13 −0
Original line number Diff line number Diff line
@@ -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);

ssl/record/ssl3_record.c

+7 −1
Original line number Diff line number Diff line
@@ -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)