# HG changeset patch # User Daniel Atallah # Date 1206120380 0 # Node ID 5a0186edda975b29f8704ee04101c35ccfe3befa # Parent 9693a727e7efd3e58b3f48ca18ed07309d4a00aa 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. diff -r 9693a727e7ef -r 5a0186edda97 libpurple/proxy.c --- 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