Fix OOB write on URL callback with 2GB+ response. Add new size limit.

The OOB write did not happen on file-backed downloads, such as remote
includes. It only happened for memory-backed requests, which are only
these 4 in standard UnrealIRCd: centralblocklist, central spam report,
other spamreport blocks (eg to dronebl) and the log block with
destination webhook. All those 4 cases are very likely to be trusted
web servers, given the nature of the data you are sending to them.

The fix was to extend the size fields everywhere to 64 bits. It was
applied to both URL backends: url_unreal.c and url_curl.c.

The new API feature is a 'max_size' in OutgoingWebRequest, which
defaults to 1MB. This is only used for memory-backed responses,
so not for real file downloads. This fixes not only the reported
bug but also the case where a rogue webserver was unbounded in
terms of what response it could send back, potentially filling
up gigabytes of server memory.

Reported by Link420.

Author: syzop

include/config.h

@@ -210,6 +210,13 @@
  */
 #define DOWNLOAD_MAX_REDIRECTS 2
 
+/* Default maximum size (in bytes) for memory-backed HTTP responses
+ * (i.e. when OutgoingWebRequest.store_in_file == 0). Responses exceeding
+ * this are rejected and the transfer is aborted. Callers can override
+ * by setting OutgoingWebRequest.max_size before url_start_async().
+ */
+#define DOWNLOAD_MAX_SIZE 1048576
+
 /*
  * Max time from the nickname change that still causes KILL
  * automaticly to switch for the current nick of that user. (seconds)

include/h.h

@@ -1561,7 +1561,7 @@ extern int valid_operclass_name(const char *str);
 #define safe_free_outgoingwebrequest(x)	do { if (x) { free_outgoingwebrequest(x); x = NULL; } } while(0)
 extern void free_outgoingwebrequest(OutgoingWebRequest *r);
 extern OutgoingWebRequest *duplicate_outgoingwebrequest(OutgoingWebRequest *orig);
-extern void url_callback(OutgoingWebRequest *r, const char *file, const char *memory, int memory_len, const char *errorbuf, int cached, void *ptr);
+extern void url_callback(OutgoingWebRequest *r, const char *file, const char *memory, long long memory_len, const char *errorbuf, int cached, void *ptr);
 extern const char *synchronous_http_request(const char *url, int max_redirects, int connect_timeout, int transfer_timeout);
 extern int update_known_user_cache(Client *client);
 extern MODVAR SecurityGroup *known_users;

include/struct.h

@@ -1983,6 +1983,8 @@ struct OutgoingWebRequest
 	int connect_timeout; /**< How many seconds to wait for the (TLS) connect to succeed */
 	int transfer_timeout; /**< How many seconds the total transfer may take (connect+reading everything) */
 	int minimum_tls_version;
+	long long max_size; /**< Max response size for memory-backed downloads, in bytes.
+	                     *   0 = use DOWNLOAD_MAX_SIZE. Ignored for file-backed. */
 	// If you are adding fields here:
 	// 1) update duplicate_outgoingwebrequest() in src/misc.c
 	// 2) and update free_outgoingwebrequest() there as well (if something needs to be freed)
@@ -1993,7 +1995,7 @@ struct OutgoingWebResponse
 {
 	const char *file; /**< The temporary file of the download, or NULL. This is only set if OutgoingWebRequest had 'store_in_file' set to 1 and the download was succesful. */
 	const char *memory; /**< The memory buffer of the response, or NULL if an error occured (see errorbuf) */
-	int memory_len; /**< The length of 'memory', since the response may contain binary data. */
+	long long memory_len; /**< The length of 'memory', since the response may contain binary data. */
 	const char *errorbuf; /**< If this is non-NULL then an error occured and this is the error string. Check this member before checking any others! */
 	int cached; /**< Set to 1 if OutgoingWebRequest had 'cachetime' set and we have a cache hit on the webserver. The file and errobuf will be NULL since there was no data transfer. */
 	void *ptr; /**< The OutgoingWebRequest 'callback_data' */

src/misc.c

@@ -3055,6 +3055,7 @@ OutgoingWebRequest *duplicate_outgoingwebrequest(OutgoingWebRequest *orig)
 	e->connect_timeout = orig->connect_timeout;
 	e->transfer_timeout = orig->transfer_timeout;
 	e->minimum_tls_version = orig->minimum_tls_version;
+	e->max_size = orig->max_size;
 	return e;
 }
 
@@ -3098,7 +3099,7 @@ void download_file_async(const char *url,
 	url_start_async(request);
 }
 
-void url_callback(OutgoingWebRequest *r, const char *file, const char *memory, int memory_len, const char *errorbuf, int cached, void *ptr)
+void url_callback(OutgoingWebRequest *r, const char *file, const char *memory, long long memory_len, const char *errorbuf, int cached, void *ptr)
 {
 	OutgoingWebResponse *response;
 

src/url_curl.c

@@ -38,8 +38,9 @@ struct Download
 	FILE *file_fd;		/**< File open for writing (otherwise NULL) */
 	char *filename;
 	char *memory_data; /**< Memory for writing response (otherwise NULL) */
-	int memory_data_len; /**< Size of memory_data */
-	int memory_data_allocated; /**< Total allocated memory for 'memory_data' */
+	long long memory_data_len; /**< Size of memory_data */
+	long long memory_data_allocated; /**< Total allocated memory for 'memory_data' */
+	int cap_exceeded; /**< Set to 1 by do_download_memory when max_size was hit */
 };
 
 CURLM *multihandle = NULL;
@@ -173,12 +174,21 @@ static size_t do_download_memory(void *ptr, size_t size, size_t nmemb, void *str
 {
 	// DUPLICATE CODE: same as src/url_unreal.c, well.. sortof
 	Download *handle = (Download *)stream;
-	int write_sz = size * nmemb;
-	int size_required = handle->memory_data_len + write_sz;
+	size_t write_sz = size * nmemb;
+	long long size_required = handle->memory_data_len + (long long)write_sz;
+
+	if (size_required > handle->request->max_size)
+	{
+		handle->cap_exceeded = 1;
+		safe_free(handle->memory_data);
+		handle->memory_data_len = 0;
+		handle->memory_data_allocated = 0;
+		return 0; /* aborts the curl transfer; real error surfaced in url_check_multi_handles */
+	}
 
 	if (size_required >= handle->memory_data_allocated - 1) // the -1 is for zero termination, even though it is binary..
 	{
-		int newsize = ((size_required / URL_MEMORY_BACKED_CHUNK_SIZE)+1)*URL_MEMORY_BACKED_CHUNK_SIZE;
+		long long newsize = ((size_required / URL_MEMORY_BACKED_CHUNK_SIZE)+1)*URL_MEMORY_BACKED_CHUNK_SIZE;
 		char *newptr = realloc(handle->memory_data, newsize);
 		if (!newptr)
 		{
@@ -248,7 +258,16 @@ static void url_check_multi_handles(void)
 			}
 			else
 			{
-				url_callback(handle->request, NULL, NULL, 0, handle->errorbuf, 0, handle->request->callback_data);
+				char capbuf[128];
+				const char *err = handle->errorbuf;
+				if (handle->cap_exceeded)
+				{
+					snprintf(capbuf, sizeof(capbuf),
+					         "Response too large (maximum: %lld bytes)",
+					         handle->request->max_size);
+					err = capbuf;
+				}
+				url_callback(handle->request, NULL, NULL, 0, err, 0, handle->request->callback_data);
 			}
 
 			if (handle->filename && !handle->request->keep_file)
@@ -349,6 +368,10 @@ void url_start_async(OutgoingWebRequest *request)
 	if (!request->url || !request->http_method)
 		abort();
 
+	/* Set request defaults */
+	if (request->max_size <= 0)
+		request->max_size = DOWNLOAD_MAX_SIZE;
+
 	curl = curl_easy_init();
 	if (!curl)
 	{

src/url_unreal.c

@@ -51,8 +51,8 @@ struct Download
 	FILE *file_fd;		/**< File open for writing (otherwise NULL) */
 	char *filename;
 	char *memory_data; /**< Memory for writing response (otherwise NULL) */
-	int memory_data_len; /**< Size of memory_data */
-	int memory_data_allocated; /**< Total allocated memory for 'memory_data' */
+	long long memory_data_len; /**< Size of memory_data */
+	long long memory_data_allocated; /**< Total allocated memory for 'memory_data' */
 	char errorbuf[512];
 	char *hostname;		/**< Parsed hostname (from 'url') */
 	int port;		/**< Parsed port (from 'url') */
@@ -73,7 +73,7 @@ struct Download
 	time_t download_started;
 	int dns_refcnt;
 	TransferEncoding transfer_encoding;
-	long chunk_remaining;
+	long long chunk_remaining;
 	char *redirect_new_location;
 };
 
@@ -179,6 +179,8 @@ void url_start_async(OutgoingWebRequest *request)
 		request->connect_timeout = DOWNLOAD_CONNECT_TIMEOUT;
 	if (request->transfer_timeout == 0)
 		request->transfer_timeout = DOWNLOAD_TRANSFER_TIMEOUT;
+	if (request->max_size <= 0)
+		request->max_size = DOWNLOAD_MAX_SIZE;
 
 	handle = safe_alloc(sizeof(Download));
 	handle->download_started = TStime();
@@ -813,17 +815,17 @@ int https_handle_response_header(Download *handle, char *readbuf, int n)
 	return 1;
 }
 
-int https_handle_response_body_memory(Download *handle, const char *ptr, int write_sz)
+long long https_handle_response_body_memory(Download *handle, const char *ptr, long long write_sz)
 {
 	// DUPLICATE CODE: same as src/url_curl.c, well... sortof
-	int size_required = handle->memory_data_len + write_sz;
+	long long size_required = handle->memory_data_len + write_sz;
 
 	if (handle->memory_data == NULL)
 		return 0; /* Normally does not happen as it is preallocated, but could happen upon unwinding cancels.. */
 
 	if (size_required >= handle->memory_data_allocated - 1) // the -1 is for zero termination, even though it is binary..
 	{
-		int newsize = ((size_required / URL_MEMORY_BACKED_CHUNK_SIZE)+1)*URL_MEMORY_BACKED_CHUNK_SIZE;
+		long long newsize = ((size_required / URL_MEMORY_BACKED_CHUNK_SIZE)+1)*URL_MEMORY_BACKED_CHUNK_SIZE;
 		char *newptr = realloc(handle->memory_data, newsize);
 		if (!newptr)
 		{
@@ -858,7 +860,14 @@ int https_handle_response_body(Download *handle, char *readbuf, int pktsize)
 	{
 		/* Ohh.. so easy! */
 		if (handle->request->store_in_file == 0)
+		{
+			if (handle->memory_data_len + pktsize > handle->request->max_size)
+			{
+				https_cancel(handle, "Response too large (maximum: %lld bytes)", handle->request->max_size);
+				return 0; /* handle freed */
+			}
 			https_handle_response_body_memory(handle, readbuf, pktsize);
+		}
 		else if (handle->file_fd)
 			fwrite(readbuf, 1, pktsize, handle->file_fd);
 		return 1;
@@ -886,9 +895,17 @@ int https_handle_response_body(Download *handle, char *readbuf, int pktsize)
 		if (handle->chunk_remaining > 0)
 		{
 			/* Eat it */
-			int eat = MIN(handle->chunk_remaining, n);
+			long long eat = MIN(handle->chunk_remaining, n);
 			if (handle->request->store_in_file == 0)
+			{
+				if (handle->memory_data_len + eat > handle->request->max_size)
+				{
+					https_cancel(handle, "Response too large (maximum: %lld bytes)", handle->request->max_size);
+					safe_free(free_this_buffer);
+					return 0; /* handle freed */
+				}
 				https_handle_response_body_memory(handle, buf, eat);
+			}
 			else if (handle->file_fd)
 				fwrite(buf, 1, eat, handle->file_fd);
 			n -= eat;
@@ -943,10 +960,10 @@ int https_handle_response_body(Download *handle, char *readbuf, int pktsize)
 			}
 			buf[i] = '\0'; /* cut at LF */
 			i++; /* point to next data */
-			handle->chunk_remaining = strtol(buf, NULL, 16);
+			handle->chunk_remaining = strtoll(buf, NULL, 16);
 			if (handle->chunk_remaining < 0)
 			{
-				https_cancel(handle, "Negative chunk encountered (%ld)", handle->chunk_remaining);
+				https_cancel(handle, "Negative chunk encountered (%lld)", handle->chunk_remaining);
 				safe_free(free_this_buffer);
 				return 0;
 			}