premiere
/
premiere-libtorrent
2
2
Fork 1
Code Issues Pull Requests Releases Wiki Activity

avoid comparing unrelated pointer in the DHT routing table (which is UB)

This commit is contained in:
arvidn 2019-08-31 01:55:53 +02:00 committed by Arvid Norberg
parent d5b56ca187
commit c0c4c2083a
2 changed files with 22 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
include/libtorrent/kademlia/routing_table.hpp
View File

@ -204,8 +204,7 @@ public:
// are nearest to the given id. // are nearest to the given id.
void find_node(node_id const& id, std::vector<node_entry>& l void find_node(node_id const& id, std::vector<node_entry>& l
, int options, int count = 0); , int options, int count = 0);
void remove_node(node_entry* n void remove_node(node_entry* n, bucket_t* b);
, table_t::iterator bucket) ;
int bucket_size(int bucket) const int bucket_size(int bucket) const
{ {
@ -277,8 +276,8 @@ private:
// return a pointer the node_entry with the given endpoint // return a pointer the node_entry with the given endpoint
// or 0 if we don't have such a node. Both the address and the // or 0 if we don't have such a node. Both the address and the
// port has to match // port has to match
node_entry* find_node(udp::endpoint const& ep std::tuple<node_entry*, routing_table::table_t::iterator, bucket_t*>
, routing_table::table_t::iterator* bucket); find_node(udp::endpoint const& ep);
// if the bucket is not full, try to fill it with nodes from the // if the bucket is not full, try to fill it with nodes from the
// replacement list // replacement list

46
src/kademlia/routing_table.cpp
View File

@ -479,8 +479,8 @@ bool compare_ip_cidr(address const& lhs, address const& rhs)
} }
} }
node_entry* routing_table::find_node(udp::endpoint const& ep std::tuple<node_entry*, routing_table::table_t::iterator, bucket_t*>
, routing_table::table_t::iterator* bucket) routing_table::find_node(udp::endpoint const& ep)
{ {
for (auto i = m_buckets.begin() , end(m_buckets.end()); i != end; ++i) for (auto i = m_buckets.begin() , end(m_buckets.end()); i != end; ++i)
{ {
@ -488,19 +488,17 @@ node_entry* routing_table::find_node(udp::endpoint const& ep
{ {
if (j->addr() != ep.address()) continue; if (j->addr() != ep.address()) continue;
if (j->port() != ep.port()) continue; if (j->port() != ep.port()) continue;
*bucket = i; return std::make_tuple(&*j, i, &i->replacements);
return &*j;
} }
for (auto j = i->live_nodes.begin(); j != i->live_nodes.end(); ++j) for (auto j = i->live_nodes.begin(); j != i->live_nodes.end(); ++j)
{ {
if (j->addr() != ep.address()) continue; if (j->addr() != ep.address()) continue;
if (j->port() != ep.port()) continue; if (j->port() != ep.port()) continue;
*bucket = i; return std::make_tuple(&*j, i, &i->live_nodes);
return &*j;
} }
} }
*bucket = m_buckets.end(); return std::tuple<node_entry*, routing_table::table_t::iterator, bucket_t*>(
return nullptr; nullptr, m_buckets.end(), nullptr);
} }
// TODO: this need to take bucket "prefix" into account. It should be unified // TODO: this need to take bucket "prefix" into account. It should be unified
@ -535,26 +533,14 @@ void routing_table::prune_empty_bucket()
} }
} }
void routing_table::remove_node_internal(node_entry* n, bucket_t& b) void routing_table::remove_node(node_entry* n, bucket_t* b)
{ {
if (!b.empty() std::ptrdiff_t const idx = n - b->data();
&& n >= &b[0] TORRENT_ASSERT(idx >= 0);
&& n < &b[0] + b.size()) TORRENT_ASSERT(idx < intptr_t(b->size()));
{
std::ptrdiff_t const idx = n - &b[0];
TORRENT_ASSERT(m_ips.exists(n->addr())); TORRENT_ASSERT(m_ips.exists(n->addr()));
m_ips.erase(n->addr()); m_ips.erase(n->addr());
b.erase(b.begin() + idx); b->erase(b->begin() + idx);
}
}
void routing_table::remove_node(node_entry* n
, routing_table::table_t::iterator bucket)
{
INVARIANT_CHECK;
remove_node_internal(n, bucket->replacements);
remove_node_internal(n, bucket->live_nodes);
} }
bool routing_table::add_node(node_entry const& e) bool routing_table::add_node(node_entry const& e)
@ -645,8 +631,10 @@ routing_table::add_node_status_t routing_table::add_node_impl(node_entry e)
// a response with a correct transaction ID, i.e. it is verified to not // a response with a correct transaction ID, i.e. it is verified to not
// be the result of a poisoned routing table // be the result of a poisoned routing table
table_t::iterator existing_bucket; node_entry * existing;
node_entry* existing = find_node(e.ep(), &existing_bucket); routing_table::table_t::iterator existing_bucket;
bucket_t* bucket;
std::tie(existing, existing_bucket, bucket) = find_node(e.ep());
if (existing == nullptr) if (existing == nullptr)
{ {
// the node we're trying to add is not a match with an existing node. we // the node we're trying to add is not a match with an existing node. we
@ -686,7 +674,7 @@ routing_table::add_node_status_t routing_table::add_node_impl(node_entry e)
{ {
// this node's ID was unknown. remove the old entry and // this node's ID was unknown. remove the old entry and
// replace it with the node's real ID // replace it with the node's real ID
remove_node(existing, existing_bucket); remove_node(existing, bucket);
} }
else if (!e.pinged()) else if (!e.pinged())
{ {
@ -709,7 +697,7 @@ routing_table::add_node_status_t routing_table::add_node_impl(node_entry e)
} }
#endif #endif
remove_node(existing, existing_bucket); remove_node(existing, bucket);
fill_from_replacements(existing_bucket); fill_from_replacements(existing_bucket);
// when we detect possible malicious activity in a bucket, // when we detect possible malicious activity in a bucket,