Mirror/TrinityCore
1
0
Fork 0
mirror of https://github.com/TrinityCore/TrinityCore.git synced 2026-01-15 23:20:36 +01:00
Code Issues Packages Projects Releases Wiki Activity

Core/Networking: Fixed unsafe access to _worldSession member in WorldSocket

Browse Source
This commit is contained in:
Shauren
2015-03-28 21:45:27 +01:00
parent 801fc634b5
commit bed88e0dd4
4 changed files with 103 additions and 50 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
src/server/game/Server/WorldSession.cpp
View File

@@ -283,6 +283,7 @@ void WorldSession::SendPacket(WorldPacket const* packet, bool forced /*= false*/
sScriptMgr->OnPacketSend(this, *packet);
TC_LOG_TRACE("network.opcode", "S->C: %s %s", GetPlayerInfo().c_str(), GetOpcodeNameForLogging(static_cast<OpcodeServer>(packet->GetOpcode())).c_str());
m_Socket[conIdx]->SendPacket(*packet);
}

138
src/server/game/Server/WorldSocket.cpp
View File

@@ -57,7 +57,7 @@ uint32 const SizeOfServerHeader[2] = { sizeof(uint16) + sizeof(uint32), sizeof(u
WorldSocket::WorldSocket(tcp::socket&& socket) : Socket(std::move(socket)),
_type(CONNECTION_TYPE_REALM), _authSeed(rand32()), _OverSpeedPings(0),
_worldSession(nullptr), _compressionStream(nullptr), _initialized(false)
_worldSession(nullptr), _authed(false), _compressionStream(nullptr), _initialized(false)
{
_headerBuffer.Resize(SizeOfClientHeader[0][0]);
}
@@ -96,7 +96,15 @@ void WorldSocket::HandleSendAuthSession()
memcpy(&challenge.DosChallenge[4], _decryptSeed.AsByteArray(16).get(), 16);
challenge.DosZeroBits = 1;
SendPacket(*challenge.Write());
SendPacketAndLogOpcode(*challenge.Write());
}
void WorldSocket::OnClose()
{
{
std::lock_guard<std::mutex> sessionGuard(_worldSessionLock);
_worldSession = nullptr;
}
}
void WorldSocket::ReadHandler()
@@ -182,15 +190,8 @@ bool WorldSocket::ReadHeaderHandler()
if (!ClientPktHeader::IsValidSize(size) || (_initialized && !ClientPktHeader::IsValidOpcode(opcode)))
{
if (_worldSession)
{
Player* player = _worldSession->GetPlayer();
TC_LOG_ERROR("network", "WorldSocket::ReadHeaderHandler(): client (account: %u, char [%s, name: %s]) sent malformed packet (size: %u, cmd: %u)",
_worldSession->GetAccountId(), player ? player->GetGUID().ToString().c_str() : "GUID: Empty", player ? player->GetName().c_str() : "<none>", size, opcode);
}
else
TC_LOG_ERROR("network", "WorldSocket::ReadHeaderHandler(): client %s sent malformed packet (size: %u, cmd: %u)",
GetRemoteIpAddress().to_string().c_str(), size, opcode);
TC_LOG_ERROR("network", "WorldSocket::ReadHeaderHandler(): client %s sent malformed packet (size: %u, cmd: %u)",
GetRemoteIpAddress().to_string().c_str(), size, opcode);
CloseSocket();
return false;
@@ -217,19 +218,29 @@ bool WorldSocket::ReadDataHandler()
if (sPacketLog->CanLogPacket())
sPacketLog->LogPacket(packet, CLIENT_TO_SERVER, GetRemoteIpAddress(), GetRemotePort(), GetConnectionType());
TC_LOG_TRACE("network.opcode", "C->S: %s %s", (_worldSession ? _worldSession->GetPlayerInfo() : GetRemoteIpAddress().to_string()).c_str(), GetOpcodeNameForLogging(opcode).c_str());
std::unique_lock<std::mutex> sessionGuard(_worldSessionLock, std::defer_lock);
switch (opcode)
{
case CMSG_PING:
LogOpcodeText(opcode, sessionGuard);
HandlePing(packet);
break;
case CMSG_AUTH_SESSION:
{
if (_worldSession)
LogOpcodeText(opcode, sessionGuard);
if (_authed)
{
TC_LOG_ERROR("network", "WorldSocket::ProcessIncoming: received duplicate CMSG_AUTH_SESSION from %s", _worldSession->GetPlayerInfo().c_str());
break;
// locking just to safely log offending user is probably overkill but we are disconnecting him anyway
{
if (sessionGuard.try_lock())
{
TC_LOG_ERROR("network", "WorldSocket::ProcessIncoming: received duplicate CMSG_AUTH_SESSION from %s", _worldSession->GetPlayerInfo().c_str());
sessionGuard.unlock(); // unlock session guard to prevent deadlocking in CloseSocket
}
}
CloseSocket();
return;
}
WorldPackets::Auth::AuthSession authSession(std::move(packet));
@@ -239,10 +250,19 @@ bool WorldSocket::ReadDataHandler()
}
case CMSG_AUTH_CONTINUED_SESSION:
{
if (_worldSession)
LogOpcodeText(opcode, sessionGuard);
if (_authed)
{
TC_LOG_ERROR("network", "WorldSocket::ProcessIncoming: received duplicate CMSG_AUTH_CONTINUED_SESSION from %s", _worldSession->GetPlayerInfo().c_str());
break;
// locking just to safely log offending user is probably overkill but we are disconnecting him anyway
{
if (sessionGuard.try_lock())
{
TC_LOG_ERROR("network", "WorldSocket::ProcessIncoming: received duplicate CMSG_AUTH_CONTINUED_SESSION from %s", _worldSession->GetPlayerInfo().c_str());
sessionGuard.unlock(); // unlock session guard to prevent deadlocking in CloseSocket
}
}
CloseSocket();
return;
}
WorldPackets::Auth::AuthContinuedSession authSession(std::move(packet));
@@ -251,23 +271,21 @@ bool WorldSocket::ReadDataHandler()
break;
}
case CMSG_KEEP_ALIVE:
TC_LOG_DEBUG("network", "%s", GetOpcodeNameForLogging(opcode).c_str());
sScriptMgr->OnPacketReceive(_worldSession, packet);
LogOpcodeText(opcode, sessionGuard);
break;
case CMSG_LOG_DISCONNECT:
LogOpcodeText(opcode, sessionGuard);
packet.rfinish(); // contains uint32 disconnectReason;
TC_LOG_DEBUG("network", "%s", GetOpcodeNameForLogging(opcode).c_str());
sScriptMgr->OnPacketReceive(_worldSession, packet);
return true;
case CMSG_ENABLE_NAGLE:
{
TC_LOG_DEBUG("network", "Client %s requested enabling nagle algorithm", GetRemoteIpAddress().to_string().c_str());
sScriptMgr->OnPacketReceive(_worldSession, packet);
LogOpcodeText(opcode, sessionGuard);
SetNoDelay(false);
break;
}
case CMSG_CONNECT_TO_FAILED:
{
sessionGuard.lock();
LogOpcodeText(opcode, sessionGuard);
WorldPackets::Auth::ConnectToFailed connectToFailed(std::move(packet));
connectToFailed.Read();
HandleConnectToFailed(connectToFailed);
@@ -275,6 +293,10 @@ bool WorldSocket::ReadDataHandler()
}
default:
{
sessionGuard.lock();
LogOpcodeText(opcode, sessionGuard);
if (!_worldSession)
{
TC_LOG_ERROR("network.opcode", "ProcessIncoming: Client not authed opcode = %u", uint32(opcode));
@@ -282,13 +304,6 @@ bool WorldSocket::ReadDataHandler()
return false;
}
// prevent invalid memory access/crash with custom opcodes
if (static_cast<uint32>(opcode) >= NUM_OPCODE_HANDLERS)
{
CloseSocket();
return false;
}
OpcodeHandler const* handler = opcodeTable[opcode];
if (!handler)
{
@@ -338,6 +353,25 @@ bool WorldSocket::ReadDataHandler()
return true;
}
void WorldSocket::LogOpcodeText(OpcodeServer opcode, std::unique_lock<std::mutex> const& guard) const
{
if (!guard)
{
TC_LOG_TRACE("network.opcode", "C->S: %s %s", GetRemoteIpAddress().to_string().c_str(), GetOpcodeNameForLogging(opcode).c_str());
}
else
{
TC_LOG_TRACE("network.opcode", "C->S: %s %s", (_worldSession ? _worldSession->GetPlayerInfo() : GetRemoteIpAddress().to_string()).c_str(),
GetOpcodeNameForLogging(opcode).c_str());
}
}
void WorldSocket::SendPacketAndLogOpcode(WorldPacket const& packet)
{
TC_LOG_TRACE("network.opcode", "S->C: %s %s", GetRemoteIpAddress().to_string().c_str(), GetOpcodeNameForLogging(static_cast<OpcodeServer>(packet.GetOpcode())).c_str());
SendPacket(packet);
}
void WorldSocket::SendPacket(WorldPacket const& packet)
{
if (!IsOpen())
@@ -346,8 +380,6 @@ void WorldSocket::SendPacket(WorldPacket const& packet)
if (sPacketLog->CanLogPacket())
sPacketLog->LogPacket(packet, SERVER_TO_CLIENT, GetRemoteIpAddress(), GetRemotePort(), GetConnectionType());
TC_LOG_TRACE("network.opcode", "S->C: %s %s", (_worldSession ? _worldSession->GetPlayerInfo() : GetRemoteIpAddress().to_string()).c_str(), GetOpcodeNameForLogging(static_cast<OpcodeServer>(packet.GetOpcode())).c_str());
uint32 packetSize = packet.size();
uint32 sizeOfHeader = SizeOfServerHeader[_authCrypt.IsInitialized()];
if (packetSize > 0x400)
@@ -655,6 +687,7 @@ void WorldSocket::HandleAuthSession(WorldPackets::Auth::AuthSession& authSession
// At this point, we can safely hook a successful login
sScriptMgr->OnAccountLogin(id);
_authed = true;
_worldSession = new WorldSession(id, battlenetAccountId, shared_from_this(), AccountTypes(security), expansion, mutetime, locale, recruiter, isRecruiter);
_worldSession->LoadGlobalAccountData();
_worldSession->LoadTutorialsData();
@@ -714,8 +747,10 @@ void WorldSocket::HandleAuthContinuedSession(WorldPackets::Auth::AuthContinuedSe
return;
}
_authed = true;
WorldPackets::Auth::ResumeComms resumeComms;
SendPacket(*resumeComms.Write());
SendPacketAndLogOpcode(*resumeComms.Write());
_worldSession->AddInstanceConnection(shared_from_this());
_worldSession->HandleContinuePlayerLogin();
@@ -754,10 +789,9 @@ void WorldSocket::HandleConnectToFailed(WorldPackets::Auth::ConnectToFailed& con
//else
//{
// transfer_aborted when/if we get map node redirection
// SendPacket(*WorldPackets::Auth::ResumeComms().Write());
// SendPacketAndLogOpcode(*WorldPackets::Auth::ResumeComms().Write());
//}
}
}
void WorldSocket::SendAuthResponseError(uint8 code)
@@ -766,7 +800,7 @@ void WorldSocket::SendAuthResponseError(uint8 code)
response.SuccessInfo.HasValue = false;
response.WaitInfo.HasValue = false;
response.Result = code;
SendPacket(*response.Write());
SendPacketAndLogOpcode(*response.Write());
}
void WorldSocket::HandlePing(WorldPacket& recvPacket)
@@ -798,6 +832,8 @@ void WorldSocket::HandlePing(WorldPacket& recvPacket)
if (maxAllowed && _OverSpeedPings > maxAllowed)
{
std::lock_guard<std::mutex> sessionGuard(_worldSessionLock);
if (_worldSession && !_worldSession->HasPermission(rbac::RBAC_PERM_SKIP_CHECK_OVERSPEED_PING))
{
TC_LOG_ERROR("network", "WorldSocket::HandlePing: %s kicked for over-speed pings (address: %s)",
@@ -812,20 +848,24 @@ void WorldSocket::HandlePing(WorldPacket& recvPacket)
_OverSpeedPings = 0;
}
if (_worldSession)
{
_worldSession->SetLatency(latency);
_worldSession->ResetClientTimeDelay();
}
else
{
TC_LOG_ERROR("network", "WorldSocket::HandlePing: peer sent CMSG_PING, but is not authenticated or got recently kicked, address = %s", GetRemoteIpAddress().to_string().c_str());
std::lock_guard<std::mutex> sessionGuard(_worldSessionLock);
CloseSocket();
return;
if (_worldSession)
{
_worldSession->SetLatency(latency);
_worldSession->ResetClientTimeDelay();
}
else
{
TC_LOG_ERROR("network", "WorldSocket::HandlePing: peer sent CMSG_PING, but is not authenticated or got recently kicked, address = %s", GetRemoteIpAddress().to_string().c_str());
CloseSocket();
return;
}
}
WorldPacket packet(SMSG_PONG, 4);
packet << ping;
return SendPacket(packet);
return SendPacketAndLogOpcode(packet);
}

8
src/server/game/Server/WorldSocket.h
View File

@@ -86,11 +86,17 @@ public:
ConnectionType GetConnectionType() const { return _type; }
protected:
void OnClose() override;
void ReadHandler() override;
bool ReadHeaderHandler();
bool ReadDataHandler();
private:
/// writes network.opcode log
/// accessing WorldSession is not threadsafe, only do it when holding _worldSessionLock
void LogOpcodeText(OpcodeServer opcode, std::unique_lock<std::mutex> const& guard) const;
/// sends and logs network.opcode without accessing WorldSession
void SendPacketAndLogOpcode(WorldPacket const& packet);
void WritePacketToBuffer(WorldPacket const& packet, MessageBuffer& buffer);
uint32 CompressPacket(uint8* buffer, WorldPacket const& packet);
@@ -114,7 +120,9 @@ private:
std::chrono::steady_clock::time_point _LastPingTime;
uint32 _OverSpeedPings;
std::mutex _worldSessionLock;
WorldSession* _worldSession;
bool _authed;
MessageBuffer _headerBuffer;
MessageBuffer _packetBuffer;

6
src/server/shared/Networking/Socket.h
View File

@@ -62,7 +62,7 @@ public:
return false;
#ifndef TC_SOCKET_USE_IOCP
std::unique_lock<std::mutex> guard(_writeLock, std::try_to_lock);
std::unique_lock<std::mutex> guard(_writeLock);
if (!guard)
return true;
@@ -140,6 +140,8 @@ public:
if (shutdownError)
TC_LOG_DEBUG("network", "Socket::CloseSocket: %s errored when shutting down socket: %i (%s)", GetRemoteIpAddress().to_string().c_str(),
shutdownError.value(), shutdownError.message().c_str());
OnClose();
}
/// Marks the socket for closing after write buffer becomes empty
@@ -148,6 +150,8 @@ public:
MessageBuffer& GetReadBuffer() { return _readBuffer; }
protected:
virtual void OnClose() { }
virtual void ReadHandler() = 0;
bool AsyncProcessQueue(std::unique_lock<std::mutex>&)