Fix purple_util_fetch_url_request variants to avoid implicitly trusting the release-2.x.y
authorDaniel Atallah <datallah@pidgin.im>
Thu, 21 Feb 2013 12:04:22 -0500
branchrelease-2.x.y
changesetcd529e1158d3 pushlog
parent 68d6df7dc69c
child 74bc66c67211
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)
libpurple/util.c
     1.1 --- a/libpurple/util.c
     1.2 +++ b/libpurple/util.c
     1.3 @@ -33,6 +33,10 @@
     1.4  #include "prefs.h"
     1.5  #include "util.h"
     1.6  
     1.7 +/* 512KiB Default value for maximum HTTP download size (when the client hasn't
     1.8 +   specified a length) */
     1.9 +#define DEFAULT_MAX_HTTP_DOWNLOAD (512 * 1024)
    1.10 +
    1.11  struct _PurpleUtilFetchUrlData
    1.12  {
    1.13  	PurpleUtilFetchUrlCallback callback;
    1.14 @@ -68,7 +72,7 @@
    1.15  	char *webdata;
    1.16  	gsize len;
    1.17  	unsigned long data_len;
    1.18 -	gssize max_len;
    1.19 +	gsize max_len;
    1.20  	gboolean chunked;
    1.21  	PurpleAccount *account;
    1.22  };
    1.23 @@ -3239,24 +3243,26 @@
    1.24  	return ret;
    1.25  }
    1.26  
    1.27 -const char *
    1.28 -purple_strcasestr(const char *haystack, const char *needle)
    1.29 +/** TODO: Expose this when we can add API */
    1.30 +static const char *
    1.31 +purple_strcasestr_len(const char *haystack, gssize hlen, const char *needle, gssize nlen)
    1.32  {
    1.33 -	size_t hlen, nlen;
    1.34  	const char *tmp, *ret;
    1.35  
    1.36  	g_return_val_if_fail(haystack != NULL, NULL);
    1.37  	g_return_val_if_fail(needle != NULL, NULL);
    1.38  
    1.39 -	hlen = strlen(haystack);
    1.40 -	nlen = strlen(needle);
    1.41 +	if (hlen == -1)
    1.42 +		hlen = strlen(haystack);
    1.43 +	if (nlen == -1)
    1.44 +		nlen = strlen(needle);
    1.45  	tmp = haystack,
    1.46  	ret = NULL;
    1.47  
    1.48  	g_return_val_if_fail(hlen > 0, NULL);
    1.49  	g_return_val_if_fail(nlen > 0, NULL);
    1.50  
    1.51 -	while (*tmp && !ret) {
    1.52 +	while (*tmp && !ret && (hlen - (tmp - haystack)) >= nlen) {
    1.53  		if (!g_ascii_strncasecmp(needle, tmp, nlen))
    1.54  			ret = tmp;
    1.55  		else
    1.56 @@ -3266,6 +3272,12 @@
    1.57  	return ret;
    1.58  }
    1.59  
    1.60 +const char *
    1.61 +purple_strcasestr(const char *haystack, const char *needle)
    1.62 +{
    1.63 +	return purple_strcasestr_len(haystack, -1, needle, -1);
    1.64 +}
    1.65 +
    1.66  char *
    1.67  purple_str_size_to_units(size_t size)
    1.68  {
    1.69 @@ -3576,7 +3588,7 @@
    1.70  static void ssl_url_fetch_error_cb(PurpleSslConnection *ssl_connection, PurpleSslErrorType error, gpointer data);
    1.71  
    1.72  static gboolean
    1.73 -parse_redirect(const char *data, size_t data_len,
    1.74 +parse_redirect(const char *data, gsize data_len,
    1.75  			   PurpleUtilFetchUrlData *gfud)
    1.76  {
    1.77  	gchar *s;
    1.78 @@ -3681,20 +3693,21 @@
    1.79  	return TRUE;
    1.80  }
    1.81  
    1.82 +/* find the starting point of the content for the specified header and make
    1.83 + * sure that the content is safe to pass to sscanf */
    1.84  static const char *
    1.85 -find_header_content(const char *data, size_t data_len, const char *header, size_t header_len)
    1.86 +find_header_content(const char *data, gsize data_len, const char *header)
    1.87  {
    1.88  	const char *p = NULL;
    1.89  
    1.90 -	if (header_len <= 0)
    1.91 -		header_len = strlen(header);
    1.92 -
    1.93 -	/* Note: data is _not_ nul-terminated.  */
    1.94 +	gsize header_len = strlen(header);
    1.95 +
    1.96  	if (data_len > header_len) {
    1.97 +		/* Check if the first header matches (data won't start with a \n") */
    1.98  		if (header[0] == '\n')
    1.99  			p = (g_ascii_strncasecmp(data, header + 1, header_len - 1) == 0) ? data : NULL;
   1.100  		if (!p)
   1.101 -			p = purple_strcasestr(data, header);
   1.102 +			p = purple_strcasestr_len(data, data_len, header, header_len);
   1.103  		if (p)
   1.104  			p += header_len;
   1.105  	}
   1.106 @@ -3710,13 +3723,13 @@
   1.107  	return NULL;
   1.108  }
   1.109  
   1.110 -static size_t
   1.111 -parse_content_len(const char *data, size_t data_len)
   1.112 +static gsize 
   1.113 +parse_content_len(const char *data, gsize data_len)
   1.114  {
   1.115 -	size_t content_len = 0;
   1.116 +	gsize content_len = 0;
   1.117  	const char *p = NULL;
   1.118  
   1.119 -	p = find_header_content(data, data_len, "\nContent-Length: ", sizeof("\nContent-Length: ") - 1);
   1.120 +	p = find_header_content(data, data_len, "\nContent-Length: ");
   1.121  	if (p) {
   1.122  		sscanf(p, "%" G_GSIZE_FORMAT, &content_len);
   1.123  		purple_debug_misc("util", "parsed %" G_GSIZE_FORMAT "\n", content_len);
   1.124 @@ -3726,9 +3739,9 @@
   1.125  }
   1.126  
   1.127  static gboolean
   1.128 -content_is_chunked(const char *data, size_t data_len)
   1.129 +content_is_chunked(const char *data, gsize data_len)
   1.130  {
   1.131 -	const char *p = find_header_content(data, data_len, "\nTransfer-Encoding: ", sizeof("\nTransfer-Encoding: ") - 1);
   1.132 +	const char *p = find_header_content(data, data_len, "\nTransfer-Encoding: ");
   1.133  	if (p && g_ascii_strncasecmp(p, "chunked", 7) == 0)
   1.134  		return TRUE;
   1.135  
   1.136 @@ -3811,7 +3824,7 @@
   1.137  	while ((gfud->is_ssl && ((len = purple_ssl_read(gfud->ssl_connection, buf, sizeof(buf))) > 0)) ||
   1.138  			(!gfud->is_ssl && (len = read(source, buf, sizeof(buf))) > 0))
   1.139  	{
   1.140 -		if(gfud->max_len != -1 && (gfud->len + len) > gfud->max_len) {
   1.141 +		if((gfud->len + len) > gfud->max_len) {
   1.142  			purple_util_fetch_url_error(gfud, _("Error reading from %s: response too long (%d bytes limit)"),
   1.143  						    gfud->website.address, gfud->max_len);
   1.144  			return;
   1.145 @@ -3839,9 +3852,8 @@
   1.146  			/* See if we've reached the end of the headers yet */
   1.147  			end_of_headers = strstr(gfud->webdata, "\r\n\r\n");
   1.148  			if (end_of_headers) {
   1.149 -				char *new_data;
   1.150  				guint header_len = (end_of_headers + 4 - gfud->webdata);
   1.151 -				size_t content_len;
   1.152 +				gsize content_len;
   1.153  
   1.154  				purple_debug_misc("util", "Response headers: '%.*s'\n",
   1.155  					header_len, gfud->webdata);
   1.156 @@ -3861,15 +3873,36 @@
   1.157  					content_len = 8192;
   1.158  				} else {
   1.159  					gfud->has_explicit_data_len = TRUE;
   1.160 +					if (content_len > gfud->max_len) {
   1.161 +						purple_debug_error("util",
   1.162 +								"Overriding explicit Content-Length of %" G_GSIZE_FORMAT " with max of %" G_GSSIZE_FORMAT "\n",
   1.163 +								content_len, gfud->max_len);
   1.164 +						content_len = gfud->max_len;
   1.165 +					}
   1.166  				}
   1.167  
   1.168  
   1.169  				/* If we're returning the headers too, we don't need to clean them out */
   1.170  				if (gfud->include_headers) {
   1.171 +					char *new_data;
   1.172  					gfud->data_len = content_len + header_len;
   1.173 -					gfud->webdata = g_realloc(gfud->webdata, gfud->data_len);
   1.174 +					new_data = g_try_realloc(gfud->webdata, gfud->data_len);
   1.175 +					if (new_data == NULL) {
   1.176 +						purple_debug_error("util",
   1.177 +								"Failed to allocate %" G_GSIZE_FORMAT " bytes: %s\n",
   1.178 +								content_len, g_strerror(errno));
   1.179 +						purple_util_fetch_url_error(gfud,
   1.180 +								_("Unable to allocate enough memory to hold "
   1.181 +								  "the contents from %s.  The web server may "
   1.182 +								  "be trying something malicious."),
   1.183 +								gfud->website.address);
   1.184 +
   1.185 +						return;
   1.186 +					}
   1.187 +					gfud->webdata = new_data;
   1.188  				} else {
   1.189 -					size_t body_len = gfud->len - header_len;
   1.190 +					char *new_data;
   1.191 +					gsize body_len = gfud->len - header_len;
   1.192  
   1.193  					content_len = MAX(content_len, body_len);
   1.194  
   1.195 @@ -4155,7 +4188,11 @@
   1.196  	gfud->request = g_strdup(request);
   1.197  	gfud->include_headers = include_headers;
   1.198  	gfud->fd = -1;
   1.199 -	gfud->max_len = max_len;
   1.200 +	if (max_len <= 0) {
   1.201 +		max_len = DEFAULT_MAX_HTTP_DOWNLOAD;
   1.202 +		purple_debug_error("util", "Defaulting max download from %s to %" G_GSSIZE_FORMAT "\n", url, max_len);
   1.203 +	}
   1.204 +	gfud->max_len = (gsize) max_len;
   1.205  	gfud->account = account;
   1.206  
   1.207  	purple_url_parse(url, &gfud->website.address, &gfud->website.port,