phaneron/Kelsidavis-WoWee
1
0
Fork 0
mirror of https://github.com/Kelsidavis/WoWee.git synced 2026-03-22 23:30:14 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Add validation caps and in-loop size checks to gossip message parsing

Browse source 
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.
This commit is contained in:
Kelsi 2026-03-11 13:56:16 -07:00
parent d14f82cb7c
commit d7e1a3773c
2 changed files with 78 additions and 0 deletions
Download patch file Download diff file Expand all files Collapse all files

38
src/game/packet_parsers_classic.cpp
View file

@ -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();

40
src/game/packet_parsers_tbc.cpp
View file

@ -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();