
# HG changeset patch
# User Daniel Atallah <datallah@pidgin.im>
# Date 1361466262 18000
# Node ID cd529e1158d3e6a9c55eab7de6806ab0c81e233d
# Parent  68d6df7dc69c1b62c01fae2390c00f3639542793
Fix purple_util_fetch_url_request variants to avoid implicitly trusting the
Content-Length specified by the server.
This issue was identified by Jacob Appelbaum of the Tor Project

 * Add a default limit of 512KiB when no explicit download limit has been specified to the API
 ** This may be controversial, but anything that needs more than that should specify it explicitly
 * Don't allocate more memory than the maximum download limit (+ header_length, which effectively doubles the limit)
 * Use g_try_realloc() instead of g_realloc() to avoid aborting when memory can't be allocated
 * Remove spurious comment about the header data not being nul-terminated - it is (line 3846)
 * Avoid looking in the HTTP body for headers (just a correctness bug)

Index: pidgin-2.10.7/libpurple/util.c
===================================================================
--- pidgin-2.10.7.orig/libpurple/util.c	2014-02-05 15:02:52.776162037 -0500
+++ pidgin-2.10.7/libpurple/util.c	2014-02-05 15:02:52.772162037 -0500
@@ -33,6 +33,10 @@
 #include "prefs.h"
 #include "util.h"
 
+/* 512KiB Default value for maximum HTTP download size (when the client hasn't
+   specified a length) */
+#define DEFAULT_MAX_HTTP_DOWNLOAD (512 * 1024)
+
 struct _PurpleUtilFetchUrlData
 {
 	PurpleUtilFetchUrlCallback callback;
@@ -68,7 +72,7 @@
 	char *webdata;
 	gsize len;
 	unsigned long data_len;
-	gssize max_len;
+	gsize max_len;
 	gboolean chunked;
 	PurpleAccount *account;
 };
@@ -3239,24 +3243,26 @@
 	return ret;
 }
 
-const char *
-purple_strcasestr(const char *haystack, const char *needle)
+/** TODO: Expose this when we can add API */
+static const char *
+purple_strcasestr_len(const char *haystack, gssize hlen, const char *needle, gssize nlen)
 {
-	size_t hlen, nlen;
 	const char *tmp, *ret;
 
 	g_return_val_if_fail(haystack != NULL, NULL);
 	g_return_val_if_fail(needle != NULL, NULL);
 
-	hlen = strlen(haystack);
-	nlen = strlen(needle);
+	if (hlen == -1)
+		hlen = strlen(haystack);
+	if (nlen == -1)
+		nlen = strlen(needle);
 	tmp = haystack,
 	ret = NULL;
 
 	g_return_val_if_fail(hlen > 0, NULL);
 	g_return_val_if_fail(nlen > 0, NULL);
 
-	while (*tmp && !ret) {
+	while (*tmp && !ret && (hlen - (tmp - haystack)) >= nlen) {
 		if (!g_ascii_strncasecmp(needle, tmp, nlen))
 			ret = tmp;
 		else
@@ -3266,6 +3272,12 @@
 	return ret;
 }
 
+const char *
+purple_strcasestr(const char *haystack, const char *needle)
+{
+	return purple_strcasestr_len(haystack, -1, needle, -1);
+}
+
 char *
 purple_str_size_to_units(size_t size)
 {
@@ -3575,7 +3587,7 @@
 static void ssl_url_fetch_error_cb(PurpleSslConnection *ssl_connection, PurpleSslErrorType error, gpointer data);
 
 static gboolean
-parse_redirect(const char *data, size_t data_len,
+parse_redirect(const char *data, gsize data_len,
 			   PurpleUtilFetchUrlData *gfud)
 {
 	gchar *s;
@@ -3680,20 +3692,21 @@
 	return TRUE;
 }
 
+/* find the starting point of the content for the specified header and make
+ * sure that the content is safe to pass to sscanf */
 static const char *
-find_header_content(const char *data, size_t data_len, const char *header, size_t header_len)
+find_header_content(const char *data, gsize data_len, const char *header)
 {
 	const char *p = NULL;
 
-	if (header_len <= 0)
-		header_len = strlen(header);
+	gsize header_len = strlen(header);
 
-	/* Note: data is _not_ nul-terminated.  */
 	if (data_len > header_len) {
+		/* Check if the first header matches (data won't start with a \n") */
 		if (header[0] == '\n')
 			p = (g_ascii_strncasecmp(data, header + 1, header_len - 1) == 0) ? data : NULL;
 		if (!p)
-			p = purple_strcasestr(data, header);
+			p = purple_strcasestr_len(data, data_len, header, header_len);
 		if (p)
 			p += header_len;
 	}
@@ -3709,13 +3722,13 @@
 	return NULL;
 }
 
-static size_t
-parse_content_len(const char *data, size_t data_len)
+static gsize 
+parse_content_len(const char *data, gsize data_len)
 {
-	size_t content_len = 0;
+	gsize content_len = 0;
 	const char *p = NULL;
 
-	p = find_header_content(data, data_len, "\nContent-Length: ", sizeof("\nContent-Length: ") - 1);
+	p = find_header_content(data, data_len, "\nContent-Length: ");
 	if (p) {
 		sscanf(p, "%" G_GSIZE_FORMAT, &content_len);
 		purple_debug_misc("util", "parsed %" G_GSIZE_FORMAT "\n", content_len);
@@ -3725,9 +3738,9 @@
 }
 
 static gboolean
-content_is_chunked(const char *data, size_t data_len)
+content_is_chunked(const char *data, gsize data_len)
 {
-	const char *p = find_header_content(data, data_len, "\nTransfer-Encoding: ", sizeof("\nTransfer-Encoding: ") - 1);
+	const char *p = find_header_content(data, data_len, "\nTransfer-Encoding: ");
 	if (p && g_ascii_strncasecmp(p, "chunked", 7) == 0)
 		return TRUE;
 
@@ -3810,7 +3823,7 @@
 	while ((gfud->is_ssl && ((len = purple_ssl_read(gfud->ssl_connection, buf, sizeof(buf))) > 0)) ||
 			(!gfud->is_ssl && (len = read(source, buf, sizeof(buf))) > 0))
 	{
-		if(gfud->max_len != -1 && (gfud->len + len) > gfud->max_len) {
+		if((gfud->len + len) > gfud->max_len) {
 			purple_util_fetch_url_error(gfud, _("Error reading from %s: response too long (%d bytes limit)"),
 						    gfud->website.address, gfud->max_len);
 			return;
@@ -3838,9 +3851,8 @@
 			/* See if we've reached the end of the headers yet */
 			end_of_headers = strstr(gfud->webdata, "\r\n\r\n");
 			if (end_of_headers) {
-				char *new_data;
 				guint header_len = (end_of_headers + 4 - gfud->webdata);
-				size_t content_len;
+				gsize content_len;
 
 				purple_debug_misc("util", "Response headers: '%.*s'\n",
 					header_len, gfud->webdata);
@@ -3860,15 +3872,36 @@
 					content_len = 8192;
 				} else {
 					gfud->has_explicit_data_len = TRUE;
+					if (content_len > gfud->max_len) {
+						purple_debug_error("util",
+								"Overriding explicit Content-Length of %" G_GSIZE_FORMAT " with max of %" G_GSSIZE_FORMAT "\n",
+								content_len, gfud->max_len);
+						content_len = gfud->max_len;
+					}
 				}
 
 
 				/* If we're returning the headers too, we don't need to clean them out */
 				if (gfud->include_headers) {
+					char *new_data;
 					gfud->data_len = content_len + header_len;
-					gfud->webdata = g_realloc(gfud->webdata, gfud->data_len);
+					new_data = g_try_realloc(gfud->webdata, gfud->data_len);
+					if (new_data == NULL) {
+						purple_debug_error("util",
+								"Failed to allocate %" G_GSIZE_FORMAT " bytes: %s\n",
+								content_len, g_strerror(errno));
+						purple_util_fetch_url_error(gfud,
+								_("Unable to allocate enough memory to hold "
+								  "the contents from %s.  The web server may "
+								  "be trying something malicious."),
+								gfud->website.address);
+
+						return;
+					}
+					gfud->webdata = new_data;
 				} else {
-					size_t body_len = gfud->len - header_len;
+					char *new_data;
+					gsize body_len = gfud->len - header_len;
 
 					content_len = MAX(content_len, body_len);
 
@@ -4154,7 +4187,11 @@
 	gfud->request = g_strdup(request);
 	gfud->include_headers = include_headers;
 	gfud->fd = -1;
-	gfud->max_len = max_len;
+	if (max_len <= 0) {
+		max_len = DEFAULT_MAX_HTTP_DOWNLOAD;
+		purple_debug_error("util", "Defaulting max download from %s to %" G_GSSIZE_FORMAT "\n", url, max_len);
+	}
+	gfud->max_len = (gsize) max_len;
 	gfud->account = account;
 
 	purple_url_parse(url, &gfud->website.address, &gfud->website.port,
