From d93bceb9bd6ab32c614ac20dc5c87e3af2a7f85f Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Sun, 7 Sep 2008 06:10:52 +0300 Subject: [PATCH] Fix blacklist crash in BitTorrent make_bittorrent_peer_connection() used to construct a struct uri on the stack. This was hacky but worked nicely because the struct uri was not really accessed after make_connection() returned. However, since commit a83ff1f565a4a7bc25a4b8353ee26bc1b97410e3, the struct uri is also needed when the connection is being closed. Valgrind shows: Invalid read of size 2 at 0x8100764: get_blacklist_entry (blacklist.c:33) by 0x8100985: del_blacklist_entry (blacklist.c:64) by 0x80DA579: complete_connect_socket (socket.c:448) by 0x80DA84A: connected (socket.c:513) by 0x80D0DDF: select_loop (select.c:297) by 0x80D00C6: main (main.c:353) Address 0xBEC3BFAE is just below the stack ptr. To suppress, use: --workaround-gcc296-bugs=yes To fix this, allocate the struct uri on the heap instead, by constructing a string and giving that to get_uri(). This string cannot use the "bittorrent" URI scheme because parse_uri() does not recognize the host and port fields in that. (The "bittorrent" scheme has protocol_backend.free_syntax = 1 in order to support strings like "bittorrent:http://beta.legaltorrents.com/get/159-noisome-beasts".) Instead, define a new "bittorrent-peer" URI scheme for this purpose. If the user attempts to use this URI scheme, its handler aborts the connection with an error; but when make_bittorrent_peer_connection() uses a bittorrent-peer URI, the handler is not called. This change also lets get_uri() set the ipv6 flag if peer_info->ip is an IPv6 address literal. Reported by Witold Filipczyk. --- src/network/state.c | 1 + src/network/state.h | 1 + src/protocol/bittorrent/connection.c | 6 ++++ src/protocol/bittorrent/connection.h | 2 ++ src/protocol/bittorrent/peerconnect.c | 41 ++++++++++++++++----------- src/protocol/protocol.c | 1 + src/protocol/protocol.h | 1 + 7 files changed, 37 insertions(+), 16 deletions(-) diff --git a/src/network/state.c b/src/network/state.c index 3a949829b..9ecd6f31e 100644 --- a/src/network/state.c +++ b/src/network/state.c @@ -124,6 +124,7 @@ static const struct s_msg_dsc msg_dsc[] = { {S_BITTORRENT_METAINFO, N_("The BitTorrent metainfo file contained errors")}, {S_BITTORRENT_TRACKER, N_("The tracker requesting failed")}, {S_BITTORRENT_BAD_URL, N_("The BitTorrent URL does not point to a valid URL")}, + {S_BITTORRENT_PEER_URL, N_("The bittorrent-peer URL scheme is for internal use only")}, #endif /* fsp_open_session() failed but did not set errno. diff --git a/src/network/state.h b/src/network/state.h index 16faf418e..f8358b25d 100644 --- a/src/network/state.h +++ b/src/network/state.h @@ -103,6 +103,7 @@ enum connection_basic_state { S_BITTORRENT_METAINFO = -100801, S_BITTORRENT_TRACKER = -100802, S_BITTORRENT_BAD_URL = -100803, + S_BITTORRENT_PEER_URL = -100804, S_FSP_OPEN_SESSION_UNKN = -100900, }; diff --git a/src/protocol/bittorrent/connection.c b/src/protocol/bittorrent/connection.c index f15bfe84d..132283bd2 100644 --- a/src/protocol/bittorrent/connection.c +++ b/src/protocol/bittorrent/connection.c @@ -414,3 +414,9 @@ bittorrent_protocol_handler(struct connection *conn) bittorrent_metainfo_callback, conn, 0); done_uri(uri); } + +void +bittorrent_peer_protocol_handler(struct connection *conn) +{ + abort_connection(conn, connection_state(S_BITTORRENT_PEER_URL)); +} diff --git a/src/protocol/bittorrent/connection.h b/src/protocol/bittorrent/connection.h index af6c89d08..13aeca031 100644 --- a/src/protocol/bittorrent/connection.h +++ b/src/protocol/bittorrent/connection.h @@ -9,8 +9,10 @@ struct connection; #ifdef CONFIG_BITTORRENT extern protocol_handler_T bittorrent_protocol_handler; +extern protocol_handler_T bittorrent_peer_protocol_handler; #else #define bittorrent_protocol_handler NULL +#define bittorrent_peer_protocol_handler NULL #endif void update_bittorrent_connection_state(struct connection *conn); diff --git a/src/protocol/bittorrent/peerconnect.c b/src/protocol/bittorrent/peerconnect.c index 8db721116..7f4158c1d 100644 --- a/src/protocol/bittorrent/peerconnect.c +++ b/src/protocol/bittorrent/peerconnect.c @@ -271,12 +271,13 @@ enum bittorrent_state make_bittorrent_peer_connection(struct bittorrent_connection *bittorrent, struct bittorrent_peer *peer_info) { - struct uri uri; + enum bittorrent_state result = BITTORRENT_STATE_OUT_OF_MEM; + struct uri *uri = NULL; + struct string uri_string = NULL_STRING; struct bittorrent_peer_connection *peer; - unsigned char port[5]; peer = init_bittorrent_peer_connection(-1); - if (!peer) return BITTORRENT_STATE_OUT_OF_MEM; + if (!peer) goto out; peer->local.initiater = 1; @@ -284,10 +285,7 @@ make_bittorrent_peer_connection(struct bittorrent_connection *bittorrent, peer->bittorrent = bittorrent; peer->bitfield = init_bitfield(bittorrent->meta.pieces); - if (!peer->bitfield) { - done_bittorrent_peer_connection(peer); - return BITTORRENT_STATE_OUT_OF_MEM; - } + if (!peer->bitfield) goto out; memcpy(peer->id, peer_info->id, sizeof(peer->id)); @@ -295,17 +293,28 @@ make_bittorrent_peer_connection(struct bittorrent_connection *bittorrent, * can extract the IP address and port number. */ /* 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); + if (!init_string(&uri_string)) goto out; + if (!add_format_to_string(&uri_string, +#ifdef CONFIG_IPV6 + strchr(peer_info->ip, ':') ? + "bittorrent-peer://[%s]:%u/" : +#endif + "bittorrent-peer://%s:%u/", + peer_info->ip, (unsigned) peer_info->port)) + goto out; + uri = get_uri(uri_string.source, 0); + if (!uri) goto out; - make_connection(peer->socket, &uri, send_bittorrent_peer_handshake, 1); + make_connection(peer->socket, uri, send_bittorrent_peer_handshake, 1); + result = BITTORRENT_STATE_OK; - return BITTORRENT_STATE_OK; +out: + if (uri) + done_uri(uri); + done_string(&uri_string); + if (peer && result != BITTORRENT_STATE_OK) + done_bittorrent_peer_connection(peer); + return result; } diff --git a/src/protocol/protocol.c b/src/protocol/protocol.c index 333036566..e43adc2ee 100644 --- a/src/protocol/protocol.c +++ b/src/protocol/protocol.c @@ -58,6 +58,7 @@ struct protocol_backend { static const struct protocol_backend protocol_backends[] = { { "about", 0, about_protocol_handler, 0, 0, 1, 0, 1 }, { "bittorrent", 0, bittorrent_protocol_handler, 0, 0, 1, 0, 1 }, + { "bittorrent-peer",0,bittorrent_peer_protocol_handler, 1, 1, 0, 0, 1 }, { "data", 0, data_protocol_handler, 0, 0, 1, 0, 1 }, { "file", 0, file_protocol_handler, 1, 0, 0, 0, 0 }, { "finger", 79, finger_protocol_handler, 1, 1, 0, 0, 1 }, diff --git a/src/protocol/protocol.h b/src/protocol/protocol.h index c02e67166..d009bc0c1 100644 --- a/src/protocol/protocol.h +++ b/src/protocol/protocol.h @@ -11,6 +11,7 @@ struct uri; enum protocol { PROTOCOL_ABOUT, PROTOCOL_BITTORRENT, + PROTOCOL_BITTORRENT_PEER, PROTOCOL_DATA, PROTOCOL_FILE, PROTOCOL_FINGER,