Inbuf for outgoing SOCKS 4 proxy not cleared before reading from OR connection

When configured to use an outgoing SOCKS 4 proxy (as set with Socks4Proxy ip:port), tor does not clear the connection buffer after the SOCKS handshake and before reading bytes from the OR connection. A proxy can send extra characters at the end of the handshake which will be stored on the OR connection inbuf as if they were authenticated bytes from the first hop.

This is not actually exploitable because of a check in connection_or_process_inbuf(), which will close the connection if it sees that there are bytes on the inbuf while the connection is in the TLS handshaking state. This check was originally designed to protect relays and not clients, but luckily it actually protects the client in this case as well.

/* This check was necessary with 0.2.2, when the TLS_SERVER_RENEGOTIATING
 * check would otherwise just let data accumulate.  It serves no purpose
 * in 0.2.3.
 *
 * XXXX Remove this check once we verify that the above paragraph is
 * 100% true. */

if (buf_datalen(conn->base_.inbuf) > MAX_OR_INBUF_WHEN_NONOPEN) {
  log_fn(LOG_PROTOCOL_WARN, LD_NET, "Accumulated too much data (%d bytes) "
         "on nonopen OR connection %s %s:%u in state %s; closing.",
         (int)buf_datalen(conn->base_.inbuf),
         connection_or_nonopen_was_started_here(conn) ? "to" : "from",
         conn->base_.address, conn->base_.port,
         conn_state_to_string(conn->base_.type, conn->base_.state));
  connection_or_close_for_error(conn, 0);
  ret = -1;
}

I think that the comments here should be changed to show that this check is still important, and it would be good to also clear the inbuf when transitioning out of the OR_CONN_STATE_PROXY_HANDSHAKING state. The best solution would probably be to not use a single buffer for both authenticated and unauthenticated data, but this would probably take more work. I have also not looked at the other proxy types, but the SOCKS 5 proxy looks okay at first glance.

If you remove the check above (as mentioned in the comment), a SOCKS 4 proxy that appends for example \x00\x00\x07\x00\x00\x01 (a VERSIONS cell) to the proxy reply will cause the tor client to log:

channel_tls_process_versions_cell(): Couldn't find a version in common between my version list and the list in the VERSIONS cell; closing connection.

This only causes the client to drop the connection (so the client can't connect to the tor network), but as the proxy would be able to bypass the TLS authentication for the first cells on a connection (if the check above were removed) and make the client think that it received cells that the first hop didn't send, there could potentially be other interesting things you could do with other cell types.