From d7e1a3773cb30593b2af9c49a5310845ed67b3b6 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 13:56:16 -0700 Subject: [PATCH] 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();