Delaying hostname send in SOCKS5 `CONNECT` command causes failure in hostname resolution

Tested on systems

• Linux 5.8.0-7625-generic amd64, Tor 0.4.2.7
• FreeBSD 11.2-RELEASE-p15 amd64, Tor 0.4.4.5

Replication

SOCKS5 has the below specification for the request (RFC1928)

   The SOCKS request is formed as follows:

        +----+-----+-------+------+----------+----------+
        |VER | CMD |  RSV  | ATYP | DST.ADDR | DST.PORT |
        +----+-----+-------+------+----------+----------+
        | 1  |  1  | X'00' |  1   | Variable |    2     |
        +----+-----+-------+------+----------+----------+

     Where:

          o  VER    protocol version: X'05'
          o  CMD
             o  CONNECT X'01'
             o  BIND X'02'
             o  UDP ASSOCIATE X'03'
          o  RSV    RESERVED
          o  ATYP   address type of following address
             o  IP V4 address: X'01'
             o  DOMAINNAME: X'03'
             o  IP V6 address: X'04'
          o  DST.ADDR       desired destination address
          o  DST.PORT desired destination port in network octet
             order

To replicate:

• Send the VER, CMD, RSV, and ATYP bytes (4 bytes) in a single write.
• Wait a short while.
• Send the rest of the request (DST.ADDR, DST.PORT) in another write.

This consistently leads to hostname resolution failure as logged in the Tor logs.

Test Code

#include<netinet/in.h>
#include<netinet/ip.h>
#include<stdint.h>
#include<stdio.h>
#include<string.h>
#include<sys/socket.h>
#include<sys/types.h>
#include<unistd.h>

static
const short proxy_port = 9050;
#define PROXY_HOST htonl(INADDR_LOOPBACK)

static
const char* hostname = "www.google.com";
static
const short port = 443;

int main() {
	uint8_t buffer[1024];
	int fd;
	struct sockaddr_in si;
	short n_port;

	int success = 0;

	fd = socket(AF_INET, SOCK_STREAM, 0);

	si.sin_family = AF_INET;
	si.sin_port = htons(proxy_port);
	si.sin_addr.s_addr = PROXY_HOST;
	connect(fd, (const struct sockaddr*) &si, sizeof(si));

	buffer[0] = 0x05; /* SOCKS5 */
	buffer[1] = 0x01; /* Number of authentication methods*/
	buffer[2] = 0x00; /* No authentication.  */
	write(fd, buffer, 3);

	buffer[0] = ~0x05;
	buffer[1] = ~0x00;
	read(fd, buffer, 2);
	if (buffer[0] != 0x05)
		fprintf(stderr, "Unexpected response from proxy, expecting SOCKS5\n");
	if (buffer[1] != 0x00)
		fprintf(stderr, "Unexpected response from proxy, expecting unauthenticated\n");

	buffer[0] = 0x05; /* SOCKS5 */
	buffer[1] = 0x01; /* CMD CONNECT */
	buffer[2] = 0x00; /* RSV */
	buffer[3] = 0x03; /* ATYP DOMAINNAME */
	write(fd, buffer, 4);

	/* This delay, should not affect the behavior of a proxy!  */
	usleep(1000); /* 1ms */
	/* If the above delay is commented out, *sometimes* on my
	 * system this program succeeds and prints "connected",
	 * *sometimes* it fails and prints "not connected".
	 * With the above 1-millisecond delay, it always prints
	 * "not connected".
	 * Without the above delay, if I run in any kind of test
	 * harness (`strace`, `valgrind`) it always says "not
	 * connected", suggesting that any minor delay after
	 * `ATYP` can trigger this.
	 */

	buffer[0] = (uint8_t)strlen(hostname);
	memcpy(&buffer[1], hostname, (size_t) buffer[0]);
	write(fd, buffer, 1 + ((size_t) buffer[0]));

	n_port = htons(port);
	memcpy(&buffer[0], &n_port, 2);
	write(fd, buffer, 2);

	buffer[0] = ~0x05;
	buffer[1] = ~0x00;
	read(fd, buffer, 2);
	if (buffer[0] != 0x05)
		fprintf(stderr, "Unexpected response from proxy, expecting SOCKS5\n");
	success = (buffer[1] == 0x00);

	close(fd);

	if (success)
		fprintf(stdout, "Connected.\n");
	else
		fprintf(stdout, "Not connected.\n");

	return 0;
}

Notes

In principle, it should not matter if I send all of the data in a single write, or if I want to send each individual byte into its own write with a 100-millisecond delay each. This is supposedly a TCP stream socket, after all; such timing delays may matter on datagram-based sockets, but in principle not to stream sockets (where all that should matter is ordering of bytes).

curl --socks5-hostname and torify wget do not tickle this bug since they consistently send the entire request in a single write/sendto.

This suggests to me that some layer inside Tor is triggering the hostname lookup as soon as it receives ATYP and simply assumes that the entire hostname has been received, without actually verifying that the DST.ADDRESS after ATYP was actually received. The problem is that this layer inside Tor may be reading an uninitialized buffer with all the security problems that implies, and sending it out to a remote resolver. This was not easy to spot as well, since Tor logs will show hostnames it failed to look up as [Scrubbed], and I spent a lot of time looking for other bugs in my code before I thought to look into the behavior of Tor.

This might not be a high priority issue, since it seems most commodity libraries and tools send the entire request as a single write, it was only my own internal library that, for code simplicicty, wrote the hostname (stored in a different buffer) separately from the VER CMD RSV ATYP sequence. However, others reimplementing a simple SOCKS5 client might stumble into this as well, since typically in TCP it does not matter how many writes you use to send the data, as long as the order of bytes is correct.

Marked as confidential as I suspect Tor may be triggered into reading uninitialized buffers and possibly sending the contents of that buffer to a remote node.