Skip to content

Commit

Permalink
[8850] Check packet opcode for >= NUM_MSG_TYPES before queueing.
Browse files Browse the repository at this point in the history 
Also add check for wrong packet status requirement in code.
  • Loading branch information
XTZGZoReX committed Nov 20, 2009
1 parent fdb060c commit 5651043
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 83 deletions.
160 changes: 78 additions & 82 deletions src/game/WorldSession.cpp
View file
Expand Up @@ -169,97 +169,93 @@ bool WorldSession::Update(uint32 /*diff*/)
packet->GetOpcode());
#endif*/

if(packet->GetOpcode() >= NUM_MSG_TYPES)
OpcodeHandler& opHandle = opcodeTable[packet->GetOpcode()];
try
{
sLog.outError( "SESSION: received non-existed opcode %s (0x%.4X)",
LookupOpcodeName(packet->GetOpcode()),
packet->GetOpcode());
}
else
{
OpcodeHandler& opHandle = opcodeTable[packet->GetOpcode()];
try
switch (opHandle.status)
{
switch (opHandle.status)
{
case STATUS_LOGGEDIN:
if(!_player)
{
// skip STATUS_LOGGEDIN opcode unexpected errors if player logout sometime ago - this can be network lag delayed packets
if(!m_playerRecentlyLogout)
LogUnexpectedOpcode(packet, "the player has not logged in yet");
}
else if(_player->IsInWorld())
{
(this->*opHandle.handler)(*packet);
if (sLog.IsOutDebug() && packet->rpos() < packet->wpos())
LogUnprocessedTail(packet);
}
// lag can cause STATUS_LOGGEDIN opcodes to arrive after the player started a transfer
break;
case STATUS_LOGGEDIN_OR_RECENTLY_LOGGEDOUT:
if(!_player && !m_playerRecentlyLogout)
{
LogUnexpectedOpcode(packet, "the player has not logged in yet and not recently logout");
}
else
{
// not expected _player or must checked in packet hanlder
(this->*opHandle.handler)(*packet);
if (sLog.IsOutDebug() && packet->rpos() < packet->wpos())
LogUnprocessedTail(packet);
}
break;
case STATUS_TRANSFER:
if(!_player)
case STATUS_LOGGEDIN:
if(!_player)
{
// skip STATUS_LOGGEDIN opcode unexpected errors if player logout sometime ago - this can be network lag delayed packets
if(!m_playerRecentlyLogout)
LogUnexpectedOpcode(packet, "the player has not logged in yet");
else if(_player->IsInWorld())
LogUnexpectedOpcode(packet, "the player is still in world");
else
{
(this->*opHandle.handler)(*packet);
if (sLog.IsOutDebug() && packet->rpos() < packet->wpos())
LogUnprocessedTail(packet);
}
break;
case STATUS_AUTHED:
// prevent cheating with skip queue wait
if(m_inQueue)
{
LogUnexpectedOpcode(packet, "the player not pass queue yet");
break;
}

// single from authed time opcodes send in to after logout time
// and before other STATUS_LOGGEDIN_OR_RECENTLY_LOGGOUT opcodes.
if (packet->GetOpcode() != CMSG_SET_ACTIVE_VOICE_CHANNEL)
m_playerRecentlyLogout = false;

}
else if(_player->IsInWorld())
{
(this->*opHandle.handler)(*packet);
if (sLog.IsOutDebug() && packet->rpos() < packet->wpos())
LogUnprocessedTail(packet);
}
// lag can cause STATUS_LOGGEDIN opcodes to arrive after the player started a transfer
break;
case STATUS_LOGGEDIN_OR_RECENTLY_LOGGEDOUT:
if(!_player && !m_playerRecentlyLogout)
{
LogUnexpectedOpcode(packet, "the player has not logged in yet and not recently logout");
}
else
{
// not expected _player or must checked in packet hanlder
(this->*opHandle.handler)(*packet);
if (sLog.IsOutDebug() && packet->rpos() < packet->wpos())
LogUnprocessedTail(packet);
}
break;
case STATUS_TRANSFER:
if(!_player)
LogUnexpectedOpcode(packet, "the player has not logged in yet");
else if(_player->IsInWorld())
LogUnexpectedOpcode(packet, "the player is still in world");
else
{
(this->*opHandle.handler)(*packet);
if (sLog.IsOutDebug() && packet->rpos() < packet->wpos())
LogUnprocessedTail(packet);
}
break;
case STATUS_AUTHED:
// prevent cheating with skip queue wait
if(m_inQueue)
{
LogUnexpectedOpcode(packet, "the player not pass queue yet");
break;
case STATUS_NEVER:
sLog.outError( "SESSION: received not allowed opcode %s (0x%.4X)",
LookupOpcodeName(packet->GetOpcode()),
packet->GetOpcode());
break;
case STATUS_UNHANDLED:
sLog.outDebug("SESSION: received not handled opcode %s (0x%.4X)",
LookupOpcodeName(packet->GetOpcode()),
packet->GetOpcode());
break;
}
}

// single from authed time opcodes send in to after logout time
// and before other STATUS_LOGGEDIN_OR_RECENTLY_LOGGOUT opcodes.
if (packet->GetOpcode() != CMSG_SET_ACTIVE_VOICE_CHANNEL)
m_playerRecentlyLogout = false;

(this->*opHandle.handler)(*packet);
if (sLog.IsOutDebug() && packet->rpos() < packet->wpos())
LogUnprocessedTail(packet);
break;
case STATUS_NEVER:
sLog.outError( "SESSION: received not allowed opcode %s (0x%.4X)",
LookupOpcodeName(packet->GetOpcode()),
packet->GetOpcode());
break;
case STATUS_UNHANDLED:
sLog.outDebug("SESSION: received not handled opcode %s (0x%.4X)",
LookupOpcodeName(packet->GetOpcode()),
packet->GetOpcode());
break;
default:
sLog.outError("SESSION: received wrong-status-req opcode %s (0x%.4X)",
LookupOpcodeName(packet->GetOpcode()),
packet->GetOpcode());
break;
}
catch(ByteBufferException &)
}
catch(ByteBufferException &)
{
sLog.outError("WorldSession::Update ByteBufferException occured while parsing a packet (opcode: %u) from client %s, accountid=%i. Skipped packet.",
packet->GetOpcode(), GetRemoteAddress().c_str(), GetAccountId());
if(sLog.IsOutDebug())
{
sLog.outError("WorldSession::Update ByteBufferException occured while parsing a packet (opcode: %u) from client %s, accountid=%i. Skipped packet.",
packet->GetOpcode(), GetRemoteAddress().c_str(), GetAccountId());
if(sLog.IsOutDebug())
{
sLog.outDebug("Dumping error causing packet:");
packet->hexlike();
}
sLog.outDebug("Dumping error causing packet:");
packet->hexlike();
}
}

Expand Down
7 changes: 7 additions & 0 deletions src/game/WorldSocket.cpp
View file
Expand Up @@ -659,6 +659,13 @@ int WorldSocket::ProcessIncoming (WorldPacket* new_pct)

const ACE_UINT16 opcode = new_pct->GetOpcode ();

if (opcode >= NUM_MSG_TYPES)
{
sLog.outError( "SESSION: received non-existed opcode %s (0x%.4X)",
LookupOpcodeName(opcode), opcode);
return -1;
}

if (closing_)
return -1;

Expand Down
2 changes: 1 addition & 1 deletion src/shared/revision_nr.h
View file
@@ -1,4 +1,4 @@
#ifndef __REVISION_NR_H__
#define __REVISION_NR_H__
#define REVISION_NR "8849"
#define REVISION_NR "8850"
#endif // __REVISION_NR_H__

4 comments on commit 5651043

@VladimirMangos
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this just speedup (more early) check for not existed packets send by client or cheating tools.

@gaussianrecurrence
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sLog.outError( "SESSION: received non-existed opcode %s (0x%.4X)",LookupOpcodeName(opcode), opcode);
If opcode is greater that list LookupOpcodeName will return null and then server could crash

@alexrp
Copy link

@alexrp alexrp commented on 5651043 Nov 22, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, copy-paste mistake :). Thanks.

@VladimirMangos
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in fact this exist before your patch and if this is problem then this problem exist before your commit.
In nay case i drop attempt output name for not existed opcode in one from next commits.

Please sign in to comment.