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

Add packet size validation to SMSG_INITIAL_SPELLS parsing

Browse source 
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.
This commit is contained in:
Kelsi 2026-03-11 14:10:20 -07:00
parent f472ee3be8
commit d1414b6a46
1 changed files with 46 additions and 0 deletions
Download patch file Download diff file Expand all files Collapse all files

46
src/game/world_packets.cpp
View file

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