[tor-commits] [torsocks/master] Fix: socks5 resolve wasn't sending data correctly

[dgoulet at torproject.org]

commit 72039624bb190dfec3e758b7389410a987adc8a3
Author: David Goulet <dgoulet at ev0ke.net>
Date:   Wed May 27 13:20:03 2015 -0400

    Fix: socks5 resolve wasn't sending data correctly
    
    The resolve function was sending uninitialized data to tor and sometimes
    too much data than needed. Furthermore, a valid SOCKS5 request for that
    needs a port so add one in both resolve and resolve_ptr.
    
    Reported-by: Yawning Angel <yawning at schwanenlied.me>
    Signed-off-by: David Goulet <dgoulet at ev0ke.net>
---
 src/common/socks5.c |   43 +++++++++++++++++++++++++++++++------------
 src/common/socks5.h |    2 ++
 2 files changed, 33 insertions(+), 12 deletions(-)

diff --git a/src/common/socks5.c b/src/common/socks5.c
index 3c9544a..3f284ca 100644
--- a/src/common/socks5.c
+++ b/src/common/socks5.c
@@ -564,20 +564,19 @@ ATTR_HIDDEN
 int socks5_send_resolve_request(const char *hostname, struct connection *conn)
 {
 	int ret, ret_send;
-	/*
-	 * Can't go bigger than that. 4 bytes for the header, 1 for the name len
-	 * and 255 for the name.
-	 */
-	unsigned char buffer[260];
 	size_t name_len, msg_len, data_len;
 	struct socks5_request msg;
 	struct socks5_request_resolve req;
+	/* Can't go bigger than that. 4 bytes for the header, 1 for the name len,
+	 * 255 for the name and 2 bytes for the port. */
+	unsigned char buffer[sizeof(msg) + sizeof(req)];
 
 	assert(hostname);
 	assert(conn);
 	assert(conn->fd >= 0);
 
 	memset(buffer, 0, sizeof(buffer));
+	memset(&req, 0, sizeof(req));
 	msg_len = sizeof(msg);
 
 	msg.ver = SOCKS5_VERSION;
@@ -596,11 +595,21 @@ int socks5_send_resolve_request(const char *hostname, struct connection *conn)
 	/* Setup resolve request. */
 	req.len = name_len;
 	memcpy(req.name, hostname, name_len);
+	/* Dummy port, tor doesn't need it. */
+	req.port = htons(42);
 
 	/* Copy final buffer. */
 	memcpy(buffer, &msg, msg_len);
-	memcpy(buffer + msg_len, &req, sizeof(req));
-	data_len = msg_len + sizeof(req);
+	data_len = msg_len;
+	/* Add the length of hostname. */
+	memcpy(buffer + data_len, &req.len, sizeof(req.len));
+	data_len += sizeof(req.len);
+	/* Add hostname without NULL terminated byte. */
+	memcpy(buffer + data_len, req.name, req.len);
+	data_len += req.len;
+	/* Add the dummy port at the end. */
+	memcpy(buffer + data_len, &req.port, sizeof(req.port));
+	data_len += sizeof(req.port);
 
 	ret_send = send_data(conn->fd, &buffer, data_len);
 	if (ret_send < 0) {
@@ -703,10 +712,11 @@ ATTR_HIDDEN
 int socks5_send_resolve_ptr_request(struct connection *conn, const void *ip, int af)
 {
 	int ret, ret_send;
-	unsigned char buffer[20];	/* Can't go higher than that (with IPv6). */
 	size_t msg_len, data_len;
 	struct socks5_request msg;
 	struct socks5_request_resolve_ptr req;
+	/* Can't go higher than that (with IPv6). */
+	unsigned char buffer[sizeof(msg) + sizeof(req)];
 
 	assert(conn);
 	assert(conn->fd >= 0);
@@ -724,11 +734,17 @@ int socks5_send_resolve_ptr_request(struct connection *conn, const void *ip, int
 	switch (af) {
 	case AF_INET:
 		msg.atyp = SOCKS5_ATYP_IPV4;
-		memcpy(req.addr.ipv4, ip, 4);
+		memcpy(req.addr.ipv4, ip, sizeof(req.addr.ipv4));
+		/* Copy right away the IP since we know the family type. */
+		memcpy(buffer + msg_len, &req.addr, sizeof(req.addr.ipv4));
+		data_len = msg_len + sizeof(req.addr.ipv4);
 		break;
 	case AF_INET6:
 		msg.atyp = SOCKS5_ATYP_IPV6;
-		memcpy(req.addr.ipv6, ip, 16);
+		memcpy(req.addr.ipv6, ip, sizeof(req.addr.ipv6));
+		/* Copy right away the IP since we know the family type. */
+		memcpy(buffer + msg_len, &req.addr, sizeof(req.addr.ipv6));
+		data_len = msg_len + sizeof(req.addr.ipv6);
 		break;
 	default:
 		ERR("Unknown address domain of %d", ip);
@@ -736,10 +752,13 @@ int socks5_send_resolve_ptr_request(struct connection *conn, const void *ip, int
 		goto error;
 	}
 
+	/* Dummy port, tor doesn't need it. */
+	req.port = htons(42);
+
 	/* Copy final buffer. */
 	memcpy(buffer, &msg, msg_len);
-	memcpy(buffer + msg_len, &req, sizeof(req));
-	data_len = msg_len + sizeof(req);
+	memcpy(buffer + data_len, &req.port, sizeof(req.port));
+	data_len += sizeof(req.port);
 
 	ret_send = send_data(conn->fd, &buffer, data_len);
 	if (ret_send < 0) {
diff --git a/src/common/socks5.h b/src/common/socks5.h
index 35c7d2b..41da679 100644
--- a/src/common/socks5.h
+++ b/src/common/socks5.h
@@ -108,6 +108,7 @@ struct socks5_request_domain {
 struct socks5_request_resolve {
 	uint8_t len;
 	unsigned char name[UINT8_MAX];
+	uint16_t port;
 };
 
 /* Use for the Tor resolve ptr command. */
@@ -116,6 +117,7 @@ struct socks5_request_resolve_ptr {
 		uint8_t ipv4[4];
 		uint8_t ipv6[16];
 	} addr;
+	uint16_t port;
 };
 
 /* Non variable part of a reply. */