pidgin: 150db837: A few changes to our code to handle chun...

[markdoliner at pidgin.im]

-----------------------------------------------------------------
Revision: 150db837b012d157fec1d94919808a3125b7d1a4
Ancestor: bbca51d4ecb2cb1f3eb8ebf0de5f8c8ad6d0893e
Author: markdoliner at pidgin.im
Date: 2009-07-02T07:33:17
Branch: im.pidgin.pidgin
URL: http://d.pidgin.im/viewmtn/revision/info/150db837b012d157fec1d94919808a3125b7d1a4

Modified files:
        libpurple/util.c

ChangeLog: 

A few changes to our code to handle chunked transfer-encodings (technically
required for http 1.1 clients, but I guess it hasn't been a problem until now?)

* Most importantly, check that the size of this chunk is less than the size
  of the data remaining in the buffer.  Otherwise malicious servers could
  cause us to crash
* Handle reading chunks that have a semi-colon after them.  It seems like
  maybe this is allowed but almost never used?
* Assume len is non-NULL (it always is in our case)
* Add some comments

-------------- next part --------------
============================================================
--- libpurple/util.c	1a552bb60cb8f950190972e5245f16443fce7cc8
+++ libpurple/util.c	10ff4dd275fe591ee44e41da047659cd7fbe5cf6
@@ -3788,32 +3788,56 @@ process_chunked_data(char *data, gsize *
 process_chunked_data(char *data, gsize *len)
 {
 	gsize sz;
-	gsize nlen = 0;
+	gsize newlen = 0;
 	char *p = data;
 	char *s = data;
 
 	while (*s) {
-		if (sscanf(s, "%" G_GSIZE_MODIFIER "x\r\n", &sz) != 1) {
-			purple_debug_error("util", "Error processing chunked data. Expected data length, found: %s\n", s);
+		/* Read the size of this chunk */
+		if (sscanf(s, "%" G_GSIZE_MODIFIER "x\r\n", &sz) != 1 &&
+			sscanf(s, "%" G_GSIZE_MODIFIER "x;", &sz) != 1)
+		{
+			purple_debug_error("util", "Error processing chunked data: "
+					"Expected data length, found: %s\n", s);
 			break;
 		}
-		if (sz == 0)
+		if (sz == 0) {
+			/* We've reached the last chunk */
+			/*
+			 * TODO: The spec allows "footers" to follow the last chunk.
+			 *       If there is more data after this line then we should
+			 *       treat it like a header.
+			 */
 			break;
+		}
+
+		/* Advance to the start of the data */
 		s = strstr(s, "\r\n") + 2;
+
+		if (s + sz > data + *len) {
+			purple_debug_error("util", "Error processing chunked data: "
+					"Chunk size %" G_GSIZE_FORMAT " bytes was longer "
+					"than the data remaining in the buffer (%"
+					G_GSIZE_FORMAT " bytes)\n", sz, data + *len - s);
+		}
+
+		/* Move all data overtop of the chunk length that we read in earlier */
 		g_memmove(p, s, sz);
 		p += sz;
 		s += sz;
-		nlen += sz;
+		newlen += sz;
 		if (*s != '\r' && *(s + 1) != '\n') {
-			purple_debug_error("util", "Error processing chunked data. Expected \\r\\n, found: %s\n", s);
+			purple_debug_error("util", "Error processing chunked data: "
+					"Expected \\r\\n, found: %s\n", s);
 			break;
 		}
 		s += 2;
 	}
+
+	/* NULL terminate the data */
 	*p = 0;
 
-	if (len)
-		*len = nlen;
+	*len = newlen;
 }
 
 static void