Witold Filipczyk <[EMAIL PROTECTED]> writes:
> + 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->protocol = PROTOCOL_BITTORRENT;Changing uri->protocol like that is not safe, I think. get_uri() may have returned a cached URI to which there are other references. Because a malicious BitTorrent tracker can return arbitrary hostnames in the peer list, and this code will then change the protocol of the corresponding HTTP URIs to BitTorrent and affect future accesses to those hosts, I think this needs to be fixed. A pretty clear solution is to add a "bittorrent-peer" URI scheme for use in these connections, but other fixes are fine too. The following patch is also available from a Git repository: git://repo.or.cz/elinks/kon.git 0.12/bittorrent-peer-uri
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. --- commit d93bceb9bd6ab32c614ac20dc5c87e3af2a7f85f tree a32c808927944db6717398c8595626c958008579 parent 7de8b9940c96f51c55c2706ed9aa6ad11257d7ee author Kalle Olavi Niemitalo <[EMAIL PROTECTED]> Sun, 07 Sep 2008 06:10:52 +0300 committer Kalle Olavi Niemitalo <[EMAIL PROTECTED]> Sun, 07 Sep 2008 06:31:36 +0300 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 | 45 ++++++++++++++++++++------------- src/protocol/protocol.c | 1 + src/protocol/protocol.h | 1 + 7 files changed, 39 insertions(+), 18 deletions(-) diff --git a/src/network/state.c b/src/network/state.c index 3a94982..9ecd6f3 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 16faf41..f8358b2 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 f15bfe8..132283b 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 af6c89d..13aeca0 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 8db7211..7f4158c 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); - - make_connection(peer->socket, &uri, send_bittorrent_peer_handshake, 1); - - return BITTORRENT_STATE_OK; + 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); + result = 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 3330365..e43adc2 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 c02e671..d009bc0 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,
pgpk6hZot3CLz.pgp
Description: PGP signature
_______________________________________________ elinks-dev mailing list elinks-dev@linuxfromscratch.org http://linuxfromscratch.org/mailman/listinfo/elinks-dev
