Re: Errors in bittorent

Witold Filipczyk <[email protected]> Sat, 30 Aug 2008 11:21:27 +0200
Newsgroups gmane.comp.web.links
Message-ID <20080830092127.GA19395@pldmachine>
On Fri, Aug 29, 2008 at 10:38:20PM +0200, Witold Filipczyk wrote:
> Hi,
> I noticed a bug in the bittorent protocol code while trying to get an ISO from
> http://torrents.gentoo.org/.
> 
> Here is a fix for it.
> 1) Before the uri was put on the stack and the access that uri later
> may trash the stack.
> 2) done_uri expects that uri->string is not NULL, so uri->string points to "".
> 

That patch wasn't good. I attached the second one.

Witek

_______________________________________________
elinks-dev mailing list
[email protected]
http://linuxfromscratch.org/mailman/listinfo/elinks-dev
elinks-bittorrent.patch (text/plain, 2.8 KB)
commit b589f19b73c65621c0a8582199509f58dbcac09f
Author:     Witold Filipczyk <[email protected]>
AuthorDate: Sat Aug 30 10:52:00 2008 +0200
Commit:     Witold Filipczyk <nobody@nowhere>
CommitDate: Sat Aug 30 10:52:00 2008 +0200

    An issue with bittorrent.
    
    It was possible that the reference to the automatic variable
    uri of the make_bittorrent_peer_connection trashed the stack.
    In addition done_uri crashed because uri->string was NULL.
    
    Now uri is allocated and unlocked to avoid memleak.

diff --git a/src/protocol/bittorrent/peerconnect.c b/src/protocol/bittorrent/peerconnect.c
index aeafbf3..5f65e68 100644
--- a/src/protocol/bittorrent/peerconnect.c
+++ b/src/protocol/bittorrent/peerconnect.c
@@ -20,6 +20,7 @@
 #include "elinks.h"
 
 #include "config/options.h"
+#include "main/object.h"
 #include "main/select.h"
 #include "main/timer.h"
 #include "network/connection.h"
@@ -271,9 +272,12 @@ enum bittorrent_state
 make_bittorrent_peer_connection(struct bittorrent_connection *bittorrent,
 				struct bittorrent_peer *peer_info)
 {
-	struct uri uri;
+	struct string uri_string;
+	struct uri *uri;
 	struct bittorrent_peer_connection *peer;
-	unsigned char port[5];
+	unsigned char port[6];
+	int ip_start, port_start;
+	int port_length;
 
 	peer = init_bittorrent_peer_connection(-1);
 	if (!peer) return BITTORRENT_STATE_OUT_OF_MEM;
@@ -296,14 +300,32 @@ make_bittorrent_peer_connection(struct bittorrent_connection *bittorrent,
 	/* FIXME: Rather change the make_connection() interface. This is an ugly
 	 * hack. */
 	/* FIXME: Set the ipv6 flag iff ... */
-	memset(&uri, 0, sizeof(uri));
-	uri.protocol = PROTOCOL_BITTORRENT;
-	uri.host     = peer_info->ip;
-	uri.hostlen  = strlen(peer_info->ip);
-	uri.port     = port;
-	uri.portlen  = snprintf(port, sizeof(port), "%u", peer_info->port);
-
-	make_connection(peer->socket, &uri, send_bittorrent_peer_handshake, 1);
+	if (!init_string(&uri_string)) {
+		done_bittorrent_peer_connection(peer);
+		return BITTORRENT_STATE_OUT_OF_MEM;
+	}
+	add_to_string(&uri_string, "bittorrent:");
+	ip_start = uri_string.length;
+	add_to_string(&uri_string, peer_info->ip);
+	add_char_to_string(&uri_string, ':');
+	port_start = uri_string.length;
+
+	port_length = snprintf(port, sizeof(port), "%u", peer_info->port);
+	add_bytes_to_string(&uri_string, port, port_length);
+	uri = get_uri(uri_string.source, URI_BASE);
+	done_string(&uri_string);
+	if (!uri) {
+		done_bittorrent_peer_connection(peer);
+		return BITTORRENT_STATE_OUT_OF_MEM;
+	}
+
+	uri->host = uri->string + ip_start;
+	uri->hostlen = port_start - ip_start - 1;
+	uri->port = uri->string + port_start;
+	uri->portlen = port_length;
+	/* Do not lock it. */
+	object_unlock(uri);
+	make_connection(peer->socket, uri, send_bittorrent_peer_handshake, 1);
 
 	return BITTORRENT_STATE_OK;
 }