changeset 22522:5a0186edda97

Fix SOCKS5 CHAP authentication to be more robust; the current stuff depends on the attributes coming in in a specific order and the whole authentication having a maximum length of 20 bytes. Big thanks to Jonathan Rudolph ("simple" on IRC) for helping figure out what was wrong.

--- a/libpurple/proxy.c	Fri Mar 21 15:18:55 2008 +0000
+++ b/libpurple/proxy.c	Fri Mar 21 17:26:20 2008 +0000
@@ -1356,11 +1356,17 @@
 	purple_debug(PURPLE_DEBUG_INFO, "socks5 proxy", "Got CHAP response.\n");
 
 	if (connect_data->read_buffer == NULL) {
-		connect_data->read_buf_len = 20;
+		/* A big enough butfer to read the message header (2 bytes) and at least one complete attribute and value (1 + 1 + 255). */
+		connect_data->read_buf_len = 259;
 		connect_data->read_buffer = g_malloc(connect_data->read_buf_len);
 		connect_data->read_len = 0;
 	}
 
+	if (connect_data->read_buf_len - connect_data->read_len == 0) {
+		/*If the stuff below is right, this shouldn't be possible. */
+		purple_debug_error("socks5 proxy", "This is about to suck because the read buffer is full (shouldn't happen).\n");
+	}
+
 	len = read(connect_data->fd, connect_data->read_buffer + connect_data->read_len,
 		connect_data->read_buf_len - connect_data->read_len);
 
@@ -1400,13 +1406,31 @@
 	cmdbuf++;
 
 	for (currentav = 0; currentav < navas; currentav++) {
-		if (connect_data->read_len - (cmdbuf - connect_data->read_buffer) < 2)
+
+		len = connect_data->read_len - (cmdbuf - connect_data->read_buffer);
+		/* We don't have enough data to even know how long the next attribute is,
+		 * or we don't have the full length of the next attribute. */
+		if (len < 2 || len < (cmdbuf[1] + 2)) {
+			/* Clear out the attributes that have been read - decrease the attribute count */
+			connect_data->read_buffer[1] = navas - currentav;
+			/* Move the unprocessed data into the first attribute position */
+			memmove((connect_data->read_buffer + 2), cmdbuf, len);
+			/* Decrease the read count accordingly */
+			connect_data->read_len = len + 2;
 			return;
-		if (connect_data->read_len - (cmdbuf - connect_data->read_buffer) < cmdbuf[1])
-			return;
+		}
+
 		buf = cmdbuf + 2;
+
+		if (cmdbuf[1] == 0) {
+			purple_debug_error("socks5 proxy", "Attribute %x Value length of 0; ignoring.\n", cmdbuf[0]);
+			cmdbuf = buf;
+			continue;
+		}
+
 		switch (cmdbuf[0]) {
 			case 0x00:
+				purple_debug_info("socks5 proxy", "Received STATUS of %x\n", buf[0]);
 				/* Did auth work? */
 				if (buf[0] == 0x00) {
 					purple_input_remove(connect_data->inpa);
@@ -1415,7 +1439,6 @@
 					connect_data->read_buffer = NULL;
 					/* Success */
 					s5_sendconnect(connect_data, connect_data->fd);
-					return;
 				} else {
 					/* Failure */
 					purple_debug_warning("proxy",
@@ -1423,10 +1446,10 @@
 						"failed.  Disconnecting...");
 					purple_proxy_connect_data_disconnect(connect_data,
 							_("Authentication failed"));
-					return;
 				}
-				break;
+				return;
 			case 0x03:
+				purple_debug_info("socks5 proxy", "Received CHALLENGE\n");
 				/* Server wants our credentials */
 
 				connect_data->write_buf_len = 16 + 4;
@@ -1451,8 +1474,9 @@
 					PURPLE_INPUT_WRITE, proxy_do_write, connect_data);
 
 				proxy_do_write(connect_data, connect_data->fd, PURPLE_INPUT_WRITE);
-				break;
+				return;
 			case 0x11:
+				purple_debug_info("socks5 proxy", "Received ALGORIGTHMS of %x\n", buf[0]);
 				/* Server wants to select an algorithm */
 				if (buf[0] != 0x85) {
 					/* Only currently support HMAC-MD5 */
@@ -1467,12 +1491,19 @@
 					return;
 				}
 				break;
+			default:
+				purple_debug_info("socks5 proxy", "Received unused command %x, length=%d\n", cmdbuf[0], cmdbuf[1]);
 		}
 		cmdbuf = buf + cmdbuf[1];
 	}
+
 	/* Fell through.  We ran out of CHAP events to process, but haven't
 	 * succeeded or failed authentication - there may be more to come.
 	 * If this is the case, come straight back here. */
+
+	/* We've processed all the available attributes, so get ready for a whole new message */
+ 	g_free(connect_data->read_buffer);
+	connect_data->read_buffer = NULL;
 }
 
 static void