Hi team,
I'm suggesting we introduce a Curl_saferealloc() function to be used instead
of plain old realloc() where suitable.
The replacement makes sure to free the passed in memory area if realloc()
fails, but it does *not* free the memory area if the realloc'ed size is zero.
The intent is to address a common problem pattern our code has shown in the
past. We've allowed the size argument to get dynamically calculated and then
passed to realloc() and in some cases it could wrap and become zero. A
realloc() to 0 returns NULL *and* frees the memory area, which easily ends up
in a free-twice scenario.
This new function is meant to help us avoid this problem.
The attached patch introduce the function and changes most uses of realloc()
to use the new function. It didn't really fit in every case so there are still
a handful calls to plain realloc() left.
Comments or thoughts on this?
--
/ daniel.haxx.se
From 9bfecad23d6f5e548223bf8d14ccfe51e1d8d981 Mon Sep 17 00:00:00 2001
From: Daniel Stenberg <[email protected]>
Date: Mon, 7 Nov 2016 10:55:25 +0100
Subject: [PATCH] realloc: use Curl_saferealloc to avoid common mistakes
---
lib/content_encoding.c | 9 +++------
lib/curl_ntlm_wb.c | 8 ++++----
lib/escape.c | 7 +++----
lib/http.c | 4 ++--
lib/http2.c | 7 +++----
lib/rtsp.c | 5 +++--
lib/security.c | 4 ++--
lib/ssh.c | 7 ++++---
lib/strdup.c | 23 +++++++++++++++++++++++
lib/strdup.h | 1 +
10 files changed, 48 insertions(+), 27 deletions(-)
diff --git a/lib/content_encoding.c b/lib/content_encoding.c
index fa36aca..5a5824d 100644
--- a/lib/content_encoding.c
+++ b/lib/content_encoding.c
@@ -3,11 +3,11 @@
* Project ___| | | | _ \| |
* / __| | | | |_) | |
* | (__| |_| | _ <| |___
* \___|\___/|_| \_\_____|
*
- * Copyright (C) 1998 - 2011, Daniel Stenberg, <[email protected]>, et al.
+ * Copyright (C) 1998 - 2016, Daniel Stenberg, <[email protected]>, et al.
*
* This software is licensed as described in the file COPYING, which
* you should have received as part of this distribution. The terms
* are also available at https://curl.haxx.se/docs/copyright.html.
*
@@ -26,12 +26,12 @@
#include "urldata.h"
#include <curl/curl.h>
#include "sendf.h"
#include "content_encoding.h"
+#include "strdup.h"
#include "curl_memory.h"
-
#include "memdebug.h"
/* Comment this out if zlib is always going to be at least ver. 1.2.0.4
(doing so will reduce code size slightly). */
#define OLD_ZLIB_SUPPORT 1
@@ -369,16 +369,13 @@ Curl_unencode_gzip_write(struct connectdata *conn,
case ZLIB_GZIP_HEADER:
{
/* Need more gzip header data state */
ssize_t hlen;
- unsigned char *oldblock = z->next_in;
-
z->avail_in += (uInt)nread;
- z->next_in = realloc(z->next_in, z->avail_in);
+ z->next_in = Curl_saferealloc(z->next_in, z->avail_in);
if(z->next_in == NULL) {
- free(oldblock);
return exit_zlib(z, &k->zlib_init, CURLE_OUT_OF_MEMORY);
}
/* Append the new block of data to the previous one */
memcpy(z->next_in + z->avail_in - nread, k->str, nread);
diff --git a/lib/curl_ntlm_wb.c b/lib/curl_ntlm_wb.c
index afdea16..1d1b3a3 100644
--- a/lib/curl_ntlm_wb.c
+++ b/lib/curl_ntlm_wb.c
@@ -49,10 +49,11 @@
#include "select.h"
#include "vauth/ntlm.h"
#include "curl_ntlm_wb.h"
#include "url.h"
#include "strerror.h"
+#include "strdup.h"
/* The last 3 #include files should be in this order */
#include "curl_printf.h"
#include "curl_memory.h"
#include "memdebug.h"
@@ -291,15 +292,14 @@ static CURLcode ntlm_wb_response(struct connectdata *conn,
len_out += size;
if(buf[len_out - 1] == '\n') {
buf[len_out - 1] = '\0';
break;
}
- newbuf = realloc(buf, len_out + NTLM_BUFSIZE);
- if(!newbuf) {
- free(buf);
+ newbuf = Curl_saferealloc(buf, len_out + NTLM_BUFSIZE);
+ if(!newbuf)
return CURLE_OUT_OF_MEMORY;
- }
+
buf = newbuf;
}
/* Samba/winbind installed but not configured */
if(state == NTLMSTATE_TYPE1 &&
diff --git a/lib/escape.c b/lib/escape.c
index 6657007..9fb8d3e 100644
--- a/lib/escape.c
+++ b/lib/escape.c
@@ -29,10 +29,11 @@
#include "urldata.h"
#include "warnless.h"
#include "non-ascii.h"
#include "escape.h"
+#include "strdup.h"
/* The last 3 #include files should be in this order */
#include "curl_printf.h"
#include "curl_memory.h"
#include "memdebug.h"
@@ -107,15 +108,13 @@ char *curl_easy_escape(struct Curl_easy *data, const char *string,
else {
/* encode it */
newlen += 2; /* the size grows with two, since this'll become a %XX */
if(newlen > alloc) {
alloc *= 2;
- testing_ptr = realloc(ns, alloc);
- if(!testing_ptr) {
- free(ns);
+ testing_ptr = Curl_saferealloc(ns, alloc);
+ if(!testing_ptr)
return NULL;
- }
else {
ns = testing_ptr;
}
}
diff --git a/lib/http.c b/lib/http.c
index e7788e7..999d6da 100644
--- a/lib/http.c
+++ b/lib/http.c
@@ -74,10 +74,11 @@
#include "non-ascii.h"
#include "conncache.h"
#include "pipeline.h"
#include "http2.h"
#include "connect.h"
+#include "strdup.h"
/* The last 3 #include files should be in this order */
#include "curl_printf.h"
#include "curl_memory.h"
#include "memdebug.h"
@@ -1253,18 +1254,17 @@ CURLcode Curl_add_buffer(Curl_send_buffer *in, const void *inptr, size_t size)
else
new_size = (in->size_used+size) * 2;
if(in->buffer)
/* we have a buffer, enlarge the existing one */
- new_rb = realloc(in->buffer, new_size);
+ new_rb = Curl_saferealloc(in->buffer, new_size);
else
/* create a new buffer */
new_rb = malloc(new_size);
if(!new_rb) {
/* If we failed, we cleanup the whole buffer and return error */
- Curl_safefree(in->buffer);
free(in);
return CURLE_OUT_OF_MEMORY;
}
in->buffer = new_rb;
diff --git a/lib/http2.c b/lib/http2.c
index b0301da..1484e15 100644
--- a/lib/http2.c
+++ b/lib/http2.c
@@ -33,11 +33,11 @@
#include "multiif.h"
#include "conncache.h"
#include "url.h"
#include "connect.h"
#include "strtoofft.h"
-
+#include "strdup.h"
/* The last 3 #include files should be in this order */
#include "curl_printf.h"
#include "curl_memory.h"
#include "memdebug.h"
@@ -839,14 +839,13 @@ static int on_header(nghttp2_session *session, const nghttp2_frame *frame,
}
else if(stream->push_headers_used ==
stream->push_headers_alloc) {
char **headp;
stream->push_headers_alloc *= 2;
- headp = realloc(stream->push_headers,
- stream->push_headers_alloc * sizeof(char *));
+ headp = Curl_saferealloc(stream->push_headers,
+ stream->push_headers_alloc * sizeof(char *));
if(!headp) {
- free(stream->push_headers);
stream->push_headers = NULL;
return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
}
stream->push_headers = headp;
}
diff --git a/lib/rtsp.c b/lib/rtsp.c
index d1bad19..5da33d4 100644
--- a/lib/rtsp.c
+++ b/lib/rtsp.c
@@ -34,10 +34,11 @@
#include "progress.h"
#include "rtsp.h"
#include "strcase.h"
#include "select.h"
#include "connect.h"
+#include "strdup.h"
/* The last 3 #include files should be in this order */
#include "curl_printf.h"
#include "curl_memory.h"
#include "memdebug.h"
@@ -612,13 +613,13 @@ static CURLcode rtsp_rtp_readwrite(struct Curl_easy *data,
char *scratch;
CURLcode result;
if(rtspc->rtp_buf) {
/* There was some leftover data the last time. Merge buffers */
- char *newptr = realloc(rtspc->rtp_buf, rtspc->rtp_bufsize + *nread);
+ char *newptr = Curl_saferealloc(rtspc->rtp_buf,
+ rtspc->rtp_bufsize + *nread);
if(!newptr) {
- Curl_safefree(rtspc->rtp_buf);
rtspc->rtp_buf = NULL;
rtspc->rtp_bufsize = 0;
return CURLE_OUT_OF_MEMORY;
}
rtspc->rtp_buf = newptr;
diff --git a/lib/security.c b/lib/security.c
index ff26066..87c22da 100644
--- a/lib/security.c
+++ b/lib/security.c
@@ -60,11 +60,11 @@
#include "curl_sec.h"
#include "ftp.h"
#include "sendf.h"
#include "strcase.h"
#include "warnless.h"
-
+#include "strdup.h"
/* The last #include file should be: */
#include "memdebug.h"
static const struct {
enum protection_level level;
@@ -200,11 +200,11 @@ static CURLcode read_data(struct connectdata *conn,
return result;
if(len) {
/* only realloc if there was a length */
len = ntohl(len);
- tmp = realloc(buf->data, len);
+ tmp = Curl_saferealloc(buf->data, len);
}
if(tmp == NULL)
return CURLE_OUT_OF_MEMORY;
buf->data = tmp;
diff --git a/lib/ssh.c b/lib/ssh.c
index 0df030d..420beb2 100644
--- a/lib/ssh.c
+++ b/lib/ssh.c
@@ -69,11 +69,11 @@
#include "http.h" /* for HTTP proxy tunnel stuff */
#include "ssh.h"
#include "url.h"
#include "speedcheck.h"
#include "getinfo.h"
-
+#include "strdup.h"
#include "strcase.h"
#include "vtls/vtls.h"
#include "connect.h"
#include "strerror.h"
#include "inet_ntop.h"
@@ -2110,13 +2110,14 @@ static CURLcode ssh_statemach_act(struct connectdata *conn, bool *block)
}
Curl_safefree(sshc->readdir_linkPath);
/* get room for the filename and extra output */
sshc->readdir_totalLen += 4 + sshc->readdir_len;
- new_readdir_line = realloc(sshc->readdir_line, sshc->readdir_totalLen);
+ new_readdir_line = Curl_saferealloc(sshc->readdir_line,
+ sshc->readdir_totalLen);
if(!new_readdir_line) {
- Curl_safefree(sshc->readdir_line);
+ sshc->readdir_line = NULL;
Curl_safefree(sshc->readdir_filename);
Curl_safefree(sshc->readdir_longentry);
state(conn, SSH_SFTP_CLOSE);
sshc->actualcode = CURLE_OUT_OF_MEMORY;
break;
diff --git a/lib/strdup.c b/lib/strdup.c
index 5a15c2b..ec1b0df 100644
--- a/lib/strdup.c
+++ b/lib/strdup.c
@@ -73,5 +73,28 @@ void *Curl_memdup(const void *src, size_t length)
memcpy(buffer, src, length);
return buffer;
}
+
+/***************************************************************************
+ *
+ * Curl_saferealloc(dataptr, newlength)
+ *
+ * Does a normal realloc(), but will free the data pointer if the realloc
+ * fails. If 'newlength' is zero, it will free the data and return a failure.
+ *
+ * This convenience function is provided and used to help us avoid a common
+ * mistake pattern when we could pass in a zero, catch the NULL return and end
+ * up free'ing the memory twice.
+ *
+ * Returns the new pointer or NULL on failure.
+ *
+ ***************************************************************************/
+void *Curl_saferealloc(void *ptr, size_t size)
+{
+ void *datap = realloc(ptr, size);
+ if(size && !datap)
+ /* only free 'ptr' if size was non-zero */
+ free(ptr);
+ return datap;
+}
diff --git a/lib/strdup.h b/lib/strdup.h
index c74a3b7..ae3d5d0 100644
--- a/lib/strdup.h
+++ b/lib/strdup.h
@@ -25,7 +25,8 @@
#ifndef HAVE_STRDUP
extern char *curlx_strdup(const char *str);
#endif
void *Curl_memdup(const void *src, size_t buffer_length);
+void *Curl_saferealloc(void *ptr, size_t size);
#endif /* HEADER_CURL_STRDUP_H */
--
2.10.2
-------------------------------------------------------------------
List admin: https://cool.haxx.se/list/listinfo/curl-library
Etiquette: https://curl.haxx.se/mail/etiquette.html
