Several fixes (#10)

* * Fixed dereferencing nullptr, when sending CMSG_LOOT guid with HIGHGUID_GAMEOBJECT, but invalid in general.

* * Don't allow sending LANG_UNIVERSAL by player
* Don't allow messages with more that 255 symbols (server side check)

* * Prevent cheaters cast honorless target on self

* * Now warden reacts on client delay when initialization in progress or strange behaviour with initialization logic

* Remove default case from switch

This allows compilers to detect if a case for the WardenState enum is missing when using -Wall flag

* Make the intention of the IF case more clear

Make the intention of the IF case more clear and suppress a compiler warning when using -Wall or /W4 flags

src/game/Object/Player.cpp

@@ -7288,7 +7288,6 @@ void Player::SendLoot(ObjectGuid guid, LootType loot_type)
         {
             DEBUG_LOG("       IS_GAMEOBJECT_GUID(guid)");
             GameObject* go = GetMap()->GetGameObject(guid);
-            GameObjectInfo const* goInfo = go->GetGOInfo();
 
             // not check distance for GO in case owned GO (fishing bobber case, for example)
             // And permit out of range GO with no owner in case fishing hole
@@ -7298,6 +7297,8 @@ void Player::SendLoot(ObjectGuid guid, LootType loot_type)
                 return;
             }
 
+            GameObjectInfo const* goInfo = go->GetGOInfo();
+
             loot = &go->loot;
 
             Player* recipient = go->GetLootRecipient();

src/game/Warden/Warden.cpp

@@ -36,7 +36,7 @@
 #include "AccountMgr.h"
 
 Warden::Warden() : _session(NULL), _inputCrypto(16), _outputCrypto(16), _checkTimer(10000/*10 sec*/), _clientResponseTimer(0),
-                   _dataSent(false), _previousTimestamp(0), _module(NULL), _initialized(false)
+                   _state(WardenState::STATE_INITIAL), _previousTimestamp(0), _module(NULL)
 {
     memset(_inputKey, 0, sizeof(_inputKey));
     memset(_outputKey, 0, sizeof(_outputKey));
@@ -48,7 +48,11 @@ Warden::~Warden()
     delete[] _module->CompressedData;
     delete _module;
     _module = NULL;
-    _initialized = false;
+}
+
+void Warden::InitializeModule()
+{
+    SetNewState(WardenState::STATE_INITIALIZE_MODULE);
 }
 
 void Warden::RequestHash()
@@ -66,6 +70,8 @@ void Warden::RequestHash()
     WorldPacket pkt(SMSG_WARDEN_DATA, sizeof(WardenHashRequest));
     pkt.append((uint8*)&Request, sizeof(WardenHashRequest));
     _session->SendPacket(&pkt);
+
+    SetNewState(WardenState::STATE_REQUESTED_HASH);
 }
 
 void Warden::SendModuleToClient()
@@ -92,6 +98,8 @@ void Warden::SendModuleToClient()
         pkt1.append((uint8*)&packet, burstSize + 3);
         _session->SendPacket(&pkt1);
     }
+
+    SetNewState(WardenState::STATE_SENDED_MODULE);
 }
 
 void Warden::RequestModule()
@@ -112,17 +120,24 @@ void Warden::RequestModule()
     WorldPacket pkt(SMSG_WARDEN_DATA, sizeof(WardenModuleUse));
     pkt.append((uint8*)&request, sizeof(WardenModuleUse));
     _session->SendPacket(&pkt);
+
+    SetNewState(WardenState::STATE_REQUESTED_MODULE);
 }
 
 void Warden::Update()
 {
-    if (_initialized)
+    uint32 currentTimestamp = WorldTimer::getMSTime();
-    {
+    uint32 diff = currentTimestamp - _previousTimestamp;
-        uint32 currentTimestamp = WorldTimer::getMSTime();
+    _previousTimestamp = currentTimestamp;
-        uint32 diff = currentTimestamp - _previousTimestamp;
-        _previousTimestamp = currentTimestamp;
 
-        if (_dataSent)
+    switch (_state)
+    {
+        case WardenState::STATE_INITIAL:
+            break;
+        case WardenState::STATE_REQUESTED_MODULE:
+        case WardenState::STATE_SENDED_MODULE:
+        case WardenState::STATE_REQUESTED_HASH:
+        case WardenState::STATE_REQUESTED_DATA:
         {
             uint32 maxClientResponseDelay = sWorld.getConfig(CONFIG_UINT32_WARDEN_CLIENT_RESPONSE_DELAY);
 
@@ -131,23 +146,34 @@ void Warden::Update()
                 // Kick player if client response delays more than set in config
                 if (_clientResponseTimer > maxClientResponseDelay * IN_MILLISECONDS)
                 {
-                    sLog.outWarden("%s (latency: %u, IP: %s) exceeded Warden module response delay for more than %s - disconnecting client",
+                    sLog.outWarden("%s (latency: %u, IP: %s) exceeded Warden module response delay on state %s for more than %s - disconnecting client",
-                        _session->GetPlayerName(), _session->GetLatency(), _session->GetRemoteAddress().c_str(), secsToTimeString(maxClientResponseDelay, true).c_str());
+                                   _session->GetPlayerName(), _session->GetLatency(), _session->GetRemoteAddress().c_str(), WardenState::to_string(_state), secsToTimeString(maxClientResponseDelay, true).c_str());
                     _session->KickPlayer();
                 }
                 else
+		{
                     _clientResponseTimer += diff;
-            }
+		}
-        }
+
-        else
+	    }
+	}
+        break;
+        case WardenState::STATE_INITIALIZE_MODULE:
+        case WardenState::STATE_RESTING:
         {
             if (diff >= _checkTimer)
             {
                 RequestData();
             }
             else
+            {
                 _checkTimer -= diff;
+            }
         }
+        break;
+        default:
+            sLog.outWarden("Unimplemented warden state!");
+            break;
     }
 }
 
@@ -161,6 +187,28 @@ void Warden::EncryptData(uint8* buffer, uint32 length)
     _outputCrypto.UpdateData(length, buffer);
 }
 
+void Warden::SetNewState(WardenState::Value state)
+{
+    //if we pass all initial checks, allow change
+    if (state < WardenState::STATE_REQUESTED_DATA)
+    {
+        if (state < _state)
+        {
+            sLog.outWarden("Warden Error: jump from %s to %s which is lower by initialization routine", WardenState::to_string(_state), WardenState::to_string(state));
+            return;
+        }
+    }
+
+    _state = state;
+
+    //Reset timers
+    // Set hold off timer, minimum timer should at least be 1 second
+    uint32 holdOff = sWorld.getConfig(CONFIG_UINT32_WARDEN_CLIENT_CHECK_HOLDOFF);
+    _checkTimer = (holdOff < 1 ? 1 : holdOff) * IN_MILLISECONDS;
+
+    _clientResponseTimer = 0;
+}
+
 bool Warden::IsValidCheckSum(uint32 checksum, const uint8* data, const uint16 length)
 {
     uint32 newChecksum = BuildChecksum(data, length);
@@ -280,11 +328,14 @@ void WorldSession::HandleWardenDataOpcode(WorldPacket& recvData)
     }
 }
 
+void Warden::RequestData()
+{
+    SetNewState(WardenState::STATE_REQUESTED_DATA);
+}
+
 void Warden::HandleData(ByteBuffer& /*buff*/)
 {
-    // Set hold off timer, minimum timer should at least be 1 second
+    SetNewState(WardenState::STATE_RESTING);
-    uint32 holdOff = sWorld.getConfig(CONFIG_UINT32_WARDEN_CLIENT_CHECK_HOLDOFF);
-    _checkTimer = (holdOff < 1 ? 1 : holdOff) * IN_MILLISECONDS;
 }
 
 void Warden::LogPositiveToDB(WardenCheck* check)

src/game/Warden/Warden.h

@@ -95,6 +95,42 @@ struct WardenHashRequest
     uint8 Seed[16];
 };
 
+namespace WardenState
+{
+    enum Value
+    {
+        STATE_INITIAL,
+        STATE_REQUESTED_MODULE,
+        STATE_SENDED_MODULE,
+        STATE_REQUESTED_HASH,
+        STATE_INITIALIZE_MODULE,
+        STATE_REQUESTED_DATA,
+        STATE_RESTING
+    };
+
+    inline char* to_string(WardenState::Value value)
+    {
+        switch (value)
+        {
+            case WardenState::STATE_INITIAL:
+                return "STATE_INITIAL";
+            case WardenState::STATE_REQUESTED_MODULE:
+                return "STATE_REQUESTED_MODULE";
+            case WardenState::STATE_SENDED_MODULE:
+                return "STATE_SENDED_MODULE";
+            case WardenState::STATE_REQUESTED_HASH:
+                return "STATE_REQUESTED_HASH";
+            case WardenState::STATE_INITIALIZE_MODULE:
+                return "STATE_INITIALIZE_MODULE";
+            case WardenState::STATE_REQUESTED_DATA:
+                return "STATE_SENDED_DATA";
+            case WardenState::STATE_RESTING:
+                return "STATE_RESTING";
+        }
+        return "UNDEFINED STATE";
+    }
+};
+
 #if defined(__GNUC__)
 #pragma pack()
 #else
@@ -119,10 +155,10 @@ class Warden
 
         virtual void Init(WorldSession* session, BigNumber* k) = 0;
         virtual ClientWardenModule* GetModuleForClient() = 0;
-        virtual void InitializeModule() = 0;
+        virtual void InitializeModule();
         virtual void RequestHash();
         virtual void HandleHashResult(ByteBuffer &buff) = 0;
-        virtual void RequestData() = 0;
+        virtual void RequestData();
         virtual void HandleData(ByteBuffer &buff);
 
         void SendModuleToClient();
@@ -131,6 +167,8 @@ class Warden
         void DecryptData(uint8* buffer, uint32 length);
         void EncryptData(uint8* buffer, uint32 length);
 
+	void SetNewState(WardenState::Value state);
+
         static bool IsValidCheckSum(uint32 checksum, const uint8 *data, const uint16 length);
         static uint32 BuildChecksum(const uint8 *data, uint32 length);
 
@@ -148,10 +186,9 @@ class Warden
         ARC4 _outputCrypto;
         uint32 _checkTimer;                          // Timer for sending check requests
         uint32 _clientResponseTimer;                 // Timer for client response delay
-        bool _dataSent;
         uint32 _previousTimestamp;
         ClientWardenModule* _module;
-        bool _initialized;
+        WardenState::Value _state;
 };
 
 #endif

src/game/Warden/WardenMac.cpp

@@ -98,6 +98,7 @@ ClientWardenModule* WardenMac::GetModuleForClient()
 void WardenMac::InitializeModule()
 {
     sLog.outWarden("Initialize module");
+    Warden::InitializeModule();
 }
 
 struct keyData {
@@ -174,8 +175,6 @@ void WardenMac::HandleHashResult(ByteBuffer &buff)
     _inputCrypto.Init(_inputKey);
     _outputCrypto.Init(_outputKey);
 
-    _initialized = true;
-
     _previousTimestamp = WorldTimer::getMSTime();
 }
 
@@ -200,16 +199,13 @@ void WardenMac::RequestData()
     pkt.append(buff);
     _session->SendPacket(&pkt);
 
-    _dataSent = true;
+    Warden::RequestData();
 }
 
 void WardenMac::HandleData(ByteBuffer &buff)
 {
     sLog.outWarden("Handle data");
 
-    _dataSent = false;
-    _clientResponseTimer = 0;
-
     //uint16 Length;
     //buff >> Length;
     //uint32 Checksum;

src/game/Warden/WardenWin.cpp

@@ -132,6 +132,8 @@ void WardenWin::InitializeModule()
     WorldPacket pkt(SMSG_WARDEN_DATA, sizeof(WardenInitModuleRequest));
     pkt.append((uint8*)&Request, sizeof(WardenInitModuleRequest));
     _session->SendPacket(&pkt);
+
+    Warden::InitializeModule();
 }
 
 void WardenWin::HandleHashResult(ByteBuffer &buff)
@@ -154,8 +156,6 @@ void WardenWin::HandleHashResult(ByteBuffer &buff)
     _inputCrypto.Init(_inputKey);
     _outputCrypto.Init(_outputKey);
 
-    _initialized = true;
-
     _previousTimestamp = WorldTimer::getMSTime();
 }
 
@@ -305,23 +305,20 @@ void WardenWin::RequestData()
     pkt.append(buff);
     _session->SendPacket(&pkt);
 
-    _dataSent = true;
-
     std::stringstream stream;
     stream << "Sent check id's: ";
     for (std::list<uint16>::iterator itr = _currentChecks.begin(); itr != _currentChecks.end(); ++itr)
         stream << *itr << " ";
 
     sLog.outWarden("%s", stream.str().c_str());
+
+    Warden::RequestData();
 }
 
 void WardenWin::HandleData(ByteBuffer &buff)
 {
     sLog.outWarden("Handle data");
 
-    _dataSent = false;
-    _clientResponseTimer = 0;
-
     uint16 Length;
     buff >> Length;
     uint32 Checksum;

src/game/WorldHandlers/ChatHandler.cpp

@@ -52,6 +52,13 @@ bool WorldSession::processChatmessageFurtherAfterSecurityChecks(std::string& msg
 {
     if (lang != LANG_ADDON)
     {
+        //client can't send more than 255 character's, so if we break limit - that's cheater
+        if (msg.size() > 255)
+        {
+            sLog.outError("Player %s (GUID: %u) tries send a chatmessage with more than 255 symbols", GetPlayer()->GetName(), GetPlayer()->GetGUIDLow());
+            return false;
+        }
+
         // strip invisible characters for non-addon messages
         if (sWorld.getConfig(CONFIG_BOOL_CHAT_FAKE_MESSAGE_PREVENTING))
             { stripLineInvisibleChars(msg); }
@@ -93,7 +100,10 @@ void WorldSession::HandleMessagechatOpcode(WorldPacket& recv_data)
         SendNotification(LANG_UNKNOWN_LANGUAGE);
         return;
     }
-    if (langDesc->skill_id != 0 && !_player->HasSkill(langDesc->skill_id))
+
+    //prevent cheating, by sending LANG_UNIVERSAL
+    if ((langDesc->lang_id == LANG_UNIVERSAL && !sWorld.getConfig(CONFIG_BOOL_ALLOW_TWO_SIDE_INTERACTION_CHAT) && GetSecurity() == SEC_PLAYER) ||
+         (langDesc->skill_id != 0 && !_player->HasSkill(langDesc->skill_id)))
     {
         SendNotification(LANG_NOT_LEARNED_LANGUAGE);
         return;
@@ -103,7 +113,7 @@ void WorldSession::HandleMessagechatOpcode(WorldPacket& recv_data)
     {
         // Disabled addon channel?
         if (!sWorld.getConfig(CONFIG_BOOL_ADDON_CHANNEL))
-            { return; }
+        { return; }
     }
     // LANG_ADDON should not be changed nor be affected by flood control
     else

src/game/WorldHandlers/Spell.cpp

@@ -4084,6 +4084,11 @@ SpellCastResult Spell::CheckCast(bool strict)
             if (bg->GetStatus() == STATUS_WAIT_LEAVE)
                 { return SPELL_FAILED_DONT_REPORT; }
 
+    if (!m_IsTriggeredSpell && m_spellInfo->Id == 2479) //honorless target as non-triggered spell
+    {
+        return SPELL_FAILED_DONT_REPORT;
+    }
+
     if (!m_IsTriggeredSpell && IsNonCombatSpell(m_spellInfo) &&
         m_caster->IsInCombat())
         { return SPELL_FAILED_AFFECTING_COMBAT; }