diff --git a/src/server/game/Handlers/MiscHandler.cpp b/src/server/game/Handlers/MiscHandler.cpp index 16d0d943e7..f28cea8b11 100644 --- a/src/server/game/Handlers/MiscHandler.cpp +++ b/src/server/game/Handlers/MiscHandler.cpp @@ -1635,35 +1635,37 @@ void WorldSession::HandleUpdateAccountData(WorldPacket& recvData) SF_LOG_DEBUG("network", "WORLD: Received CMSG_UPDATE_ACCOUNT_DATA"); uint32 timestamp = 0, decompressedSize = 0, compCount = 0; - uint8 type = 0; recvData >> decompressedSize >> timestamp >> compCount; - std::size_t const typePosition = recvData.rpos() + compCount; - if (typePosition >= recvData.size()) + auto readAccountDataType = [&recvData](AccountDataType& dataType) -> bool { - recvData.rfinish(); - SF_LOG_DEBUG("network", "UAD: Account data packet missing type bits"); - return; - } + if (recvData.rpos() >= recvData.size()) + { + recvData.rfinish(); + SF_LOG_DEBUG("network", "UAD: Account data packet missing type bits"); + return false; + } - type = recvData[typePosition] >> 5; - AccountDataType UADType = AccountDataType(type); - if (UADType >= AccountDataType::NUM_ACCOUNT_DATA_TYPES) - { - recvData.rfinish(); - SF_LOG_DEBUG("network", "UAD: Unknown account data type: %u", type); - return; - } + uint8 const type = uint8(recvData.ReadBits(3)); + dataType = AccountDataType(type); + if (dataType >= AccountDataType::NUM_ACCOUNT_DATA_TYPES) + { + recvData.rfinish(); + SF_LOG_DEBUG("network", "UAD: Unknown account data type: %u", type); + return false; + } + + return true; + }; + + AccountDataType UADType = AccountDataType::NUM_ACCOUNT_DATA_TYPES; if (decompressedSize == 0) // erase { + if (!readAccountDataType(UADType)) + return; + SetAccountData(UADType, 0, ""); - - WorldPacket data(SMSG_UPDATE_ACCOUNT_DATA_COMPLETE, 4 + 4); - data << uint32(type); - data << uint32(0); - SendPacket(&data); - return; } @@ -1674,6 +1676,13 @@ void WorldSession::HandleUpdateAccountData(WorldPacket& recvData) return; } + if (compCount > recvData.size() - recvData.rpos()) + { + recvData.rfinish(); + SF_LOG_DEBUG("network", "UAD: Account data packet too short, compressed size %u", compCount); + return; + } + ByteBuffer dest; dest.resize(decompressedSize); @@ -1685,19 +1694,14 @@ void WorldSession::HandleUpdateAccountData(WorldPacket& recvData) return; } - recvData.rpos(recvData.rpos() + compCount); + std::string adata = dest.ReadString(decompressedSize); - recvData.ReadBits(3); + recvData.read_skip(compCount); - std::string adata; - dest >> adata; + if (!readAccountDataType(UADType)) + return; SetAccountData(UADType, timestamp, adata); - - WorldPacket data(SMSG_UPDATE_ACCOUNT_DATA_COMPLETE, 4 + 4); - data << uint32(UADType); - data << uint32(0); - SendPacket(&data); } void WorldSession::HandleRequestAccountData(WorldPacket& recvData) diff --git a/src/server/game/Server/Protocol/Opcodes.cpp b/src/server/game/Server/Protocol/Opcodes.cpp index 829a98eb27..1b2809f4cc 100644 --- a/src/server/game/Server/Protocol/Opcodes.cpp +++ b/src/server/game/Server/Protocol/Opcodes.cpp @@ -1083,7 +1083,6 @@ void OpcodeTable::InitializeServerTable() DEFINE_OPCODE_HANDLER(SMSG_TUTORIAL_FLAGS, 0x1B90, STATUS_NEVER ); // 5.4.8 18414 DEFINE_OPCODE_HANDLER(SMSG_UI_TIME, 0x0027, STATUS_NEVER ); // 5.4.8 18414 DEFINE_OPCODE_HANDLER(SMSG_UPDATE_ACCOUNT_DATA, 0x0AAE, STATUS_NEVER ); // 5.4.8 18414 - DEFINE_OPCODE_HANDLER(SMSG_UPDATE_ACCOUNT_DATA_COMPLETE, 0x0000, STATUS_UNHANDLED); // 5.4.8 18414 DEFINE_OPCODE_HANDLER(SMSG_UPDATE_ACTION_BUTTONS, 0x081A, STATUS_NEVER ); // 5.4.8 18414 DEFINE_OPCODE_HANDLER(SMSG_UPDATE_COMBO_POINTS, 0x082F, STATUS_NEVER ); // 5.4.8 18414 DEFINE_OPCODE_HANDLER(SMSG_UPDATE_CURRENCY, 0x129E, STATUS_NEVER ); // 5.4.8 18414 diff --git a/src/server/game/Server/Protocol/Opcodes.h b/src/server/game/Server/Protocol/Opcodes.h index d133f978b8..8083b1a336 100644 --- a/src/server/game/Server/Protocol/Opcodes.h +++ b/src/server/game/Server/Protocol/Opcodes.h @@ -1051,7 +1051,6 @@ enum Opcodes SMSG_TUTORIAL_FLAGS, SMSG_UI_TIME, SMSG_UPDATE_ACCOUNT_DATA, - SMSG_UPDATE_ACCOUNT_DATA_COMPLETE, SMSG_UPDATE_ACTION_BUTTONS, SMSG_UPDATE_COMBO_POINTS, SMSG_UPDATE_CURRENCY, diff --git a/src/tests/AccountDataUpdateHandlerTests.cpp b/src/tests/AccountDataUpdateHandlerTests.cpp new file mode 100644 index 0000000000..4ffa96cc1c --- /dev/null +++ b/src/tests/AccountDataUpdateHandlerTests.cpp @@ -0,0 +1,96 @@ +/* +* This file is part of Project SkyFire https://www.projectskyfire.org. +* See LICENSE.md file for Copyright information +*/ + +#include +#include +#include +#include +#include + +namespace +{ + bool Expect(bool condition, char const* message) + { + if (!condition) + std::cerr << message << '\n'; + + return condition; + } + + std::string ReadFile(std::string const& path) + { + std::ifstream input(path.c_str()); + if (!input) + { + std::cerr << "Could not open file: " << path << '\n'; + return ""; + } + + std::ostringstream buffer; + buffer << input.rdbuf(); + return buffer.str(); + } + + std::string ExtractFunction(std::string const& source, char const* signature) + { + size_t const start = source.find(signature); + if (start == std::string::npos) + return ""; + + size_t const openBrace = source.find('{', start); + if (openBrace == std::string::npos) + return ""; + + uint32_t depth = 0; + for (size_t i = openBrace; i < source.size(); ++i) + { + if (source[i] == '{') + ++depth; + else if (source[i] == '}') + { + if (--depth == 0) + return source.substr(start, i - start + 1); + } + } + + return ""; + } + + bool Contains(std::string const& haystack, char const* needle) + { + return haystack.find(needle) != std::string::npos; + } +} + +int main() +{ + std::string const miscHandler = + ReadFile(std::string(SKYFIRE_SOURCE_DIR) + "/src/server/game/Handlers/MiscHandler.cpp"); + std::string const handleUpdateAccountData = + ExtractFunction(miscHandler, "void WorldSession::HandleUpdateAccountData(WorldPacket& recvData)"); + std::string const opcodes = + ReadFile(std::string(SKYFIRE_SOURCE_DIR) + "/src/server/game/Server/Protocol/Opcodes.cpp"); + std::string const opcodeHeader = + ReadFile(std::string(SKYFIRE_SOURCE_DIR) + "/src/server/game/Server/Protocol/Opcodes.h"); + + bool passed = true; + passed &= Expect(!handleUpdateAccountData.empty(), + "HandleUpdateAccountData should exist in MiscHandler.cpp."); + passed &= Expect(!Contains(handleUpdateAccountData, "SMSG_UPDATE_ACCOUNT_DATA_COMPLETE"), + "HandleUpdateAccountData should not send an account data completion response."); + passed &= Expect(!Contains(opcodes, "DEFINE_OPCODE_HANDLER(SMSG_UPDATE_ACCOUNT_DATA_COMPLETE"), + "The unused account data completion response should not be registered in the opcode table."); + passed &= Expect(!Contains(opcodeHeader, "SMSG_UPDATE_ACCOUNT_DATA_COMPLETE"), + "The unused account data completion response should not remain in the server opcode enum."); + passed &= Expect(Contains(handleUpdateAccountData, "recvData.read_skip(compCount)") && + Contains(handleUpdateAccountData, "recvData.ReadBits(3)"), + "HandleUpdateAccountData should read the account data type after the compressed payload."); + passed &= Expect(Contains(handleUpdateAccountData, "SetAccountData(UADType, timestamp, adata)"), + "HandleUpdateAccountData should still persist non-empty account data updates."); + passed &= Expect(Contains(handleUpdateAccountData, "SetAccountData(UADType, 0, \"\")"), + "HandleUpdateAccountData should still erase empty account data updates."); + + return passed ? 0 : 1; +} diff --git a/src/tests/CMakeLists.txt b/src/tests/CMakeLists.txt index 71dacb7c3d..c0f3b96b36 100644 --- a/src/tests/CMakeLists.txt +++ b/src/tests/CMakeLists.txt @@ -150,6 +150,14 @@ target_include_directories(account_data_utils_tests PRIVATE add_test(NAME account_data_utils_tests COMMAND account_data_utils_tests) +add_executable(account_data_update_handler_tests + AccountDataUpdateHandlerTests.cpp +) + +target_compile_definitions(account_data_update_handler_tests PRIVATE SKYFIRE_SOURCE_DIR=\"${CMAKE_SOURCE_DIR}\") + +add_test(NAME account_data_update_handler_tests COMMAND account_data_update_handler_tests) + add_executable(world_auth_identity_tests WorldAuthIdentityTests.cpp )