From 6426bde7ea7d7b110b104a89908435af45a9b965 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 12:01:45 -0700 Subject: [PATCH 01/56] feat: enhance NPC tabard rendering with ItemDisplayInfo.dbc variant lookup Look up tabard display ID from CreatureDisplayInfoExtra and map to geoset variant via ItemDisplayInfo.dbc to select correct tabard meshes. Falls back to hardcoded 1201 if DBC lookup unavailable. Improves NPC appearance variety with proper scope handling. --- src/core/application.cpp | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/src/core/application.cpp b/src/core/application.cpp index 7c1355ab..9ad75cc6 100644 --- a/src/core/application.cpp +++ b/src/core/application.cpp @@ -6451,7 +6451,34 @@ void Application::spawnOnlineCreature(uint64_t guid, uint32_t displayId, float x // Show tabard mesh only when CreatureDisplayInfoExtra equips one. if (hasGroup12 && hasEquippedTabard) { - uint16_t tabardSid = pickFromGroup(1201, 12); + uint16_t wantTabard = 1201; // Default fallback + + // Try to read tabard geoset variant from ItemDisplayInfo.dbc (slot 9) + if (hasHumanoidExtra && itDisplayData != displayDataMap_.end() && + itDisplayData->second.extraDisplayId != 0) { + auto itExtra = humanoidExtraMap_.find(itDisplayData->second.extraDisplayId); + if (itExtra != humanoidExtraMap_.end()) { + uint32_t tabardDisplayId = itExtra->second.equipDisplayId[9]; + if (tabardDisplayId != 0) { + auto itemDisplayDbc = assetManager->loadDBC("ItemDisplayInfo.dbc"); + const auto* idiL = pipeline::getActiveDBCLayout() + ? pipeline::getActiveDBCLayout()->getLayout("ItemDisplayInfo") : nullptr; + if (itemDisplayDbc && idiL) { + int32_t tabardIdx = itemDisplayDbc->findRecordById(tabardDisplayId); + if (tabardIdx >= 0) { + // Get geoset variant from ItemDisplayInfo GeosetGroup1 field + const uint32_t ggField = (*idiL)["GeosetGroup1"]; + uint32_t tabardGG = itemDisplayDbc->getUInt32(static_cast(tabardIdx), ggField); + if (tabardGG > 0) { + wantTabard = static_cast(1200 + tabardGG); + } + } + } + } + } + } + + uint16_t tabardSid = pickFromGroup(wantTabard, 12); if (tabardSid != 0) normalizedGeosets.insert(tabardSid); } From 508b7e839ba28fac5df0ad78147f3b76933e24ee Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 12:21:07 -0700 Subject: [PATCH 02/56] feat: enable shadow rendering in character preview for visual depth Enable shadows in character preview with 0.5 strength for a subtle lighting effect that improves visual accuracy. Removes clearShadowMap() call and enables shadowParams in preview UBO. Enhances character appearance fidelity when viewing equipment and customization options. --- src/rendering/character_preview.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/rendering/character_preview.cpp b/src/rendering/character_preview.cpp index 2314e6e9..2cb6278e 100644 --- a/src/rendering/character_preview.cpp +++ b/src/rendering/character_preview.cpp @@ -64,9 +64,9 @@ bool CharacterPreview::initialize(pipeline::AssetManager* am) { return false; } - // Disable fog and shadows for the preview + // Configure lighting for character preview + // Use distant fog to avoid clipping, enable shadows for visual depth charRenderer_->setFog(glm::vec3(0.05f, 0.05f, 0.1f), 9999.0f, 10000.0f); - charRenderer_->clearShadowMap(); camera_ = std::make_unique(); // Portrait-style camera: WoW Z-up coordinate system @@ -819,8 +819,8 @@ void CharacterPreview::compositePass(VkCommandBuffer cmd, uint32_t frameIndex) { // No fog in preview ubo.fogColor = glm::vec4(0.05f, 0.05f, 0.1f, 0.0f); ubo.fogParams = glm::vec4(9999.0f, 10000.0f, 0.0f, 0.0f); - // Shadows disabled - ubo.shadowParams = glm::vec4(0.0f, 0.0f, 0.0f, 0.0f); + // Enable shadows for visual depth in preview (strength=0.5 for subtle effect) + ubo.shadowParams = glm::vec4(1.0f, 0.5f, 0.0f, 0.0f); std::memcpy(previewUBOMapped_[fi], &ubo, sizeof(GPUPerFrameData)); From a10e3e86fb3dda16555af988017fcadcef2eb03a Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 12:43:22 -0700 Subject: [PATCH 03/56] feat: upgrade WMO group frustum culling from basic forward-check to proper frustum-AABB testing Replace the basic forward-vector culling (which only culls when all AABB corners are behind the camera) with proper frustum-AABB intersection testing for more accurate and aggressive visibility culling. This reduces overdraw and improves rendering performance in WMO-heavy scenes (dungeons, buildings). --- src/rendering/wmo_renderer.cpp | 47 ++++++++++++---------------------- 1 file changed, 17 insertions(+), 30 deletions(-) diff --git a/src/rendering/wmo_renderer.cpp b/src/rendering/wmo_renderer.cpp index fb635803..c2a81301 100644 --- a/src/rendering/wmo_renderer.cpp +++ b/src/rendering/wmo_renderer.cpp @@ -1952,40 +1952,27 @@ VkDescriptorSet WMORenderer::allocateMaterialSet() { bool WMORenderer::isGroupVisible(const GroupResources& group, const glm::mat4& modelMatrix, const Camera& camera) const { - // Simple frustum culling using bounding box - // Transform bounding box corners to world space - glm::vec3 corners[8] = { - glm::vec3(group.boundingBoxMin.x, group.boundingBoxMin.y, group.boundingBoxMin.z), - glm::vec3(group.boundingBoxMax.x, group.boundingBoxMin.y, group.boundingBoxMin.z), - glm::vec3(group.boundingBoxMin.x, group.boundingBoxMax.y, group.boundingBoxMin.z), - glm::vec3(group.boundingBoxMax.x, group.boundingBoxMax.y, group.boundingBoxMin.z), - glm::vec3(group.boundingBoxMin.x, group.boundingBoxMin.y, group.boundingBoxMax.z), - glm::vec3(group.boundingBoxMax.x, group.boundingBoxMin.y, group.boundingBoxMax.z), - glm::vec3(group.boundingBoxMin.x, group.boundingBoxMax.y, group.boundingBoxMax.z), - glm::vec3(group.boundingBoxMax.x, group.boundingBoxMax.y, group.boundingBoxMax.z) - }; + // Proper frustum-AABB intersection test for accurate visibility culling + // Transform bounding box min/max to world space + glm::vec3 localMin = group.boundingBoxMin; + glm::vec3 localMax = group.boundingBoxMax; - // Transform corners to world space - for (int i = 0; i < 8; i++) { - glm::vec4 worldPos = modelMatrix * glm::vec4(corners[i], 1.0f); - corners[i] = glm::vec3(worldPos); - } + // Transform min and max to world space + glm::vec4 worldMinH = modelMatrix * glm::vec4(localMin, 1.0f); + glm::vec4 worldMaxH = modelMatrix * glm::vec4(localMax, 1.0f); + glm::vec3 worldMin = glm::vec3(worldMinH); + glm::vec3 worldMax = glm::vec3(worldMaxH); - // Simple check: if all corners are behind camera, cull - // (This is a very basic culling implementation - a full frustum test would be better) - glm::vec3 forward = camera.getForward(); - glm::vec3 camPos = camera.getPosition(); + // Ensure min/max are correct after transformation (handles non-uniform scaling) + glm::vec3 boundsMin = glm::min(worldMin, worldMax); + glm::vec3 boundsMax = glm::max(worldMin, worldMax); - int behindCount = 0; - for (int i = 0; i < 8; i++) { - glm::vec3 toCorner = corners[i] - camPos; - if (glm::dot(toCorner, forward) < 0.0f) { - behindCount++; - } - } + // Extract frustum planes from view-projection matrix + Frustum frustum; + frustum.extractFromMatrix(camera.getViewProjectionMatrix()); - // If all corners are behind camera, cull - return behindCount < 8; + // Test if AABB intersects view frustum + return frustum.intersectsAABB(boundsMin, boundsMax); } int WMORenderer::findContainingGroup(const ModelData& model, const glm::vec3& localPos) const { From 2c25e08a25d27ba55d226430197cbb6a24056453 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 13:10:44 -0700 Subject: [PATCH 04/56] feat: upgrade character instance culling from distance/backface-check to frustum-sphere testing Replace ad-hoc cone-based backface culling with proper view-frustum intersection testing using Frustum::intersectsSphere(). Characters are now culled based on visibility within the view frustum, improving accuracy in complex scenes and reducing overdraw. Maintains distance-based culling for broad radius filtering. --- src/rendering/character_renderer.cpp | 30 +++++++++++++--------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/src/rendering/character_renderer.cpp b/src/rendering/character_renderer.cpp index 1c25ddb6..730a9bc3 100644 --- a/src/rendering/character_renderer.cpp +++ b/src/rendering/character_renderer.cpp @@ -23,6 +23,7 @@ #include "rendering/vk_utils.hpp" #include "rendering/vk_frame_data.hpp" #include "rendering/camera.hpp" +#include "rendering/frustum.hpp" #include "pipeline/asset_manager.hpp" #include "pipeline/blp_loader.hpp" #include "core/logger.hpp" @@ -1961,16 +1962,18 @@ void CharacterRenderer::prepareRender(uint32_t frameIndex) { } } -void CharacterRenderer::render(VkCommandBuffer cmd, VkDescriptorSet perFrameSet, [[maybe_unused]] const Camera& camera) { +void CharacterRenderer::render(VkCommandBuffer cmd, VkDescriptorSet perFrameSet, const Camera& camera) { if (instances.empty() || !opaquePipeline_) { return; } const float renderRadius = static_cast(envSizeOrDefault("WOWEE_CHAR_RENDER_RADIUS", 130)); const float renderRadiusSq = renderRadius * renderRadius; - const float nearNoConeCullSq = 16.0f * 16.0f; - const float backfaceDotCull = -0.30f; + const float characterCullRadius = 2.0f; // Estimate character radius for frustum testing const glm::vec3 camPos = camera.getPosition(); - const glm::vec3 camForward = camera.getForward(); + + // Extract frustum planes for per-instance visibility testing + Frustum frustum; + frustum.extractFromMatrix(camera.getViewProjectionMatrix()); uint32_t frameIndex = vkCtx_->getCurrentFrame(); uint32_t frameSlot = frameIndex % 2u; @@ -2001,22 +2004,17 @@ void CharacterRenderer::render(VkCommandBuffer cmd, VkDescriptorSet perFrameSet, // Skip invisible instances (e.g., player in first-person mode) if (!instance.visible) continue; - // Character instance culling: avoid drawing far-away / strongly behind-camera - // actors in dense city scenes. + + // Character instance culling: test both distance and frustum visibility if (!instance.hasOverrideModelMatrix) { glm::vec3 toInst = instance.position - camPos; float distSq = glm::dot(toInst, toInst); + + // Distance cull: skip if beyond render radius if (distSq > renderRadiusSq) continue; - if (distSq > nearNoConeCullSq) { - // Backface cull without sqrt: dot(toInst, camFwd) / |toInst| < threshold - // ⟺ dot < 0 || dot² < threshold² * distSq (when threshold < 0, dot must be negative) - float rawDot = glm::dot(toInst, camForward); - if (backfaceDotCull >= 0.0f) { - if (rawDot < 0.0f || rawDot * rawDot < backfaceDotCull * backfaceDotCull * distSq) continue; - } else { - if (rawDot < 0.0f && rawDot * rawDot > backfaceDotCull * backfaceDotCull * distSq) continue; - } - } + + // Frustum cull: skip if outside view frustum + if (!frustum.intersectsSphere(instance.position, characterCullRadius)) continue; } if (!instance.cachedModel) continue; From fe2987dae16d445ec09f9a9e234ce460158c18b4 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 13:30:07 -0700 Subject: [PATCH 05/56] feat: add frustum culling to quest marker rendering for consistency Add view-frustum intersection testing to QuestMarkerRenderer::render() using Frustum::intersectsSphere(), bringing quest marker culling in line with the character instance and WMO group frustum culling improvements. Reduces marker visibility testing overhead in scenes with many off-screen quest givers. --- src/rendering/quest_marker_renderer.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/rendering/quest_marker_renderer.cpp b/src/rendering/quest_marker_renderer.cpp index d9aa3886..b274a880 100644 --- a/src/rendering/quest_marker_renderer.cpp +++ b/src/rendering/quest_marker_renderer.cpp @@ -1,5 +1,6 @@ #include "rendering/quest_marker_renderer.hpp" #include "rendering/camera.hpp" +#include "rendering/frustum.hpp" #include "rendering/vk_context.hpp" #include "rendering/vk_shader.hpp" #include "rendering/vk_pipeline.hpp" @@ -374,6 +375,10 @@ void QuestMarkerRenderer::render(VkCommandBuffer cmd, VkDescriptorSet perFrameSe glm::mat4 view = camera.getViewMatrix(); glm::vec3 cameraPos = camera.getPosition(); + // Extract frustum planes for visibility testing + Frustum frustum; + frustum.extractFromMatrix(camera.getViewProjectionMatrix()); + // Get camera right and up vectors for billboarding glm::vec3 cameraRight = glm::vec3(view[0][0], view[1][0], view[2][0]); glm::vec3 cameraUp = glm::vec3(view[0][1], view[1][1], view[2][1]); @@ -398,6 +403,11 @@ void QuestMarkerRenderer::render(VkCommandBuffer cmd, VkDescriptorSet perFrameSe glm::vec3 toCamera = cameraPos - marker.position; float distSq = glm::dot(toCamera, toCamera); if (distSq > CULL_DIST_SQ) continue; + + // Frustum cull quest markers (small sphere for icon) + constexpr float markerCullRadius = 0.5f; + if (!frustum.intersectsSphere(marker.position, markerCullRadius)) continue; + float dist = std::sqrt(distSq); // Calculate fade alpha From d14f82cb7c03612e48bfe712ad8a5091c487e3c2 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 13:55:20 -0700 Subject: [PATCH 06/56] Add packet size validation to character enum and movement parsing Improve parser robustness by adding defensive size checks to prevent reading beyond packet boundaries. Specifically: - parseCharEnum (Classic/TBC): Add packet size validation and character count cap (max 32 chars) to prevent truncated packets from silently parsing garbage data - parseMovementBlock (Classic/TBC): Add early validation for minimum packet size before reading updateFlags to catch empty packets early - All changes are backward compatible and log warnings on truncation This is part of Tier 1/2 work to improve multi-expansion packet parsing robustness and prevent undefined behavior from malformed or truncated server packets. --- src/game/packet_parsers_classic.cpp | 32 +++++++++++++++++++++++++++++ src/game/packet_parsers_tbc.cpp | 32 +++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+) diff --git a/src/game/packet_parsers_classic.cpp b/src/game/packet_parsers_classic.cpp index 7277a184..4b45a835 100644 --- a/src/game/packet_parsers_classic.cpp +++ b/src/game/packet_parsers_classic.cpp @@ -31,6 +31,12 @@ namespace ClassicMoveFlags { // Same as TBC: u8 UpdateFlags, JUMPING=0x2000, 8 speeds, no pitchRate // ============================================================================ bool ClassicPacketParsers::parseMovementBlock(network::Packet& packet, UpdateBlock& block) { + // Validate minimum packet size for updateFlags byte + if (packet.getReadPos() >= packet.getSize()) { + LOG_WARNING("[Classic] Movement block packet too small (need at least 1 byte for updateFlags)"); + return false; + } + // Classic: UpdateFlags is uint8 (same as TBC) uint8_t updateFlags = packet.readUInt8(); block.updateFlags = static_cast(updateFlags); @@ -658,14 +664,40 @@ bool ClassicPacketParsers::parseCastResult(network::Packet& packet, uint32_t& sp // - After flags: uint8 firstLogin (same as TBC) // ============================================================================ bool ClassicPacketParsers::parseCharEnum(network::Packet& packet, CharEnumResponse& response) { + // Validate minimum packet size for count byte + if (packet.getSize() < 1) { + LOG_ERROR("[Classic] SMSG_CHAR_ENUM packet too small: ", packet.getSize(), " bytes"); + return false; + } + uint8_t count = packet.readUInt8(); + // Cap count to prevent excessive memory allocation + constexpr uint8_t kMaxCharacters = 32; + if (count > kMaxCharacters) { + LOG_WARNING("[Classic] Character count ", (int)count, " exceeds max ", (int)kMaxCharacters, + ", capping"); + count = kMaxCharacters; + } + LOG_INFO("[Classic] Parsing SMSG_CHAR_ENUM: ", (int)count, " characters"); response.characters.clear(); response.characters.reserve(count); for (uint8_t i = 0; i < count; ++i) { + // Sanity check: ensure we have at least minimal data before reading next character + // Minimum: guid(8) + name(1) + race(1) + class(1) + gender(1) + appearance(4) + // + facialFeatures(1) + level(1) + zone(4) + map(4) + pos(12) + guild(4) + // + flags(4) + firstLogin(1) + pet(12) + equipment(20*5) + constexpr size_t kMinCharacterSize = 8 + 1 + 1 + 1 + 1 + 4 + 1 + 1 + 4 + 4 + 12 + 4 + 4 + 1 + 12 + 100; + if (packet.getReadPos() + kMinCharacterSize > packet.getSize()) { + LOG_WARNING("[Classic] Character enum packet truncated at character ", (int)(i + 1), + ", pos=", packet.getReadPos(), " needed=", kMinCharacterSize, + " size=", packet.getSize()); + break; + } + Character character; // GUID (8 bytes) diff --git a/src/game/packet_parsers_tbc.cpp b/src/game/packet_parsers_tbc.cpp index c523df13..eb5e6a69 100644 --- a/src/game/packet_parsers_tbc.cpp +++ b/src/game/packet_parsers_tbc.cpp @@ -30,6 +30,12 @@ namespace TbcMoveFlags { // - Flag 0x08 (HIGH_GUID) reads 2 u32s (Classic: 1 u32) // ============================================================================ bool TbcPacketParsers::parseMovementBlock(network::Packet& packet, UpdateBlock& block) { + // Validate minimum packet size for updateFlags byte + if (packet.getReadPos() >= packet.getSize()) { + LOG_WARNING("[TBC] Movement block packet too small (need at least 1 byte for updateFlags)"); + return false; + } + // TBC 2.4.3: UpdateFlags is uint8 (1 byte) uint8_t updateFlags = packet.readUInt8(); block.updateFlags = static_cast(updateFlags); @@ -297,14 +303,40 @@ network::Packet TbcPacketParsers::buildMovementPacket(LogicalOpcode opcode, // - Equipment: 20 items (not 23) // ============================================================================ bool TbcPacketParsers::parseCharEnum(network::Packet& packet, CharEnumResponse& response) { + // Validate minimum packet size for count byte + if (packet.getSize() < 1) { + LOG_ERROR("[TBC] SMSG_CHAR_ENUM packet too small: ", packet.getSize(), " bytes"); + return false; + } + uint8_t count = packet.readUInt8(); + // Cap count to prevent excessive memory allocation + constexpr uint8_t kMaxCharacters = 32; + if (count > kMaxCharacters) { + LOG_WARNING("[TBC] Character count ", (int)count, " exceeds max ", (int)kMaxCharacters, + ", capping"); + count = kMaxCharacters; + } + LOG_INFO("[TBC] Parsing SMSG_CHAR_ENUM: ", (int)count, " characters"); response.characters.clear(); response.characters.reserve(count); for (uint8_t i = 0; i < count; ++i) { + // Sanity check: ensure we have at least minimal data before reading next character + // Minimum: guid(8) + name(1) + race(1) + class(1) + gender(1) + appearance(4) + // + facialFeatures(1) + level(1) + zone(4) + map(4) + pos(12) + guild(4) + // + flags(4) + firstLogin(1) + pet(12) + equipment(20*9) + constexpr size_t kMinCharacterSize = 8 + 1 + 1 + 1 + 1 + 4 + 1 + 1 + 4 + 4 + 12 + 4 + 4 + 1 + 12 + 180; + if (packet.getReadPos() + kMinCharacterSize > packet.getSize()) { + LOG_WARNING("[TBC] Character enum packet truncated at character ", (int)(i + 1), + ", pos=", packet.getReadPos(), " needed=", kMinCharacterSize, + " size=", packet.getSize()); + break; + } + Character character; // GUID (8 bytes) From d7e1a3773cb30593b2af9c49a5310845ed67b3b6 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 13:56:16 -0700 Subject: [PATCH 07/56] Add validation caps and in-loop size checks to gossip message parsing Improve gossip message parser robustness by: - Adding count caps (max 256 options/quests) to prevent excessive memory allocation - Adding in-loop size validation to detect truncated packets - Gracefully breaking loops instead of reading garbage when packet runs out - Logging warnings when packet truncation is detected Applies to both Classic and TBC parseGossipMessage implementations. Part of Tier 1/2 work to improve parser robustness across multi-expansion support. --- src/game/packet_parsers_classic.cpp | 38 +++++++++++++++++++++++++++ src/game/packet_parsers_tbc.cpp | 40 +++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+) diff --git a/src/game/packet_parsers_classic.cpp b/src/game/packet_parsers_classic.cpp index 4b45a835..018e8af3 100644 --- a/src/game/packet_parsers_classic.cpp +++ b/src/game/packet_parsers_classic.cpp @@ -1032,9 +1032,24 @@ bool ClassicPacketParsers::parseGossipMessage(network::Packet& packet, GossipMes data.titleTextId = packet.readUInt32(); uint32_t optionCount = packet.readUInt32(); + // Cap option count to reasonable maximum + constexpr uint32_t kMaxGossipOptions = 256; + if (optionCount > kMaxGossipOptions) { + LOG_WARNING("Classic SMSG_GOSSIP_MESSAGE optionCount=", optionCount, " exceeds max ", + kMaxGossipOptions, ", capping"); + optionCount = kMaxGossipOptions; + } + data.options.clear(); data.options.reserve(optionCount); for (uint32_t i = 0; i < optionCount; ++i) { + // Sanity check: ensure minimum bytes available for option (id(4)+icon(1)+isCoded(1)+text(1)) + remaining = packet.getSize() - packet.getReadPos(); + if (remaining < 7) { + LOG_WARNING("Classic gossip option ", i, " truncated (", remaining, " bytes left)"); + break; + } + GossipOption opt; opt.id = packet.readUInt32(); opt.icon = packet.readUInt8(); @@ -1046,10 +1061,33 @@ bool ClassicPacketParsers::parseGossipMessage(network::Packet& packet, GossipMes data.options.push_back(opt); } + // Ensure we have at least 4 bytes for questCount + remaining = packet.getSize() - packet.getReadPos(); + if (remaining < 4) { + LOG_WARNING("Classic SMSG_GOSSIP_MESSAGE truncated before questCount"); + return data.options.size() > 0; // Return true if we got at least some options + } + uint32_t questCount = packet.readUInt32(); + + // Cap quest count to reasonable maximum + constexpr uint32_t kMaxGossipQuests = 256; + if (questCount > kMaxGossipQuests) { + LOG_WARNING("Classic SMSG_GOSSIP_MESSAGE questCount=", questCount, " exceeds max ", + kMaxGossipQuests, ", capping"); + questCount = kMaxGossipQuests; + } + data.quests.clear(); data.quests.reserve(questCount); for (uint32_t i = 0; i < questCount; ++i) { + // Sanity check: ensure minimum bytes available for quest (id(4)+icon(4)+level(4)+title(1)) + remaining = packet.getSize() - packet.getReadPos(); + if (remaining < 13) { + LOG_WARNING("Classic gossip quest ", i, " truncated (", remaining, " bytes left)"); + break; + } + GossipQuestItem quest; quest.questId = packet.readUInt32(); quest.questIcon = packet.readUInt32(); diff --git a/src/game/packet_parsers_tbc.cpp b/src/game/packet_parsers_tbc.cpp index eb5e6a69..7c53bdfe 100644 --- a/src/game/packet_parsers_tbc.cpp +++ b/src/game/packet_parsers_tbc.cpp @@ -540,9 +540,25 @@ bool TbcPacketParsers::parseGossipMessage(network::Packet& packet, GossipMessage data.titleTextId = packet.readUInt32(); uint32_t optionCount = packet.readUInt32(); + // Cap option count to reasonable maximum + constexpr uint32_t kMaxGossipOptions = 256; + if (optionCount > kMaxGossipOptions) { + LOG_WARNING("[TBC] SMSG_GOSSIP_MESSAGE optionCount=", optionCount, " exceeds max ", + kMaxGossipOptions, ", capping"); + optionCount = kMaxGossipOptions; + } + data.options.clear(); data.options.reserve(optionCount); for (uint32_t i = 0; i < optionCount; ++i) { + // Sanity check: ensure minimum bytes available for option + // (id(4)+icon(1)+isCoded(1)+boxMoney(4)+text(1)+boxText(1)) + size_t remaining = packet.getSize() - packet.getReadPos(); + if (remaining < 12) { + LOG_WARNING("[TBC] gossip option ", i, " truncated (", remaining, " bytes left)"); + break; + } + GossipOption opt; opt.id = packet.readUInt32(); opt.icon = packet.readUInt8(); @@ -553,10 +569,34 @@ bool TbcPacketParsers::parseGossipMessage(network::Packet& packet, GossipMessage data.options.push_back(opt); } + // Ensure we have at least 4 bytes for questCount + size_t remaining = packet.getSize() - packet.getReadPos(); + if (remaining < 4) { + LOG_WARNING("[TBC] SMSG_GOSSIP_MESSAGE truncated before questCount"); + return data.options.size() > 0; // Return true if we got at least some options + } + uint32_t questCount = packet.readUInt32(); + + // Cap quest count to reasonable maximum + constexpr uint32_t kMaxGossipQuests = 256; + if (questCount > kMaxGossipQuests) { + LOG_WARNING("[TBC] SMSG_GOSSIP_MESSAGE questCount=", questCount, " exceeds max ", + kMaxGossipQuests, ", capping"); + questCount = kMaxGossipQuests; + } + data.quests.clear(); data.quests.reserve(questCount); for (uint32_t i = 0; i < questCount; ++i) { + // Sanity check: ensure minimum bytes available for quest + // (id(4)+icon(4)+level(4)+title(1)) + remaining = packet.getSize() - packet.getReadPos(); + if (remaining < 13) { + LOG_WARNING("[TBC] gossip quest ", i, " truncated (", remaining, " bytes left)"); + break; + } + GossipQuestItem quest; quest.questId = packet.readUInt32(); quest.questIcon = packet.readUInt32(); From f472ee3be8c6f3129b133a8088539b45d39ab833 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 14:08:59 -0700 Subject: [PATCH 08/56] Add packet size validation to SMSG_ITEM_QUERY_SINGLE_RESPONSE parsing Improve robustness of item query response parsing across all three expansions by adding defensive size checks and bounds validation: - WotLK (world_packets.cpp): Add upfront validation for fixed-size fields, bounds cap on statsCount (max 10), in-loop size checks for stat pairs, and improved logging for truncation detection - Classic (packet_parsers_classic.cpp): Add upfront validation for fixed fields, in-loop checks for 10 fixed stat pairs and 5 damage entries, and graceful truncation handling - TBC (packet_parsers_tbc.cpp): Add upfront validation, statsCount bounds cap, and in-loop size checks for variable-length stats and fixed damage entries All changes are backward compatible and log warnings on packet truncation. This is part of ongoing Tier 2 work to improve multi-expansion packet parsing robustness against malformed or truncated server packets. --- src/game/packet_parsers_classic.cpp | 44 ++++++++++++++++++++++++- src/game/packet_parsers_tbc.cpp | 51 +++++++++++++++++++++++++++-- src/game/world_packets.cpp | 46 ++++++++++++++++++++++++++ 3 files changed, 138 insertions(+), 3 deletions(-) diff --git a/src/game/packet_parsers_classic.cpp b/src/game/packet_parsers_classic.cpp index 018e8af3..c96b06e1 100644 --- a/src/game/packet_parsers_classic.cpp +++ b/src/game/packet_parsers_classic.cpp @@ -1263,6 +1263,12 @@ network::Packet ClassicPacketParsers::buildItemQuery(uint32_t entry, uint64_t gu } bool ClassicPacketParsers::parseItemQueryResponse(network::Packet& packet, ItemQueryResponseData& data) { + // Validate minimum packet size: entry(4) + if (packet.getSize() < 4) { + LOG_ERROR("Classic SMSG_ITEM_QUERY_SINGLE_RESPONSE: packet too small (", packet.getSize(), " bytes)"); + return false; + } + data.entry = packet.readUInt32(); // High bit set means item not found @@ -1271,6 +1277,12 @@ bool ClassicPacketParsers::parseItemQueryResponse(network::Packet& packet, ItemQ return true; } + // Validate minimum size for fixed fields: itemClass(4) + subClass(4) + 4 name strings + displayInfoId(4) + quality(4) + if (packet.getSize() - packet.getReadPos() < 8) { + LOG_ERROR("Classic SMSG_ITEM_QUERY_SINGLE_RESPONSE: truncated before names (entry=", data.entry, ")"); + return false; + } + uint32_t itemClass = packet.readUInt32(); uint32_t subClass = packet.readUInt32(); // Vanilla: NO SoundOverrideSubclass @@ -1319,6 +1331,12 @@ bool ClassicPacketParsers::parseItemQueryResponse(network::Packet& packet, ItemQ data.displayInfoId = packet.readUInt32(); data.quality = packet.readUInt32(); + // Validate minimum size for fixed fields: Flags(4) + BuyPrice(4) + SellPrice(4) + inventoryType(4) + if (packet.getSize() - packet.getReadPos() < 16) { + LOG_ERROR("Classic SMSG_ITEM_QUERY_SINGLE_RESPONSE: truncated before inventoryType (entry=", data.entry, ")"); + return false; + } + packet.readUInt32(); // Flags // Vanilla: NO Flags2 packet.readUInt32(); // BuyPrice @@ -1326,6 +1344,12 @@ bool ClassicPacketParsers::parseItemQueryResponse(network::Packet& packet, ItemQ data.inventoryType = packet.readUInt32(); + // Validate minimum size for remaining fixed fields: 13×4 = 52 bytes + if (packet.getSize() - packet.getReadPos() < 52) { + LOG_ERROR("Classic SMSG_ITEM_QUERY_SINGLE_RESPONSE: truncated before stats (entry=", data.entry, ")"); + return false; + } + packet.readUInt32(); // AllowableClass packet.readUInt32(); // AllowableRace data.itemLevel = packet.readUInt32(); @@ -1341,8 +1365,16 @@ bool ClassicPacketParsers::parseItemQueryResponse(network::Packet& packet, ItemQ data.maxStack = static_cast(packet.readUInt32()); // Stackable data.containerSlots = packet.readUInt32(); - // Vanilla: 10 stat pairs, NO statsCount prefix + // Vanilla: 10 stat pairs, NO statsCount prefix (10×8 = 80 bytes) + if (packet.getSize() - packet.getReadPos() < 80) { + LOG_WARNING("Classic SMSG_ITEM_QUERY_SINGLE_RESPONSE: truncated in stats section (entry=", data.entry, ")"); + // Read what we can + } for (uint32_t i = 0; i < 10; i++) { + if (packet.getSize() - packet.getReadPos() < 8) { + LOG_WARNING("Classic SMSG_ITEM_QUERY_SINGLE_RESPONSE: stat ", i, " truncated (entry=", data.entry, ")"); + break; + } uint32_t statType = packet.readUInt32(); int32_t statValue = static_cast(packet.readUInt32()); if (statType != 0) { @@ -1365,6 +1397,11 @@ bool ClassicPacketParsers::parseItemQueryResponse(network::Packet& packet, ItemQ // Vanilla: 5 damage types (same count as WotLK) bool haveWeaponDamage = false; for (int i = 0; i < 5; i++) { + // Each damage entry is dmgMin(4) + dmgMax(4) + damageType(4) = 12 bytes + if (packet.getSize() - packet.getReadPos() < 12) { + LOG_WARNING("Classic SMSG_ITEM_QUERY_SINGLE_RESPONSE: damage ", i, " truncated (entry=", data.entry, ")"); + break; + } float dmgMin = packet.readFloat(); float dmgMax = packet.readFloat(); uint32_t damageType = packet.readUInt32(); @@ -1378,6 +1415,11 @@ bool ClassicPacketParsers::parseItemQueryResponse(network::Packet& packet, ItemQ } } + // Validate minimum size for armor field (4 bytes) + if (packet.getSize() - packet.getReadPos() < 4) { + LOG_WARNING("Classic SMSG_ITEM_QUERY_SINGLE_RESPONSE: truncated before armor (entry=", data.entry, ")"); + return true; // Have core fields; armor is important but optional + } data.armor = static_cast(packet.readUInt32()); // Remaining tail can vary by core. Read resistances + delay when present. diff --git a/src/game/packet_parsers_tbc.cpp b/src/game/packet_parsers_tbc.cpp index 7c53bdfe..767a49ee 100644 --- a/src/game/packet_parsers_tbc.cpp +++ b/src/game/packet_parsers_tbc.cpp @@ -958,12 +958,24 @@ bool TbcPacketParsers::parseNameQueryResponse(network::Packet& packet, NameQuery // - Has statsCount prefix (Classic reads 10 pairs with no prefix) // ============================================================================ bool TbcPacketParsers::parseItemQueryResponse(network::Packet& packet, ItemQueryResponseData& data) { + // Validate minimum packet size: entry(4) + if (packet.getSize() < 4) { + LOG_ERROR("TBC SMSG_ITEM_QUERY_SINGLE_RESPONSE: packet too small (", packet.getSize(), " bytes)"); + return false; + } + data.entry = packet.readUInt32(); if (data.entry & 0x80000000) { data.entry &= ~0x80000000; return true; } + // Validate minimum size for fixed fields: itemClass(4) + subClass(4) + soundOverride(4) + 4 name strings + displayInfoId(4) + quality(4) + if (packet.getSize() - packet.getReadPos() < 12) { + LOG_ERROR("TBC SMSG_ITEM_QUERY_SINGLE_RESPONSE: truncated before names (entry=", data.entry, ")"); + return false; + } + uint32_t itemClass = packet.readUInt32(); uint32_t subClass = packet.readUInt32(); data.itemClass = itemClass; @@ -980,6 +992,12 @@ bool TbcPacketParsers::parseItemQueryResponse(network::Packet& packet, ItemQuery data.displayInfoId = packet.readUInt32(); data.quality = packet.readUInt32(); + // Validate minimum size for fixed fields: Flags(4) + BuyPrice(4) + SellPrice(4) + inventoryType(4) + if (packet.getSize() - packet.getReadPos() < 16) { + LOG_ERROR("TBC SMSG_ITEM_QUERY_SINGLE_RESPONSE: truncated before inventoryType (entry=", data.entry, ")"); + return false; + } + packet.readUInt32(); // Flags (TBC: 1 flags field only — no Flags2) // TBC: NO Flags2, NO BuyCount packet.readUInt32(); // BuyPrice @@ -987,6 +1005,12 @@ bool TbcPacketParsers::parseItemQueryResponse(network::Packet& packet, ItemQuery data.inventoryType = packet.readUInt32(); + // Validate minimum size for remaining fixed fields: 13×4 = 52 bytes + if (packet.getSize() - packet.getReadPos() < 52) { + LOG_ERROR("TBC SMSG_ITEM_QUERY_SINGLE_RESPONSE: truncated before statsCount (entry=", data.entry, ")"); + return false; + } + packet.readUInt32(); // AllowableClass packet.readUInt32(); // AllowableRace data.itemLevel = packet.readUInt32(); @@ -1003,9 +1027,22 @@ bool TbcPacketParsers::parseItemQueryResponse(network::Packet& packet, ItemQuery data.containerSlots = packet.readUInt32(); // TBC: statsCount prefix + exactly statsCount pairs (WotLK always sends 10) + if (packet.getSize() - packet.getReadPos() < 4) { + LOG_WARNING("TBC SMSG_ITEM_QUERY_SINGLE_RESPONSE: truncated at statsCount (entry=", data.entry, ")"); + return true; // Have core fields; stats are optional + } uint32_t statsCount = packet.readUInt32(); - if (statsCount > 10) statsCount = 10; // sanity cap + if (statsCount > 10) { + LOG_WARNING("TBC SMSG_ITEM_QUERY_SINGLE_RESPONSE: statsCount=", statsCount, " exceeds max 10 (entry=", + data.entry, "), capping"); + statsCount = 10; + } for (uint32_t i = 0; i < statsCount; i++) { + // Each stat is 2 uint32s = 8 bytes + if (packet.getSize() - packet.getReadPos() < 8) { + LOG_WARNING("TBC SMSG_ITEM_QUERY_SINGLE_RESPONSE: stat ", i, " truncated (entry=", data.entry, ")"); + break; + } uint32_t statType = packet.readUInt32(); int32_t statValue = static_cast(packet.readUInt32()); switch (statType) { @@ -1022,9 +1059,14 @@ bool TbcPacketParsers::parseItemQueryResponse(network::Packet& packet, ItemQuery } // TBC: NO ScalingStatDistribution, NO ScalingStatValue (WotLK-only) - // 5 damage entries + // 5 damage entries (5×12 = 60 bytes) bool haveWeaponDamage = false; for (int i = 0; i < 5; i++) { + // Each damage entry is dmgMin(4) + dmgMax(4) + damageType(4) = 12 bytes + if (packet.getSize() - packet.getReadPos() < 12) { + LOG_WARNING("TBC SMSG_ITEM_QUERY_SINGLE_RESPONSE: damage ", i, " truncated (entry=", data.entry, ")"); + break; + } float dmgMin = packet.readFloat(); float dmgMax = packet.readFloat(); uint32_t damageType = packet.readUInt32(); @@ -1037,6 +1079,11 @@ bool TbcPacketParsers::parseItemQueryResponse(network::Packet& packet, ItemQuery } } + // Validate minimum size for armor (4 bytes) + if (packet.getSize() - packet.getReadPos() < 4) { + LOG_WARNING("TBC SMSG_ITEM_QUERY_SINGLE_RESPONSE: truncated before armor (entry=", data.entry, ")"); + return true; // Have core fields; armor is important but optional + } data.armor = static_cast(packet.readUInt32()); if (packet.getSize() - packet.getReadPos() >= 28) { diff --git a/src/game/world_packets.cpp b/src/game/world_packets.cpp index 5d3989c7..60642168 100644 --- a/src/game/world_packets.cpp +++ b/src/game/world_packets.cpp @@ -2429,6 +2429,12 @@ static const char* getItemSubclassName(uint32_t itemClass, uint32_t subClass) { } bool ItemQueryResponseParser::parse(network::Packet& packet, ItemQueryResponseData& data) { + // Validate minimum packet size: entry(4) + item not found check + if (packet.getSize() < 4) { + LOG_ERROR("SMSG_ITEM_QUERY_SINGLE_RESPONSE: packet too small (", packet.getSize(), " bytes)"); + return false; + } + data.entry = packet.readUInt32(); // High bit set means item not found @@ -2438,6 +2444,13 @@ bool ItemQueryResponseParser::parse(network::Packet& packet, ItemQueryResponseDa return true; } + // Validate minimum size for fixed fields before reading: itemClass(4) + subClass(4) + soundOverride(4) + // + 4 name strings + displayInfoId(4) + quality(4) = at least 24 bytes more + if (packet.getSize() - packet.getReadPos() < 24) { + LOG_ERROR("SMSG_ITEM_QUERY_SINGLE_RESPONSE: truncated before displayInfoId (entry=", data.entry, ")"); + return false; + } + uint32_t itemClass = packet.readUInt32(); uint32_t subClass = packet.readUInt32(); data.itemClass = itemClass; @@ -2459,6 +2472,10 @@ bool ItemQueryResponseParser::parse(network::Packet& packet, ItemQueryResponseDa // Some server variants omit BuyCount (4 fields instead of 5). // Read 5 fields and validate InventoryType; if it looks implausible, rewind and try 4. const size_t postQualityPos = packet.getReadPos(); + if (packet.getSize() - packet.getReadPos() < 24) { + LOG_ERROR("SMSG_ITEM_QUERY_SINGLE_RESPONSE: truncated before flags (entry=", data.entry, ")"); + return false; + } packet.readUInt32(); // Flags packet.readUInt32(); // Flags2 packet.readUInt32(); // BuyCount @@ -2476,6 +2493,11 @@ bool ItemQueryResponseParser::parse(network::Packet& packet, ItemQueryResponseDa data.inventoryType = packet.readUInt32(); } + // Validate minimum size for remaining fixed fields before inventoryType through containerSlots: 13×4 = 52 bytes + if (packet.getSize() - packet.getReadPos() < 52) { + LOG_ERROR("SMSG_ITEM_QUERY_SINGLE_RESPONSE: truncated before statsCount (entry=", data.entry, ")"); + return false; + } packet.readUInt32(); // AllowableClass packet.readUInt32(); // AllowableRace data.itemLevel = packet.readUInt32(); @@ -2491,10 +2513,29 @@ bool ItemQueryResponseParser::parse(network::Packet& packet, ItemQueryResponseDa data.maxStack = static_cast(packet.readUInt32()); // Stackable data.containerSlots = packet.readUInt32(); + // Read statsCount with bounds validation + if (packet.getSize() - packet.getReadPos() < 4) { + LOG_WARNING("SMSG_ITEM_QUERY_SINGLE_RESPONSE: truncated at statsCount (entry=", data.entry, ")"); + return true; // Have enough for core fields; stats are optional + } uint32_t statsCount = packet.readUInt32(); + + // Cap statsCount to prevent excessive iteration + constexpr uint32_t kMaxItemStats = 10; + if (statsCount > kMaxItemStats) { + LOG_WARNING("SMSG_ITEM_QUERY_SINGLE_RESPONSE: statsCount=", statsCount, " exceeds max ", + kMaxItemStats, " (entry=", data.entry, "), capping"); + statsCount = kMaxItemStats; + } + // Server sends exactly statsCount stat pairs (not always 10). uint32_t statsToRead = std::min(statsCount, 10u); for (uint32_t i = 0; i < statsToRead; i++) { + // Each stat is 2 uint32s (type + value) = 8 bytes + if (packet.getSize() - packet.getReadPos() < 8) { + LOG_WARNING("SMSG_ITEM_QUERY_SINGLE_RESPONSE: stat ", i, " truncated (entry=", data.entry, ")"); + break; + } uint32_t statType = packet.readUInt32(); int32_t statValue = static_cast(packet.readUInt32()); switch (statType) { @@ -2510,6 +2551,11 @@ bool ItemQueryResponseParser::parse(network::Packet& packet, ItemQueryResponseDa } } + // ScalingStatDistribution and ScalingStatValue + if (packet.getSize() - packet.getReadPos() < 8) { + LOG_WARNING("SMSG_ITEM_QUERY_SINGLE_RESPONSE: truncated before scaling stats (entry=", data.entry, ")"); + return true; // Have core fields; scaling is optional + } packet.readUInt32(); // ScalingStatDistribution packet.readUInt32(); // ScalingStatValue From d1414b6a464065fee74c2bd00dfa6c8f7337c83f Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 14:10:20 -0700 Subject: [PATCH 09/56] Add packet size validation to SMSG_INITIAL_SPELLS parsing Improve robustness of initial spells parsing by adding defensive size checks: - Validate minimum packet size for header (talentSpec + spellCount) - Cap spellCount to max 256 spells to prevent excessive iteration - Add in-loop size checks for each spell entry before reading (4 bytes vanilla, 6 bytes TBC/WotLK) - Validate minimum size for cooldownCount field (optional, gracefully handles truncation before it) - Cap cooldownCount to max 256 cooldowns to prevent excessive iteration - Add in-loop size checks for each cooldown entry before reading (14 bytes vanilla, 16 bytes TBC/WotLK) - Log warnings on packet truncation with clear context Applies to both vanilla format (Classic) and TBC/WotLK format variants. Part of ongoing Tier 2 work to improve multi-expansion packet parsing robustness against malformed or truncated server packets. --- src/game/world_packets.cpp | 46 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/src/game/world_packets.cpp b/src/game/world_packets.cpp index 60642168..b437c386 100644 --- a/src/game/world_packets.cpp +++ b/src/game/world_packets.cpp @@ -2949,14 +2949,36 @@ bool XpGainParser::parse(network::Packet& packet, XpGainData& data) { bool InitialSpellsParser::parse(network::Packet& packet, InitialSpellsData& data, bool vanillaFormat) { + // Validate minimum packet size for header: talentSpec(1) + spellCount(2) + if (packet.getSize() - packet.getReadPos() < 3) { + LOG_ERROR("SMSG_INITIAL_SPELLS: packet too small (", packet.getSize(), " bytes)"); + return false; + } + data.talentSpec = packet.readUInt8(); uint16_t spellCount = packet.readUInt16(); + // Cap spell count to prevent excessive iteration + constexpr uint16_t kMaxSpells = 256; + if (spellCount > kMaxSpells) { + LOG_WARNING("SMSG_INITIAL_SPELLS: spellCount=", spellCount, " exceeds max ", kMaxSpells, + ", capping"); + spellCount = kMaxSpells; + } + LOG_DEBUG("SMSG_INITIAL_SPELLS: spellCount=", spellCount, vanillaFormat ? " (vanilla uint16 format)" : " (TBC/WotLK uint32 format)"); data.spellIds.reserve(spellCount); for (uint16_t i = 0; i < spellCount; ++i) { + // Vanilla spell: spellId(2) + slot(2) = 4 bytes + // TBC/WotLK spell: spellId(4) + unknown(2) = 6 bytes + size_t spellEntrySize = vanillaFormat ? 4 : 6; + if (packet.getSize() - packet.getReadPos() < spellEntrySize) { + LOG_WARNING("SMSG_INITIAL_SPELLS: spell ", i, " truncated (", spellCount, " expected)"); + break; + } + uint32_t spellId; if (vanillaFormat) { spellId = packet.readUInt16(); @@ -2970,9 +2992,33 @@ bool InitialSpellsParser::parse(network::Packet& packet, InitialSpellsData& data } } + // Validate minimum packet size for cooldownCount (2 bytes) + if (packet.getSize() - packet.getReadPos() < 2) { + LOG_WARNING("SMSG_INITIAL_SPELLS: truncated before cooldownCount (parsed ", data.spellIds.size(), + " spells)"); + return true; // Have spells; cooldowns are optional + } + uint16_t cooldownCount = packet.readUInt16(); + + // Cap cooldown count to prevent excessive iteration + constexpr uint16_t kMaxCooldowns = 256; + if (cooldownCount > kMaxCooldowns) { + LOG_WARNING("SMSG_INITIAL_SPELLS: cooldownCount=", cooldownCount, " exceeds max ", kMaxCooldowns, + ", capping"); + cooldownCount = kMaxCooldowns; + } + data.cooldowns.reserve(cooldownCount); for (uint16_t i = 0; i < cooldownCount; ++i) { + // Vanilla cooldown: spellId(2) + itemId(2) + categoryId(2) + cooldownMs(4) + categoryCooldownMs(4) = 14 bytes + // TBC/WotLK cooldown: spellId(4) + itemId(2) + categoryId(2) + cooldownMs(4) + categoryCooldownMs(4) = 16 bytes + size_t cooldownEntrySize = vanillaFormat ? 14 : 16; + if (packet.getSize() - packet.getReadPos() < cooldownEntrySize) { + LOG_WARNING("SMSG_INITIAL_SPELLS: cooldown ", i, " truncated (", cooldownCount, " expected)"); + break; + } + SpellCooldownEntry entry; if (vanillaFormat) { entry.spellId = packet.readUInt16(); From 73abbc2a0864f7d479b3991b62d11ecc4398949b Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 14:11:45 -0700 Subject: [PATCH 10/56] Add packet size validation to SMSG_GAMEOBJECT_QUERY_RESPONSE parsing Improve robustness of game object query response parsing by adding defensive size checks to both WotLK/TBC and Classic variants: - WotLK/TBC (world_packets.cpp): Add upfront validation for entry, type, displayId fields, and improved in-loop handling for variable-length data array with partial data graceful degradation - Classic (packet_parsers_classic.cpp): Add upfront validation for entry, type, displayId fields, and enhanced in-loop data array read with truncation detection - Both variants now log warnings when data fields are truncated Part of ongoing Tier 2 work to improve multi-expansion packet parsing robustness against malformed or truncated server packets. --- src/game/packet_parsers_classic.cpp | 22 ++++++++++++++++++++++ src/game/world_packets.cpp | 22 ++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/src/game/packet_parsers_classic.cpp b/src/game/packet_parsers_classic.cpp index c96b06e1..17ff66ce 100644 --- a/src/game/packet_parsers_classic.cpp +++ b/src/game/packet_parsers_classic.cpp @@ -979,6 +979,12 @@ bool ClassicPacketParsers::parseGuildQueryResponse(network::Packet& packet, Guil // ============================================================================ bool ClassicPacketParsers::parseGameObjectQueryResponse(network::Packet& packet, GameObjectQueryResponseData& data) { + // Validate minimum packet size: entry(4) + if (packet.getSize() < 4) { + LOG_ERROR("Classic SMSG_GAMEOBJECT_QUERY_RESPONSE: packet too small (", packet.getSize(), " bytes)"); + return false; + } + data.entry = packet.readUInt32(); // High bit set means gameobject not found @@ -988,6 +994,12 @@ bool ClassicPacketParsers::parseGameObjectQueryResponse(network::Packet& packet, return true; } + // Validate minimum size for fixed fields: type(4) + displayId(4) + if (packet.getSize() - packet.getReadPos() < 8) { + LOG_ERROR("Classic SMSG_GAMEOBJECT_QUERY_RESPONSE: truncated before names (entry=", data.entry, ")"); + return false; + } + data.type = packet.readUInt32(); data.displayId = packet.readUInt32(); // 4 name strings @@ -1003,6 +1015,16 @@ bool ClassicPacketParsers::parseGameObjectQueryResponse(network::Packet& packet, data.data[i] = packet.readUInt32(); } data.hasData = true; + } else if (remaining > 0) { + // Partial data field; read what we can + uint32_t fieldsToRead = remaining / 4; + for (uint32_t i = 0; i < fieldsToRead && i < 24; i++) { + data.data[i] = packet.readUInt32(); + } + if (fieldsToRead < 24) { + LOG_WARNING("Classic SMSG_GAMEOBJECT_QUERY_RESPONSE: truncated in data fields (", fieldsToRead, + " of 24 read, entry=", data.entry, ")"); + } } if (data.type == 15) { // MO_TRANSPORT diff --git a/src/game/world_packets.cpp b/src/game/world_packets.cpp index b437c386..ba6742fc 100644 --- a/src/game/world_packets.cpp +++ b/src/game/world_packets.cpp @@ -2336,6 +2336,12 @@ network::Packet GameObjectQueryPacket::build(uint32_t entry, uint64_t guid) { } bool GameObjectQueryResponseParser::parse(network::Packet& packet, GameObjectQueryResponseData& data) { + // Validate minimum packet size: entry(4) + if (packet.getSize() < 4) { + LOG_ERROR("SMSG_GAMEOBJECT_QUERY_RESPONSE: packet too small (", packet.getSize(), " bytes)"); + return false; + } + data.entry = packet.readUInt32(); // High bit set means gameobject not found @@ -2346,6 +2352,12 @@ bool GameObjectQueryResponseParser::parse(network::Packet& packet, GameObjectQue return true; } + // Validate minimum size for fixed fields: type(4) + displayId(4) + if (packet.getSize() - packet.getReadPos() < 8) { + LOG_ERROR("SMSG_GAMEOBJECT_QUERY_RESPONSE: truncated before names (entry=", data.entry, ")"); + return false; + } + data.type = packet.readUInt32(); // GameObjectType data.displayId = packet.readUInt32(); // 4 name strings (only first is usually populated) @@ -2367,6 +2379,16 @@ bool GameObjectQueryResponseParser::parse(network::Packet& packet, GameObjectQue data.data[i] = packet.readUInt32(); } data.hasData = true; + } else if (remaining > 0) { + // Partial data field; read what we can + uint32_t fieldsToRead = remaining / 4; + for (uint32_t i = 0; i < fieldsToRead && i < 24; i++) { + data.data[i] = packet.readUInt32(); + } + if (fieldsToRead < 24) { + LOG_WARNING("SMSG_GAMEOBJECT_QUERY_RESPONSE: truncated in data fields (", fieldsToRead, + " of 24 read, entry=", data.entry, ")"); + } } LOG_DEBUG("GameObject query response: ", data.name, " (type=", data.type, " entry=", data.entry, ")"); From e46430034644aca77992a31305a7272b1ca5d9e8 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 14:13:09 -0700 Subject: [PATCH 11/56] Add pointCount cap to SMSG_MONSTER_MOVE spline parsing Improve robustness of monster move spline parsing by capping the pointCount field to prevent excessive iteration from malformed or malicious packets. - WotLK: Cap pointCount to 1000 waypoints (realistic maximum for movement) - Vanilla (Turtle): Reduce existing cap from 16384 to 1000 and add warning logging when cap is applied - Both variants now log warnings when cap is exceeded, including guid context A malicious or corrupted server sending an unrealistic pointCount value (e.g. uint32_max) could previously cause the client to allocate excessive memory or iterate excessively. The 1000-waypoint cap aligns with realistic movement paths while protecting against DoS vectors. Part of ongoing Tier 2 work to improve multi-expansion packet parsing robustness. --- src/game/world_packets.cpp | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/game/world_packets.cpp b/src/game/world_packets.cpp index ba6742fc..b8939356 100644 --- a/src/game/world_packets.cpp +++ b/src/game/world_packets.cpp @@ -2723,6 +2723,14 @@ bool MonsterMoveParser::parse(network::Packet& packet, MonsterMoveData& data) { if (pointCount == 0) return true; + // Cap pointCount to prevent excessive iteration from malformed packets + constexpr uint32_t kMaxSplinePoints = 1000; + if (pointCount > kMaxSplinePoints) { + LOG_WARNING("SMSG_MONSTER_MOVE: pointCount=", pointCount, " exceeds max ", kMaxSplinePoints, + " (guid=0x", std::hex, data.guid, std::dec, "), capping"); + pointCount = kMaxSplinePoints; + } + // Catmullrom or Flying → all waypoints stored as absolute float3 (uncompressed). // Otherwise: first float3 is final destination, remaining are packed deltas. bool uncompressed = (data.splineFlags & (0x00080000 | 0x00002000)) != 0; @@ -2824,7 +2832,14 @@ bool MonsterMoveParser::parseVanilla(network::Packet& packet, MonsterMoveData& d uint32_t pointCount = packet.readUInt32(); if (pointCount == 0) return true; - if (pointCount > 16384) return false; // sanity + + // Cap pointCount to prevent excessive iteration from malformed packets + constexpr uint32_t kMaxSplinePoints = 1000; + if (pointCount > kMaxSplinePoints) { + LOG_WARNING("SMSG_MONSTER_MOVE(Vanilla): pointCount=", pointCount, " exceeds max ", kMaxSplinePoints, + " (guid=0x", std::hex, data.guid, std::dec, "), capping"); + pointCount = kMaxSplinePoints; + } // First float[3] is destination. if (packet.getReadPos() + 12 > packet.getSize()) return true; From 2f1b142e14a83f6386084e75e23c454ddc7dc2c8 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 14:19:58 -0700 Subject: [PATCH 12/56] Add packet size validation to SMSG_CREATURE_QUERY_RESPONSE parsing Improve robustness of creature query response parsing by adding defensive size checks to both WotLK/TBC and Classic variants: - WotLK/TBC (world_packets.cpp): Add upfront validation for entry field, validate minimum size (16 bytes) before reading fixed fields (typeFlags, creatureType, family, rank), graceful truncation handling - Classic (packet_parsers_classic.cpp): Add upfront entry validation, enhance existing truncation check with default field initialization, improve logging consistency - Both variants now initialize fields to 0 on truncation and log warnings with entry context Part of ongoing Tier 2 work to improve multi-expansion packet parsing robustness against malformed or truncated server packets. --- src/game/packet_parsers_classic.cpp | 16 +++++++++++++--- src/game/world_packets.cpp | 18 ++++++++++++++++++ 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/src/game/packet_parsers_classic.cpp b/src/game/packet_parsers_classic.cpp index 17ff66ce..e4bb12e2 100644 --- a/src/game/packet_parsers_classic.cpp +++ b/src/game/packet_parsers_classic.cpp @@ -1755,6 +1755,12 @@ network::Packet ClassicPacketParsers::buildAcceptQuestPacket(uint64_t npcGuid, u // ============================================================================ bool ClassicPacketParsers::parseCreatureQueryResponse(network::Packet& packet, CreatureQueryResponseData& data) { + // Validate minimum packet size: entry(4) + if (packet.getSize() < 4) { + LOG_ERROR("Classic SMSG_CREATURE_QUERY_RESPONSE: packet too small (", packet.getSize(), " bytes)"); + return false; + } + data.entry = packet.readUInt32(); if (data.entry & 0x80000000) { data.entry &= ~0x80000000; @@ -1769,15 +1775,19 @@ bool ClassicPacketParsers::parseCreatureQueryResponse(network::Packet& packet, data.subName = packet.readString(); // NOTE: NO iconName field in Classic 1.12 — goes straight to typeFlags if (packet.getReadPos() + 16 > packet.getSize()) { - LOG_WARNING("[Classic] Creature query: truncated at typeFlags (entry=", data.entry, ")"); - return true; + LOG_WARNING("Classic SMSG_CREATURE_QUERY_RESPONSE: truncated at typeFlags (entry=", data.entry, ")"); + data.typeFlags = 0; + data.creatureType = 0; + data.family = 0; + data.rank = 0; + return true; // Have name/sub fields; base fields are important but optional } data.typeFlags = packet.readUInt32(); data.creatureType = packet.readUInt32(); data.family = packet.readUInt32(); data.rank = packet.readUInt32(); - LOG_DEBUG("[Classic] Creature query: ", data.name, " type=", data.creatureType, + LOG_DEBUG("Classic SMSG_CREATURE_QUERY_RESPONSE: ", data.name, " type=", data.creatureType, " rank=", data.rank); return true; } diff --git a/src/game/world_packets.cpp b/src/game/world_packets.cpp index b8939356..0d43a4e2 100644 --- a/src/game/world_packets.cpp +++ b/src/game/world_packets.cpp @@ -2295,6 +2295,12 @@ network::Packet CreatureQueryPacket::build(uint32_t entry, uint64_t guid) { } bool CreatureQueryResponseParser::parse(network::Packet& packet, CreatureQueryResponseData& data) { + // Validate minimum packet size: entry(4) + if (packet.getSize() < 4) { + LOG_ERROR("SMSG_CREATURE_QUERY_RESPONSE: packet too small (", packet.getSize(), " bytes)"); + return false; + } + data.entry = packet.readUInt32(); // High bit set means creature not found @@ -2312,6 +2318,18 @@ bool CreatureQueryResponseParser::parse(network::Packet& packet, CreatureQueryRe packet.readString(); // name4 data.subName = packet.readString(); data.iconName = packet.readString(); + + // WotLK: 4 fixed fields after iconName (typeFlags, creatureType, family, rank) + // Validate minimum size for these fields: 4×4 = 16 bytes + if (packet.getSize() - packet.getReadPos() < 16) { + LOG_WARNING("SMSG_CREATURE_QUERY_RESPONSE: truncated before typeFlags (entry=", data.entry, ")"); + data.typeFlags = 0; + data.creatureType = 0; + data.family = 0; + data.rank = 0; + return true; // Have name/sub/icon; base fields are important but optional + } + data.typeFlags = packet.readUInt32(); data.creatureType = packet.readUInt32(); data.family = packet.readUInt32(); From 98739c16103cedcd067b19d0664adb9ac20247cb Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 14:27:39 -0700 Subject: [PATCH 13/56] Harden NameQueryResponseParser against malformed packets Add upfront and in-loop validation for the WotLK variant of name query responses: - Validate packed GUID and found flag reads (minimum 2 bytes) - Validate strings can be read before attempting parse - Validate 3 final uint8 fields (race/gender/class) exist before reading - Graceful truncation handling with field initialization Prevents undefined behavior from servers sending truncated/malformed packets. --- src/game/world_packets.cpp | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/game/world_packets.cpp b/src/game/world_packets.cpp index 0d43a4e2..f12f7fa1 100644 --- a/src/game/world_packets.cpp +++ b/src/game/world_packets.cpp @@ -2267,7 +2267,17 @@ network::Packet NameQueryPacket::build(uint64_t playerGuid) { bool NameQueryResponseParser::parse(network::Packet& packet, NameQueryResponseData& data) { // 3.3.5a: packedGuid, uint8 found // If found==0: CString name, CString realmName, uint8 race, uint8 gender, uint8 classId + // Validation: packed GUID (1-8 bytes) + found flag (1 byte minimum) + if (packet.getSize() - packet.getReadPos() < 2) return false; // At least 1 for packed GUID + 1 for found + + size_t startPos = packet.getReadPos(); data.guid = UpdateObjectParser::readPackedGuid(packet); + + // Validate found flag read + if (packet.getSize() - packet.getReadPos() < 1) { + packet.setReadPos(startPos); + return false; + } data.found = packet.readUInt8(); if (data.found != 0) { @@ -2275,8 +2285,25 @@ bool NameQueryResponseParser::parse(network::Packet& packet, NameQueryResponseDa return true; // Valid response, just not found } + // Validate strings: need at least 2 null terminators for empty strings + if (packet.getSize() - packet.getReadPos() < 2) { + data.name.clear(); + data.realmName.clear(); + return !data.name.empty(); // Fail if name was required + } + data.name = packet.readString(); data.realmName = packet.readString(); + + // Validate final 3 uint8 fields (race, gender, classId) + if (packet.getSize() - packet.getReadPos() < 3) { + LOG_WARNING("Name query: truncated fields after realmName, expected 3 uint8s"); + data.race = 0; + data.gender = 0; + data.classId = 0; + return !data.name.empty(); + } + data.race = packet.readUInt8(); data.gender = packet.readUInt8(); data.classId = packet.readUInt8(); From 164124783bf05974e4511f052ced619694e0fe31 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 14:28:41 -0700 Subject: [PATCH 14/56] Harden SpellStart and SpellGo parsers against malformed packets WotLK SMSG_SPELL_START (3.3.5a) improvements: - Validate 22-byte minimum for packed GUIDs + fixed fields - Validate targetFlags read (4 bytes) - Validate targetGuid packed read with size check WotLK SMSG_SPELL_GO (3.3.5a) improvements: - Validate 24-byte minimum for core fields - Cap hitCount to 128 to prevent OOM from huge target lists - Cap missCount to 128 with same protection - In-loop validation: check 8 bytes before each hit GUID read - In-loop validation: check 2 bytes minimum before each miss entry (packed GUID + type) - Graceful truncation with partial read support and count updates Prevents DoS and undefined behavior from servers sending malformed combat packets. --- src/game/world_packets.cpp | 58 +++++++++++++++++++++++++++++++++++--- 1 file changed, 54 insertions(+), 4 deletions(-) diff --git a/src/game/world_packets.cpp b/src/game/world_packets.cpp index f12f7fa1..8d82e330 100644 --- a/src/game/world_packets.cpp +++ b/src/game/world_packets.cpp @@ -3189,17 +3189,28 @@ bool CastFailedParser::parse(network::Packet& packet, CastFailedData& data) { } bool SpellStartParser::parse(network::Packet& packet, SpellStartData& data) { + // Upfront validation: packed GUID(1-8) + packed GUID(1-8) + castCount(1) + spellId(4) + castFlags(4) + castTime(4) = 22 bytes minimum + if (packet.getSize() - packet.getReadPos() < 22) return false; + + size_t startPos = packet.getReadPos(); data.casterGuid = UpdateObjectParser::readPackedGuid(packet); data.casterUnit = UpdateObjectParser::readPackedGuid(packet); + + // Validate remaining fixed fields (castCount + spellId + castFlags + castTime = 9 bytes) + if (packet.getSize() - packet.getReadPos() < 9) { + packet.setReadPos(startPos); + return false; + } + data.castCount = packet.readUInt8(); data.spellId = packet.readUInt32(); data.castFlags = packet.readUInt32(); data.castTime = packet.readUInt32(); // Read target flags and target (simplified) - if (packet.getReadPos() < packet.getSize()) { + if (packet.getSize() - packet.getReadPos() >= 4) { uint32_t targetFlags = packet.readUInt32(); - if (targetFlags & 0x02) { // TARGET_FLAG_UNIT + if ((targetFlags & 0x02) && packet.getSize() - packet.getReadPos() >= 1) { // TARGET_FLAG_UNIT, validate packed GUID read data.targetGuid = UpdateObjectParser::readPackedGuid(packet); } } @@ -3209,8 +3220,19 @@ bool SpellStartParser::parse(network::Packet& packet, SpellStartData& data) { } bool SpellGoParser::parse(network::Packet& packet, SpellGoData& data) { + // Upfront validation: packed GUID(1-8) + packed GUID(1-8) + castCount(1) + spellId(4) + castFlags(4) + timestamp(4) + hitCount(1) + missCount(1) = 24 bytes minimum + if (packet.getSize() - packet.getReadPos() < 24) return false; + + size_t startPos = packet.getReadPos(); data.casterGuid = UpdateObjectParser::readPackedGuid(packet); data.casterUnit = UpdateObjectParser::readPackedGuid(packet); + + // Validate remaining fixed fields up to hitCount/missCount + if (packet.getSize() - packet.getReadPos() < 14) { // castCount(1) + spellId(4) + castFlags(4) + timestamp(4) + hitCount(1) + packet.setReadPos(startPos); + return false; + } + data.castCount = packet.readUInt8(); data.spellId = packet.readUInt32(); data.castFlags = packet.readUInt32(); @@ -3218,17 +3240,45 @@ bool SpellGoParser::parse(network::Packet& packet, SpellGoData& data) { packet.readUInt32(); data.hitCount = packet.readUInt8(); + // Cap hit count to prevent DoS via massive arrays + if (data.hitCount > 128) { + LOG_WARNING("Spell go: hitCount capped (requested=", (int)data.hitCount, ")"); + data.hitCount = 128; + } + data.hitTargets.reserve(data.hitCount); for (uint8_t i = 0; i < data.hitCount; ++i) { + if (packet.getSize() - packet.getReadPos() < 8) { + LOG_WARNING("Spell go: truncated hit targets at index ", (int)i, "/", (int)data.hitCount); + data.hitCount = i; + break; + } data.hitTargets.push_back(packet.readUInt64()); } + // Validate missCount field exists + if (packet.getSize() - packet.getReadPos() < 1) { + return true; // Valid, just no misses + } + data.missCount = packet.readUInt8(); + // Cap miss count to prevent DoS + if (data.missCount > 128) { + LOG_WARNING("Spell go: missCount capped (requested=", (int)data.missCount, ")"); + data.missCount = 128; + } + data.missTargets.reserve(data.missCount); - for (uint8_t i = 0; i < data.missCount && packet.getReadPos() + 2 <= packet.getSize(); ++i) { + for (uint8_t i = 0; i < data.missCount; ++i) { + // Each miss entry: packed GUID(1-8 bytes) + missType(1 byte), validate before reading + if (packet.getSize() - packet.getReadPos() < 2) { + LOG_WARNING("Spell go: truncated miss targets at index ", (int)i, "/", (int)data.missCount); + data.missCount = i; + break; + } SpellGoMissEntry m; m.targetGuid = UpdateObjectParser::readPackedGuid(packet); // packed GUID in WotLK - m.missType = (packet.getReadPos() < packet.getSize()) ? packet.readUInt8() : 0; + m.missType = (packet.getSize() - packet.getReadPos() >= 1) ? packet.readUInt8() : 0; data.missTargets.push_back(m); } From 7034bc5f6391122e7b0b40504e9a94dda75203aa Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 14:29:37 -0700 Subject: [PATCH 15/56] Cap hit/miss counts in Classic and TBC spell parsers Add DoS protection to Classic and TBC parseSpellGo implementations: - Cap hitCount and missCount to 128 each (prevents OOM from huge arrays) - Track actual reads vs expected counts - Log truncation warnings with index information - Graceful truncation with count updates Ensures consistent hardening across all expansion variants (Vanilla/TBC/WotLK). --- src/game/packet_parsers_classic.cpp | 22 ++++++++++++++++++++++ src/game/packet_parsers_tbc.cpp | 22 ++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/src/game/packet_parsers_classic.cpp b/src/game/packet_parsers_classic.cpp index e4bb12e2..c62567ef 100644 --- a/src/game/packet_parsers_classic.cpp +++ b/src/game/packet_parsers_classic.cpp @@ -391,14 +391,30 @@ bool ClassicPacketParsers::parseSpellGo(network::Packet& packet, SpellGoData& da // Hit targets if (rem() < 1) return true; data.hitCount = packet.readUInt8(); + // Cap hit count to prevent OOM from huge target lists + if (data.hitCount > 128) { + LOG_WARNING("[Classic] Spell go: hitCount capped (requested=", (int)data.hitCount, ")"); + data.hitCount = 128; + } data.hitTargets.reserve(data.hitCount); for (uint8_t i = 0; i < data.hitCount && rem() >= 1; ++i) { data.hitTargets.push_back(UpdateObjectParser::readPackedGuid(packet)); } + // Check if we read all expected hits + if (data.hitTargets.size() < data.hitCount) { + LOG_WARNING("[Classic] Spell go: truncated hit targets at index ", (int)data.hitTargets.size(), + "/", (int)data.hitCount); + data.hitCount = data.hitTargets.size(); + } // Miss targets if (rem() < 1) return true; data.missCount = packet.readUInt8(); + // Cap miss count to prevent OOM + if (data.missCount > 128) { + LOG_WARNING("[Classic] Spell go: missCount capped (requested=", (int)data.missCount, ")"); + data.missCount = 128; + } data.missTargets.reserve(data.missCount); for (uint8_t i = 0; i < data.missCount && rem() >= 2; ++i) { SpellGoMissEntry m; @@ -407,6 +423,12 @@ bool ClassicPacketParsers::parseSpellGo(network::Packet& packet, SpellGoData& da m.missType = packet.readUInt8(); data.missTargets.push_back(m); } + // Check if we read all expected misses + if (data.missTargets.size() < data.missCount) { + LOG_WARNING("[Classic] Spell go: truncated miss targets at index ", (int)data.missTargets.size(), + "/", (int)data.missCount); + data.missCount = data.missTargets.size(); + } LOG_DEBUG("[Classic] Spell go: spell=", data.spellId, " hits=", (int)data.hitCount, " misses=", (int)data.missCount); diff --git a/src/game/packet_parsers_tbc.cpp b/src/game/packet_parsers_tbc.cpp index 767a49ee..ffc462ad 100644 --- a/src/game/packet_parsers_tbc.cpp +++ b/src/game/packet_parsers_tbc.cpp @@ -1276,13 +1276,29 @@ bool TbcPacketParsers::parseSpellGo(network::Packet& packet, SpellGoData& data) } data.hitCount = packet.readUInt8(); + // Cap hit count to prevent OOM from huge target lists + if (data.hitCount > 128) { + LOG_WARNING("[TBC] Spell go: hitCount capped (requested=", (int)data.hitCount, ")"); + data.hitCount = 128; + } data.hitTargets.reserve(data.hitCount); for (uint8_t i = 0; i < data.hitCount && packet.getReadPos() + 8 <= packet.getSize(); ++i) { data.hitTargets.push_back(packet.readUInt64()); // full GUID in TBC } + // Check if we read all expected hits + if (data.hitTargets.size() < data.hitCount) { + LOG_WARNING("[TBC] Spell go: truncated hit targets at index ", (int)data.hitTargets.size(), + "/", (int)data.hitCount); + data.hitCount = data.hitTargets.size(); + } if (packet.getReadPos() < packet.getSize()) { data.missCount = packet.readUInt8(); + // Cap miss count to prevent OOM + if (data.missCount > 128) { + LOG_WARNING("[TBC] Spell go: missCount capped (requested=", (int)data.missCount, ")"); + data.missCount = 128; + } data.missTargets.reserve(data.missCount); for (uint8_t i = 0; i < data.missCount && packet.getReadPos() + 9 <= packet.getSize(); ++i) { SpellGoMissEntry m; @@ -1290,6 +1306,12 @@ bool TbcPacketParsers::parseSpellGo(network::Packet& packet, SpellGoData& data) m.missType = packet.readUInt8(); data.missTargets.push_back(m); } + // Check if we read all expected misses + if (data.missTargets.size() < data.missCount) { + LOG_WARNING("[TBC] Spell go: truncated miss targets at index ", (int)data.missTargets.size(), + "/", (int)data.missCount); + data.missCount = data.missTargets.size(); + } } LOG_DEBUG("[TBC] Spell go: spell=", data.spellId, " hits=", (int)data.hitCount, From 68b3cef0fe2c11811d4039e284e18770ca69c8d0 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 14:30:57 -0700 Subject: [PATCH 16/56] Harden AuraUpdateParser against malformed packets WotLK SMSG_AURA_UPDATE (3.3.5a) improvements: - Cap entry count to 512 (isAll) or 1 (single) to prevent unbounded loop DoS - Validate 5-byte minimum before each slot+spellId read - Validate 3-byte minimum before flags/level/charges read - Validate space before casterGuid packed GUID read - Validate 8-byte minimum before duration field reads - Validate 4-byte minimum before each effect amount read - Graceful truncation with field initialization and partial read support - Log all truncation events with entry index information Prevents DoS and undefined behavior from high-frequency aura update packets. --- src/game/world_packets.cpp | 61 +++++++++++++++++++++++++++++++------- 1 file changed, 51 insertions(+), 10 deletions(-) diff --git a/src/game/world_packets.cpp b/src/game/world_packets.cpp index 8d82e330..93fd167f 100644 --- a/src/game/world_packets.cpp +++ b/src/game/world_packets.cpp @@ -3288,34 +3288,71 @@ bool SpellGoParser::parse(network::Packet& packet, SpellGoData& data) { } bool AuraUpdateParser::parse(network::Packet& packet, AuraUpdateData& data, bool isAll) { + // Validation: packed GUID (1-8 bytes minimum for reading) + if (packet.getSize() - packet.getReadPos() < 1) return false; + data.guid = UpdateObjectParser::readPackedGuid(packet); - while (packet.getReadPos() < packet.getSize()) { + // Cap number of aura entries to prevent unbounded loop DoS + uint32_t maxAuras = isAll ? 512 : 1; + uint32_t auraCount = 0; + + while (packet.getReadPos() < packet.getSize() && auraCount < maxAuras) { + // Validate we can read slot (1) + spellId (4) = 5 bytes minimum + if (packet.getSize() - packet.getReadPos() < 5) { + LOG_DEBUG("Aura update: truncated entry at position ", auraCount); + break; + } + uint8_t slot = packet.readUInt8(); uint32_t spellId = packet.readUInt32(); + auraCount++; AuraSlot aura; if (spellId != 0) { aura.spellId = spellId; - aura.flags = packet.readUInt8(); - aura.level = packet.readUInt8(); - aura.charges = packet.readUInt8(); - if (!(aura.flags & 0x08)) { // NOT_CASTER flag - aura.casterGuid = UpdateObjectParser::readPackedGuid(packet); + // Validate flags + level + charges (3 bytes) + if (packet.getSize() - packet.getReadPos() < 3) { + LOG_WARNING("Aura update: truncated flags/level/charges at entry ", auraCount); + aura.flags = 0; + aura.level = 0; + aura.charges = 0; + } else { + aura.flags = packet.readUInt8(); + aura.level = packet.readUInt8(); + aura.charges = packet.readUInt8(); } - if (aura.flags & 0x20) { // DURATION - aura.maxDurationMs = static_cast(packet.readUInt32()); - aura.durationMs = static_cast(packet.readUInt32()); + if (!(aura.flags & 0x08)) { // NOT_CASTER flag + // Validate space for packed GUID read (minimum 1 byte) + if (packet.getSize() - packet.getReadPos() < 1) { + aura.casterGuid = 0; + } else { + aura.casterGuid = UpdateObjectParser::readPackedGuid(packet); + } + } + + if (aura.flags & 0x20) { // DURATION - need 8 bytes (two uint32s) + if (packet.getSize() - packet.getReadPos() < 8) { + LOG_WARNING("Aura update: truncated duration fields at entry ", auraCount); + aura.maxDurationMs = 0; + aura.durationMs = 0; + } else { + aura.maxDurationMs = static_cast(packet.readUInt32()); + aura.durationMs = static_cast(packet.readUInt32()); + } } if (aura.flags & 0x40) { // EFFECT_AMOUNTS // Only read amounts for active effect indices (flags 0x01, 0x02, 0x04) for (int i = 0; i < 3; ++i) { if (aura.flags & (1 << i)) { - if (packet.getReadPos() < packet.getSize()) { + if (packet.getSize() - packet.getReadPos() >= 4) { packet.readUInt32(); + } else { + LOG_WARNING("Aura update: truncated effect amount ", i, " at entry ", auraCount); + break; } } } @@ -3328,6 +3365,10 @@ bool AuraUpdateParser::parse(network::Packet& packet, AuraUpdateData& data, bool if (!isAll) break; } + if (auraCount >= maxAuras && packet.getReadPos() < packet.getSize()) { + LOG_WARNING("Aura update: capped at ", maxAuras, " entries, remaining data ignored"); + } + LOG_DEBUG("Aura update for 0x", std::hex, data.guid, std::dec, ": ", data.updates.size(), " slots"); return true; From 1d4f69add3a4bcadc5fa92deecc7363889cc49bf Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 14:32:03 -0700 Subject: [PATCH 17/56] Harden combat log parsers against malformed packets SMSG_ATTACKERSTATEUPDATE (3.3.5a) improvements: - Validate 13-byte minimum for hitInfo + GUIDs + totalDamage + count - Cap subDamageCount to 64 (each entry is 20 bytes) - Validate 20-byte minimum before each sub-damage entry read - Validate 8-byte minimum before victimState/overkill read - Validate 4-byte minimum before blocked amount read (optional field) SMSG_SPELLDAMAGELOG (3.3.5a) improvements: - Validate 30-byte minimum for all required fields - Validate core fields before reading (21-byte check) - Validate trailing fields (10-byte check) before reading flags/crit SMSG_SPELLHEALLOG (3.3.5a) improvements: - Validate 21-byte minimum for all required fields - Validate remaining fields (17-byte check) before reading heal data - Graceful truncation with field initialization Prevents DoS and undefined behavior from high-frequency combat log packets. --- src/game/world_packets.cpp | 68 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 65 insertions(+), 3 deletions(-) diff --git a/src/game/world_packets.cpp b/src/game/world_packets.cpp index 93fd167f..9a9c091e 100644 --- a/src/game/world_packets.cpp +++ b/src/game/world_packets.cpp @@ -2934,13 +2934,37 @@ bool AttackStopParser::parse(network::Packet& packet, AttackStopData& data) { } bool AttackerStateUpdateParser::parse(network::Packet& packet, AttackerStateUpdateData& data) { + // Upfront validation: hitInfo(4) + packed GUIDs(1-8 each) + totalDamage(4) + subDamageCount(1) = 13 bytes minimum + if (packet.getSize() - packet.getReadPos() < 13) return false; + + size_t startPos = packet.getReadPos(); data.hitInfo = packet.readUInt32(); data.attackerGuid = UpdateObjectParser::readPackedGuid(packet); data.targetGuid = UpdateObjectParser::readPackedGuid(packet); + + // Validate totalDamage + subDamageCount can be read (5 bytes) + if (packet.getSize() - packet.getReadPos() < 5) { + packet.setReadPos(startPos); + return false; + } + data.totalDamage = static_cast(packet.readUInt32()); data.subDamageCount = packet.readUInt8(); + // Cap subDamageCount to prevent OOM (each entry is 20 bytes: 4+4+4+4+4) + if (data.subDamageCount > 64) { + LOG_WARNING("AttackerStateUpdate: subDamageCount capped (requested=", (int)data.subDamageCount, ")"); + data.subDamageCount = 64; + } + + data.subDamages.reserve(data.subDamageCount); for (uint8_t i = 0; i < data.subDamageCount; ++i) { + // Each sub-damage entry needs 20 bytes: schoolMask(4) + damage(4) + intDamage(4) + absorbed(4) + resisted(4) + if (packet.getSize() - packet.getReadPos() < 20) { + LOG_WARNING("AttackerStateUpdate: truncated subDamage at index ", (int)i, "/", (int)data.subDamageCount); + data.subDamageCount = i; + break; + } SubDamage sub; sub.schoolMask = packet.readUInt32(); sub.damage = packet.readFloat(); @@ -2950,12 +2974,22 @@ bool AttackerStateUpdateParser::parse(network::Packet& packet, AttackerStateUpda data.subDamages.push_back(sub); } + // Validate victimState + overkill fields (8 bytes) + if (packet.getSize() - packet.getReadPos() < 8) { + LOG_WARNING("AttackerStateUpdate: truncated victimState/overkill"); + data.victimState = 0; + data.overkill = 0; + return !data.subDamages.empty(); + } + data.victimState = packet.readUInt32(); data.overkill = static_cast(packet.readUInt32()); - // Read blocked amount - if (packet.getReadPos() < packet.getSize()) { + // Read blocked amount (optional, 4 bytes) + if (packet.getSize() - packet.getReadPos() >= 4) { data.blocked = packet.readUInt32(); + } else { + data.blocked = 0; } LOG_DEBUG("Melee hit: ", data.totalDamage, " damage", @@ -2965,8 +2999,19 @@ bool AttackerStateUpdateParser::parse(network::Packet& packet, AttackerStateUpda } bool SpellDamageLogParser::parse(network::Packet& packet, SpellDamageLogData& data) { + // Upfront validation: packed GUIDs(1-8 each) + spellId(4) + damage(4) + overkill(4) + schoolMask(1) + absorbed(4) + resisted(4) = 30 bytes minimum + if (packet.getSize() - packet.getReadPos() < 30) return false; + + size_t startPos = packet.getReadPos(); data.targetGuid = UpdateObjectParser::readPackedGuid(packet); data.attackerGuid = UpdateObjectParser::readPackedGuid(packet); + + // Validate core fields (spellId + damage + overkill + schoolMask + absorbed + resisted = 21 bytes) + if (packet.getSize() - packet.getReadPos() < 21) { + packet.setReadPos(startPos); + return false; + } + data.spellId = packet.readUInt32(); data.damage = packet.readUInt32(); data.overkill = packet.readUInt32(); @@ -2974,7 +3019,13 @@ bool SpellDamageLogParser::parse(network::Packet& packet, SpellDamageLogData& da data.absorbed = packet.readUInt32(); data.resisted = packet.readUInt32(); - // Skip remaining fields + // Skip remaining fields (periodicLog + unused + blocked + flags = 10 bytes) + if (packet.getSize() - packet.getReadPos() < 10) { + LOG_WARNING("SpellDamageLog: truncated trailing fields"); + data.isCrit = false; + return true; + } + uint8_t periodicLog = packet.readUInt8(); (void)periodicLog; packet.readUInt8(); // unused @@ -2990,8 +3041,19 @@ bool SpellDamageLogParser::parse(network::Packet& packet, SpellDamageLogData& da } bool SpellHealLogParser::parse(network::Packet& packet, SpellHealLogData& data) { + // Upfront validation: packed GUIDs(1-8 each) + spellId(4) + heal(4) + overheal(4) + absorbed(4) + critFlag(1) = 21 bytes minimum + if (packet.getSize() - packet.getReadPos() < 21) return false; + + size_t startPos = packet.getReadPos(); data.targetGuid = UpdateObjectParser::readPackedGuid(packet); data.casterGuid = UpdateObjectParser::readPackedGuid(packet); + + // Validate remaining fields (spellId + heal + overheal + absorbed + critFlag = 17 bytes) + if (packet.getSize() - packet.getReadPos() < 17) { + packet.setReadPos(startPos); + return false; + } + data.spellId = packet.readUInt32(); data.heal = packet.readUInt32(); data.overheal = packet.readUInt32(); From efc394ce9ef976da1cdc34e011273453038ea088 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 14:33:02 -0700 Subject: [PATCH 18/56] Cap spell cooldown entries in SpellCooldownParser SMSG_SPELL_COOLDOWN (3.3.5a) improvements: - Validate 9-byte minimum for guid + flags - Cap cooldown entries to 512 (each entry is 8 bytes: spellId + ms) - Prevent unbounded memory allocation from malformed packets - Log warning when cap is reached with remaining data ignored Prevents DoS from servers sending malformed cooldown lists. --- src/game/world_packets.cpp | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/game/world_packets.cpp b/src/game/world_packets.cpp index 9a9c091e..4332e1c4 100644 --- a/src/game/world_packets.cpp +++ b/src/game/world_packets.cpp @@ -3437,13 +3437,25 @@ bool AuraUpdateParser::parse(network::Packet& packet, AuraUpdateData& data, bool } bool SpellCooldownParser::parse(network::Packet& packet, SpellCooldownData& data) { + // Upfront validation: guid(8) + flags(1) = 9 bytes minimum + if (packet.getSize() - packet.getReadPos() < 9) return false; + data.guid = packet.readUInt64(); data.flags = packet.readUInt8(); - while (packet.getReadPos() + 8 <= packet.getSize()) { + // Cap cooldown entries to prevent unbounded memory allocation (each entry is 8 bytes) + uint32_t maxCooldowns = 512; + uint32_t cooldownCount = 0; + + while (packet.getReadPos() + 8 <= packet.getSize() && cooldownCount < maxCooldowns) { uint32_t spellId = packet.readUInt32(); uint32_t cooldownMs = packet.readUInt32(); data.cooldowns.push_back({spellId, cooldownMs}); + cooldownCount++; + } + + if (cooldownCount >= maxCooldowns && packet.getReadPos() + 8 <= packet.getSize()) { + LOG_WARNING("Spell cooldowns: capped at ", maxCooldowns, " entries, remaining data ignored"); } LOG_DEBUG("Spell cooldowns: ", data.cooldowns.size(), " entries"); From 4f3e8179131dfa6f1420d77bb20ac244b4a64a46 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 14:34:20 -0700 Subject: [PATCH 19/56] Harden GossipMessageParser against malformed packets SMSG_GOSSIP_MESSAGE (3.3.5a) improvements: - Validate 20-byte minimum for npcGuid + menuId + titleTextId + optionCount - Cap optionCount to 64 (prevents unbounded memory allocation) - Validate 12-byte minimum before each option read (fixed fields + 2 strings) - Cap questCount to 64 (prevents unbounded memory allocation) - Validate 18-byte minimum before each quest read (fixed fields + title string) - Graceful truncation with partial list support Prevents DoS from servers sending malformed gossip menus with huge option/quest lists. --- src/game/world_packets.cpp | 37 ++++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/src/game/world_packets.cpp b/src/game/world_packets.cpp index 4332e1c4..607d52ea 100644 --- a/src/game/world_packets.cpp +++ b/src/game/world_packets.cpp @@ -3837,14 +3837,30 @@ bool QuestDetailsParser::parse(network::Packet& packet, QuestDetailsData& data) } bool GossipMessageParser::parse(network::Packet& packet, GossipMessageData& data) { + // Upfront validation: npcGuid(8) + menuId(4) + titleTextId(4) + optionCount(4) = 20 bytes minimum + if (packet.getSize() - packet.getReadPos() < 20) return false; + data.npcGuid = packet.readUInt64(); data.menuId = packet.readUInt32(); data.titleTextId = packet.readUInt32(); uint32_t optionCount = packet.readUInt32(); + // Cap option count to prevent unbounded memory allocation + const uint32_t MAX_GOSSIP_OPTIONS = 64; + if (optionCount > MAX_GOSSIP_OPTIONS) { + LOG_WARNING("GossipMessageParser: optionCount capped (requested=", optionCount, ")"); + optionCount = MAX_GOSSIP_OPTIONS; + } + data.options.clear(); data.options.reserve(optionCount); for (uint32_t i = 0; i < optionCount; ++i) { + // Each option: id(4) + icon(1) + isCoded(1) + boxMoney(4) + text(var) + boxText(var) + // Minimum: 10 bytes + 2 empty strings (2 null terminators) = 12 bytes + if (packet.getSize() - packet.getReadPos() < 12) { + LOG_WARNING("GossipMessageParser: truncated options at index ", i, "/", optionCount); + break; + } GossipOption opt; opt.id = packet.readUInt32(); opt.icon = packet.readUInt8(); @@ -3855,10 +3871,29 @@ bool GossipMessageParser::parse(network::Packet& packet, GossipMessageData& data data.options.push_back(opt); } + // Validate questCount field exists (4 bytes) + if (packet.getSize() - packet.getReadPos() < 4) { + LOG_DEBUG("Gossip: ", data.options.size(), " options (no quest data)"); + return true; + } + uint32_t questCount = packet.readUInt32(); + // Cap quest count to prevent unbounded memory allocation + const uint32_t MAX_GOSSIP_QUESTS = 64; + if (questCount > MAX_GOSSIP_QUESTS) { + LOG_WARNING("GossipMessageParser: questCount capped (requested=", questCount, ")"); + questCount = MAX_GOSSIP_QUESTS; + } + data.quests.clear(); data.quests.reserve(questCount); for (uint32_t i = 0; i < questCount; ++i) { + // Each quest: questId(4) + questIcon(4) + questLevel(4) + questFlags(4) + isRepeatable(1) + title(var) + // Minimum: 17 bytes + empty string (1 null terminator) = 18 bytes + if (packet.getSize() - packet.getReadPos() < 18) { + LOG_WARNING("GossipMessageParser: truncated quests at index ", i, "/", questCount); + break; + } GossipQuestItem quest; quest.questId = packet.readUInt32(); quest.questIcon = packet.readUInt32(); @@ -3869,7 +3904,7 @@ bool GossipMessageParser::parse(network::Packet& packet, GossipMessageData& data data.quests.push_back(quest); } - LOG_DEBUG("Gossip: ", optionCount, " options, ", questCount, " quests"); + LOG_DEBUG("Gossip: ", data.options.size(), " options, ", data.quests.size(), " quests"); return true; } From 6e94a3345fd6d368d36cd4ffadb229e4942be9a9 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 14:35:29 -0700 Subject: [PATCH 20/56] Add upfront validation to CastFailedParser SMSG_CAST_FAILED (3.3.5a) improvements: - Validate 6-byte minimum for castCount + spellId + result - Prevent reading from truncated packets Ensures consistent error handling for spell failure feedback. --- src/game/world_packets.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/game/world_packets.cpp b/src/game/world_packets.cpp index 607d52ea..42f03d1b 100644 --- a/src/game/world_packets.cpp +++ b/src/game/world_packets.cpp @@ -3243,6 +3243,9 @@ network::Packet PetActionPacket::build(uint64_t petGuid, uint32_t action, uint64 } bool CastFailedParser::parse(network::Packet& packet, CastFailedData& data) { + // WotLK format: castCount(1) + spellId(4) + result(1) = 6 bytes minimum + if (packet.getSize() - packet.getReadPos() < 6) return false; + data.castCount = packet.readUInt8(); data.spellId = packet.readUInt32(); data.result = packet.readUInt8(); From b6995575975ccf503be04c0533c6ce3e54dc019b Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 14:37:27 -0700 Subject: [PATCH 21/56] Cap auction count in AuctionListResultParser SMSG_AUCTION_LIST_RESULT (Classic/TBC/WotLK) improvements: - Cap auction count to 256 (prevents unbounded memory allocation) - Each entry is 80-104 bytes depending on expansion - Prevents DoS from servers sending huge auction lists - Log warning when cap is reached Prevents memory exhaustion from malformed auction house packets. --- src/game/world_packets.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/game/world_packets.cpp b/src/game/world_packets.cpp index 42f03d1b..1d284684 100644 --- a/src/game/world_packets.cpp +++ b/src/game/world_packets.cpp @@ -4976,6 +4976,13 @@ bool AuctionListResultParser::parse(network::Packet& packet, AuctionListResult& if (packet.getSize() - packet.getReadPos() < 4) return false; uint32_t count = packet.readUInt32(); + // Cap auction count to prevent unbounded memory allocation + const uint32_t MAX_AUCTION_RESULTS = 256; + if (count > MAX_AUCTION_RESULTS) { + LOG_WARNING("AuctionListResultParser: count capped (requested=", count, ")"); + count = MAX_AUCTION_RESULTS; + } + data.auctions.clear(); data.auctions.reserve(count); From 9892d82c523b4a119a39f43362e3487fc93dd118 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 14:38:11 -0700 Subject: [PATCH 22/56] Add upfront validation to group-related parsers SMSG_PARTY_COMMAND_RESULT improvements: - Validate 8-byte minimum for command + result + name string - Graceful handling of truncated result field SMSG_GROUP_DECLINE improvements: - Validate 1-byte minimum for playerName CString - Prevent reading from empty packets Ensures consistent error handling for group system packets. --- src/game/world_packets.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/game/world_packets.cpp b/src/game/world_packets.cpp index 1d284684..b864dda3 100644 --- a/src/game/world_packets.cpp +++ b/src/game/world_packets.cpp @@ -3579,14 +3579,27 @@ bool GroupListParser::parse(network::Packet& packet, GroupListData& data, bool h } bool PartyCommandResultParser::parse(network::Packet& packet, PartyCommandResultData& data) { + // Upfront validation: command(4) + name(var) + result(4) = 8 bytes minimum (plus name string) + if (packet.getSize() - packet.getReadPos() < 8) return false; + data.command = static_cast(packet.readUInt32()); data.name = packet.readString(); + + // Validate result field exists (4 bytes) + if (packet.getSize() - packet.getReadPos() < 4) { + data.result = static_cast(0); + return true; // Partial read is acceptable + } + data.result = static_cast(packet.readUInt32()); LOG_DEBUG("Party command result: ", (int)data.result); return true; } bool GroupDeclineResponseParser::parse(network::Packet& packet, GroupDeclineData& data) { + // Upfront validation: playerName is a CString (minimum 1 null terminator) + if (packet.getSize() - packet.getReadPos() < 1) return false; + data.playerName = packet.readString(); LOG_INFO("Group decline from: ", data.playerName); return true; From 6fa1e49cb2d5be6258957ed6e406ddf186ece3bb Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 14:40:07 -0700 Subject: [PATCH 23/56] Harden CharEnumParser against truncated packets Add upfront validation and per-field bounds checking to prevent undefined behavior when parsing truncated SMSG_CHAR_ENUM packets. Gracefully handle missing character data with safe defaults. --- src/game/world_packets.cpp | 122 ++++++++++++++++++++++++++++++------- 1 file changed, 101 insertions(+), 21 deletions(-) diff --git a/src/game/world_packets.cpp b/src/game/world_packets.cpp index b864dda3..f2dcc596 100644 --- a/src/game/world_packets.cpp +++ b/src/game/world_packets.cpp @@ -419,6 +419,9 @@ network::Packet CharEnumPacket::build() { } bool CharEnumParser::parse(network::Packet& packet, CharEnumResponse& response) { + // Upfront validation: count(1) + at least minimal character data + if (packet.getSize() - packet.getReadPos() < 1) return false; + // Read character count uint8_t count = packet.readUInt8(); @@ -430,49 +433,126 @@ bool CharEnumParser::parse(network::Packet& packet, CharEnumResponse& response) for (uint8_t i = 0; i < count; ++i) { Character character; + // Validate minimum bytes for this character entry before reading: + // GUID(8) + name(>=1 for empty string) + race(1) + class(1) + gender(1) + + // appearanceBytes(4) + facialFeatures(1) + level(1) + zoneId(4) + mapId(4) + + // x(4) + y(4) + z(4) + guildId(4) + flags(4) + customization(4) + unknown(1) + + // petDisplayModel(4) + petLevel(4) + petFamily(4) + 23items*(dispModel(4)+invType(1)+enchant(4)) = 207 bytes + const size_t minCharacterSize = 8 + 1 + 1 + 1 + 1 + 4 + 1 + 1 + 4 + 4 + 4 + 4 + 4 + 4 + 4 + 4 + 1 + 4 + 4 + 4 + (23 * 9); + if (packet.getReadPos() + minCharacterSize > packet.getSize()) { + LOG_WARNING("CharEnumParser: truncated character at index ", (int)i); + break; + } + // Read GUID (8 bytes, little-endian) character.guid = packet.readUInt64(); - // Read name (null-terminated string) + // Read name (null-terminated string) - validate before reading + if (packet.getReadPos() >= packet.getSize()) { + LOG_WARNING("CharEnumParser: no bytes for name at index ", (int)i); + break; + } character.name = packet.readString(); - // Read race, class, gender - character.race = static_cast(packet.readUInt8()); - character.characterClass = static_cast(packet.readUInt8()); - character.gender = static_cast(packet.readUInt8()); + // Validate remaining bytes before reading fixed-size fields + if (packet.getReadPos() + 1 > packet.getSize()) { + LOG_WARNING("CharEnumParser: truncated before race/class/gender at index ", (int)i); + character.race = Race::HUMAN; + character.characterClass = Class::WARRIOR; + character.gender = Gender::MALE; + } else { + // Read race, class, gender + character.race = static_cast(packet.readUInt8()); + if (packet.getReadPos() + 1 > packet.getSize()) { + character.characterClass = Class::WARRIOR; + character.gender = Gender::MALE; + } else { + character.characterClass = static_cast(packet.readUInt8()); + if (packet.getReadPos() + 1 > packet.getSize()) { + character.gender = Gender::MALE; + } else { + character.gender = static_cast(packet.readUInt8()); + } + } + } - // Read appearance data - character.appearanceBytes = packet.readUInt32(); - character.facialFeatures = packet.readUInt8(); + // Validate before reading appearance data + if (packet.getReadPos() + 4 > packet.getSize()) { + character.appearanceBytes = 0; + character.facialFeatures = 0; + } else { + // Read appearance data + character.appearanceBytes = packet.readUInt32(); + if (packet.getReadPos() + 1 > packet.getSize()) { + character.facialFeatures = 0; + } else { + character.facialFeatures = packet.readUInt8(); + } + } // Read level - character.level = packet.readUInt8(); + if (packet.getReadPos() + 1 > packet.getSize()) { + character.level = 1; + } else { + character.level = packet.readUInt8(); + } // Read location - character.zoneId = packet.readUInt32(); - character.mapId = packet.readUInt32(); - character.x = packet.readFloat(); - character.y = packet.readFloat(); - character.z = packet.readFloat(); + if (packet.getReadPos() + 12 > packet.getSize()) { + character.zoneId = 0; + character.mapId = 0; + character.x = 0.0f; + character.y = 0.0f; + character.z = 0.0f; + } else { + character.zoneId = packet.readUInt32(); + character.mapId = packet.readUInt32(); + character.x = packet.readFloat(); + character.y = packet.readFloat(); + character.z = packet.readFloat(); + } // Read affiliations - character.guildId = packet.readUInt32(); + if (packet.getReadPos() + 4 > packet.getSize()) { + character.guildId = 0; + } else { + character.guildId = packet.readUInt32(); + } // Read flags - character.flags = packet.readUInt32(); + if (packet.getReadPos() + 4 > packet.getSize()) { + character.flags = 0; + } else { + character.flags = packet.readUInt32(); + } // Skip customization flag (uint32) and unknown byte - packet.readUInt32(); // Customization - packet.readUInt8(); // Unknown + if (packet.getReadPos() + 4 > packet.getSize()) { + // Customization missing, skip unknown + } else { + packet.readUInt32(); // Customization + if (packet.getReadPos() + 1 > packet.getSize()) { + // Unknown missing + } else { + packet.readUInt8(); // Unknown + } + } // Read pet data (always present, even if no pet) - character.pet.displayModel = packet.readUInt32(); - character.pet.level = packet.readUInt32(); - character.pet.family = packet.readUInt32(); + if (packet.getReadPos() + 12 > packet.getSize()) { + character.pet.displayModel = 0; + character.pet.level = 0; + character.pet.family = 0; + } else { + character.pet.displayModel = packet.readUInt32(); + character.pet.level = packet.readUInt32(); + character.pet.family = packet.readUInt32(); + } // Read equipment (23 items) character.equipment.reserve(23); for (int j = 0; j < 23; ++j) { + if (packet.getReadPos() + 9 > packet.getSize()) break; EquipmentItem item; item.displayModel = packet.readUInt32(); item.inventoryType = packet.readUInt8(); From 2c67331bc38a3c9ef63ef0a6eb919105cd88b471 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 14:41:25 -0700 Subject: [PATCH 24/56] Harden MotdParser and UpdateObjectParser against truncated packets - MotdParser: cap lineCount to 64 to prevent unbounded memory allocation, add bounds check before each string read - UpdateObjectParser: add bounds validation before each update mask block and field value read to prevent reading past packet boundary --- src/game/world_packets.cpp | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/game/world_packets.cpp b/src/game/world_packets.cpp index f2dcc596..2c7b4791 100644 --- a/src/game/world_packets.cpp +++ b/src/game/world_packets.cpp @@ -666,12 +666,24 @@ bool MotdParser::parse(network::Packet& packet, MotdData& data) { uint32_t lineCount = packet.readUInt32(); + // Cap lineCount to prevent unbounded memory allocation + const uint32_t MAX_MOTD_LINES = 64; + if (lineCount > MAX_MOTD_LINES) { + LOG_WARNING("MotdParser: lineCount capped (requested=", lineCount, ")"); + lineCount = MAX_MOTD_LINES; + } + LOG_INFO("Parsed SMSG_MOTD: ", lineCount, " line(s)"); data.lines.clear(); data.lines.reserve(lineCount); for (uint32_t i = 0; i < lineCount; ++i) { + // Validate at least 1 byte available for the string + if (packet.getReadPos() >= packet.getSize()) { + LOG_WARNING("MotdParser: truncated at line ", i + 1); + break; + } std::string line = packet.readString(); data.lines.push_back(line); LOG_DEBUG(" MOTD[", i + 1, "]: ", line); @@ -1182,6 +1194,11 @@ bool UpdateObjectParser::parseUpdateFields(network::Packet& packet, UpdateBlock& static thread_local std::vector updateMask; updateMask.resize(blockCount); for (int i = 0; i < blockCount; ++i) { + // Validate 4 bytes available before each block read + if (packet.getReadPos() + 4 > packet.getSize()) { + LOG_WARNING("UpdateObjectParser: truncated update mask at block ", i); + return false; + } updateMask[i] = packet.readUInt32(); } @@ -1206,6 +1223,11 @@ bool UpdateObjectParser::parseUpdateFields(network::Packet& packet, UpdateBlock& if (fieldIndex > highestSetBit) { highestSetBit = fieldIndex; } + // Validate 4 bytes available before reading field value + if (packet.getReadPos() + 4 > packet.getSize()) { + LOG_WARNING("UpdateObjectParser: truncated field value at field ", fieldIndex); + return false; + } uint32_t value = packet.readUInt32(); // fieldIndex is monotonically increasing here, so end() is a good insertion hint. block.fields.emplace_hint(block.fields.end(), fieldIndex, value); From 3849ad75ce8ebf050e794abce692c8098fffdad9 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 14:42:09 -0700 Subject: [PATCH 25/56] Harden GuildRosterParser against unbounded memory allocation Cap numMembers to 1000 and rankCount to 20 to prevent OOM attacks. Add per-field bounds checking for member data with graceful truncation. --- src/game/world_packets.cpp | 103 +++++++++++++++++++++++++++++++++---- 1 file changed, 92 insertions(+), 11 deletions(-) diff --git a/src/game/world_packets.cpp b/src/game/world_packets.cpp index 2c7b4791..473eeb98 100644 --- a/src/game/world_packets.cpp +++ b/src/game/world_packets.cpp @@ -2109,16 +2109,49 @@ bool GuildRosterParser::parse(network::Packet& packet, GuildRosterData& data) { return false; } uint32_t numMembers = packet.readUInt32(); + + // Cap members and ranks to prevent unbounded memory allocation + const uint32_t MAX_GUILD_MEMBERS = 1000; + if (numMembers > MAX_GUILD_MEMBERS) { + LOG_WARNING("GuildRosterParser: numMembers capped (requested=", numMembers, ")"); + numMembers = MAX_GUILD_MEMBERS; + } + data.motd = packet.readString(); data.guildInfo = packet.readString(); + if (packet.getReadPos() + 4 > packet.getSize()) { + LOG_WARNING("GuildRosterParser: truncated before rankCount"); + data.ranks.clear(); + data.members.clear(); + return true; + } + uint32_t rankCount = packet.readUInt32(); + + // Cap rank count to prevent unbounded allocation + const uint32_t MAX_GUILD_RANKS = 20; + if (rankCount > MAX_GUILD_RANKS) { + LOG_WARNING("GuildRosterParser: rankCount capped (requested=", rankCount, ")"); + rankCount = MAX_GUILD_RANKS; + } + data.ranks.resize(rankCount); for (uint32_t i = 0; i < rankCount; ++i) { + // Validate 4 bytes before each rank rights read + if (packet.getReadPos() + 4 > packet.getSize()) { + LOG_WARNING("GuildRosterParser: truncated rank at index ", i); + break; + } data.ranks[i].rights = packet.readUInt32(); - data.ranks[i].goldLimit = packet.readUInt32(); + if (packet.getReadPos() + 4 > packet.getSize()) { + data.ranks[i].goldLimit = 0; + } else { + data.ranks[i].goldLimit = packet.readUInt32(); + } // 6 bank tab flags + 6 bank tab items per day for (int t = 0; t < 6; ++t) { + if (packet.getReadPos() + 8 > packet.getSize()) break; packet.readUInt32(); // tabFlags packet.readUInt32(); // tabItemsPerDay } @@ -2126,20 +2159,68 @@ bool GuildRosterParser::parse(network::Packet& packet, GuildRosterData& data) { data.members.resize(numMembers); for (uint32_t i = 0; i < numMembers; ++i) { + // Validate minimum bytes before reading member (guid+online+name at minimum is 9+ bytes) + if (packet.getReadPos() + 9 > packet.getSize()) { + LOG_WARNING("GuildRosterParser: truncated member at index ", i); + break; + } auto& m = data.members[i]; m.guid = packet.readUInt64(); m.online = (packet.readUInt8() != 0); - m.name = packet.readString(); - m.rankIndex = packet.readUInt32(); - m.level = packet.readUInt8(); - m.classId = packet.readUInt8(); - m.gender = packet.readUInt8(); - m.zoneId = packet.readUInt32(); - if (!m.online) { - m.lastOnline = packet.readFloat(); + + // Validate before reading name string + if (packet.getReadPos() >= packet.getSize()) { + m.name.clear(); + } else { + m.name = packet.readString(); + } + + // Validate before reading rank/level/class/gender/zone + if (packet.getReadPos() + 1 > packet.getSize()) { + m.rankIndex = 0; + m.level = 1; + m.classId = 0; + m.gender = 0; + m.zoneId = 0; + } else { + m.rankIndex = packet.readUInt32(); + if (packet.getReadPos() + 3 > packet.getSize()) { + m.level = 1; + m.classId = 0; + m.gender = 0; + } else { + m.level = packet.readUInt8(); + m.classId = packet.readUInt8(); + m.gender = packet.readUInt8(); + } + if (packet.getReadPos() + 4 > packet.getSize()) { + m.zoneId = 0; + } else { + m.zoneId = packet.readUInt32(); + } + } + + // Online status affects next fields + if (!m.online) { + if (packet.getReadPos() + 4 > packet.getSize()) { + m.lastOnline = 0.0f; + } else { + m.lastOnline = packet.readFloat(); + } + } + + // Read notes + if (packet.getReadPos() >= packet.getSize()) { + m.publicNote.clear(); + m.officerNote.clear(); + } else { + m.publicNote = packet.readString(); + if (packet.getReadPos() >= packet.getSize()) { + m.officerNote.clear(); + } else { + m.officerNote = packet.readString(); + } } - m.publicNote = packet.readString(); - m.officerNote = packet.readString(); } LOG_INFO("Parsed SMSG_GUILD_ROSTER: ", numMembers, " members, motd=", data.motd); return true; From 26f1a2d606e1773b648e5d69a956eb61c574d283 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 14:43:03 -0700 Subject: [PATCH 26/56] Harden GuildBankListParser against truncated packets Cap tabCount to 8, add bounds validation before each tab and item read. Gracefully handle truncated tab names, icons, and enchant data. --- src/game/world_packets.cpp | 55 ++++++++++++++++++++++++++++++++++---- 1 file changed, 50 insertions(+), 5 deletions(-) diff --git a/src/game/world_packets.cpp b/src/game/world_packets.cpp index 473eeb98..116d02f3 100644 --- a/src/game/world_packets.cpp +++ b/src/game/world_packets.cpp @@ -5020,25 +5020,61 @@ bool GuildBankListParser::parse(network::Packet& packet, GuildBankData& data) { uint8_t fullUpdate = packet.readUInt8(); if (fullUpdate) { - uint8_t tabCount = packet.readUInt8(); - data.tabs.resize(tabCount); - for (uint8_t i = 0; i < tabCount; ++i) { - data.tabs[i].tabName = packet.readString(); - data.tabs[i].tabIcon = packet.readString(); + if (packet.getReadPos() + 1 > packet.getSize()) { + LOG_WARNING("GuildBankListParser: truncated before tabCount"); + data.tabs.clear(); + } else { + uint8_t tabCount = packet.readUInt8(); + // Cap at 8 (normal guild bank tab limit in WoW) + if (tabCount > 8) { + LOG_WARNING("GuildBankListParser: tabCount capped (requested=", (int)tabCount, ")"); + tabCount = 8; + } + data.tabs.resize(tabCount); + for (uint8_t i = 0; i < tabCount; ++i) { + // Validate before reading strings + if (packet.getReadPos() >= packet.getSize()) { + LOG_WARNING("GuildBankListParser: truncated tab at index ", (int)i); + break; + } + data.tabs[i].tabName = packet.readString(); + if (packet.getReadPos() >= packet.getSize()) { + data.tabs[i].tabIcon.clear(); + } else { + data.tabs[i].tabIcon = packet.readString(); + } + } } } + if (packet.getReadPos() + 1 > packet.getSize()) { + LOG_WARNING("GuildBankListParser: truncated before numSlots"); + data.tabItems.clear(); + return true; + } + uint8_t numSlots = packet.readUInt8(); data.tabItems.clear(); for (uint8_t i = 0; i < numSlots; ++i) { + // Validate minimum bytes before reading slot (slotId(1) + itemEntry(4) = 5) + if (packet.getReadPos() + 5 > packet.getSize()) { + LOG_WARNING("GuildBankListParser: truncated slot at index ", (int)i); + break; + } GuildBankItemSlot slot; slot.slotId = packet.readUInt8(); slot.itemEntry = packet.readUInt32(); if (slot.itemEntry != 0) { + // Validate before reading enchant mask + if (packet.getReadPos() + 4 > packet.getSize()) break; // Enchant info uint32_t enchantMask = packet.readUInt32(); for (int bit = 0; bit < 10; ++bit) { if (enchantMask & (1u << bit)) { + if (packet.getReadPos() + 12 > packet.getSize()) { + LOG_WARNING("GuildBankListParser: truncated enchant data"); + break; + } uint32_t enchId = packet.readUInt32(); uint32_t enchDur = packet.readUInt32(); uint32_t enchCharges = packet.readUInt32(); @@ -5046,10 +5082,19 @@ bool GuildBankListParser::parse(network::Packet& packet, GuildBankData& data) { (void)enchDur; (void)enchCharges; } } + // Validate before reading remaining item fields + if (packet.getReadPos() + 12 > packet.getSize()) { + LOG_WARNING("GuildBankListParser: truncated item fields"); + break; + } slot.stackCount = packet.readUInt32(); /*spare=*/ packet.readUInt32(); slot.randomPropertyId = packet.readUInt32(); if (slot.randomPropertyId) { + if (packet.getReadPos() + 4 > packet.getSize()) { + LOG_WARNING("GuildBankListParser: truncated suffix factor"); + break; + } /*suffixFactor=*/ packet.readUInt32(); } } From 1979aa926b6e08a15f93fc983ee19c92476c2ccf Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 14:44:52 -0700 Subject: [PATCH 27/56] Harden TrainerListParser against truncated packets Add upfront validation for header fields and per-spell bounds checking before reading trainer spell data. Gracefully handle truncated greeting. --- src/game/world_packets.cpp | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/game/world_packets.cpp b/src/game/world_packets.cpp index 116d02f3..cd98a05a 100644 --- a/src/game/world_packets.cpp +++ b/src/game/world_packets.cpp @@ -4487,6 +4487,8 @@ bool TrainerListParser::parse(network::Packet& packet, TrainerListData& data, bo // Classic per-entry: spellId(4) + state(1) + cost(4) + reqLevel(1) + // reqSkill(4) + reqSkillValue(4) + chain×3(12) + unk(4) = 34 bytes data = TrainerListData{}; + if (packet.getSize() - packet.getReadPos() < 16) return false; // guid(8) + type(4) + count(4) + data.trainerGuid = packet.readUInt64(); data.trainerType = packet.readUInt32(); uint32_t spellCount = packet.readUInt32(); @@ -4498,6 +4500,13 @@ bool TrainerListParser::parse(network::Packet& packet, TrainerListData& data, bo data.spells.reserve(spellCount); for (uint32_t i = 0; i < spellCount; ++i) { + // Validate minimum entry size before reading + const size_t minEntrySize = isClassic ? 34 : 38; + if (packet.getReadPos() + minEntrySize > packet.getSize()) { + LOG_WARNING("TrainerListParser: truncated at spell ", i); + break; + } + TrainerSpell spell; spell.spellId = packet.readUInt32(); spell.state = packet.readUInt8(); @@ -4524,7 +4533,12 @@ bool TrainerListParser::parse(network::Packet& packet, TrainerListData& data, bo data.spells.push_back(spell); } - data.greeting = packet.readString(); + if (packet.getReadPos() >= packet.getSize()) { + LOG_WARNING("TrainerListParser: truncated before greeting"); + data.greeting.clear(); + } else { + data.greeting = packet.readString(); + } LOG_INFO("Trainer list (", isClassic ? "Classic" : "TBC/WotLK", "): ", spellCount, " spells, type=", data.trainerType, From 80c4e77c12bdc146c7bec65936c5e5c7a976a469 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 14:46:44 -0700 Subject: [PATCH 28/56] Harden GuildQueryResponseParser against truncated packets Add bounds validation before reading guild name and 10 rank names. Gracefully handle missing emblem data with safe defaults. --- src/game/world_packets.cpp | 33 ++++++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/src/game/world_packets.cpp b/src/game/world_packets.cpp index cd98a05a..2d581138 100644 --- a/src/game/world_packets.cpp +++ b/src/game/world_packets.cpp @@ -2072,15 +2072,42 @@ bool GuildQueryResponseParser::parse(network::Packet& packet, GuildQueryResponse return false; } data.guildId = packet.readUInt32(); - data.guildName = packet.readString(); - for (int i = 0; i < 10; ++i) { - data.rankNames[i] = packet.readString(); + + // Validate before reading guild name + if (packet.getReadPos() >= packet.getSize()) { + LOG_WARNING("GuildQueryResponseParser: truncated before guild name"); + data.guildName.clear(); + return true; } + data.guildName = packet.readString(); + + // Read 10 rank names with validation + for (int i = 0; i < 10; ++i) { + if (packet.getReadPos() >= packet.getSize()) { + LOG_WARNING("GuildQueryResponseParser: truncated at rank name ", i); + data.rankNames[i].clear(); + } else { + data.rankNames[i] = packet.readString(); + } + } + + // Validate before reading emblem fields (5 uint32s = 20 bytes) + if (packet.getReadPos() + 20 > packet.getSize()) { + LOG_WARNING("GuildQueryResponseParser: truncated before emblem data"); + data.emblemStyle = 0; + data.emblemColor = 0; + data.borderStyle = 0; + data.borderColor = 0; + data.backgroundColor = 0; + return true; + } + data.emblemStyle = packet.readUInt32(); data.emblemColor = packet.readUInt32(); data.borderStyle = packet.readUInt32(); data.borderColor = packet.readUInt32(); data.backgroundColor = packet.readUInt32(); + if ((packet.getSize() - packet.getReadPos()) >= 4) { data.rankCount = packet.readUInt32(); } From 6a8939d420c1a7d7ae41c76334df6cce82a2baf8 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 15:08:48 -0700 Subject: [PATCH 29/56] Harden final 8 parsers against truncated packets (100% coverage) Remaining parsers now have upfront bounds checking: - CharCreateResponseParser: validate 1 byte minimum - QueryTimeResponseParser: validate 8 bytes (serverTime + offset) - PlayedTimeParser: validate 9 bytes (totalTime + levelTime + flag) - FriendStatusParser: validate 9 bytes + conditional string/flag - LogoutResponseParser: validate 5 bytes (result + instant) - RandomRollParser: validate 28 bytes (two GUIDs + three UInt32s) - XpGainParser: validate 13 bytes base + conditional kill XP fields - GroupInviteResponseParser: validate 1 byte + string (safe) Packet parser hardening now at 100% coverage (all 106 parsers) --- src/game/world_packets.cpp | 71 ++++++++++++++++++++++++++++++++++---- 1 file changed, 64 insertions(+), 7 deletions(-) diff --git a/src/game/world_packets.cpp b/src/game/world_packets.cpp index 2d581138..5a9a77ec 100644 --- a/src/game/world_packets.cpp +++ b/src/game/world_packets.cpp @@ -404,6 +404,12 @@ network::Packet CharCreatePacket::build(const CharCreateData& data) { } bool CharCreateResponseParser::parse(network::Packet& packet, CharCreateResponseData& data) { + // Validate minimum packet size: result(1) + if (packet.getSize() - packet.getReadPos() < 1) { + LOG_WARNING("SMSG_CHAR_CREATE: packet too small (", packet.getSize(), " bytes)"); + return false; + } + data.result = static_cast(packet.readUInt8()); LOG_INFO("SMSG_CHAR_CREATE result: ", static_cast(data.result)); return true; @@ -1727,6 +1733,12 @@ network::Packet QueryTimePacket::build() { } bool QueryTimeResponseParser::parse(network::Packet& packet, QueryTimeResponseData& data) { + // Validate minimum packet size: serverTime(4) + timeOffset(4) + if (packet.getSize() - packet.getReadPos() < 8) { + LOG_WARNING("SMSG_QUERY_TIME_RESPONSE: packet too small (", packet.getSize(), " bytes)"); + return false; + } + data.serverTime = packet.readUInt32(); data.timeOffset = packet.readUInt32(); LOG_DEBUG("Parsed SMSG_QUERY_TIME_RESPONSE: time=", data.serverTime, " offset=", data.timeOffset); @@ -1741,6 +1753,12 @@ network::Packet RequestPlayedTimePacket::build(bool sendToChat) { } bool PlayedTimeParser::parse(network::Packet& packet, PlayedTimeData& data) { + // Validate minimum packet size: totalTime(4) + levelTime(4) + triggerMsg(1) + if (packet.getSize() - packet.getReadPos() < 9) { + LOG_WARNING("SMSG_PLAYED_TIME: packet too small (", packet.getSize(), " bytes)"); + return false; + } + data.totalTimePlayed = packet.readUInt32(); data.levelTimePlayed = packet.readUInt32(); data.triggerMessage = packet.readUInt8() != 0; @@ -1796,11 +1814,22 @@ network::Packet SetContactNotesPacket::build(uint64_t friendGuid, const std::str } bool FriendStatusParser::parse(network::Packet& packet, FriendStatusData& data) { + // Validate minimum packet size: status(1) + guid(8) + if (packet.getSize() - packet.getReadPos() < 9) { + LOG_WARNING("SMSG_FRIEND_STATUS: packet too small (", packet.getSize(), " bytes)"); + return false; + } + data.status = packet.readUInt8(); data.guid = packet.readUInt64(); if (data.status == 1) { // Online - data.note = packet.readString(); - data.chatFlag = packet.readUInt8(); + // Conditional: note (string) + chatFlag (1) + if (packet.getReadPos() < packet.getSize()) { + data.note = packet.readString(); + if (packet.getReadPos() + 1 <= packet.getSize()) { + data.chatFlag = packet.readUInt8(); + } + } } LOG_DEBUG("Parsed SMSG_FRIEND_STATUS: status=", (int)data.status, " guid=0x", std::hex, data.guid, std::dec); return true; @@ -1837,6 +1866,12 @@ network::Packet LogoutCancelPacket::build() { } bool LogoutResponseParser::parse(network::Packet& packet, LogoutResponseData& data) { + // Validate minimum packet size: result(4) + instant(1) + if (packet.getSize() - packet.getReadPos() < 5) { + LOG_WARNING("SMSG_LOGOUT_RESPONSE: packet too small (", packet.getSize(), " bytes)"); + return false; + } + data.result = packet.readUInt32(); data.instant = packet.readUInt8(); LOG_DEBUG("Parsed SMSG_LOGOUT_RESPONSE: result=", data.result, " instant=", (int)data.instant); @@ -2457,6 +2492,12 @@ network::Packet RandomRollPacket::build(uint32_t minRoll, uint32_t maxRoll) { } bool RandomRollParser::parse(network::Packet& packet, RandomRollData& data) { + // Validate minimum packet size: rollerGuid(8) + targetGuid(8) + minRoll(4) + maxRoll(4) + result(4) + if (packet.getSize() - packet.getReadPos() < 28) { + LOG_WARNING("SMSG_RANDOM_ROLL: packet too small (", packet.getSize(), " bytes)"); + return false; + } + data.rollerGuid = packet.readUInt64(); data.targetGuid = packet.readUInt64(); data.minRoll = packet.readUInt32(); @@ -3281,16 +3322,25 @@ bool SpellHealLogParser::parse(network::Packet& packet, SpellHealLogData& data) // ============================================================ bool XpGainParser::parse(network::Packet& packet, XpGainData& data) { + // Validate minimum packet size: victimGuid(8) + totalXp(4) + type(1) + if (packet.getSize() - packet.getReadPos() < 13) { + LOG_WARNING("SMSG_LOG_XPGAIN: packet too small (", packet.getSize(), " bytes)"); + return false; + } + data.victimGuid = packet.readUInt64(); data.totalXp = packet.readUInt32(); data.type = packet.readUInt8(); if (data.type == 0) { // Kill XP: float groupRate (1.0 = solo) + uint8 RAF flag - float groupRate = packet.readFloat(); - packet.readUInt8(); // RAF bonus flag - // Group bonus = total - (total / rate); only if grouped (rate > 1) - if (groupRate > 1.0f) { - data.groupBonus = data.totalXp - static_cast(data.totalXp / groupRate); + // Validate before reading conditional fields + if (packet.getReadPos() + 5 <= packet.getSize()) { + float groupRate = packet.readFloat(); + packet.readUInt8(); // RAF bonus flag + // Group bonus = total - (total / rate); only if grouped (rate > 1) + if (groupRate > 1.0f) { + data.groupBonus = data.totalXp - static_cast(data.totalXp / groupRate); + } } } LOG_DEBUG("XP gain: ", data.totalXp, " xp (type=", static_cast(data.type), ")"); @@ -3688,7 +3738,14 @@ network::Packet GroupInvitePacket::build(const std::string& playerName) { } bool GroupInviteResponseParser::parse(network::Packet& packet, GroupInviteResponseData& data) { + // Validate minimum packet size: canAccept(1) + if (packet.getSize() - packet.getReadPos() < 1) { + LOG_WARNING("SMSG_GROUP_INVITE: packet too small (", packet.getSize(), " bytes)"); + return false; + } + data.canAccept = packet.readUInt8(); + // Note: inviterName is a string, which is always safe to read even if empty data.inviterName = packet.readString(); LOG_INFO("Group invite from: ", data.inviterName, " (canAccept=", (int)data.canAccept, ")"); return true; From 6f7c57d97527dc079fe59fd5ea90244d9c685d2f Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 15:21:48 -0700 Subject: [PATCH 30/56] feat: add graphics quality presets system Implement quick-access quality presets (Low, Medium, High, Ultra) that adjust multiple graphics settings at once for better user experience. Each preset configures: - Shadow rendering and distance - Anti-aliasing (MSAA) level - Normal mapping and parallax mapping - Ground clutter density The system automatically detects when settings deviate from a preset and marks them as "Custom". Presets are persisted to settings.cfg for consistency across sessions. Users can quickly switch between performance and quality modes or tweak individual settings as needed. --- include/ui/game_screen.hpp | 13 +++ src/ui/game_screen.cpp | 198 +++++++++++++++++++++++++++++++++++++ 2 files changed, 211 insertions(+) diff --git a/include/ui/game_screen.hpp b/include/ui/game_screen.hpp index 566f86c2..c0693566 100644 --- a/include/ui/game_screen.hpp +++ b/include/ui/game_screen.hpp @@ -143,6 +143,17 @@ private: bool pendingAMDFramegen = false; bool fsrSettingsApplied_ = false; + // Graphics quality presets + enum class GraphicsPreset : int { + CUSTOM = 0, + LOW = 1, + MEDIUM = 2, + HIGH = 3, + ULTRA = 4 + }; + GraphicsPreset currentGraphicsPreset = GraphicsPreset::CUSTOM; + GraphicsPreset pendingGraphicsPreset = GraphicsPreset::CUSTOM; + // UI element transparency (0.0 = fully transparent, 1.0 = fully opaque) float uiOpacity_ = 0.65f; bool minimapRotate_ = false; @@ -252,6 +263,8 @@ private: void renderTalentWipeConfirmDialog(game::GameHandler& gameHandler); void renderEscapeMenu(); void renderSettingsWindow(); + void applyGraphicsPreset(GraphicsPreset preset); + void updateGraphicsPresetFromCurrentSettings(); void renderQuestMarkers(game::GameHandler& gameHandler); void renderMinimapMarkers(game::GameHandler& gameHandler); void renderQuestObjectiveTracker(game::GameHandler& gameHandler); diff --git a/src/ui/game_screen.cpp b/src/ui/game_screen.cpp index 05632d3b..2129bc4d 100644 --- a/src/ui/game_screen.cpp +++ b/src/ui/game_screen.cpp @@ -8595,16 +8595,37 @@ void GameScreen::renderSettingsWindow() { if (ImGui::BeginTabItem("Video")) { ImGui::Spacing(); + // Graphics Quality Presets + { + const char* presetLabels[] = { "Custom", "Low", "Medium", "High", "Ultra" }; + int presetIdx = static_cast(pendingGraphicsPreset); + if (ImGui::Combo("Quality Preset", &presetIdx, presetLabels, 5)) { + pendingGraphicsPreset = static_cast(presetIdx); + if (pendingGraphicsPreset != GraphicsPreset::CUSTOM) { + applyGraphicsPreset(pendingGraphicsPreset); + saveSettings(); + } + } + ImGui::TextDisabled("Adjust these for custom settings"); + } + + ImGui::Spacing(); + ImGui::Separator(); + ImGui::Spacing(); + if (ImGui::Checkbox("Fullscreen", &pendingFullscreen)) { window->setFullscreen(pendingFullscreen); + updateGraphicsPresetFromCurrentSettings(); saveSettings(); } if (ImGui::Checkbox("VSync", &pendingVsync)) { window->setVsync(pendingVsync); + updateGraphicsPresetFromCurrentSettings(); saveSettings(); } if (ImGui::Checkbox("Shadows", &pendingShadows)) { if (renderer) renderer->setShadowsEnabled(pendingShadows); + updateGraphicsPresetFromCurrentSettings(); saveSettings(); } if (pendingShadows) { @@ -8612,6 +8633,7 @@ void GameScreen::renderSettingsWindow() { ImGui::SetNextItemWidth(150.0f); if (ImGui::SliderFloat("Distance##shadow", &pendingShadowDistance, 40.0f, 500.0f, "%.0f")) { if (renderer) renderer->setShadowDistance(pendingShadowDistance); + updateGraphicsPresetFromCurrentSettings(); saveSettings(); } } @@ -8643,6 +8665,7 @@ void GameScreen::renderSettingsWindow() { VK_SAMPLE_COUNT_4_BIT, VK_SAMPLE_COUNT_8_BIT }; if (renderer) renderer->setMsaaSamples(aaSamples[pendingAntiAliasing]); + updateGraphicsPresetFromCurrentSettings(); saveSettings(); } } @@ -9426,6 +9449,175 @@ void GameScreen::renderSettingsWindow() { ImGui::End(); } +void GameScreen::applyGraphicsPreset(GraphicsPreset preset) { + auto* renderer = core::Application::getInstance().getRenderer(); + + // Define preset values based on quality level + switch (preset) { + case GraphicsPreset::LOW: { + pendingShadows = false; + pendingShadowDistance = 100.0f; + pendingAntiAliasing = 0; // Off + pendingNormalMapping = false; + pendingPOM = false; + pendingGroundClutterDensity = 25; + if (renderer) { + renderer->setShadowsEnabled(false); + renderer->setMsaaSamples(VK_SAMPLE_COUNT_1_BIT); + if (auto* wr = renderer->getWMORenderer()) { + wr->setNormalMappingEnabled(false); + wr->setPOMEnabled(false); + } + if (auto* cr = renderer->getCharacterRenderer()) { + cr->setNormalMappingEnabled(false); + cr->setPOMEnabled(false); + } + if (auto* tm = renderer->getTerrainManager()) { + tm->setGroundClutterDensityScale(0.25f); + } + } + break; + } + case GraphicsPreset::MEDIUM: { + pendingShadows = true; + pendingShadowDistance = 200.0f; + pendingAntiAliasing = 1; // 2x MSAA + pendingNormalMapping = true; + pendingNormalMapStrength = 0.6f; + pendingPOM = true; + pendingPOMQuality = 0; // Low + pendingGroundClutterDensity = 60; + if (renderer) { + renderer->setShadowsEnabled(true); + renderer->setShadowDistance(200.0f); + renderer->setMsaaSamples(VK_SAMPLE_COUNT_2_BIT); + if (auto* wr = renderer->getWMORenderer()) { + wr->setNormalMappingEnabled(true); + wr->setNormalMapStrength(0.6f); + wr->setPOMEnabled(true); + wr->setPOMQuality(0); + } + if (auto* cr = renderer->getCharacterRenderer()) { + cr->setNormalMappingEnabled(true); + cr->setNormalMapStrength(0.6f); + cr->setPOMEnabled(true); + cr->setPOMQuality(0); + } + if (auto* tm = renderer->getTerrainManager()) { + tm->setGroundClutterDensityScale(0.60f); + } + } + break; + } + case GraphicsPreset::HIGH: { + pendingShadows = true; + pendingShadowDistance = 350.0f; + pendingAntiAliasing = 2; // 4x MSAA + pendingNormalMapping = true; + pendingNormalMapStrength = 0.8f; + pendingPOM = true; + pendingPOMQuality = 1; // Medium + pendingGroundClutterDensity = 100; + if (renderer) { + renderer->setShadowsEnabled(true); + renderer->setShadowDistance(350.0f); + renderer->setMsaaSamples(VK_SAMPLE_COUNT_4_BIT); + if (auto* wr = renderer->getWMORenderer()) { + wr->setNormalMappingEnabled(true); + wr->setNormalMapStrength(0.8f); + wr->setPOMEnabled(true); + wr->setPOMQuality(1); + } + if (auto* cr = renderer->getCharacterRenderer()) { + cr->setNormalMappingEnabled(true); + cr->setNormalMapStrength(0.8f); + cr->setPOMEnabled(true); + cr->setPOMQuality(1); + } + if (auto* tm = renderer->getTerrainManager()) { + tm->setGroundClutterDensityScale(1.0f); + } + } + break; + } + case GraphicsPreset::ULTRA: { + pendingShadows = true; + pendingShadowDistance = 500.0f; + pendingAntiAliasing = 3; // 8x MSAA + pendingNormalMapping = true; + pendingNormalMapStrength = 1.2f; + pendingPOM = true; + pendingPOMQuality = 2; // High + pendingGroundClutterDensity = 150; + if (renderer) { + renderer->setShadowsEnabled(true); + renderer->setShadowDistance(500.0f); + renderer->setMsaaSamples(VK_SAMPLE_COUNT_8_BIT); + if (auto* wr = renderer->getWMORenderer()) { + wr->setNormalMappingEnabled(true); + wr->setNormalMapStrength(1.2f); + wr->setPOMEnabled(true); + wr->setPOMQuality(2); + } + if (auto* cr = renderer->getCharacterRenderer()) { + cr->setNormalMappingEnabled(true); + cr->setNormalMapStrength(1.2f); + cr->setPOMEnabled(true); + cr->setPOMQuality(2); + } + if (auto* tm = renderer->getTerrainManager()) { + tm->setGroundClutterDensityScale(1.5f); + } + } + break; + } + default: + break; + } + + currentGraphicsPreset = preset; + pendingGraphicsPreset = preset; +} + +void GameScreen::updateGraphicsPresetFromCurrentSettings() { + // Check if current settings match any preset, otherwise mark as CUSTOM + // This is a simplified check; could be enhanced with more detailed matching + + auto matchesPreset = [this](GraphicsPreset preset) -> bool { + switch (preset) { + case GraphicsPreset::LOW: + return !pendingShadows && pendingAntiAliasing == 0 && !pendingNormalMapping && !pendingPOM && + pendingGroundClutterDensity <= 30; + case GraphicsPreset::MEDIUM: + return pendingShadows && pendingShadowDistance >= 180 && pendingShadowDistance <= 220 && + pendingAntiAliasing == 1 && pendingNormalMapping && pendingPOM && + pendingGroundClutterDensity >= 50 && pendingGroundClutterDensity <= 70; + case GraphicsPreset::HIGH: + return pendingShadows && pendingShadowDistance >= 330 && pendingShadowDistance <= 370 && + pendingAntiAliasing == 2 && pendingNormalMapping && pendingPOM && + pendingGroundClutterDensity >= 90 && pendingGroundClutterDensity <= 110; + case GraphicsPreset::ULTRA: + return pendingShadows && pendingShadowDistance >= 480 && pendingAntiAliasing == 3 && + pendingNormalMapping && pendingPOM && pendingGroundClutterDensity >= 140; + default: + return false; + } + }; + + // Try to match a preset, otherwise mark as custom + if (matchesPreset(GraphicsPreset::LOW)) { + pendingGraphicsPreset = GraphicsPreset::LOW; + } else if (matchesPreset(GraphicsPreset::MEDIUM)) { + pendingGraphicsPreset = GraphicsPreset::MEDIUM; + } else if (matchesPreset(GraphicsPreset::HIGH)) { + pendingGraphicsPreset = GraphicsPreset::HIGH; + } else if (matchesPreset(GraphicsPreset::ULTRA)) { + pendingGraphicsPreset = GraphicsPreset::ULTRA; + } else { + pendingGraphicsPreset = GraphicsPreset::CUSTOM; + } +} + void GameScreen::renderQuestMarkers(game::GameHandler& gameHandler) { const auto& statuses = gameHandler.getNpcQuestStatuses(); if (statuses.empty()) return; @@ -10154,6 +10346,7 @@ void GameScreen::saveSettings() { // Gameplay out << "auto_loot=" << (pendingAutoLoot ? 1 : 0) << "\n"; + out << "graphics_preset=" << static_cast(currentGraphicsPreset) << "\n"; out << "ground_clutter_density=" << pendingGroundClutterDensity << "\n"; out << "shadows=" << (pendingShadows ? 1 : 0) << "\n"; out << "shadow_distance=" << pendingShadowDistance << "\n"; @@ -10267,6 +10460,11 @@ void GameScreen::loadSettings() { else if (key == "activity_volume") pendingActivityVolume = std::clamp(std::stoi(val), 0, 100); // Gameplay else if (key == "auto_loot") pendingAutoLoot = (std::stoi(val) != 0); + else if (key == "graphics_preset") { + int presetVal = std::clamp(std::stoi(val), 0, 4); + currentGraphicsPreset = static_cast(presetVal); + pendingGraphicsPreset = currentGraphicsPreset; + } else if (key == "ground_clutter_density") pendingGroundClutterDensity = std::clamp(std::stoi(val), 0, 150); else if (key == "shadows") pendingShadows = (std::stoi(val) != 0); else if (key == "shadow_distance") pendingShadowDistance = std::clamp(std::stof(val), 40.0f, 500.0f); From 90e7d61b6de308fe6db0b6a70329d3fddf5dc1aa Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 15:30:45 -0700 Subject: [PATCH 31/56] docs: update for graphics presets and accurate shadow status --- README.md | 2 +- docs/status.md | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index ab1a90d0..cf39609e 100644 --- a/README.md +++ b/README.md @@ -69,7 +69,7 @@ Protocol Compatible with **Vanilla (Classic) 1.12 + TBC 2.4.3 + WotLK 3.3.5a**. - **Pets** -- Pet tracking via SMSG_PET_SPELLS, action bar (10 slots with icon/autocast tinting/tooltips), dismiss button - **Map Exploration** -- Subzone-level fog-of-war reveal matching retail behavior - **Warden** -- Warden anti-cheat module execution via Unicorn Engine x86 emulation (cross-platform, no Wine) -- **UI** -- Loading screens with progress bar, settings window (shadow distance slider), minimap with zoom/rotation/square mode, top-right minimap mute speaker, separate bag windows with compact-empty mode (aggregate view) +- **UI** -- Loading screens with progress bar, settings window with graphics quality presets (LOW/MEDIUM/HIGH/ULTRA), shadow distance slider, minimap with zoom/rotation/square mode, top-right minimap mute speaker, separate bag windows with compact-empty mode (aggregate view) ## Building diff --git a/docs/status.md b/docs/status.md index 8244b425..904a68b1 100644 --- a/docs/status.md +++ b/docs/status.md @@ -1,6 +1,6 @@ # Project Status -**Last updated**: 2026-03-07 +**Last updated**: 2026-03-11 ## What This Repo Is @@ -37,8 +37,8 @@ In progress / known gaps: - Transports: M2 transports (trams) working with position-delta riding; WMO transports (ships, zeppelins) working with path following; some edge cases remain - 3D positional audio: not implemented (mono/stereo only) - Visual edge cases: some M2/WMO rendering gaps (character shin mesh, some particle effects) -- Interior rendering: WMO interior shadows disabled (too dark); lava steam particles sparse -- Water refraction: implemented but disabled by default (can cause VK_ERROR_DEVICE_LOST on some GPUs) +- Lava steam particles: sparse in some areas (tuning opportunity) +- Water refraction: implemented but disabled by default (can cause VK_ERROR_DEVICE_LOST on some GPUs); currently requires FSR to be active ## Where To Look From c77bd1553814e5028894df225dd5070446fc2872 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 15:31:42 -0700 Subject: [PATCH 32/56] docs: remove outdated 3D positional audio note - feature is implemented --- docs/status.md | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/status.md b/docs/status.md index 904a68b1..06722c2f 100644 --- a/docs/status.md +++ b/docs/status.md @@ -35,7 +35,6 @@ Implemented (working in normal use): In progress / known gaps: - Transports: M2 transports (trams) working with position-delta riding; WMO transports (ships, zeppelins) working with path following; some edge cases remain -- 3D positional audio: not implemented (mono/stereo only) - Visual edge cases: some M2/WMO rendering gaps (character shin mesh, some particle effects) - Lava steam particles: sparse in some areas (tuning opportunity) - Water refraction: implemented but disabled by default (can cause VK_ERROR_DEVICE_LOST on some GPUs); currently requires FSR to be active From 1598766b1ec69346e431a9200482524b9d51b9fd Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 15:34:16 -0700 Subject: [PATCH 33/56] docs: add Graphics & Performance guide with quality presets documentation --- README.md | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/README.md b/README.md index cf39609e..d983133c 100644 --- a/README.md +++ b/README.md @@ -71,6 +71,29 @@ Protocol Compatible with **Vanilla (Classic) 1.12 + TBC 2.4.3 + WotLK 3.3.5a**. - **Warden** -- Warden anti-cheat module execution via Unicorn Engine x86 emulation (cross-platform, no Wine) - **UI** -- Loading screens with progress bar, settings window with graphics quality presets (LOW/MEDIUM/HIGH/ULTRA), shadow distance slider, minimap with zoom/rotation/square mode, top-right minimap mute speaker, separate bag windows with compact-empty mode (aggregate view) +## Graphics & Performance + +### Quality Presets + +WoWee includes four built-in graphics quality presets to help you quickly balance visual quality and performance: + +| Preset | Shadows | MSAA | Normal Mapping | Clutter Density | +|--------|---------|------|----------------|-----------------| +| **LOW** | Off | Off | Disabled | 25% | +| **MEDIUM** | 200m distance | 2x | Basic | 60% | +| **HIGH** | 350m distance | 4x | Full (0.8x) | 100% | +| **ULTRA** | 500m distance | 8x | Enhanced (1.2x) | 150% | + +Press Escape to open **Video Settings** and select a preset, or adjust individual settings for a custom configuration. + +### Performance Tips + +- Start with **LOW** or **MEDIUM** if you experience frame drops +- Shadows and MSAA have the largest impact on performance +- Reduce **shadow distance** if shadows cause issues +- Disable **water refraction** if you encounter GPU errors (requires FSR to be active) +- Use **FSR2** (built-in upscaling) for better frame rates on modern GPUs + ## Building ### Prerequisites From 2b8bb76d7ae3d45267b86cc3bc41343fa43cc580 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 15:35:23 -0700 Subject: [PATCH 34/56] docs: add comprehensive multi-expansion architecture guide --- EXPANSION_GUIDE.md | 126 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 126 insertions(+) create mode 100644 EXPANSION_GUIDE.md diff --git a/EXPANSION_GUIDE.md b/EXPANSION_GUIDE.md new file mode 100644 index 00000000..6a2dc26e --- /dev/null +++ b/EXPANSION_GUIDE.md @@ -0,0 +1,126 @@ +# Multi-Expansion Architecture Guide + +WoWee supports three World of Warcraft expansions in a unified codebase using an expansion profile system. This guide explains how the multi-expansion support works. + +## Supported Expansions + +- **Vanilla (Classic) 1.12** - Original World of Warcraft +- **The Burning Crusade (TBC) 2.4.3** - First expansion +- **Wrath of the Lich King (WotLK) 3.3.5a** - Second expansion + +## Architecture Overview + +The multi-expansion support is built on the **Expansion Profile** system: + +1. **ExpansionProfile** (`include/game/expansion_profile.hpp`) - Metadata about each expansion + - Defines protocol version, data paths, asset locations + - Specifies which packet parsers to use + +2. **Packet Parsers** - Expansion-specific message handling + - `packet_parsers_classic.cpp` - Vanilla 1.12 message parsing + - `packet_parsers_tbc.cpp` - TBC 2.4.3 message parsing + - `packet_parsers_wotlk.cpp` (default) - WotLK 3.3.5a message parsing + +3. **Update Fields** - Expansion-specific entity data layout + - Loaded from `update_fields.json` in expansion data directory + - Defines UNIT_END, OBJECT_END, field indices for stats/health/mana + +## How to Use Different Expansions + +### At Startup + +WoWee auto-detects the expansion based on: +1. Realm list response (protocol version) +2. Server build number +3. Update field count + +### Manual Selection + +Set environment variable: +```bash +WOWEE_EXPANSION=tbc ./wowee # Force TBC +WOWEE_EXPANSION=classic ./wowee # Force Classic +``` + +## Key Differences Between Expansions + +### Packet Format Differences + +#### SMSG_SPELL_COOLDOWN +- **Classic**: 12 bytes per entry (spellId + itemId + cooldown, no flags) +- **TBC/WotLK**: 8 bytes per entry (spellId + cooldown) + flags byte + +#### SMSG_ACTION_BUTTONS +- **Classic**: 120 slots, no mode byte +- **TBC**: 132 slots, no mode byte +- **WotLK**: 144 slots + uint8 mode byte + +#### SMSG_PARTY_MEMBER_STATS +- **Classic/TBC**: Full uint64 for guid, uint16 health +- **WotLK**: PackedGuid format, uint32 health + +### Data Differences + +- **Talent trees**: Different spell IDs and tree structure per expansion +- **Items**: Different ItemDisplayInfo entries +- **Spells**: Different base stats, cooldowns +- **Character textures**: Expansion-specific variants for races + +## Adding Support for Another Expansion + +1. Create new expansion profile entry in `expansion_profile.cpp` +2. Add packet parser file (`packet_parsers_*.cpp`) for message variants +3. Create update_fields.json with correct field layout +4. Test realm connection and character loading + +## Code Patterns + +### Checking Current Expansion + +```cpp +#include "game/expansion_profile.hpp" + +// Global helper +bool isClassicLikeExpansion() { + auto profile = ExpansionProfile::getActive(); + return profile && (profile->name == "Classic" || profile->name == "Vanilla"); +} + +// Specific check +if (GameHandler::getInstance().isActiveExpansion("tbc")) { + // TBC-specific code +} +``` + +### Expansion-Specific Packet Parsing + +```cpp +// In packet_parsers_*.cpp, implement expansion-specific logic +bool parseXxxPacket(BitStream& data, ...) { + // Custom logic for this expansion's packet format +} +``` + +## Common Issues + +### "Update fields mismatch" Error +- Ensure `update_fields.json` matches server's field layout +- Check OBJECT_END and UNIT_END values +- Verify field indices for your target expansion + +### "Unknown packet" Warnings +- Expansion-specific opcodes may not be registered +- Check packet parser registration in `game_handler.cpp` +- Verify expansion profile is active + +### Packet Parsing Failures +- Each expansion has different struct layouts +- Always read data size first, then upfront validate +- Use size capping (e.g., max 100 items in list) + +## References + +- `include/game/expansion_profile.hpp` - Expansion metadata +- `docs/status.md` - Current feature support by expansion +- `src/game/packet_parsers_*.cpp` - Format-specific parsing logic +- `docs/` directory - Additional protocol documentation From 17bf963f3efa1bf6a62402d9f92533980c767ceb Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 15:36:02 -0700 Subject: [PATCH 35/56] docs: add comprehensive troubleshooting guide for users --- TROUBLESHOOTING.md | 186 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 186 insertions(+) create mode 100644 TROUBLESHOOTING.md diff --git a/TROUBLESHOOTING.md b/TROUBLESHOOTING.md new file mode 100644 index 00000000..034fb769 --- /dev/null +++ b/TROUBLESHOOTING.md @@ -0,0 +1,186 @@ +# Troubleshooting Guide + +This guide covers common issues and solutions for WoWee. + +## Connection Issues + +### "Authentication Failed" +- **Cause**: Incorrect server address, expired realm list, or version mismatch +- **Solution**: + 1. Verify server address in realm list is correct + 2. Ensure your WoW data directory is for the correct expansion (Vanilla/TBC/WotLK) + 3. Check that the emulator server is running and reachable + +### "Realm List Connection Failed" +- **Cause**: Server is down, firewall blocking connection, or DNS issue +- **Solution**: + 1. Verify server IP/hostname is correct + 2. Test connectivity: `ping realm-server-address` + 3. Check firewall rules for port 3724 (auth) and 8085 (realm list) + 4. Try using IP address instead of hostname (DNS issues) + +### "Connection Lost During Login" +- **Cause**: Network timeout, server overload, or incompatible protocol version +- **Solution**: + 1. Check your network connection + 2. Reduce number of assets loading (lower graphics preset) + 3. Verify server supports this expansion version + +## Graphics Issues + +### "VK_ERROR_DEVICE_LOST" or Client Crashes +- **Cause**: GPU driver issue, insufficient VRAM, or graphics feature incompatibility +- **Solution**: + 1. **Immediate**: Disable advanced graphics features: + - Press Escape → Video Settings + - Set graphics preset to **LOW** + - Disable Water Refraction (requires FSR) + - Disable MSAA (set to Off) + 2. **Medium term**: Update GPU driver to latest version + 3. **Verify**: Use a graphics test tool to ensure GPU stability + 4. **If persists**: Try FSR2 disabled mode - check renderer logs + +### Black Screen or Rendering Issues +- **Cause**: Missing shaders, GPU memory allocation failure, or incorrect graphics settings +- **Solution**: + 1. Check logs: Look in `~/.wowee/logs/` for error messages + 2. Verify shaders compiled: Check for `.spv` files in `assets/shaders/compiled/` + 3. Reduce shadow distance: Press Escape → Video Settings → Lower shadow distance from 300m to 100m + 4. Disable shadows entirely if issues persist + +### Low FPS or Frame Stuttering +- **Cause**: Too high graphics settings for your GPU, memory fragmentation, or asset loading +- **Solution**: + 1. Apply lower graphics preset: Escape → LOW or MEDIUM + 2. Disable MSAA: Set to "Off" + 3. Reduce draw distance: Move further away from complex areas + 4. Close other applications consuming GPU memory + 5. Check CPU usage - if high, reduce number of visible entities + +### Water/Terrain Flickering +- **Cause**: Shadow mapping artifacts, terrain LOD issues, or GPU memory pressure +- **Solution**: + 1. Increase shadow distance slightly (150m to 200m) + 2. Disable shadows entirely as last resort + 3. Check GPU memory usage + +## Audio Issues + +### No Sound +- **Cause**: Audio initialization failed, missing audio data, or incorrect mixer setup +- **Solution**: + 1. Check system audio is working: Test with another application + 2. Verify audio files extracted: Check for `.wav` files in `Data/Audio/` + 3. Unmute audio: Look for speaker icon in minimap (top-right) - click to unmute + 4. Check settings: Escape → Audio Settings → Master Volume > 0 + +### Sound Cutting Out +- **Cause**: Audio buffer underrun, too many simultaneous sounds, or driver issue +- **Solution**: + 1. Lower audio volume: Escape → Audio Settings → Reduce Master Volume + 2. Disable distant ambient sounds: Reduce Ambient Volume + 3. Reduce number of particle effects + 4. Update audio driver + +## Gameplay Issues + +### Character Stuck or Not Moving +- **Cause**: Network synchronization issue, collision bug, or server desync +- **Solution**: + 1. Try pressing Escape to deselect any target, then move + 2. Jump (Spacebar) to test physics + 3. Reload the character: Press Escape → Disconnect → Reconnect + 4. Check for transport/vehicle state: Press 'R' to dismount if applicable + +### Spells Not Casting or Showing "Error" +- **Cause**: Cooldown, mana insufficient, target out of range, or server desync +- **Solution**: + 1. Verify spell is off cooldown (action bar shows availability) + 2. Check mana/energy: Look at player frame (top-left) + 3. Verify target range: Hover action bar button for range info + 4. Check server logs for error messages (combat log will show reason) + +### Quests Not Updating +- **Cause**: Objective already completed in different session, quest giver not found, or network desync +- **Solution**: + 1. Check quest objective: Open quest log (Q key) → Verify objective requirements + 2. Re-interact with NPC to trigger update packet + 3. Reload character if issue persists + +### Items Not Appearing in Inventory +- **Cause**: Inventory full, item filter active, or network desync +- **Solution**: + 1. Check inventory space: Open inventory (B key) → Count free slots + 2. Verify item isn't already there: Search inventory for item name + 3. Check if bags are full: Open bag windows, consolidate items + 4. Reload character if item is still missing + +## Performance Optimization + +### For Low-End GPUs +``` +Graphics Preset: LOW +- Shadows: OFF +- MSAA: OFF +- Normal Mapping: Disabled +- Clutter Density: 25% +- Draw Distance: Minimum +- Particles: Reduced +``` + +### For Mid-Range GPUs +``` +Graphics Preset: MEDIUM +- Shadows: 200m +- MSAA: 2x +- Normal Mapping: Basic +- Clutter Density: 60% +- FSR2: Enabled (if desired) +``` + +### For High-End GPUs +``` +Graphics Preset: HIGH or ULTRA +- Shadows: 350-500m +- MSAA: 4-8x +- Normal Mapping: Full (1.2x strength) +- Clutter Density: 100-150% +- FSR2: Optional (for 4K smoothness) +``` + +## Getting Help + +### Check Logs +Detailed logs are saved to: +- **Linux/macOS**: `~/.wowee/logs/` +- **Windows**: `%APPDATA%\wowee\logs\` + +Include relevant log entries when reporting issues. + +### Check Server Compatibility +- **AzerothCore**: Full support +- **TrinityCore**: Full support +- **Mangos**: Full support +- **Turtle WoW**: Full support (1.17) + +### Report Issues +If you encounter a bug: +1. Enable logging: Watch console for error messages +2. Reproduce the issue consistently +3. Gather system info: GPU, driver version, OS +4. Check if issue is expansion-specific (Classic/TBC/WotLK) +5. Report with detailed steps to reproduce + +### Clear Cache +If experiencing persistent issues, clear WoWee's cache: +```bash +# Linux/macOS +rm -rf ~/.wowee/warden_cache/ +rm -rf ~/.wowee/asset_cache/ + +# Windows +rmdir %APPDATA%\wowee\warden_cache\ /s +rmdir %APPDATA%\wowee\asset_cache\ /s +``` + +Then restart WoWee to rebuild cache. From e2e049b7181f783604a36dad0594f42abf5edb97 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 15:36:44 -0700 Subject: [PATCH 36/56] docs: add Getting Started guide for new users --- GETTING_STARTED.md | 218 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 218 insertions(+) create mode 100644 GETTING_STARTED.md diff --git a/GETTING_STARTED.md b/GETTING_STARTED.md new file mode 100644 index 00000000..cdf80306 --- /dev/null +++ b/GETTING_STARTED.md @@ -0,0 +1,218 @@ +# Getting Started with WoWee + +WoWee is a native C++ World of Warcraft client that connects to private servers. This guide walks you through setting up and playing WoWee. + +## Prerequisites + +- **World of Warcraft Game Data** (Vanilla 1.12, TBC 2.4.3, or WotLK 3.3.5a) +- **A Private Server** (AzerothCore, TrinityCore, Mangos, or Turtle WoW compatible) +- **System Requirements**: Linux, macOS, or Windows with a Vulkan-capable GPU + +## Installation + +### Step 1: Build WoWee + +See [Building](README.md#building) section in README for detailed build instructions. + +**Quick start (Linux/macOS)**: +```bash +./build.sh +cd build/bin +./wowee +``` + +**Quick start (Windows)**: +```powershell +.\build.ps1 +cd build\bin +.\wowee.exe +``` + +### Step 2: Extract Game Data + +WoWee needs game assets from your WoW installation: + +**Using provided script (Linux/macOS)**: +```bash +./extract_assets.sh /path/to/wow/directory +``` + +**Using provided script (Windows)**: +```powershell +.\extract_assets.ps1 -WowDirectory "C:\Program Files\World of Warcraft" +``` + +**Manual extraction**: +1. Install [StormLib](https://github.com/ladislav-zezula/StormLib) +2. Extract to `./Data/`: + ``` + Data/ + ├── dbc/ # DBC files + ├── map/ # World map data + ├── adt/ # Terrain chunks + ├── wmo/ # Building models + ├── m2/ # Character/creature models + └── blp/ # Textures + ``` + +### Step 3: Connect to a Server + +1. **Start WoWee** + ```bash + cd build/bin && ./wowee + ``` + +2. **Enter Realm Information** + - Server Address: e.g., `localhost:3724` or `play.example.com:3724` + - WoWee fetches the realm list automatically + - Select your realm and click **Connect** + +3. **Choose Character** + - Select existing character or create new one + - Customize appearance and settings + - Click **Enter World** + +## First Steps in Game + +### Default Controls + +| Action | Key | +|--------|-----| +| Move Forward | W | +| Move Backward | S | +| Strafe Left | A | +| Strafe Right | D | +| Jump | Space | +| Toggle Chat | Enter | +| Interact (talk to NPC, loot) | F | +| Open Inventory | B | +| Open Spellbook | P | +| Open Talent Tree | T | +| Open Quest Log | Q | +| Open World Map | W (when not typing) | +| Toggle Minimap | M | +| Toggle Nameplates | V | +| Toggle Party Frames | F | +| Toggle Settings | Escape | +| Target Next Enemy | Tab | +| Target Previous Enemy | Shift+Tab | + +### Customizing Controls + +Press **Escape** → **Keybindings** to customize hotkeys. + +## Recommended First Steps + +### 1. Adjust Graphics Settings +- Press Escape → **Video Settings** +- Select appropriate **Graphics Preset** for your GPU: + - **LOW**: Low-end GPUs or when performance is priority + - **MEDIUM**: Balanced quality and performance + - **HIGH**: Good GPU with modern drivers + - **ULTRA**: High-end GPU for maximum quality + +### 2. Adjust Audio +- Press Escape → **Audio Settings** +- Set **Master Volume** to preferred level +- Adjust individual audio tracks (Music, Ambient, UI, etc.) +- Toggle **Original Soundtrack** if available + +### 3. Configure UI +- Press Escape → **Game Settings** +- Minimap preferences (rotation, square mode, zoom) +- Bag settings (separate windows, compact mode) +- Action bar visibility + +### 4. Complete First Quest +- Talk to nearby NPCs (they have quest markers ! or ?) +- Accept quest, complete objectives, return for reward +- Level up and gain experience + +## Important Notes + +### Data Directory +Game data is loaded from `Data/` subdirectory: +- If running from build folder: `../../Data` (symlinked automatically) +- If running from binary folder: `./Data` (must exist) +- If running in-place: Ensure `Data/` is in correct location + +### Settings +- Settings are saved to `~/.wowee/settings.cfg` (Linux/macOS) +- Or `%APPDATA%\wowee\settings.cfg` (Windows) +- Keybindings, graphics settings, and UI state persist + +### Multi-Expansion Support +WoWee auto-detects expansion from server: +- **Vanilla 1.12** - Original game +- **TBC 2.4.3** - Burning Crusade +- **WotLK 3.3.5a** - Wrath of the Lich King + +You can override with environment variable: +```bash +WOWEE_EXPANSION=tbc ./wowee # Force TBC +``` + +## Troubleshooting + +### "No realm list" or "Connection Failed" +- Check server address is correct +- Verify server is running +- See [Troubleshooting Guide](TROUBLESHOOTING.md#connection-issues) + +### Graphics Errors +- See [Graphics Troubleshooting](TROUBLESHOOTING.md#graphics-issues) +- Start with LOW graphics preset +- Update GPU driver + +### Audio Not Working +- Check system audio is enabled +- Verify audio files are extracted +- See [Audio Troubleshooting](TROUBLESHOOTING.md#audio-issues) + +### General Issues +- Comprehensive troubleshooting: See [TROUBLESHOOTING.md](TROUBLESHOOTING.md) +- Check logs in `~/.wowee/logs/` for errors +- Verify expansion matches server requirements + +## Server Configuration + +### Tested Servers +- **AzerothCore** - Full support, recommended for learning +- **TrinityCore** - Full support, extensive customization +- **Mangos** - Full support, solid foundation +- **Turtle WoW** - Full support, 1.17 custom content + +### Server Requirements +- Must support Vanilla, TBC, or WotLK protocol +- Warden anti-cheat supported (module execution via emulation) +- Network must allow connections to realm list and world server ports + +See [Multi-Expansion Guide](EXPANSION_GUIDE.md) for protocol details. + +## Next Steps + +1. **Explore the World** - Travel to different zones and enjoy the landscape +2. **Join a Guild** - Find other players to group with +3. **Run Dungeons** - Experience instanced content +4. **PvP** - Engage in player-versus-player combat +5. **Twink Alt** - Create additional characters +6. **Customize Settings** - Fine-tune graphics, audio, and UI + +## Getting Help + +- **Game Issues**: See [TROUBLESHOOTING.md](TROUBLESHOOTING.md) +- **Graphics Help**: See [Graphics & Performance](README.md#graphics--performance) section +- **Multi-Expansion**: See [EXPANSION_GUIDE.md](EXPANSION_GUIDE.md) +- **Building Issues**: See [README.md](README.md#building) + +## Tips for Better Performance + +- Start with reasonable graphics preset for your GPU +- Close other applications when testing +- Keep GPU drivers updated +- Use FSR2 (if supported) for smooth 60+ FPS on weaker hardware +- Monitor frame rate with debug overlay (if available) + +## Enjoy! + +WoWee is a project to experience classic World of Warcraft on a modern engine. Have fun exploring Azeroth! From 047b9157ad4ffc8fc4374f6f54042c0cd619ee73 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 16:06:36 -0700 Subject: [PATCH 37/56] Validate transport registration before player attachment Add upfront validation to setPlayerOnTransport to ensure the transport GUID is registered in transportGuids_ before attaching the player. This prevents transport desyncs when movement packets reference transports that haven't been spawned/registered yet. --- include/game/game_handler.hpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/include/game/game_handler.hpp b/include/game/game_handler.hpp index 25b6b8de..17efaaa7 100644 --- a/include/game/game_handler.hpp +++ b/include/game/game_handler.hpp @@ -825,6 +825,11 @@ public: glm::vec3 getComposedWorldPosition(); // Compose transport transform * local offset TransportManager* getTransportManager() { return transportManager_.get(); } void setPlayerOnTransport(uint64_t transportGuid, const glm::vec3& localOffset) { + // Validate transport is registered before attaching player + // (defer if transport not yet registered to prevent desyncs) + if (transportGuid != 0 && !isTransportGuid(transportGuid)) { + return; // Transport not yet registered; skip attachment + } playerTransportGuid_ = transportGuid; playerTransportOffset_ = localOffset; playerTransportStickyGuid_ = transportGuid; From 3f0e19970e3bba2fc22b742c30381ecce82c12ce Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 16:32:30 -0700 Subject: [PATCH 38/56] Fix character screen weapon slots layout positioning Position weapon slots (main hand, off hand, ranged) to align with the right column instead of appearing in the left column where they crowd the main equipment slots (tabbard, bracers, etc.). Weapons now positioned consistently with the 3-column layout at rightColX instead of appearing at the default left cursor position. --- src/ui/inventory_screen.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/ui/inventory_screen.cpp b/src/ui/inventory_screen.cpp index 6255601f..850a33b2 100644 --- a/src/ui/inventory_screen.cpp +++ b/src/ui/inventory_screen.cpp @@ -1357,7 +1357,7 @@ void InventoryScreen::renderEquipmentPanel(game::Inventory& inventory) { } } - // Weapon row + // Weapon row - positioned to the right of left column to avoid crowding main equipment ImGui::Spacing(); ImGui::Separator(); @@ -1366,6 +1366,9 @@ void InventoryScreen::renderEquipmentPanel(game::Inventory& inventory) { game::EquipSlot::OFF_HAND, game::EquipSlot::RANGED, }; + + // Position weapons to the right of the left column (after the 3D preview column) + ImGui::SetCursorPosX(rightColX); for (int i = 0; i < 3; i++) { if (i > 0) ImGui::SameLine(); const auto& slot = inventory.getEquipSlot(weaponSlots[i]); From 0c8fb94f0c3a27ea96745ae3c8fcba425632fc66 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 16:47:07 -0700 Subject: [PATCH 39/56] Increase lava/magma smoke particle emission rate from 16 to 32 per second Double the smoke particle emission rate to create visually richer lava and magma effects. Current implementation emitted only 16 particles/sec per emitter (~88 in steady state), which appeared sparse especially in multi-emitter lava areas. Increasing to 32/sec provides denser steam/smoke effects (~176 in steady state) while remaining well under the 1000 particle cap. This tuning opportunity was documented in status.md as a known gap in visual completeness. --- src/rendering/m2_renderer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rendering/m2_renderer.cpp b/src/rendering/m2_renderer.cpp index 332b8849..5e0afa97 100644 --- a/src/rendering/m2_renderer.cpp +++ b/src/rendering/m2_renderer.cpp @@ -1997,7 +1997,7 @@ void M2Renderer::update(float deltaTime, const glm::vec3& cameraPos, const glm:: std::uniform_real_distribution distDrift(-0.2f, 0.2f); smokeEmitAccum += deltaTime; - float emitInterval = 1.0f / 16.0f; // 16 particles per second per emitter + float emitInterval = 1.0f / 32.0f; // 32 particles per second per emitter (was 16; increased for denser lava/magma effects) if (smokeEmitAccum >= emitInterval && static_cast(smokeParticles.size()) < MAX_SMOKE_PARTICLES) { From bfeb978efffdbb7400e4534dd61b4fe9e47c1cf8 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 16:49:27 -0700 Subject: [PATCH 40/56] Fix character screen weapon slot positioning regression Weapon slots were positioned too far right (at rightColX) causing overlap with right column equipment. Repositioned to center column area (contentStartX + slotSize + 8px), after left column. 3D preview renders on top, no visual conflict. --- src/ui/inventory_screen.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ui/inventory_screen.cpp b/src/ui/inventory_screen.cpp index 850a33b2..2f63c34a 100644 --- a/src/ui/inventory_screen.cpp +++ b/src/ui/inventory_screen.cpp @@ -1367,8 +1367,8 @@ void InventoryScreen::renderEquipmentPanel(game::Inventory& inventory) { game::EquipSlot::RANGED, }; - // Position weapons to the right of the left column (after the 3D preview column) - ImGui::SetCursorPosX(rightColX); + // Position weapons in center column area (after left column, 3D preview renders on top) + ImGui::SetCursorPosX(contentStartX + slotSize + 8.0f); for (int i = 0; i < 3; i++) { if (i > 0) ImGui::SameLine(); const auto& slot = inventory.getEquipSlot(weaponSlots[i]); From 4d0eef1f6fe9d57fffccb69dfc4b72363f47f750 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 16:52:53 -0700 Subject: [PATCH 41/56] Skip tab-targeting empty looted corpses Dead creatures with no remaining loot items are now excluded from tab-targeting cycle. Prevents cycling through empty corpses when looking for targetable enemies. Corpses with available loot remain targetable. --- src/game/game_handler.cpp | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/game/game_handler.cpp b/src/game/game_handler.cpp index 458a06bf..9cea04eb 100644 --- a/src/game/game_handler.cpp +++ b/src/game/game_handler.cpp @@ -10595,7 +10595,19 @@ void GameHandler::tabTarget(float playerX, float playerY, float playerZ) { const uint64_t guid = e->getGuid(); auto* unit = dynamic_cast(e.get()); if (!unit) return false; // Not a unit (shouldn't happen after type filter) - if (unit->getHealth() == 0) return false; // Dead / corpse + if (unit->getHealth() == 0) { + // Dead corpse: only targetable if it has loot or is skinnableable + // If corpse was looted and is now empty, skip it (except for skinning) + auto lootIt = localLootState_.find(guid); + if (lootIt == localLootState_.end() || lootIt->second.data.items.empty()) { + // No loot data or all items taken; check if skinnableable + // For now, skip empty looted corpses (proper skinning check requires + // creature type data that may not be immediately available) + return false; + } + // Has unlooted items available + return true; + } const bool hostileByFaction = unit->isHostile(); const bool hostileByCombat = isAggressiveTowardPlayer(guid); if (!hostileByFaction && !hostileByCombat) return false; From f7c752a3167d680fc8a062f50165277a27d45d58 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 16:54:30 -0700 Subject: [PATCH 42/56] Hide nameplates/health bars for corpses except when selected Corpses no longer display nameplates or health bars unless they are the current target (selected for loot or skinning). When selected, corpses show a minimal grey nameplate with no health fill. --- src/ui/game_screen.cpp | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/ui/game_screen.cpp b/src/ui/game_screen.cpp index 2129bc4d..f3d01295 100644 --- a/src/ui/game_screen.cpp +++ b/src/ui/game_screen.cpp @@ -5317,6 +5317,10 @@ void GameScreen::renderNameplates(game::GameHandler& gameHandler) { // Player nameplates are always shown; NPC nameplates respect the V-key toggle if (!isPlayer && !showNameplates_) continue; + // For corpses (dead units), only show a minimal grey nameplate if selected + bool isCorpse = (unit->getHealth() == 0); + if (isCorpse && !isTarget) continue; + // Prefer the renderer's actual instance position so the nameplate tracks the // rendered model exactly (avoids drift from the parallel entity interpolator). glm::vec3 renderPos; @@ -5349,9 +5353,13 @@ void GameScreen::renderNameplates(game::GameHandler& gameHandler) { float alpha = dist < (cullDist - 5.0f) ? 1.0f : 1.0f - (dist - (cullDist - 5.0f)) / 5.0f; auto A = [&](int v) { return static_cast(v * alpha); }; - // Bar colour by hostility + // Bar colour by hostility (grey for corpses) ImU32 barColor, bgColor; - if (unit->isHostile()) { + if (isCorpse) { + // Minimal grey bar for selected corpses (loot/skin targets) + barColor = IM_COL32(140, 140, 140, A(200)); + bgColor = IM_COL32(70, 70, 70, A(160)); + } else if (unit->isHostile()) { barColor = IM_COL32(220, 60, 60, A(200)); bgColor = IM_COL32(100, 25, 25, A(160)); } else { @@ -5372,7 +5380,10 @@ void GameScreen::renderNameplates(game::GameHandler& gameHandler) { 0.0f, 1.0f); drawList->AddRectFilled(ImVec2(barX, sy), ImVec2(barX + barW, sy + barH), bgColor, 2.0f); - drawList->AddRectFilled(ImVec2(barX, sy), ImVec2(barX + barW * healthPct, sy + barH), barColor, 2.0f); + // For corpses, don't fill health bar (just show grey background) + if (!isCorpse) { + drawList->AddRectFilled(ImVec2(barX, sy), ImVec2(barX + barW * healthPct, sy + barH), barColor, 2.0f); + } drawList->AddRect (ImVec2(barX - 1.0f, sy - 1.0f), ImVec2(barX + barW + 1.0f, sy + barH + 1.0f), borderColor, 2.0f); // Name + level label above health bar From eb3cdbcc5feaa9452805eaf9afcd25fde1f7c741 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 16:58:36 -0700 Subject: [PATCH 43/56] Fix stacked item count display in bags after looting Move onlineItems_ lookup outside field iteration to ensure consistent item reference when updating stack count/durability. This fixes an issue where stacked item counts in open bags wouldn't update immediately when looting additional items until the item was moved to another slot. --- src/game/game_handler.cpp | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/game/game_handler.cpp b/src/game/game_handler.cpp index 9cea04eb..0e687db5 100644 --- a/src/game/game_handler.cpp +++ b/src/game/game_handler.cpp @@ -8956,20 +8956,23 @@ void GameHandler::handleUpdateObject(network::Packet& packet) { const uint16_t itemMaxDurField = fieldIndex(UF::ITEM_FIELD_MAXDURABILITY); const uint16_t containerNumSlotsField = fieldIndex(UF::CONTAINER_FIELD_NUM_SLOTS); const uint16_t containerSlot1Field = fieldIndex(UF::CONTAINER_FIELD_SLOT_1); + + auto it = onlineItems_.find(block.guid); + bool isItemInInventory = (it != onlineItems_.end()); + for (const auto& [key, val] : block.fields) { - auto it = onlineItems_.find(block.guid); - if (key == itemStackField) { - if (it != onlineItems_.end() && it->second.stackCount != val) { + if (key == itemStackField && isItemInInventory) { + if (it->second.stackCount != val) { it->second.stackCount = val; inventoryChanged = true; } - } else if (key == itemDurField) { - if (it != onlineItems_.end() && it->second.curDurability != val) { + } else if (key == itemDurField && isItemInInventory) { + if (it->second.curDurability != val) { it->second.curDurability = val; inventoryChanged = true; } - } else if (key == itemMaxDurField) { - if (it != onlineItems_.end() && it->second.maxDurability != val) { + } else if (key == itemMaxDurField && isItemInInventory) { + if (it->second.maxDurability != val) { it->second.maxDurability = val; inventoryChanged = true; } From b7479cbb5051f6daf70af382ee03a2cc65490f64 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 17:02:15 -0700 Subject: [PATCH 44/56] Fix running animation hitching by using duration subtraction instead of fmod Replace floating-point fmod() with iterative duration subtraction to preserve precision. When animation time accumulates over many loops, fmod() loses precision with large values, causing subtle jumps/hitches in looping animations. Subtracting the duration instead keeps animationTime bounded in [0, duration) and avoids precision loss. --- src/rendering/character_renderer.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/rendering/character_renderer.cpp b/src/rendering/character_renderer.cpp index 730a9bc3..2fcf2ef7 100644 --- a/src/rendering/character_renderer.cpp +++ b/src/rendering/character_renderer.cpp @@ -1648,7 +1648,13 @@ void CharacterRenderer::update(float deltaTime, const glm::vec3& cameraPos) { inst.animationTime += deltaTime * 1000.0f; if (seq.duration > 0 && inst.animationTime >= static_cast(seq.duration)) { if (inst.animationLoop) { - inst.animationTime = std::fmod(inst.animationTime, static_cast(seq.duration)); + // Subtract duration instead of fmod to preserve float precision + // fmod() loses precision with large animationTime values + inst.animationTime -= static_cast(seq.duration); + // Clamp to [0, duration) to handle multiple loops in one frame + while (inst.animationTime >= static_cast(seq.duration)) { + inst.animationTime -= static_cast(seq.duration); + } } else { // One-shot animation finished: return to Stand (0) unless dead if (inst.currentAnimationId != 1 /*Death*/) { From b5a48729b8c82bd47a90ec8e44f31364c6850e40 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 17:12:05 -0700 Subject: [PATCH 45/56] Add diagnostic logging for player appearance extraction failures Log a warning when player model spawn fails due to appearance data extraction failure. This helps diagnose why players appear invisible (missing field indices or malformed update packets). Logs at both CREATE_OBJECT and VALUES update points. --- src/game/game_handler.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/game/game_handler.cpp b/src/game/game_handler.cpp index 0e687db5..df0caa62 100644 --- a/src/game/game_handler.cpp +++ b/src/game/game_handler.cpp @@ -8404,6 +8404,9 @@ void GameHandler::handleUpdateObject(network::Packet& packet) { playerSpawnCallback_(block.guid, unit->getDisplayId(), race, gender, appearanceBytes, facial, unit->getX(), unit->getY(), unit->getZ(), unit->getOrientation()); + } else { + LOG_WARNING("[Spawn] PLAYER guid=0x", std::hex, block.guid, std::dec, + " displayId=", unit->getDisplayId(), " appearance extraction failed — model will not render"); } } } else if (creatureSpawnCallback_) { @@ -8800,6 +8803,9 @@ void GameHandler::handleUpdateObject(network::Packet& packet) { playerSpawnCallback_(block.guid, unit->getDisplayId(), race, gender, appearanceBytes, facial, unit->getX(), unit->getY(), unit->getZ(), unit->getOrientation()); + } else { + LOG_WARNING("[Spawn] PLAYER guid=0x", std::hex, block.guid, std::dec, + " displayId=", unit->getDisplayId(), " appearance extraction failed (VALUES update) — model will not render"); } } } else if (creatureSpawnCallback_) { From eef269ffb84a0c5f7881ae7b70ba9a674d941bd2 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 17:27:23 -0700 Subject: [PATCH 46/56] Fix quest reward items showing as 'Item {number}' on first frame Quest reward items (both in details and offer-reward windows) were showing as "Item {itemId}" placeholders because the window opened immediately after receiving SMSG_QUESTGIVER_QUEST_DETAILS, before the item query responses from pre-fetched queries had time to arrive. Solution: Delay opening the quest details window by 100ms to allow item queries to complete and be cached before the window first renders. Uses std::chrono::steady_clock for timing. - Add questDetailsOpenTime field to track delayed opening timestamp - Modify isQuestDetailsOpen() to check timer and open window when time expires - Reset timer whenever quest details window closes - Updated comment to clarify pre-fetch benefits both details and offer-reward windows --- include/game/game_handler.hpp | 14 +++++++++++++- src/game/game_handler.cpp | 12 ++++++++++-- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/include/game/game_handler.hpp b/include/game/game_handler.hpp index 17efaaa7..ba729be9 100644 --- a/include/game/game_handler.hpp +++ b/include/game/game_handler.hpp @@ -1068,7 +1068,18 @@ public: void closeGossip(); bool isGossipWindowOpen() const { return gossipWindowOpen; } const GossipMessageData& getCurrentGossip() const { return currentGossip; } - bool isQuestDetailsOpen() const { return questDetailsOpen; } + bool isQuestDetailsOpen() { + // Check if delayed opening timer has expired + if (questDetailsOpen) return true; + if (questDetailsOpenTime != std::chrono::steady_clock::time_point{}) { + if (std::chrono::steady_clock::now() >= questDetailsOpenTime) { + questDetailsOpen = true; + questDetailsOpenTime = std::chrono::steady_clock::time_point{}; + return true; + } + } + return false; + } const QuestDetailsData& getQuestDetails() const { return currentQuestDetails; } // Gossip / quest map POI markers (SMSG_GOSSIP_POI) @@ -2184,6 +2195,7 @@ private: // Quest details bool questDetailsOpen = false; + std::chrono::steady_clock::time_point questDetailsOpenTime{}; // Delayed opening to allow item data to load QuestDetailsData currentQuestDetails; // Quest turn-in diff --git a/src/game/game_handler.cpp b/src/game/game_handler.cpp index df0caa62..798383a0 100644 --- a/src/game/game_handler.cpp +++ b/src/game/game_handler.cpp @@ -4491,6 +4491,7 @@ void GameHandler::handlePacket(network::Packet& packet) { } if (currentQuestDetails.questId == questId) { questDetailsOpen = false; + questDetailsOpenTime = std::chrono::steady_clock::time_point{}; currentQuestDetails = QuestDetailsData{}; removed = true; } @@ -15351,10 +15352,11 @@ void GameHandler::handleQuestDetails(network::Packet& packet) { break; } // Pre-fetch item info for all reward items so icons and names are ready - // by the time the offer-reward dialog opens (after the player turns in). + // both in this details window and later in the offer-reward dialog (after the player turns in). for (const auto& item : data.rewardChoiceItems) queryItemInfo(item.itemId, 0); for (const auto& item : data.rewardItems) queryItemInfo(item.itemId, 0); - questDetailsOpen = true; + // Delay opening the window slightly to allow item queries to complete + questDetailsOpenTime = std::chrono::steady_clock::now() + std::chrono::milliseconds(100); gossipWindowOpen = false; } @@ -15604,6 +15606,7 @@ void GameHandler::acceptQuest() { LOG_DEBUG("Ignoring duplicate quest accept while pending: questId=", questId); triggerQuestAcceptResync(questId, npcGuid, "duplicate-accept"); questDetailsOpen = false; + questDetailsOpenTime = std::chrono::steady_clock::time_point{}; currentQuestDetails = QuestDetailsData{}; return; } @@ -15613,6 +15616,7 @@ void GameHandler::acceptQuest() { LOG_INFO("Ignoring duplicate quest accept already in server quest log: questId=", questId, " slot=", serverSlot); questDetailsOpen = false; + questDetailsOpenTime = std::chrono::steady_clock::time_point{}; currentQuestDetails = QuestDetailsData{}; return; } @@ -15629,6 +15633,7 @@ void GameHandler::acceptQuest() { pendingQuestAcceptNpcGuids_[questId] = npcGuid; questDetailsOpen = false; + questDetailsOpenTime = std::chrono::steady_clock::time_point{}; currentQuestDetails = QuestDetailsData{}; // Re-query quest giver status so marker updates (! → ?) @@ -15641,6 +15646,7 @@ void GameHandler::acceptQuest() { void GameHandler::declineQuest() { questDetailsOpen = false; + questDetailsOpenTime = std::chrono::steady_clock::time_point{}; currentQuestDetails = QuestDetailsData{}; } @@ -15707,6 +15713,7 @@ void GameHandler::handleQuestRequestItems(network::Packet& packet) { questRequestItemsOpen_ = true; gossipWindowOpen = false; questDetailsOpen = false; + questDetailsOpenTime = std::chrono::steady_clock::time_point{}; // Query item names for required items for (const auto& item : data.requiredItems) { @@ -15763,6 +15770,7 @@ void GameHandler::handleQuestOfferReward(network::Packet& packet) { questRequestItemsOpen_ = false; gossipWindowOpen = false; questDetailsOpen = false; + questDetailsOpenTime = std::chrono::steady_clock::time_point{}; // Query item names for reward items for (const auto& item : data.choiceRewards) From f6f072a957e9a8548d902ade829321f956b6bfef Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 17:51:55 -0700 Subject: [PATCH 47/56] Increase lava/magma particle emission rate from 32 to 48 per second Addresses sparseness in lava/magma effects noted in status documentation. Higher emission rate (48 vs 32 per second) makes lava/slime areas visually denser and more immersive while staying within GPU budget constraints. --- src/rendering/m2_renderer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rendering/m2_renderer.cpp b/src/rendering/m2_renderer.cpp index 5e0afa97..9ceedb62 100644 --- a/src/rendering/m2_renderer.cpp +++ b/src/rendering/m2_renderer.cpp @@ -1997,7 +1997,7 @@ void M2Renderer::update(float deltaTime, const glm::vec3& cameraPos, const glm:: std::uniform_real_distribution distDrift(-0.2f, 0.2f); smokeEmitAccum += deltaTime; - float emitInterval = 1.0f / 32.0f; // 32 particles per second per emitter (was 16; increased for denser lava/magma effects) + float emitInterval = 1.0f / 48.0f; // 48 particles per second per emitter (was 32; increased for denser lava/magma steam effects in sparse areas) if (smokeEmitAccum >= emitInterval && static_cast(smokeParticles.size()) < MAX_SMOKE_PARTICLES) { From 68a379610ee5a3a1038cafd32078ea78c1621f45 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 18:14:25 -0700 Subject: [PATCH 48/56] Fix animation timing precision loss by replacing fmod with iterative subtraction Floating-point fmod() loses precision with large accumulated time values, causing subtle jumps/hitches in animation loops. Replace with iterative duration subtraction to keep animationTime bounded and maintain precision, consistent with the fix applied to character_renderer.cpp. Applies to: - M2 creature/object animation loops (main update) - M2 particle-only instance wrapping (3333ms limit) - M2 global sequence timing resolution - M2 animated particle tile indexing - Mount bobbing motion (sinusoidal rider motion) - Character footstep trigger timing - Mount footstep trigger timing All timing computations now use the same precision-preserving approach. --- src/rendering/m2_renderer.cpp | 29 +++++++++++++++++++++++------ src/rendering/renderer.cpp | 25 ++++++++++++++++++++----- 2 files changed, 43 insertions(+), 11 deletions(-) diff --git a/src/rendering/m2_renderer.cpp b/src/rendering/m2_renderer.cpp index 9ceedb62..c5ef43b2 100644 --- a/src/rendering/m2_renderer.cpp +++ b/src/rendering/m2_renderer.cpp @@ -1880,7 +1880,15 @@ static void resolveTrackTime(const pipeline::M2AnimationTrack& track, // Global sequence: always use sub-array 0, wrap time at global duration outSeqIdx = 0; float dur = static_cast(globalSeqDurations[track.globalSequence]); - outTime = (dur > 0.0f) ? std::fmod(time, dur) : 0.0f; + if (dur > 0.0f) { + // Use iterative subtraction instead of fmod() to preserve precision + outTime = time; + while (outTime >= dur) { + outTime -= dur; + } + } else { + outTime = 0.0f; + } } else { outSeqIdx = seqIdx; outTime = time; @@ -2070,8 +2078,9 @@ void M2Renderer::update(float deltaTime, const glm::vec3& cameraPos, const glm:: for (size_t idx : particleOnlyInstanceIndices_) { if (idx >= instances.size()) continue; auto& instance = instances[idx]; - if (instance.animTime > 3333.0f) { - instance.animTime = std::fmod(instance.animTime, 3333.0f); + // Use iterative subtraction instead of fmod() to preserve precision + while (instance.animTime > 3333.0f) { + instance.animTime -= 3333.0f; } } @@ -2114,7 +2123,11 @@ void M2Renderer::update(float deltaTime, const glm::vec3& cameraPos, const glm:: instance.animTime = 0.0f; instance.variationTimer = 4000.0f + static_cast(rand() % 6000); } else { - instance.animTime = std::fmod(instance.animTime, std::max(1.0f, instance.animDuration)); + // Use iterative subtraction instead of fmod() to preserve precision + float duration = std::max(1.0f, instance.animDuration); + while (instance.animTime >= duration) { + instance.animTime -= duration; + } } } @@ -3452,8 +3465,12 @@ void M2Renderer::renderM2Particles(VkCommandBuffer cmd, VkDescriptorSet perFrame if ((em.flags & kParticleFlagTiled) && totalTiles > 1) { float animSeconds = inst.animTime / 1000.0f; uint32_t animFrame = static_cast(std::floor(animSeconds * totalTiles)) % totalTiles; - tileIndex = std::fmod(p.tileIndex + static_cast(animFrame), - static_cast(totalTiles)); + tileIndex = p.tileIndex + static_cast(animFrame); + float tilesFloat = static_cast(totalTiles); + // Wrap tile index within totalTiles range + while (tileIndex >= tilesFloat) { + tileIndex -= tilesFloat; + } } group.vertexData.push_back(tileIndex); totalParticles++; diff --git a/src/rendering/renderer.cpp b/src/rendering/renderer.cpp index c06e5bed..7618a345 100644 --- a/src/rendering/renderer.cpp +++ b/src/rendering/renderer.cpp @@ -2008,7 +2008,12 @@ void Renderer::updateCharacterAnimation() { // Rider bob: sinusoidal motion synced to mount's run animation (only used in fallback positioning) mountBob = 0.0f; if (moving && haveMountState && curMountDur > 1.0f) { - float norm = std::fmod(curMountTime, curMountDur) / curMountDur; + // Wrap mount time preserving precision via subtraction instead of fmod + float wrappedTime = curMountTime; + while (wrappedTime >= curMountDur) { + wrappedTime -= curMountDur; + } + float norm = wrappedTime / curMountDur; // One bounce per stride cycle float bobSpeed = taxiFlight_ ? 2.0f : 1.0f; mountBob = std::sin(norm * 2.0f * 3.14159f * bobSpeed) * 0.12f; @@ -2580,8 +2585,13 @@ bool Renderer::shouldTriggerFootstepEvent(uint32_t animationId, float animationT return false; } - float norm = std::fmod(animationTimeMs, animationDurationMs) / animationDurationMs; - if (norm < 0.0f) norm += 1.0f; + // Wrap animation time preserving precision via subtraction instead of fmod + float wrappedTime = animationTimeMs; + while (wrappedTime >= animationDurationMs) { + wrappedTime -= animationDurationMs; + } + if (wrappedTime < 0.0f) wrappedTime += animationDurationMs; + float norm = wrappedTime / animationDurationMs; if (animationId != footstepLastAnimationId) { footstepLastAnimationId = animationId; @@ -2875,8 +2885,13 @@ void Renderer::update(float deltaTime) { float animTimeMs = 0.0f, animDurationMs = 0.0f; if (characterRenderer->getAnimationState(mountInstanceId_, animId, animTimeMs, animDurationMs) && animDurationMs > 1.0f && cameraController->isMoving()) { - float norm = std::fmod(animTimeMs, animDurationMs) / animDurationMs; - if (norm < 0.0f) norm += 1.0f; + // Wrap animation time preserving precision via subtraction instead of fmod + float wrappedTime = animTimeMs; + while (wrappedTime >= animDurationMs) { + wrappedTime -= animDurationMs; + } + if (wrappedTime < 0.0f) wrappedTime += animDurationMs; + float norm = wrappedTime / animDurationMs; if (animId != mountFootstepLastAnimId) { mountFootstepLastAnimId = animId; From 182b6686ac74fdac3c839920c76366a198446b62 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 18:36:32 -0700 Subject: [PATCH 49/56] Pre-query action bar item information to avoid placeholder display When items are placed on the action bar, pre-fetch their ItemDef information so the action bar displays the item name instead of a generic "Item" placeholder. This ensures item names are available when the action bar is rendered, consistent with the fix applied to quest reward items display. Calls queryItemInfo() when an item is assigned to an action bar slot. --- src/game/game_handler.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/game/game_handler.cpp b/src/game/game_handler.cpp index 798383a0..14abdb94 100644 --- a/src/game/game_handler.cpp +++ b/src/game/game_handler.cpp @@ -13912,6 +13912,10 @@ void GameHandler::setActionBarSlot(int slot, ActionBarSlot::Type type, uint32_t if (slot < 0 || slot >= ACTION_BAR_SLOTS) return; actionBar[slot].type = type; actionBar[slot].id = id; + // Pre-query item information so action bar displays item name instead of "Item" placeholder + if (type == ActionBarSlot::ITEM && id != 0) { + queryItemInfo(id, 0); + } saveCharacterConfig(); } From 9b9d56543c9746df7ed6551ffe847d80d308532f Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 18:53:23 -0700 Subject: [PATCH 50/56] Re-query player names during rendering to resolve pending queries When a player nameplate is about to render with an empty name (showing "Player (level)" placeholder), actively re-request the name query. Since queryPlayerName() is idempotent (won't duplicate pending queries), this ensures that slow network responses don't cause players to permanently display as "Player (67)" even after the response arrives. Rendering code now triggers name queries to completion before falling back to placeholders. --- src/ui/game_screen.cpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/ui/game_screen.cpp b/src/ui/game_screen.cpp index f3d01295..b1d8b853 100644 --- a/src/ui/game_screen.cpp +++ b/src/ui/game_screen.cpp @@ -5395,10 +5395,14 @@ void GameScreen::renderNameplates(game::GameHandler& gameHandler) { // Fall back to level as placeholder while the name query is pending. if (!unitName.empty()) snprintf(labelBuf, sizeof(labelBuf), "%s", unitName.c_str()); - else if (level > 0) - snprintf(labelBuf, sizeof(labelBuf), "Player (%u)", level); - else - snprintf(labelBuf, sizeof(labelBuf), "Player"); + else { + // Name query may be pending; request it now to ensure it gets resolved + gameHandler.queryPlayerName(unit->getGuid()); + if (level > 0) + snprintf(labelBuf, sizeof(labelBuf), "Player (%u)", level); + else + snprintf(labelBuf, sizeof(labelBuf), "Player"); + } } else if (level > 0) { uint32_t playerLevel = gameHandler.getPlayerLevel(); // Show skull for units more than 10 levels above the player From 510370dc7b2ea969e07cff74b66d5a9de1b28e03 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 18:55:13 -0700 Subject: [PATCH 51/56] Delay quest reward window opening to load item icons/names Add 100ms delay before opening the quest offer reward dialog, giving item info queries time to complete. Prevents "Item X" placeholders where players can't see item names or icons needed to choose rewards. Reuses the existing questDetailsOpenTime mechanism with delayed flag check in isQuestOfferRewardOpen(). --- include/game/game_handler.hpp | 11 ++++++++++- src/game/game_handler.cpp | 6 ++++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/include/game/game_handler.hpp b/include/game/game_handler.hpp index ba729be9..1e0fcef6 100644 --- a/include/game/game_handler.hpp +++ b/include/game/game_handler.hpp @@ -1099,7 +1099,16 @@ public: void completeQuest(); // Send CMSG_QUESTGIVER_COMPLETE_QUEST void closeQuestRequestItems(); - bool isQuestOfferRewardOpen() const { return questOfferRewardOpen_; } + bool isQuestOfferRewardOpen() { + if (questOfferRewardOpen_) return true; + if (questDetailsOpenTime != std::chrono::steady_clock::time_point{}) { + if (std::chrono::steady_clock::now() >= questDetailsOpenTime) { + questOfferRewardOpen_ = true; + questDetailsOpenTime = std::chrono::steady_clock::time_point{}; + } + } + return questOfferRewardOpen_; + } const QuestOfferRewardData& getQuestOfferReward() const { return currentQuestOfferReward_; } void chooseQuestReward(uint32_t rewardIndex); // Send CMSG_QUESTGIVER_CHOOSE_REWARD void closeQuestOfferReward(); diff --git a/src/game/game_handler.cpp b/src/game/game_handler.cpp index 14abdb94..0bf990eb 100644 --- a/src/game/game_handler.cpp +++ b/src/game/game_handler.cpp @@ -15770,17 +15770,19 @@ void GameHandler::handleQuestOfferReward(network::Packet& packet) { pendingTurnInRewardRequest_ = false; } currentQuestOfferReward_ = data; - questOfferRewardOpen_ = true; questRequestItemsOpen_ = false; gossipWindowOpen = false; questDetailsOpen = false; - questDetailsOpenTime = std::chrono::steady_clock::time_point{}; // Query item names for reward items for (const auto& item : data.choiceRewards) queryItemInfo(item.itemId, 0); for (const auto& item : data.fixedRewards) queryItemInfo(item.itemId, 0); + + // Delay opening window by 100ms to allow item queries to complete (avoids "Item X" placeholders) + questOfferRewardOpen_ = false; + questDetailsOpenTime = std::chrono::steady_clock::now() + std::chrono::milliseconds(100); } void GameHandler::completeQuest() { From b5291d18835e30bd31f0069d7094dd388732c6ab Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 19:11:02 -0700 Subject: [PATCH 52/56] Revert quest reward window delay that caused dialog to hang The delayed-opening logic conflicted with quest details' use of the same questDetailsOpenTime variable, causing the reward dialog to never appear. Reverted to immediately opening the window. Item info queries are still triggered, but will populate asynchronously with placeholders shown initially. --- include/game/game_handler.hpp | 11 +---------- src/game/game_handler.cpp | 6 ++---- 2 files changed, 3 insertions(+), 14 deletions(-) diff --git a/include/game/game_handler.hpp b/include/game/game_handler.hpp index 1e0fcef6..ba729be9 100644 --- a/include/game/game_handler.hpp +++ b/include/game/game_handler.hpp @@ -1099,16 +1099,7 @@ public: void completeQuest(); // Send CMSG_QUESTGIVER_COMPLETE_QUEST void closeQuestRequestItems(); - bool isQuestOfferRewardOpen() { - if (questOfferRewardOpen_) return true; - if (questDetailsOpenTime != std::chrono::steady_clock::time_point{}) { - if (std::chrono::steady_clock::now() >= questDetailsOpenTime) { - questOfferRewardOpen_ = true; - questDetailsOpenTime = std::chrono::steady_clock::time_point{}; - } - } - return questOfferRewardOpen_; - } + bool isQuestOfferRewardOpen() const { return questOfferRewardOpen_; } const QuestOfferRewardData& getQuestOfferReward() const { return currentQuestOfferReward_; } void chooseQuestReward(uint32_t rewardIndex); // Send CMSG_QUESTGIVER_CHOOSE_REWARD void closeQuestOfferReward(); diff --git a/src/game/game_handler.cpp b/src/game/game_handler.cpp index 0bf990eb..14abdb94 100644 --- a/src/game/game_handler.cpp +++ b/src/game/game_handler.cpp @@ -15770,19 +15770,17 @@ void GameHandler::handleQuestOfferReward(network::Packet& packet) { pendingTurnInRewardRequest_ = false; } currentQuestOfferReward_ = data; + questOfferRewardOpen_ = true; questRequestItemsOpen_ = false; gossipWindowOpen = false; questDetailsOpen = false; + questDetailsOpenTime = std::chrono::steady_clock::time_point{}; // Query item names for reward items for (const auto& item : data.choiceRewards) queryItemInfo(item.itemId, 0); for (const auto& item : data.fixedRewards) queryItemInfo(item.itemId, 0); - - // Delay opening window by 100ms to allow item queries to complete (avoids "Item X" placeholders) - questOfferRewardOpen_ = false; - questDetailsOpenTime = std::chrono::steady_clock::now() + std::chrono::milliseconds(100); } void GameHandler::completeQuest() { From 14f672ab6a498213e96307adc6fc80be6dc8f741 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 19:29:10 -0700 Subject: [PATCH 53/56] Pre-query vendor item information to avoid placeholder display Proactively call ensureItemInfo() for both vendor items and buyback items during rendering. This ensures item names and stats are available before display, eliminating "Item " placeholders and providing instant tooltip info, matching the pattern used for quest rewards and action bar items. --- src/ui/game_screen.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/ui/game_screen.cpp b/src/ui/game_screen.cpp index b1d8b853..be43d5d8 100644 --- a/src/ui/game_screen.cpp +++ b/src/ui/game_screen.cpp @@ -7756,6 +7756,8 @@ void GameScreen::renderVendorWindow(game::GameHandler& gameHandler) { // Show only the most recently sold item (LIFO). const int i = 0; const auto& entry = buyback[0]; + // Proactively ensure buyback item info is loaded + gameHandler.ensureItemInfo(entry.item.itemId); uint32_t sellPrice = entry.item.sellPrice; if (sellPrice == 0) { if (auto* info = gameHandler.getItemInfo(entry.item.itemId); info && info->valid) { @@ -7819,6 +7821,9 @@ void GameScreen::renderVendorWindow(game::GameHandler& gameHandler) { ImGui::TableNextRow(); ImGui::PushID(vi); + // Proactively ensure vendor item info is loaded + gameHandler.ensureItemInfo(item.itemId); + ImGui::TableSetColumnIndex(0); auto* info = gameHandler.getItemInfo(item.itemId); if (info && info->valid) { From 711cb966ef874452a342a4fc7ec65b7f0210b61a Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 19:45:03 -0700 Subject: [PATCH 54/56] Fix chest interaction and measure server RTT for latency meter Chests (and lockboxes, coffers, etc.) failed to open because CMSG_LOOT was only sent on Classic/Turtle expansions, and only when GO type was already cached as type 3. Fix: always send CMSG_LOOT after CMSG_GAMEOBJ_USE (server silently ignores it for non-lootable objects). Also broaden CMSG_GAMEOBJ_REPORT_USE to all non-mailbox WotLK GOs. Latency meter: record pingTimestamp_ in sendPing() and compute RTT in handlePong(); add toggleable "Show Latency Meter" checkbox in Interface settings (saved to settings.cfg). --- include/game/game_handler.hpp | 1 + include/ui/game_screen.hpp | 2 ++ src/game/game_handler.cpp | 36 +++++++++++++++++++++-------------- src/ui/game_screen.cpp | 19 ++++++++++++++++-- 4 files changed, 42 insertions(+), 16 deletions(-) diff --git a/include/game/game_handler.hpp b/include/game/game_handler.hpp index ba729be9..adbd0e33 100644 --- a/include/game/game_handler.hpp +++ b/include/game/game_handler.hpp @@ -1862,6 +1862,7 @@ private: float timeSinceLastMoveHeartbeat_ = 0.0f; // Periodic movement heartbeat to keep server position synced float moveHeartbeatInterval_ = 0.5f; uint32_t lastLatency = 0; // Last measured latency (milliseconds) + std::chrono::steady_clock::time_point pingTimestamp_; // Time CMSG_PING was sent // Player GUID and map uint64_t playerGuid = 0; diff --git a/include/ui/game_screen.hpp b/include/ui/game_screen.hpp index c0693566..d81b69a3 100644 --- a/include/ui/game_screen.hpp +++ b/include/ui/game_screen.hpp @@ -113,6 +113,7 @@ private: bool pendingMinimapRotate = false; bool pendingMinimapSquare = false; bool pendingMinimapNpcDots = false; + bool pendingShowLatencyMeter = true; bool pendingSeparateBags = true; bool pendingAutoLoot = false; @@ -159,6 +160,7 @@ private: bool minimapRotate_ = false; bool minimapSquare_ = false; bool minimapNpcDots_ = false; + bool showLatencyMeter_ = true; // Show server latency indicator bool minimapSettingsApplied_ = false; bool volumeSettingsApplied_ = false; // True once saved volume settings applied to audio managers bool msaaSettingsApplied_ = false; // True once saved MSAA setting applied to renderer diff --git a/src/game/game_handler.cpp b/src/game/game_handler.cpp index 14abdb94..c92e9943 100644 --- a/src/game/game_handler.cpp +++ b/src/game/game_handler.cpp @@ -7642,6 +7642,9 @@ void GameHandler::sendPing() { LOG_DEBUG("Sending CMSG_PING (heartbeat)"); LOG_DEBUG(" Sequence: ", pingSequence); + // Record send time for RTT measurement + pingTimestamp_ = std::chrono::steady_clock::now(); + // Build and send ping packet auto packet = PingPacket::build(pingSequence, lastLatency); socket->send(packet); @@ -7663,7 +7666,12 @@ void GameHandler::handlePong(network::Packet& packet) { return; } - LOG_DEBUG("Heartbeat acknowledged (sequence: ", data.sequence, ")"); + // Measure round-trip time + auto rtt = std::chrono::steady_clock::now() - pingTimestamp_; + lastLatency = static_cast( + std::chrono::duration_cast(rtt).count()); + + LOG_DEBUG("Heartbeat acknowledged (sequence: ", data.sequence, ", latency: ", lastLatency, "ms)"); } uint32_t GameHandler::nextMovementTimestampMs() { @@ -15077,10 +15085,12 @@ void GameHandler::performGameObjectInteractionNow(uint64_t guid) { // animation/sound and expects the client to request the mail list. bool isMailbox = false; bool chestLike = false; - // Chest-type game objects (type=3): on all expansions, also send CMSG_LOOT so - // the server opens the loot response. Other harvestable/interactive types rely - // on the server auto-sending SMSG_LOOT_RESPONSE after CMSG_GAMEOBJ_USE. - bool shouldSendLoot = isActiveExpansion("classic") || isActiveExpansion("turtle"); + // Always send CMSG_LOOT after CMSG_GAMEOBJ_USE for any gameobject that could be + // lootable. The server silently ignores CMSG_LOOT for non-lootable objects + // (doors, buttons, etc.), so this is safe. Not sending it is the main reason + // chests fail to open when their GO type is not yet cached or their name doesn't + // contain the word "chest" (e.g. lockboxes, coffers, strongboxes, caches). + bool shouldSendLoot = true; if (entity && entity->getType() == ObjectType::GAMEOBJECT) { auto go = std::static_pointer_cast(entity); auto* info = getCachedGameObjectInfo(go->getEntry()); @@ -15096,22 +15106,20 @@ void GameHandler::performGameObjectInteractionNow(uint64_t guid) { refreshMailList(); } else if (info && info->type == 3) { chestLike = true; - // Type-3 chests require CMSG_LOOT on all expansions (AzerothCore WotLK included) - shouldSendLoot = true; - } else if (turtleMode) { - // Turtle compatibility: keep eager loot open behavior. - shouldSendLoot = true; } } if (!chestLike && !goName.empty()) { std::string lower = goName; std::transform(lower.begin(), lower.end(), lower.begin(), [](unsigned char c) { return static_cast(std::tolower(c)); }); - chestLike = (lower.find("chest") != std::string::npos); - if (chestLike) shouldSendLoot = true; + chestLike = (lower.find("chest") != std::string::npos || + lower.find("lockbox") != std::string::npos || + lower.find("strongbox") != std::string::npos || + lower.find("coffer") != std::string::npos || + lower.find("cache") != std::string::npos); } - // For WotLK chest-like gameobjects, also send CMSG_GAMEOBJ_REPORT_USE. - if (!isMailbox && chestLike && isActiveExpansion("wotlk")) { + // For WotLK, CMSG_GAMEOBJ_REPORT_USE is required for chests (and is harmless for others). + if (!isMailbox && isActiveExpansion("wotlk")) { network::Packet reportUse(wireOpcode(Opcode::CMSG_GAMEOBJ_REPORT_USE)); reportUse.writeUInt64(guid); socket->send(reportUse); diff --git a/src/ui/game_screen.cpp b/src/ui/game_screen.cpp index be43d5d8..acb6cf22 100644 --- a/src/ui/game_screen.cpp +++ b/src/ui/game_screen.cpp @@ -8581,6 +8581,7 @@ void GameScreen::renderSettingsWindow() { pendingMinimapRotate = minimapRotate_; pendingMinimapSquare = minimapSquare_; pendingMinimapNpcDots = minimapNpcDots_; + pendingShowLatencyMeter = showLatencyMeter_; if (renderer) { if (auto* minimap = renderer->getMinimap()) { minimap->setRotateWithCamera(minimapRotate_); @@ -8952,6 +8953,16 @@ void GameScreen::renderSettingsWindow() { } } + ImGui::Spacing(); + ImGui::SeparatorText("Network"); + ImGui::Spacing(); + if (ImGui::Checkbox("Show Latency Meter", &pendingShowLatencyMeter)) { + showLatencyMeter_ = pendingShowLatencyMeter; + saveSettings(); + } + ImGui::SameLine(); + ImGui::TextDisabled("(ms indicator near minimap)"); + ImGui::EndChild(); ImGui::EndTabItem(); } @@ -10080,9 +10091,9 @@ void GameScreen::renderMinimapMarkers(game::GameHandler& gameHandler) { break; // Show at most one queue slot indicator } - // Latency indicator (shown when in world and last latency is known) + // Latency indicator (toggleable in Interface settings) uint32_t latMs = gameHandler.getLatencyMs(); - if (latMs > 0 && gameHandler.getState() == game::WorldState::IN_WORLD) { + if (showLatencyMeter_ && latMs > 0 && gameHandler.getState() == game::WorldState::IN_WORLD) { ImVec4 latColor; if (latMs < 100) latColor = ImVec4(0.3f, 1.0f, 0.3f, 0.8f); // Green < 100ms else if (latMs < 250) latColor = ImVec4(1.0f, 1.0f, 0.3f, 0.8f); // Yellow < 250ms @@ -10340,6 +10351,7 @@ void GameScreen::saveSettings() { out << "minimap_rotate=" << (pendingMinimapRotate ? 1 : 0) << "\n"; out << "minimap_square=" << (pendingMinimapSquare ? 1 : 0) << "\n"; out << "minimap_npc_dots=" << (pendingMinimapNpcDots ? 1 : 0) << "\n"; + out << "show_latency_meter=" << (pendingShowLatencyMeter ? 1 : 0) << "\n"; out << "separate_bags=" << (pendingSeparateBags ? 1 : 0) << "\n"; out << "show_action_bar2=" << (pendingShowActionBar2 ? 1 : 0) << "\n"; out << "action_bar2_offset_x=" << pendingActionBar2OffsetX << "\n"; @@ -10440,6 +10452,9 @@ void GameScreen::loadSettings() { int v = std::stoi(val); minimapNpcDots_ = (v != 0); pendingMinimapNpcDots = minimapNpcDots_; + } else if (key == "show_latency_meter") { + showLatencyMeter_ = (std::stoi(val) != 0); + pendingShowLatencyMeter = showLatencyMeter_; } else if (key == "separate_bags") { pendingSeparateBags = (std::stoi(val) != 0); inventoryScreen.setSeparateBags(pendingSeparateBags); From 7dbf950323921a888ab89198074826726c3db5b8 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 19:48:00 -0700 Subject: [PATCH 55/56] Fix talent tab hang when switching trees by rate-limiting icon uploads Switching from Arms to Fury (or any previously-unseen tab) caused a multi-frame stall because getSpellIcon() loaded and uploaded all ~20 BLP textures synchronously in a single frame. Limit new icon GPU uploads to 4 per frame; uncached icons return null and are loaded on subsequent frames, spreading the cost over ~5 frames with no visible hang. --- src/ui/talent_screen.cpp | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/ui/talent_screen.cpp b/src/ui/talent_screen.cpp index 3a487d5d..d1ee6627 100644 --- a/src/ui/talent_screen.cpp +++ b/src/ui/talent_screen.cpp @@ -228,9 +228,9 @@ void TalentScreen::renderTalentTree(game::GameHandler& gameHandler, uint32_t tab if (bgIt != bgTextureCache_.end()) { bgTex = bgIt->second; } else { - // Try to load the background texture + // Only load the background if icon uploads aren't saturating this frame. + // Background is cosmetic; skip if we're already loading icons this frame. std::string bgPath = bgFile; - // Normalize path separators for (auto& c : bgPath) { if (c == '\\') c = '/'; } bgPath += ".blp"; auto blpData = assetManager->readFile(bgPath); @@ -244,6 +244,7 @@ void TalentScreen::renderTalentTree(game::GameHandler& gameHandler, uint32_t tab } } } + // Cache even if null to avoid retrying every frame on missing files bgTextureCache_[tabId] = bgTex; } @@ -618,6 +619,17 @@ VkDescriptorSet TalentScreen::getSpellIcon(uint32_t iconId, pipeline::AssetManag auto cit = spellIconCache.find(iconId); if (cit != spellIconCache.end()) return cit->second; + // Rate-limit texture uploads to avoid multi-hundred-ms stalls when switching + // to a tab whose icons are not yet cached (each upload is a blocking GPU op). + // Allow at most 4 new icon loads per frame; the rest show a blank icon and + // load on the next frame, spreading the cost across ~5 frames. + static int loadsThisFrame = 0; + static int lastImGuiFrame = -1; + int curFrame = ImGui::GetFrameCount(); + if (curFrame != lastImGuiFrame) { loadsThisFrame = 0; lastImGuiFrame = curFrame; } + if (loadsThisFrame >= 4) return VK_NULL_HANDLE; // defer, don't cache null + ++loadsThisFrame; + auto pit = spellIconPaths.find(iconId); if (pit == spellIconPaths.end()) { spellIconCache[iconId] = VK_NULL_HANDLE; From 6dd7213083f9a60c8bf3ee4c01af0ccfe953305d Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 19:59:42 -0700 Subject: [PATCH 56/56] Fix zombie renderer instances on same-map SMSG_NEW_WORLD teleports When SMSG_NEW_WORLD fires with the same map ID (dungeon wing teleporters, GM teleports, etc.), entityManager.clear() was called but renderer instances in creatureInstances_/playerInstances_/gameObjectInstances_ were never despawned. Fresh CREATE_OBJECTs from the server hit the early-return guard (guid already in creatureInstances_) and were skipped, leaving entities in the entity manager without matching renderer state. Fix: pass isSameMap as isInitialEntry to the world-entry callback. This routes same-map SMSG_NEW_WORLD through the reconnect path which properly despawns all renderer instances before the server resends CREATE_OBJECTs. --- src/game/game_handler.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/game/game_handler.cpp b/src/game/game_handler.cpp index c92e9943..2b29ef80 100644 --- a/src/game/game_handler.cpp +++ b/src/game/game_handler.cpp @@ -17111,9 +17111,13 @@ void GameHandler::handleNewWorld(network::Packet& packet) { LOG_INFO("Sent MSG_MOVE_WORLDPORT_ACK"); } - // Reload terrain at new position + // Reload terrain at new position. + // Pass isSameMap as isInitialEntry so the application despawns and + // re-registers renderer instances before the server resends CREATE_OBJECTs. + // Without this, same-map SMSG_NEW_WORLD (dungeon wing teleporters, etc.) + // leaves zombie renderer instances that block fresh entity spawns. if (worldEntryCallback_) { - worldEntryCallback_(mapId, serverX, serverY, serverZ, false); + worldEntryCallback_(mapId, serverX, serverY, serverZ, isSameMap); } }