Eliminate use-after-free in DHT Server (#120)

Currently when a connection triggers a failed_transaction it gets
deleted, which is the expected behavior. Unfortunately any packets
left in the DHT packet priority queue will now have been free'd
leading to a scenario where the process_queue is working on memory
which may contain unpredictable data.

The new behavior proposed by this patch is to also drop any queued
packets to prevent future processing since we're throwing out that
object for probelmatic behavior already.

Notably this seems to reflect the same behavior seen in issue #68
however I do not believe that to be a complete fix. It seems to
decrease the probability of the issue occurring. I personally
believe this fix should replace the one applied after #68 but am
certainly open to further discussion on the matter.

--------

READ of size 4 thread 0 (rtorrent main)

0. rak::socket_address_inet::address_n() const ../../rak/socket_address.h:167
1. torrent::DhtTransaction::key(rak::socket_address const*, int) (/usr/lib64/libtorrent.so.19+0x2d9936)
2. torrent::DhtTransaction::key(int) const (/usr/lib64/libtorrent.so.19+0x2d8d3f)
3. torrent::DhtServer::process_queue(std::deque<torrent::DhtTransactionPacket*, std::allocator<torrent::DhtTransactionPacket*> >&, unsigned int*) libtorrent/src/dht/dht_server.cc:801
4. torrent::DhtServer::event_write() libtorrent/src/dht/dht_server.cc:866
5. torrent::PollEPoll::perform() libtorrent/src/torrent/poll_epoll.cc:190
6. torrent::PollEPoll::do_poll(long, int) libtorrent/src/torrent/poll_epoll.cc:219
7. torrent::thread_base::event_loop(torrent::thread_base*) libtorrent/src/torrent/utils/thread_base.cc:174
8. main rtorrent/src/main.cc:867
9. __libc_start_main (/lib64/libc.so.6+0x20733)
10. _start (/usr/bin/rtorrent+0x9bce8)

freed by thread 0 (rtorrent main) here:

0. operator delete(void*) (/usr/lib/gcc/x86_64-pc-linux-gnu/4.9.3/libasan.so.1+0x62e47)
1. torrent::DhtTransactionPing::~DhtTransactionPing() libtorrent/src/dht/dht_transaction.h:359
2. torrent::DhtServer::failed_transaction(std::_Rb_tree_iterator<std::pair<unsigned long const, torrent::DhtTransaction*> >, bool) libtorrent/src/dht/dht_server.cc:672
3. torrent::DhtServer::receive_timeout() libtorrent/src/dht/dht_server.cc:899
4. std::tr1::_Mem_fn<void (torrent::DhtServer::*)()>::operator()(torrent::DhtServer*) const /usr/lib/gcc/x86_64-pc-linux-gnu/4.9.3/include/g++-v4/tr1/functional:585
5. std::tr1::result_of<std::tr1::_Mem_fn<void (torrent::DhtServer::*)()> (std::tr1::result_of<std::tr1::_Mu<torrent::DhtServer*, false, false> (torrent::DhtServer*,
std::tr1::tuple<>)>::type)>::type std::tr1::_Bind<std::tr1::_Mem_fn<void (torrent::DhtServer::*)()> (torrent::DhtServer*)>::__call<, 0>(std::tr1::tuple<> const&,
std::tr1::_Index_tuple<0>) /usr/lib/gcc/x86_64-pc-linux-gnu/4.9.3/include/g++-v4/tr1/functional:1178
6. std::tr1::result_of<std::tr1::_Mem_fn<void (torrent::DhtServer::*)()> (std::tr1::result_of<std::tr1::_Mu<torrent::DhtServer*, false, false> (torrent::DhtServer*,
std::tr1::tuple<>)>::type)>::type std::tr1::_Bind<std::tr1::_Mem_fn<void (torrent::DhtServer::*)()> (torrent::DhtServer*)>::operator()<>()
(/usr/lib64/libtorrent.so.19+0x2e1e5d)
7. std::tr1::_Function_handler<void (), std::tr1::_Bind<std::tr1::_Mem_fn<void (torrent::DhtServer::*)()> (torrent::DhtServer*)> >::_M_invoke(std::tr1::_Any_data const&) /usr/lib/gcc/x86_64-pc-linux-gnu/4.9.3/include/g++-v4/tr1/functional:1796
8. std::tr1::function<void ()>::operator()() const (/usr/bin/rtorrent+0xa7b7b)
9. torrent::thread_main::call_events() libtorrent/src/thread_main.cc:82
10. torrent::thread_base::event_loop(torrent::thread_base*) libtorrent/src/torrent/utils/thread_base.cc:141
11. main rtorrent/src/main.cc:867
12. __libc_start_main (/lib64/libc.so.6+0x20733)
13. _start (/usr/bin/rtorrent+0x9bce8)

src/dht/dht_server.cc

@@ -205,7 +205,7 @@ DhtServer::stop() {
 }
 
 void
-DhtServer::reset_statistics() { 
+DhtServer::reset_statistics() {
   m_queriesReceived = 0;
   m_queriesSent = 0;
   m_repliesReceived = 0;
@@ -531,6 +531,12 @@ DhtServer::add_packet(DhtTransactionPacket* packet, int priority) {
   }
 }
 
+void
+DhtServer::drop_packet(DhtTransactionPacket* packet) {
+    m_highQueue.erase(std::remove(m_highQueue.begin(), m_highQueue.end(), packet), m_highQueue.end());
+    m_lowQueue.erase(std::remove(m_lowQueue.begin(), m_lowQueue.end(), packet), m_lowQueue.end());
+}
+
 void
 DhtServer::create_query(transaction_itr itr, int tID, const rak::socket_address* sa, int priority) {
   if (itr->second->id() == m_router->id())
@@ -670,6 +676,7 @@ DhtServer::failed_transaction(transaction_itr itr, bool quick) {
 
     } catch (std::exception& e) {
       if (!quick) {
+        drop_packet(transaction->packet());
         delete itr->second;
         m_transactions.erase(itr);
       }
@@ -682,6 +689,7 @@ DhtServer::failed_transaction(transaction_itr itr, bool quick) {
     return ++itr;         // don't actually delete the transaction until the final timeout
 
   } else {
+    drop_packet(transaction->packet());
     delete itr->second;
     m_transactions.erase(itr++);
     return itr;

src/dht/dht_server.h

@@ -149,6 +149,7 @@ class DhtServer : public SocketDatagram {
   void                find_node_next(DhtTransactionSearch* t);
 
   void                add_packet(DhtTransactionPacket* packet, int priority);
+  void                drop_packet(DhtTransactionPacket* packet);
   void                create_query(transaction_itr itr, int tID, const rak::socket_address* sa, int priority);
   void                create_response(const DhtMessage& req, const rak::socket_address* sa, DhtMessage& reply);
   void                create_error(const DhtMessage& req, const rak::socket_address* sa, int num, const char* msg);