From 4d89c7da973298d30ce22972abc4a895e096f80b Mon Sep 17 00:00:00 2001 From: arvidn Date: Sat, 18 Jul 2015 21:55:26 -0400 Subject: [PATCH] fixed uTP vulnerability --- ChangeLog | 1 + src/settings_pack.cpp | 2 +- src/utp_stream.cpp | 40 +++++++++++++++++++++++++--------------- 3 files changed, 27 insertions(+), 16 deletions(-) diff --git a/ChangeLog b/ChangeLog index 16b6fe9ae..dd1f96240 100644 --- a/ChangeLog +++ b/ChangeLog @@ -74,6 +74,7 @@ 1.0.6 release + * fixed uTP vulnerability * make utf8 conversions more lenient * fix loading of piece priorities from resume data * improved seed-mode handling (seed-mode will now automatically be left when diff --git a/src/settings_pack.cpp b/src/settings_pack.cpp index 5f8c1504f..cbc95f13c 100644 --- a/src/settings_pack.cpp +++ b/src/settings_pack.cpp @@ -308,7 +308,7 @@ namespace libtorrent SET(utp_min_timeout, 500, 0), SET(utp_syn_resends, 2, 0), SET(utp_fin_resends, 2, 0), - SET(utp_num_resends, 6, 0), + SET(utp_num_resends, 3, 0), SET(utp_connect_timeout, 3000, 0), SET(utp_delayed_ack, 0, 0), SET(utp_loss_multiplier, 50, 0), diff --git a/src/utp_stream.cpp b/src/utp_stream.cpp index 136ea9640..46d3e3748 100644 --- a/src/utp_stream.cpp +++ b/src/utp_stream.cpp @@ -280,6 +280,7 @@ struct utp_socket_impl , m_deferred_ack(false) , m_subscribe_drained(false) , m_stalled(false) + , m_confirmed(false) { m_sm->inc_stats_counter(counters::num_utp_idle); TORRENT_ASSERT(m_userdata); @@ -405,7 +406,7 @@ public: // refer to the unwritten portions of the buffers, and the // ones that fill up are erased from the vector std::vector m_read_buffer; - + // packets we've received without a read operation // active. Store them here until the client triggers // an async_read_some @@ -491,7 +492,7 @@ public: // the sum of all packets stored in m_receive_buffer boost::int32_t m_receive_buffer_size; - + // the sum of all buffers in m_read_buffer boost::int32_t m_read_buffer_size; @@ -675,6 +676,11 @@ public: // of sockets in the utp_socket_manager to be notified of // the socket being writable again bool m_stalled:1; + + // this is false by default and set to true once we've received a non-SYN + // packet for this connection with a correct ack_nr, confirming that the + // other end is not spoofing its source IP + bool m_confirmed:1; }; utp_socket_impl* construct_utp_impl(boost::uint16_t recv_id @@ -2736,11 +2742,14 @@ bool utp_socket_impl::incoming_packet(boost::uint8_t const* buf, int size if (m_state == UTP_STATE_SYN_SENT && ph->get_type() == ST_STATE) cmp_seq_nr = m_seq_nr; #endif - if (m_state != UTP_STATE_NONE - && compare_less_wrap(cmp_seq_nr, ph->ack_nr, ACK_MASK)) + if ((m_state != UTP_STATE_NONE || ph->get_type() != ST_SYN) + && (compare_less_wrap(cmp_seq_nr, ph->ack_nr, ACK_MASK) + || compare_less_wrap(ph->ack_nr, m_acked_seq_nr + - dup_ack_limit, ACK_MASK))) { - UTP_LOG("%8p: ERROR: incoming packet ack_nr:%d our seq_nr:%d (ignored)\n" - , this, int(ph->ack_nr), m_seq_nr); + UTP_LOG("%8p: ERROR: incoming packet ack_nr:%d our seq_nr:%d our " + "acked_seq_nr:%d (ignored)\n" + , this, int(ph->ack_nr), m_seq_nr, m_acked_seq_nr); m_sm->inc_stats_counter(counters::utp_redundant_pkts_in); return true; } @@ -2805,12 +2814,6 @@ bool utp_socket_impl::incoming_packet(boost::uint8_t const* buf, int size , this, int(ph->ack_nr), m_seq_nr); return true; } - if (compare_less_wrap(ph->ack_nr, m_acked_seq_nr , ACK_MASK)) - { - UTP_LOG("%8p: ERROR: invalid RESET packet, ack_nr:%d our acked_seq_nr:%d (ignored)\n" - , this, int(ph->ack_nr), m_acked_seq_nr); - return true; - } UTP_LOGV("%8p: incoming packet type:RESET\n", this); m_error = boost::asio::error::connection_reset; set_state(UTP_STATE_ERROR_WAIT); @@ -2927,7 +2930,7 @@ bool utp_socket_impl::incoming_packet(boost::uint8_t const* buf, int size ptr += len; extension = next_extension; } - + // the send operation in parse_sack() may have set the socket to an error // state, in which case we shouldn't continue if (m_state == UTP_STATE_ERROR_WAIT || m_state == UTP_STATE_DELETE) return true; @@ -3118,6 +3121,10 @@ bool utp_socket_impl::incoming_packet(boost::uint8_t const* buf, int size bool has_ack = ph->get_type() == ST_DATA || ph->get_type() == ST_FIN || ph->get_type() == ST_SYN; boost::uint32_t prev_out_packets = m_out_packets; + // the connection is connected and this packet made it past all the + // checks. We can now assume the other end is not spoofing it's IP. + if (ph->get_type() != ST_SYN) m_confirmed = true; + // try to send more data as long as we can // if send_pkt returns true while (send_pkt()); @@ -3504,7 +3511,10 @@ void utp_socket_impl::tick(time_point now) if (m_outbuf.size()) ++m_num_timeouts; - if (m_num_timeouts > m_sm->num_resends()) + // a socket that has not been confirmed to actually have a live remote end + // (the IP may have been spoofed) fail on the first timeout. If we had + // heard anything from this peer, it would have been confirmed. + if (m_num_timeouts > m_sm->num_resends() || !m_confirmed) { // the connection is dead m_error = boost::asio::error::timed_out; @@ -3541,7 +3551,7 @@ void utp_socket_impl::tick(time_point now) TORRENT_ASSERT(m_cwnd >= 0); m_timeout = now + milliseconds(packet_timeout()); - + UTP_LOGV("%8p: timeout resetting cwnd:%d\n" , this, int(m_cwnd >> 16));