fix(combatlog): consume full spell go target lists when capped

This commit is contained in:
Kelsi 2026-03-14 10:09:04 -07:00
parent c90c8fb8cf
commit 5ecc46623a
3 changed files with 74 additions and 66 deletions

View file

@ -410,33 +410,34 @@ bool ClassicPacketParsers::parseSpellGo(network::Packet& packet, SpellGoData& da
// Hit targets // Hit targets
if (rem() < 1) return true; if (rem() < 1) return true;
data.hitCount = packet.readUInt8(); const uint8_t rawHitCount = packet.readUInt8();
// Cap hit count to prevent OOM from huge target lists if (rawHitCount > 128) {
if (data.hitCount > 128) { LOG_WARNING("[Classic] Spell go: hitCount capped (requested=", (int)rawHitCount, ")");
LOG_WARNING("[Classic] Spell go: hitCount capped (requested=", (int)data.hitCount, ")");
data.hitCount = 128;
} }
data.hitTargets.reserve(data.hitCount); const uint8_t storedHitLimit = std::min<uint8_t>(rawHitCount, 128);
for (uint8_t i = 0; i < data.hitCount && rem() >= 1; ++i) { data.hitTargets.reserve(storedHitLimit);
data.hitTargets.push_back(UpdateObjectParser::readPackedGuid(packet)); for (uint16_t i = 0; i < rawHitCount && rem() >= 1; ++i) {
const uint64_t targetGuid = UpdateObjectParser::readPackedGuid(packet);
if (i < storedHitLimit) {
data.hitTargets.push_back(targetGuid);
}
} }
data.hitCount = static_cast<uint8_t>(data.hitTargets.size());
// Check if we read all expected hits // Check if we read all expected hits
if (data.hitTargets.size() < data.hitCount) { if (data.hitTargets.size() < rawHitCount) {
LOG_WARNING("[Classic] Spell go: truncated hit targets at index ", (int)data.hitTargets.size(), LOG_WARNING("[Classic] Spell go: truncated hit targets at index ", (int)data.hitTargets.size(),
"/", (int)data.hitCount); "/", (int)rawHitCount);
data.hitCount = data.hitTargets.size();
} }
// Miss targets // Miss targets
if (rem() < 1) return true; if (rem() < 1) return true;
data.missCount = packet.readUInt8(); const uint8_t rawMissCount = packet.readUInt8();
// Cap miss count to prevent OOM if (rawMissCount > 128) {
if (data.missCount > 128) { LOG_WARNING("[Classic] Spell go: missCount capped (requested=", (int)rawMissCount, ")");
LOG_WARNING("[Classic] Spell go: missCount capped (requested=", (int)data.missCount, ")");
data.missCount = 128;
} }
data.missTargets.reserve(data.missCount); const uint8_t storedMissLimit = std::min<uint8_t>(rawMissCount, 128);
for (uint8_t i = 0; i < data.missCount && rem() >= 2; ++i) { data.missTargets.reserve(storedMissLimit);
for (uint16_t i = 0; i < rawMissCount && rem() >= 2; ++i) {
SpellGoMissEntry m; SpellGoMissEntry m;
m.targetGuid = UpdateObjectParser::readPackedGuid(packet); m.targetGuid = UpdateObjectParser::readPackedGuid(packet);
if (rem() < 1) break; if (rem() < 1) break;
@ -446,13 +447,15 @@ bool ClassicPacketParsers::parseSpellGo(network::Packet& packet, SpellGoData& da
(void)packet.readUInt32(); (void)packet.readUInt32();
(void)packet.readUInt8(); (void)packet.readUInt8();
} }
data.missTargets.push_back(m); if (i < storedMissLimit) {
data.missTargets.push_back(m);
}
} }
data.missCount = static_cast<uint8_t>(data.missTargets.size());
// Check if we read all expected misses // Check if we read all expected misses
if (data.missTargets.size() < data.missCount) { if (data.missTargets.size() < rawMissCount) {
LOG_WARNING("[Classic] Spell go: truncated miss targets at index ", (int)data.missTargets.size(), LOG_WARNING("[Classic] Spell go: truncated miss targets at index ", (int)data.missTargets.size(),
"/", (int)data.missCount); "/", (int)rawMissCount);
data.missCount = data.missTargets.size();
} }
LOG_DEBUG("[Classic] Spell go: spell=", data.spellId, " hits=", (int)data.hitCount, LOG_DEBUG("[Classic] Spell go: spell=", data.spellId, " hits=", (int)data.hitCount,

View file

@ -1277,32 +1277,33 @@ bool TbcPacketParsers::parseSpellGo(network::Packet& packet, SpellGoData& data)
return true; return true;
} }
data.hitCount = packet.readUInt8(); const uint8_t rawHitCount = packet.readUInt8();
// Cap hit count to prevent OOM from huge target lists if (rawHitCount > 128) {
if (data.hitCount > 128) { LOG_WARNING("[TBC] Spell go: hitCount capped (requested=", (int)rawHitCount, ")");
LOG_WARNING("[TBC] Spell go: hitCount capped (requested=", (int)data.hitCount, ")");
data.hitCount = 128;
} }
data.hitTargets.reserve(data.hitCount); const uint8_t storedHitLimit = std::min<uint8_t>(rawHitCount, 128);
for (uint8_t i = 0; i < data.hitCount && packet.getReadPos() + 8 <= packet.getSize(); ++i) { data.hitTargets.reserve(storedHitLimit);
data.hitTargets.push_back(packet.readUInt64()); // full GUID in TBC for (uint16_t i = 0; i < rawHitCount && packet.getReadPos() + 8 <= packet.getSize(); ++i) {
const uint64_t targetGuid = packet.readUInt64(); // full GUID in TBC
if (i < storedHitLimit) {
data.hitTargets.push_back(targetGuid);
}
} }
data.hitCount = static_cast<uint8_t>(data.hitTargets.size());
// Check if we read all expected hits // Check if we read all expected hits
if (data.hitTargets.size() < data.hitCount) { if (data.hitTargets.size() < rawHitCount) {
LOG_WARNING("[TBC] Spell go: truncated hit targets at index ", (int)data.hitTargets.size(), LOG_WARNING("[TBC] Spell go: truncated hit targets at index ", (int)data.hitTargets.size(),
"/", (int)data.hitCount); "/", (int)rawHitCount);
data.hitCount = data.hitTargets.size();
} }
if (packet.getReadPos() < packet.getSize()) { if (packet.getReadPos() < packet.getSize()) {
data.missCount = packet.readUInt8(); const uint8_t rawMissCount = packet.readUInt8();
// Cap miss count to prevent OOM if (rawMissCount > 128) {
if (data.missCount > 128) { LOG_WARNING("[TBC] Spell go: missCount capped (requested=", (int)rawMissCount, ")");
LOG_WARNING("[TBC] Spell go: missCount capped (requested=", (int)data.missCount, ")");
data.missCount = 128;
} }
data.missTargets.reserve(data.missCount); const uint8_t storedMissLimit = std::min<uint8_t>(rawMissCount, 128);
for (uint8_t i = 0; i < data.missCount && packet.getReadPos() + 9 <= packet.getSize(); ++i) { data.missTargets.reserve(storedMissLimit);
for (uint16_t i = 0; i < rawMissCount && packet.getReadPos() + 9 <= packet.getSize(); ++i) {
SpellGoMissEntry m; SpellGoMissEntry m;
m.targetGuid = packet.readUInt64(); // full GUID in TBC m.targetGuid = packet.readUInt64(); // full GUID in TBC
m.missType = packet.readUInt8(); m.missType = packet.readUInt8();
@ -1313,13 +1314,15 @@ bool TbcPacketParsers::parseSpellGo(network::Packet& packet, SpellGoData& data)
(void)packet.readUInt32(); (void)packet.readUInt32();
(void)packet.readUInt8(); (void)packet.readUInt8();
} }
data.missTargets.push_back(m); if (i < storedMissLimit) {
data.missTargets.push_back(m);
}
} }
data.missCount = static_cast<uint8_t>(data.missTargets.size());
// Check if we read all expected misses // Check if we read all expected misses
if (data.missTargets.size() < data.missCount) { if (data.missTargets.size() < rawMissCount) {
LOG_WARNING("[TBC] Spell go: truncated miss targets at index ", (int)data.missTargets.size(), LOG_WARNING("[TBC] Spell go: truncated miss targets at index ", (int)data.missTargets.size(),
"/", (int)data.missCount); "/", (int)rawMissCount);
data.missCount = data.missTargets.size();
} }
} }

View file

@ -3714,43 +3714,43 @@ bool SpellGoParser::parse(network::Packet& packet, SpellGoData& data) {
// Timestamp in 3.3.5a // Timestamp in 3.3.5a
packet.readUInt32(); packet.readUInt32();
data.hitCount = packet.readUInt8(); const uint8_t rawHitCount = packet.readUInt8();
// Cap hit count to prevent DoS via massive arrays if (rawHitCount > 128) {
if (data.hitCount > 128) { LOG_WARNING("Spell go: hitCount capped (requested=", (int)rawHitCount, ")");
LOG_WARNING("Spell go: hitCount capped (requested=", (int)data.hitCount, ")");
data.hitCount = 128;
} }
const uint8_t storedHitLimit = std::min<uint8_t>(rawHitCount, 128);
data.hitTargets.reserve(data.hitCount); data.hitTargets.reserve(storedHitLimit);
for (uint8_t i = 0; i < data.hitCount; ++i) { for (uint16_t i = 0; i < rawHitCount; ++i) {
// WotLK hit targets are packed GUIDs, like the caster and miss targets. // WotLK hit targets are packed GUIDs, like the caster and miss targets.
if (packet.getSize() - packet.getReadPos() < 1) { if (packet.getSize() - packet.getReadPos() < 1) {
LOG_WARNING("Spell go: truncated hit targets at index ", (int)i, "/", (int)data.hitCount); LOG_WARNING("Spell go: truncated hit targets at index ", i, "/", (int)rawHitCount);
data.hitCount = i;
break; break;
} }
data.hitTargets.push_back(UpdateObjectParser::readPackedGuid(packet)); const uint64_t targetGuid = UpdateObjectParser::readPackedGuid(packet);
if (i < storedHitLimit) {
data.hitTargets.push_back(targetGuid);
}
} }
data.hitCount = static_cast<uint8_t>(data.hitTargets.size());
// Validate missCount field exists // Validate missCount field exists
if (packet.getSize() - packet.getReadPos() < 1) { if (packet.getSize() - packet.getReadPos() < 1) {
return true; // Valid, just no misses return true; // Valid, just no misses
} }
data.missCount = packet.readUInt8(); const uint8_t rawMissCount = packet.readUInt8();
// Cap miss count to prevent DoS if (rawMissCount > 128) {
if (data.missCount > 128) { LOG_WARNING("Spell go: missCount capped (requested=", (int)rawMissCount, ")");
LOG_WARNING("Spell go: missCount capped (requested=", (int)data.missCount, ")");
data.missCount = 128;
} }
const uint8_t storedMissLimit = std::min<uint8_t>(rawMissCount, 128);
data.missTargets.reserve(data.missCount); data.missTargets.reserve(storedMissLimit);
for (uint8_t i = 0; i < data.missCount; ++i) { for (uint16_t i = 0; i < rawMissCount; ++i) {
// Each miss entry: packed GUID(1-8 bytes) + missType(1 byte). // Each miss entry: packed GUID(1-8 bytes) + missType(1 byte).
// REFLECT additionally appends uint32 reflectSpellId + uint8 reflectResult. // REFLECT additionally appends uint32 reflectSpellId + uint8 reflectResult.
if (packet.getSize() - packet.getReadPos() < 2) { if (packet.getSize() - packet.getReadPos() < 2) {
LOG_WARNING("Spell go: truncated miss targets at index ", (int)i, "/", (int)data.missCount); LOG_WARNING("Spell go: truncated miss targets at index ", i, "/", (int)rawMissCount);
data.missCount = i;
break; break;
} }
SpellGoMissEntry m; SpellGoMissEntry m;
@ -3758,15 +3758,17 @@ bool SpellGoParser::parse(network::Packet& packet, SpellGoData& data) {
m.missType = (packet.getSize() - packet.getReadPos() >= 1) ? packet.readUInt8() : 0; m.missType = (packet.getSize() - packet.getReadPos() >= 1) ? packet.readUInt8() : 0;
if (m.missType == 11) { if (m.missType == 11) {
if (packet.getSize() - packet.getReadPos() < 5) { if (packet.getSize() - packet.getReadPos() < 5) {
LOG_WARNING("Spell go: truncated reflect payload at miss index ", (int)i, "/", (int)data.missCount); LOG_WARNING("Spell go: truncated reflect payload at miss index ", i, "/", (int)rawMissCount);
data.missCount = i;
break; break;
} }
(void)packet.readUInt32(); (void)packet.readUInt32();
(void)packet.readUInt8(); (void)packet.readUInt8();
} }
data.missTargets.push_back(m); if (i < storedMissLimit) {
data.missTargets.push_back(m);
}
} }
data.missCount = static_cast<uint8_t>(data.missTargets.size());
LOG_DEBUG("Spell go: spell=", data.spellId, " hits=", (int)data.hitCount, LOG_DEBUG("Spell go: spell=", data.spellId, " hits=", (int)data.hitCount,
" misses=", (int)data.missCount); " misses=", (int)data.missCount);