From a27cf7b6020014ce9c3f338606e8c364a104bce9 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sun, 3 Jan 2021 01:35:46 +0000 Subject: [PATCH] fix accessing an network address in a deleted context Both drop_connection and add_host_fail can drop the connection, which invalidates the context, and thus the address it contains. Thanks to wfaressuissia[m] for lots of help and prodding when debugging this --- .../cryptonote_protocol_handler.inl | 9 ++++++--- src/p2p/net_node.inl | 15 +++++++++------ 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/cryptonote_protocol/cryptonote_protocol_handler.inl b/src/cryptonote_protocol/cryptonote_protocol_handler.inl index eb9aa3f7f..0dd290e8e 100644 --- a/src/cryptonote_protocol/cryptonote_protocol_handler.inl +++ b/src/cryptonote_protocol/cryptonote_protocol_handler.inl @@ -2835,12 +2835,15 @@ skip: epee::string_tools::to_string_hex(context.m_pruning_seed) << "), score " << score << ", flush_all_spans " << flush_all_spans); - if (score > 0) - m_p2p->add_host_fail(context.m_remote_address, score); - m_block_queue.flush_spans(context.m_connection_id, flush_all_spans); + // copy since dropping the connection will invalidate the context, and thus the address + const auto remote_address = context.m_remote_address; + m_p2p->drop_connection(context); + + if (score > 0) + m_p2p->add_host_fail(remote_address, score); } //------------------------------------------------------------------------------------------------------------------------ template diff --git a/src/p2p/net_node.inl b/src/p2p/net_node.inl index b4ded70dc..ee61aec46 100644 --- a/src/p2p/net_node.inl +++ b/src/p2p/net_node.inl @@ -1181,8 +1181,9 @@ namespace nodetool if(!handle_remote_peerlist(rsp.local_peerlist_new, context)) { LOG_WARNING_CC(context, "COMMAND_TIMED_SYNC: failed to handle_remote_peerlist(...), closing connection."); + const auto remote_address = context.m_remote_address; m_network_zones.at(context.m_remote_address.get_zone()).m_net_server.get_config_object().close(context.m_connection_id ); - add_host_fail(context.m_remote_address); + add_host_fail(remote_address); } if(!context.m_is_income) m_network_zones.at(context.m_remote_address.get_zone()).m_peerlist.set_peer_just_seen(context.peer_id, context.m_remote_address, context.m_pruning_seed, context.m_rpc_port, context.m_rpc_credits_per_hash); @@ -1349,7 +1350,7 @@ namespace nodetool if(just_take_peerlist) { zone.m_net_server.get_config_object().close(con->m_connection_id); - LOG_DEBUG_CC(*con, "CONNECTION HANDSHAKED OK AND CLOSED."); + MDEBUG(na.str() << "CONNECTION HANDSHAKED OK AND CLOSED."); return true; } @@ -1411,7 +1412,7 @@ namespace nodetool zone.m_net_server.get_config_object().close(con->m_connection_id); - LOG_DEBUG_CC(*con, "CONNECTION HANDSHAKED OK AND CLOSED."); + MDEBUG(na.str() << "CONNECTION HANDSHAKED OK AND CLOSED."); return true; } @@ -2434,12 +2435,14 @@ namespace nodetool template int node_server::handle_handshake(int command, typename COMMAND_HANDSHAKE::request& arg, typename COMMAND_HANDSHAKE::response& rsp, p2p_connection_context& context) { + // copy since dropping the connection will invalidate the context, and thus the address + const auto remote_address = context.m_remote_address; + if(arg.node_data.network_id != m_network_id) { - LOG_INFO_CC(context, "WRONG NETWORK AGENT CONNECTED! id=" << arg.node_data.network_id); drop_connection(context); - add_host_fail(context.m_remote_address); + add_host_fail(remote_address); return 1; } @@ -2447,7 +2450,7 @@ namespace nodetool { LOG_WARNING_CC(context, "COMMAND_HANDSHAKE came not from incoming connection"); drop_connection(context); - add_host_fail(context.m_remote_address); + add_host_fail(remote_address); return 1; }