[8389] Implement check really read received packet size and warning it not all data read.

* This let more easy catch packet structure chnages at client switch.
* Fixed structure CMSG_GUILD_BANK_SWAP_ITEMS
* Fixed structure CMSG_SPLIT_ITEM, CMSG_SELL_ITEM
* Added read data amount fixes for some other packets.

Thanks to TOM_RUS in help check correct packets structure.

Note: not all packets possible fixed. Please report for not fixed cases at errors:
"opcode %s (0x%.4X) have unprocessed tail data (read stop at %u from %u)"
This commit is contained in:
VladimirMangos 2009-08-18 23:11:11 +04:00
parent 17b94e1e09
commit 35121cdd34
12 changed files with 82 additions and 41 deletions

View file

@ -576,6 +576,8 @@ void WorldSession::HandleAuctionListItems( WorldPacket & recv_data )
recv_data >> auctionSlotID >> auctionMainCategory >> auctionSubCategory;
recv_data >> quality >> usable;
recv_data.read_skip(16); // unknown 16 bytes: 00 07 01 00 00 01 05 00 06 00 09 01 08 00 03 00
Creature *pCreature = GetPlayer()->GetNPCIfCanInteractWith(guid,UNIT_NPC_FLAG_AUCTIONEER);
if (!pCreature)
{
@ -618,7 +620,8 @@ void WorldSession::HandleAuctionListItems( WorldPacket & recv_data )
void WorldSession::HandleAuctionListPendingSales( WorldPacket & recv_data )
{
sLog.outDebug("CMSG_AUCTION_LIST_PENDING_SALES");
recv_data.hexlike();
recv_data.read_skip<uint64>(); // auctioner guid
uint32 count = 0;

View file

@ -96,6 +96,7 @@ void WorldSession::HandleCalendarAddEvent(WorldPacket &recv_data)
{
sLog.outDebug("WORLD: CMSG_CALENDAR_ADD_EVENT");
recv_data.hexlike();
recv_data.rpos(recv_data.wpos()); // set to end to avoid warnings spam
//std::string unk1, unk2;
//recv_data >> (std::string)unk1;
@ -131,6 +132,7 @@ void WorldSession::HandleCalendarUpdateEvent(WorldPacket &recv_data)
{
sLog.outDebug("WORLD: CMSG_CALENDAR_UPDATE_EVENT");
recv_data.hexlike();
recv_data.rpos(recv_data.wpos()); // set to end to avoid warnings spam
//recv_data >> uint64
//recv_data >> uint64
@ -149,6 +151,7 @@ void WorldSession::HandleCalendarRemoveEvent(WorldPacket &recv_data)
{
sLog.outDebug("WORLD: CMSG_CALENDAR_REMOVE_EVENT");
recv_data.hexlike();
recv_data.rpos(recv_data.wpos()); // set to end to avoid warnings spam
//recv_data >> uint64
//recv_data >> uint64
@ -160,6 +163,7 @@ void WorldSession::HandleCalendarCopyEvent(WorldPacket &recv_data)
{
sLog.outDebug("WORLD: CMSG_CALENDAR_COPY_EVENT");
recv_data.hexlike();
recv_data.rpos(recv_data.wpos()); // set to end to avoid warnings spam
//recv_data >> uint64
//recv_data >> uint64
@ -171,6 +175,7 @@ void WorldSession::HandleCalendarEventInvite(WorldPacket &recv_data)
{
sLog.outDebug("WORLD: CMSG_CALENDAR_EVENT_INVITE");
recv_data.hexlike();
recv_data.rpos(recv_data.wpos()); // set to end to avoid warnings spam
//recv_data >> uint64
//recv_data >> uint64
@ -184,6 +189,7 @@ void WorldSession::HandleCalendarEventRsvp(WorldPacket &recv_data)
{
sLog.outDebug("WORLD: CMSG_CALENDAR_EVENT_RSVP");
recv_data.hexlike();
recv_data.rpos(recv_data.wpos()); // set to end to avoid warnings spam
//recv_data >> uint64
//recv_data >> uint64
@ -195,6 +201,7 @@ void WorldSession::HandleCalendarEventRemoveInvite(WorldPacket &recv_data)
{
sLog.outDebug("WORLD: CMSG_CALENDAR_EVENT_REMOVE_INVITE");
recv_data.hexlike();
recv_data.rpos(recv_data.wpos()); // set to end to avoid warnings spam
//recv_data.readPackGUID(guid)
//recv_data >> uint64
@ -206,6 +213,7 @@ void WorldSession::HandleCalendarEventStatus(WorldPacket &recv_data)
{
sLog.outDebug("WORLD: CMSG_CALENDAR_EVENT_STATUS");
recv_data.hexlike();
recv_data.rpos(recv_data.wpos()); // set to end to avoid warnings spam
//recv_data.readPackGUID(guid)
//recv_data >> uint64
@ -218,6 +226,7 @@ void WorldSession::HandleCalendarEventModeratorStatus(WorldPacket &recv_data)
{
sLog.outDebug("WORLD: CMSG_CALENDAR_EVENT_MODERATOR_STATUS");
recv_data.hexlike();
recv_data.rpos(recv_data.wpos()); // set to end to avoid warnings spam
//recv_data.readPackGUID(guid)
//recv_data >> uint64
@ -230,6 +239,7 @@ void WorldSession::HandleCalendarComplain(WorldPacket &recv_data)
{
sLog.outDebug("WORLD: CMSG_CALENDAR_COMPLAIN");
recv_data.hexlike();
recv_data.rpos(recv_data.wpos()); // set to end to avoid warnings spam
//recv_data >> uint64
//recv_data >> uint64

View file

@ -84,15 +84,15 @@ void WorldSession::HandleGMTicketCreateOpcode( WorldPacket & recv_data )
uint32 map;
float x, y, z;
std::string ticketText = "";
uint32 unk1, unk2;
recv_data >> map >> x >> y >> z; // last check 2.4.3
recv_data >> ticketText;
recv_data >> unk1 >> unk2;
// note: the packet might contain more data, but the exact structure of that is unknown
recv_data.read_skip<uint32>(); // unk1, 0
recv_data.read_skip<uint32>(); // unk2, 1
recv_data.read_skip<uint32>(); // unk3, 0
sLog.outDebug("TicketCreate: map %u, x %f, y %f, z %f, text %s, unk1 %u, unk2 %u", map, x, y, z, ticketText.c_str(), unk1, unk2);
sLog.outDebug("TicketCreate: map %u, x %f, y %f, z %f, text %s", map, x, y, z, ticketText.c_str());
if(ticketmgr.GetGMTicket(GetPlayer()->GetGUIDLow()))
{

View file

@ -614,12 +614,14 @@ void WorldSession::HandleGuildRankOpcode(WorldPacket& recvPacket)
guild = objmgr.GetGuildById(GetPlayer()->GetGuildId());
if(!guild)
{
recvPacket.rpos(recvPacket.wpos()); // set to end to avoid warnings spam
SendGuildCommandResult(GUILD_CREATE_S, "", GUILD_PLAYER_NOT_IN_GUILD);
return;
}
else if(GetPlayer()->GetGUID() != guild->GetLeader())
{
recvPacket.rpos(recvPacket.wpos()); // set to end to avoid warnings spam
SendGuildCommandResult(GUILD_INVITE_S, "", GUILD_PERMISSIONS);
return;
}
@ -1026,9 +1028,14 @@ void WorldSession::HandleGuildBankSwapItems( WorldPacket & recv_data )
uint64 GoGuid;
uint8 BankToBank;
uint8 BankTab, BankTabSlot, AutoStore, AutoStoreCount, PlayerSlot, PlayerBag, SplitedAmount = 0;
uint8 BankTabDst, BankTabSlotDst, unk2, ToChar = 1;
uint8 BankTab, BankTabSlot, AutoStore;
uint8 PlayerSlot = NULL_SLOT;
uint8 PlayerBag = NULL_BAG;
uint8 BankTabDst, BankTabSlotDst, unk2;
uint8 ToChar = 1;
uint32 ItemEntry, unk1;
uint32 AutoStoreCount = 0;
uint32 SplitedAmount = 0;
recv_data >> GoGuid >> BankToBank;
if (BankToBank)
@ -1042,11 +1049,12 @@ void WorldSession::HandleGuildBankSwapItems( WorldPacket & recv_data )
recv_data >> unk2; // always 0
recv_data >> SplitedAmount;
if (BankTabSlotDst >= GUILD_BANK_MAX_SLOTS)
return;
if (BankTabDst == BankTab && BankTabSlotDst == BankTabSlot)
if (BankTabSlotDst >= GUILD_BANK_MAX_SLOTS || BankTabDst == BankTab && BankTabSlotDst == BankTabSlot)
{
recv_data.rpos(recv_data.wpos()); // prevent additional spam at rejected packet
return;
}
}
else
{
recv_data >> BankTab;
@ -1056,18 +1064,23 @@ void WorldSession::HandleGuildBankSwapItems( WorldPacket & recv_data )
if (AutoStore)
{
recv_data >> AutoStoreCount;
recv_data.read_skip<uint8>(); // ToChar (?), always and expected to be 1 (autostore only triggered in guild->ToChar)
recv_data.read_skip<uint32>(); // unknown, always 0
}
else
{
recv_data >> PlayerBag;
recv_data >> PlayerSlot;
if (!AutoStore)
{
recv_data >> ToChar;
recv_data >> SplitedAmount;
}
if (BankTabSlot >= GUILD_BANK_MAX_SLOTS && BankTabSlot != 0xFF)
{
recv_data.rpos(recv_data.wpos()); // prevent additional spam at rejected packet
return;
}
}
if (!GetPlayer()->GetGameObjectIfCanInteractWith(GoGuid, GAMEOBJECT_TYPE_GUILD_BANK))
return;
@ -1201,14 +1214,6 @@ void WorldSession::HandleGuildBankSwapItems( WorldPacket & recv_data )
// Player <-> Bank
// char->bank autostore click return BankTabSlot = 255 = NULL_SLOT
// do similar for bank->char
if(AutoStore && ToChar)
{
PlayerBag = NULL_BAG;
PlayerSlot = NULL_SLOT;
}
// allow work with inventory only
if(!Player::IsInventoryPos(PlayerBag,PlayerSlot) && !(PlayerBag == NULL_BAG && PlayerSlot == NULL_SLOT) )
{

View file

@ -30,7 +30,8 @@
void WorldSession::HandleSplitItemOpcode( WorldPacket & recv_data )
{
//sLog.outDebug("WORLD: CMSG_SPLIT_ITEM");
uint8 srcbag, srcslot, dstbag, dstslot, count;
uint8 srcbag, srcslot, dstbag, dstslot;
uint32 count;
recv_data >> srcbag >> srcslot >> dstbag >> dstslot >> count;
//sLog.outDebug("STORAGE: receive srcbag = %u, srcslot = %u, dstbag = %u, dstslot = %u, count = %u", srcbag, srcslot, dstbag, dstslot, count);
@ -488,12 +489,9 @@ void WorldSession::HandleSellItemOpcode( WorldPacket & recv_data )
{
sLog.outDebug( "WORLD: Received CMSG_SELL_ITEM" );
uint64 vendorguid, itemguid;
uint8 _count;
uint32 count;
recv_data >> vendorguid >> itemguid >> _count;
// prevent possible overflow, as mangos uses uint32 for item count
uint32 count = _count;
recv_data >> vendorguid >> itemguid >> count;
if(!itemguid)
return;
@ -969,6 +967,8 @@ void WorldSession::HandleItemNameQueryOpcode(WorldPacket & recv_data)
{
uint32 itemid;
recv_data >> itemid;
recv_data.read_skip<uint64>(); // guid
sLog.outDebug("WORLD: CMSG_ITEM_NAME_QUERY %u", itemid);
ItemPrototype const *pProto = objmgr.GetItemPrototype( itemid );
if( pProto )

View file

@ -878,6 +878,7 @@ void WorldSession::HandleUpdateAccountData(WorldPacket &recv_data)
if(decompressedSize > 0xFFFF)
{
recv_data.rpos(recv_data.wpos()); // unnneded warning spam in this case
sLog.outError("UAD: Account data packet too big, size %u", decompressedSize);
return;
}
@ -888,10 +889,13 @@ void WorldSession::HandleUpdateAccountData(WorldPacket &recv_data)
uLongf realSize = decompressedSize;
if(uncompress(const_cast<uint8*>(dest.contents()), &realSize, const_cast<uint8*>(recv_data.contents() + recv_data.rpos()), recv_data.size() - recv_data.rpos()) != Z_OK)
{
recv_data.rpos(recv_data.wpos()); // unnneded warning spam in this case
sLog.outError("UAD: Failed to decompress account data");
return;
}
recv_data.rpos(recv_data.wpos()); // uncompress read (recv_data.size() - recv_data.rpos())
std::string adata;
dest >> adata;
@ -997,7 +1001,8 @@ void WorldSession::HandleMoveTimeSkippedOpcode( WorldPacket & recv_data )
/* WorldSession::Update( getMSTime() );*/
DEBUG_LOG( "WORLD: Time Lag/Synchronization Resent/Update" );
recv_data.read_skip2<uint64,uint32>();
recv_data.read_skip<uint64>();
recv_data.read_skip<uint32>();
/*
uint64 guid;
uint32 time_skipped;

View file

@ -26,7 +26,8 @@ void WorldSession::HandleVoiceSessionEnableOpcode( WorldPacket & recv_data )
{
sLog.outDebug("WORLD: CMSG_VOICE_SESSION_ENABLE");
// uint8 isVoiceEnabled, uint8 isMicrophoneEnabled
recv_data.read_skip2<uint8,uint8>();
recv_data.read_skip<uint8>();
recv_data.read_skip<uint8>();
recv_data.hexlike();
}

View file

@ -141,7 +141,7 @@ void WorldSession::QueuePacket(WorldPacket* new_packet)
}
/// Logging helper for unexpected opcodes
void WorldSession::logUnexpectedOpcode(WorldPacket* packet, const char *reason)
void WorldSession::LogUnexpectedOpcode(WorldPacket* packet, const char *reason)
{
sLog.outError( "SESSION: received unexpected opcode %s (0x%.4X) %s",
LookupOpcodeName(packet->GetOpcode()),
@ -149,6 +149,15 @@ void WorldSession::logUnexpectedOpcode(WorldPacket* packet, const char *reason)
reason);
}
/// Logging helper for unexpected opcodes
void WorldSession::LogUnprocessedTail(WorldPacket *packet)
{
sLog.outError( "SESSION: opcode %s (0x%.4X) have unprocessed tail data (read stop at %u from %u)",
LookupOpcodeName(packet->GetOpcode()),
packet->GetOpcode(),
packet->rpos(),packet->wpos());
}
/// Update the WorldSession (triggered by World update)
bool WorldSession::Update(uint32 /*diff*/)
{
@ -182,30 +191,40 @@ bool WorldSession::Update(uint32 /*diff*/)
{
// 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");
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_TRANSFER:
if(!_player)
logUnexpectedOpcode(packet, "the player has not logged in yet");
LogUnexpectedOpcode(packet, "the player has not logged in yet");
else if(_player->IsInWorld())
logUnexpectedOpcode(packet, "the player is still in world");
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");
LogUnexpectedOpcode(packet, "the player not pass queue yet");
break;
}
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)",
@ -214,7 +233,7 @@ bool WorldSession::Update(uint32 /*diff*/)
break;
}
}
catch(ByteBufferException &exception)
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());

View file

@ -714,7 +714,8 @@ class MANGOS_DLL_SPEC WorldSession
void moveItems(Item* myItems[], Item* hisItems[]);
// logging helper
void logUnexpectedOpcode(WorldPacket *packet, const char * reason);
void LogUnexpectedOpcode(WorldPacket *packet, const char * reason);
void LogUnprocessedTail(WorldPacket *packet);
Player *_player;
WorldSocket *m_Socket;

View file

@ -732,7 +732,7 @@ int WorldSocket::ProcessIncoming (WorldPacket* new_pct)
}
}
}
catch(ByteBufferException &exception)
catch(ByteBufferException &)
{
sLog.outError("WorldSocket::ProcessIncoming ByteBufferException occured while parsing an instant handled packet (opcode: %u) from client %s, accountid=%i. Disconnected client.",
opcode, GetRemoteAddress().c_str(), m_Session?m_Session->GetAccountId():-1);

View file

@ -263,9 +263,6 @@ class ByteBuffer
template<typename T>
void read_skip() { read_skip(sizeof(T)); }
template<typename T1, typename T2>
void read_skip2() { read_skip(sizeof(T1)+sizeof(T2)); }
void read_skip(size_t skip)
{
if(_rpos + skip > size())

View file

@ -1,4 +1,4 @@
#ifndef __REVISION_NR_H__
#define __REVISION_NR_H__
#define REVISION_NR "8388"
#define REVISION_NR "8389"
#endif // __REVISION_NR_H__