From 1ac0d4a5b2c8f5e5c9a4629091c81e3f41a4c126 Mon Sep 17 00:00:00 2001 From: Jonathan G Rennison Date: Wed, 6 May 2020 23:23:03 +0100 Subject: Fix: Thread unsafe use of NetworkAddress::GetAddressAsString Remove static buffer form of NetworkAddress::GetAddressAsString. This is used in multiple threads concurrently, and is not thread-safe. Replace it with a form returning std::string. --- src/network/core/address.cpp | 13 +++++++------ src/network/core/address.h | 4 +++- src/network/core/tcp_content.cpp | 6 +++--- src/network/core/udp.cpp | 12 ++++++------ 4 files changed, 19 insertions(+), 16 deletions(-) (limited to 'src/network/core') diff --git a/src/network/core/address.cpp b/src/network/core/address.cpp index 44ba45385..c2fecc7ff 100644 --- a/src/network/core/address.cpp +++ b/src/network/core/address.cpp @@ -96,12 +96,11 @@ void NetworkAddress::GetAddressAsString(char *buffer, const char *last, bool wit * Get the address as a string, e.g. 127.0.0.1:12345. * @param with_family whether to add the family (e.g. IPvX). * @return the address - * @note NOT thread safe */ -const char *NetworkAddress::GetAddressAsString(bool with_family) +std::string NetworkAddress::GetAddressAsString(bool with_family) { /* 6 = for the : and 5 for the decimal port number */ - static char buf[NETWORK_HOSTNAME_LENGTH + 6 + 7]; + char buf[NETWORK_HOSTNAME_LENGTH + 6 + 7]; this->GetAddressAsString(buf, lastof(buf), with_family); return buf; } @@ -289,7 +288,8 @@ static SOCKET ConnectLoopProc(addrinfo *runp) { const char *type = NetworkAddress::SocketTypeAsString(runp->ai_socktype); const char *family = NetworkAddress::AddressFamilyAsString(runp->ai_family); - const char *address = NetworkAddress(runp->ai_addr, (int)runp->ai_addrlen).GetAddressAsString(); + char address[NETWORK_HOSTNAME_LENGTH + 6 + 7]; + NetworkAddress(runp->ai_addr, (int)runp->ai_addrlen).GetAddressAsString(address, lastof(address)); SOCKET sock = socket(runp->ai_family, runp->ai_socktype, runp->ai_protocol); if (sock == INVALID_SOCKET) { @@ -319,7 +319,7 @@ static SOCKET ConnectLoopProc(addrinfo *runp) */ SOCKET NetworkAddress::Connect() { - DEBUG(net, 1, "Connecting to %s", this->GetAddressAsString()); + DEBUG(net, 1, "Connecting to %s", this->GetAddressAsString().c_str()); return this->Resolve(AF_UNSPEC, SOCK_STREAM, AI_ADDRCONFIG, nullptr, ConnectLoopProc); } @@ -333,7 +333,8 @@ static SOCKET ListenLoopProc(addrinfo *runp) { const char *type = NetworkAddress::SocketTypeAsString(runp->ai_socktype); const char *family = NetworkAddress::AddressFamilyAsString(runp->ai_family); - const char *address = NetworkAddress(runp->ai_addr, (int)runp->ai_addrlen).GetAddressAsString(); + char address[NETWORK_HOSTNAME_LENGTH + 6 + 7]; + NetworkAddress(runp->ai_addr, (int)runp->ai_addrlen).GetAddressAsString(address, lastof(address)); SOCKET sock = socket(runp->ai_family, runp->ai_socktype, runp->ai_protocol); if (sock == INVALID_SOCKET) { diff --git a/src/network/core/address.h b/src/network/core/address.h index 980d7227d..65546818b 100644 --- a/src/network/core/address.h +++ b/src/network/core/address.h @@ -15,6 +15,8 @@ #include "../../string_func.h" #include "../../core/smallmap_type.hpp" +#include + class NetworkAddress; typedef std::vector NetworkAddressList; ///< Type for a list of addresses. typedef SmallMap SocketList; ///< Type for a mapping between address and socket. @@ -91,7 +93,7 @@ public: const char *GetHostname(); void GetAddressAsString(char *buffer, const char *last, bool with_family = true); - const char *GetAddressAsString(bool with_family = true); + std::string GetAddressAsString(bool with_family = true); const sockaddr_storage *GetAddress(); /** diff --git a/src/network/core/tcp_content.cpp b/src/network/core/tcp_content.cpp index 989fbefaf..6fb3b3379 100644 --- a/src/network/core/tcp_content.cpp +++ b/src/network/core/tcp_content.cpp @@ -171,9 +171,9 @@ bool NetworkContentSocketHandler::HandlePacket(Packet *p) default: if (this->HasClientQuit()) { - DEBUG(net, 0, "[tcp/content] received invalid packet type %d from %s", type, this->client_addr.GetAddressAsString()); + DEBUG(net, 0, "[tcp/content] received invalid packet type %d from %s", type, this->client_addr.GetAddressAsString().c_str()); } else { - DEBUG(net, 0, "[tcp/content] received illegal packet from %s", this->client_addr.GetAddressAsString()); + DEBUG(net, 0, "[tcp/content] received illegal packet from %s", this->client_addr.GetAddressAsString().c_str()); } return false; } @@ -224,7 +224,7 @@ bool NetworkContentSocketHandler::ReceivePackets() */ bool NetworkContentSocketHandler::ReceiveInvalidPacket(PacketContentType type) { - DEBUG(net, 0, "[tcp/content] received illegal packet type %d from %s", type, this->client_addr.GetAddressAsString()); + DEBUG(net, 0, "[tcp/content] received illegal packet type %d from %s", type, this->client_addr.GetAddressAsString().c_str()); return false; } diff --git a/src/network/core/udp.cpp b/src/network/core/udp.cpp index 123683873..57352412b 100644 --- a/src/network/core/udp.cpp +++ b/src/network/core/udp.cpp @@ -100,10 +100,10 @@ void NetworkUDPSocketHandler::SendPacket(Packet *p, NetworkAddress *recv, bool a /* Send the buffer */ int res = sendto(s.second, (const char*)p->buffer, p->size, 0, (const struct sockaddr *)send.GetAddress(), send.GetAddressLength()); - DEBUG(net, 7, "[udp] sendto(%s)", send.GetAddressAsString()); + DEBUG(net, 7, "[udp] sendto(%s)", send.GetAddressAsString().c_str()); /* Check for any errors, but ignore it otherwise */ - if (res == -1) DEBUG(net, 1, "[udp] sendto(%s) failed with: %i", send.GetAddressAsString(), GET_LAST_ERROR()); + if (res == -1) DEBUG(net, 1, "[udp] sendto(%s) failed with: %i", send.GetAddressAsString().c_str(), GET_LAST_ERROR()); if (!all) break; } @@ -136,7 +136,7 @@ void NetworkUDPSocketHandler::ReceivePackets() /* If the size does not match the packet must be corrupted. * Otherwise it will be marked as corrupted later on. */ if (nbytes != p.size) { - DEBUG(net, 1, "received a packet with mismatching size from %s", address.GetAddressAsString()); + DEBUG(net, 1, "received a packet with mismatching size from %s", address.GetAddressAsString().c_str()); continue; } @@ -313,9 +313,9 @@ void NetworkUDPSocketHandler::HandleUDPPacket(Packet *p, NetworkAddress *client_ default: if (this->HasClientQuit()) { - DEBUG(net, 0, "[udp] received invalid packet type %d from %s", type, client_addr->GetAddressAsString()); + DEBUG(net, 0, "[udp] received invalid packet type %d from %s", type, client_addr->GetAddressAsString().c_str()); } else { - DEBUG(net, 0, "[udp] received illegal packet from %s", client_addr->GetAddressAsString()); + DEBUG(net, 0, "[udp] received illegal packet from %s", client_addr->GetAddressAsString().c_str()); } break; } @@ -328,7 +328,7 @@ void NetworkUDPSocketHandler::HandleUDPPacket(Packet *p, NetworkAddress *client_ */ void NetworkUDPSocketHandler::ReceiveInvalidPacket(PacketUDPType type, NetworkAddress *client_addr) { - DEBUG(net, 0, "[udp] received packet type %d on wrong port from %s", type, client_addr->GetAddressAsString()); + DEBUG(net, 0, "[udp] received packet type %d on wrong port from %s", type, client_addr->GetAddressAsString().c_str()); } void NetworkUDPSocketHandler::Receive_CLIENT_FIND_SERVER(Packet *p, NetworkAddress *client_addr) { this->ReceiveInvalidPacket(PACKET_UDP_CLIENT_FIND_SERVER, client_addr); } -- cgit v1.2.3-54-g00ecf