From 212e3e26bc711c4bd2e0a1c2bbc14b4e40506917 Mon Sep 17 00:00:00 2001 From: Marc Hoersken Date: Sun, 14 Dec 2014 17:27:20 +0100 Subject: curl_schannel: Improvements to memory re-allocation strategy - do not grow memory by doubling its size - do not leak previously allocated memory if reallocation fails - replace while-loop with a single check to make sure that the requested amount of data fits into the buffer Bug: http://curl.haxx.se/bug/view.cgi?id=1450 Reported-by: Warren Menzer --- lib/vtls/curl_schannel.c | 58 +++++++++++++++++++++++++++++++++++------------- lib/vtls/curl_schannel.h | 1 - 2 files changed, 42 insertions(+), 17 deletions(-) diff --git a/lib/vtls/curl_schannel.c b/lib/vtls/curl_schannel.c index f31e9c273..ebdd5c6c9 100644 --- a/lib/vtls/curl_schannel.c +++ b/lib/vtls/curl_schannel.c @@ -294,6 +294,8 @@ schannel_connect_step2(struct connectdata *conn, int sockindex) ssize_t nread = -1, written = -1; struct SessionHandle *data = conn->data; struct ssl_connect_data *connssl = &conn->ssl[sockindex]; + unsigned char *reallocated_buffer; + size_t reallocated_length; SecBuffer outbuf[2]; SecBufferDesc outbuf_desc; SecBuffer inbuf[2]; @@ -326,14 +328,19 @@ schannel_connect_step2(struct connectdata *conn, int sockindex) if(connssl->encdata_length - connssl->encdata_offset < CURL_SCHANNEL_BUFFER_FREE_SIZE) { /* increase internal encrypted data buffer */ - connssl->encdata_length *= CURL_SCHANNEL_BUFFER_STEP_FACTOR; - connssl->encdata_buffer = realloc(connssl->encdata_buffer, - connssl->encdata_length); + reallocated_length = connssl->encdata_offset + + CURL_SCHANNEL_BUFFER_FREE_SIZE; + reallocated_buffer = realloc(connssl->encdata_buffer, + reallocated_length); - if(connssl->encdata_buffer == NULL) { + if(reallocated_buffer == NULL) { failf(data, "schannel: unable to re-allocate memory"); return CURLE_OUT_OF_MEMORY; } + else { + connssl->encdata_buffer = reallocated_buffer; + connssl->encdata_length = reallocated_length; + } } for(;;) { @@ -828,6 +835,8 @@ schannel_recv(struct connectdata *conn, int sockindex, CURLcode retcode; struct SessionHandle *data = conn->data; struct ssl_connect_data *connssl = &conn->ssl[sockindex]; + unsigned char *reallocated_buffer; + size_t reallocated_length; bool done = FALSE; SecBuffer inbuf[4]; SecBufferDesc inbuf_desc; @@ -849,18 +858,27 @@ schannel_recv(struct connectdata *conn, int sockindex, } /* increase buffer in order to fit the requested amount of data */ - while(connssl->encdata_length - connssl->encdata_offset < - CURL_SCHANNEL_BUFFER_FREE_SIZE || connssl->encdata_length < len) { + if(connssl->encdata_length - connssl->encdata_offset < + CURL_SCHANNEL_BUFFER_FREE_SIZE || connssl->encdata_length < len) { /* increase internal encrypted data buffer */ - connssl->encdata_length *= CURL_SCHANNEL_BUFFER_STEP_FACTOR; - connssl->encdata_buffer = realloc(connssl->encdata_buffer, - connssl->encdata_length); + reallocated_length = connssl->encdata_offset + + CURL_SCHANNEL_BUFFER_FREE_SIZE; + /* make sure that the requested amount of data fits */ + if(reallocated_length < len) { + reallocated_length = len; + } + reallocated_buffer = realloc(connssl->encdata_buffer, + reallocated_length); - if(connssl->encdata_buffer == NULL) { + if(reallocated_buffer == NULL) { failf(data, "schannel: unable to re-allocate memory"); *err = CURLE_OUT_OF_MEMORY; return -1; } + else { + connssl->encdata_buffer = reallocated_buffer; + connssl->encdata_length = reallocated_length; + } } /* read encrypted data from socket */ @@ -924,18 +942,26 @@ schannel_recv(struct connectdata *conn, int sockindex, /* increase buffer in order to fit the received amount of data */ size = inbuf[1].cbBuffer > CURL_SCHANNEL_BUFFER_FREE_SIZE ? inbuf[1].cbBuffer : CURL_SCHANNEL_BUFFER_FREE_SIZE; - while(connssl->decdata_length - connssl->decdata_offset < size || - connssl->decdata_length < len) { + if(connssl->decdata_length - connssl->decdata_offset < size || + connssl->decdata_length < len) { /* increase internal decrypted data buffer */ - connssl->decdata_length *= CURL_SCHANNEL_BUFFER_STEP_FACTOR; - connssl->decdata_buffer = realloc(connssl->decdata_buffer, - connssl->decdata_length); + reallocated_length = connssl->decdata_offset + size; + /* make sure that the requested amount of data fits */ + if(reallocated_length < len) { + reallocated_length = len; + } + reallocated_buffer = realloc(connssl->decdata_buffer, + reallocated_length); - if(connssl->decdata_buffer == NULL) { + if(reallocated_buffer == NULL) { failf(data, "schannel: unable to re-allocate memory"); *err = CURLE_OUT_OF_MEMORY; return -1; } + else { + connssl->decdata_buffer = reallocated_buffer; + connssl->decdata_length = reallocated_length; + } } /* copy decrypted data to internal buffer */ diff --git a/lib/vtls/curl_schannel.h b/lib/vtls/curl_schannel.h index 700516b35..d4a41440a 100644 --- a/lib/vtls/curl_schannel.h +++ b/lib/vtls/curl_schannel.h @@ -95,7 +95,6 @@ #define CURL_SCHANNEL_BUFFER_INIT_SIZE 4096 #define CURL_SCHANNEL_BUFFER_FREE_SIZE 1024 -#define CURL_SCHANNEL_BUFFER_STEP_FACTOR 2 CURLcode Curl_schannel_connect(struct connectdata *conn, int sockindex); -- cgit v1.2.3