fix(combatlog): reject truncated spell go packets missing counts

This commit is contained in:
Kelsi 2026-03-14 13:37:28 -07:00
parent 4e97a19b23
commit fbcbdc2935
3 changed files with 52 additions and 37 deletions

View file

@ -414,8 +414,12 @@ bool ClassicPacketParsers::parseSpellGo(network::Packet& packet, SpellGoData& da
data.spellId = packet.readUInt32(); data.spellId = packet.readUInt32();
data.castFlags = packet.readUInt16(); // uint16 in Vanilla (uint32 in TBC/WotLK) data.castFlags = packet.readUInt16(); // uint16 in Vanilla (uint32 in TBC/WotLK)
// Hit targets // hitCount is mandatory in SMSG_SPELL_GO. Missing byte means truncation.
if (rem() < 1) return true; if (rem() < 1) {
LOG_WARNING("[Classic] Spell go: missing hitCount after fixed fields");
packet.setReadPos(startPos);
return false;
}
const uint8_t rawHitCount = packet.readUInt8(); const uint8_t rawHitCount = packet.readUInt8();
if (rawHitCount > 128) { if (rawHitCount > 128) {
LOG_WARNING("[Classic] Spell go: hitCount capped (requested=", (int)rawHitCount, ")"); LOG_WARNING("[Classic] Spell go: hitCount capped (requested=", (int)rawHitCount, ")");
@ -441,8 +445,12 @@ bool ClassicPacketParsers::parseSpellGo(network::Packet& packet, SpellGoData& da
} }
data.hitCount = static_cast<uint8_t>(data.hitTargets.size()); data.hitCount = static_cast<uint8_t>(data.hitTargets.size());
// Miss targets // missCount is mandatory in SMSG_SPELL_GO. Missing byte means truncation.
if (rem() < 1) return true; if (rem() < 1) {
LOG_WARNING("[Classic] Spell go: missing missCount after hit target list");
packet.setReadPos(startPos);
return false;
}
const uint8_t rawMissCount = packet.readUInt8(); const uint8_t rawMissCount = packet.readUInt8();
if (rawMissCount > 128) { if (rawMissCount > 128) {
LOG_WARNING("[Classic] Spell go: missCount capped (requested=", (int)rawMissCount, ")"); LOG_WARNING("[Classic] Spell go: missCount capped (requested=", (int)rawMissCount, ")");

View file

@ -1277,8 +1277,9 @@ bool TbcPacketParsers::parseSpellGo(network::Packet& packet, SpellGoData& data)
// NOTE: NO timestamp field here in TBC (WotLK added packet.readUInt32()) // NOTE: NO timestamp field here in TBC (WotLK added packet.readUInt32())
if (packet.getReadPos() >= packet.getSize()) { if (packet.getReadPos() >= packet.getSize()) {
LOG_DEBUG("[TBC] Spell go: spell=", data.spellId, " (no hit data)"); LOG_WARNING("[TBC] Spell go: missing hitCount after fixed fields");
return true; packet.setReadPos(startPos);
return false;
} }
const uint8_t rawHitCount = packet.readUInt8(); const uint8_t rawHitCount = packet.readUInt8();
@ -1306,43 +1307,47 @@ bool TbcPacketParsers::parseSpellGo(network::Packet& packet, SpellGoData& data)
} }
data.hitCount = static_cast<uint8_t>(data.hitTargets.size()); data.hitCount = static_cast<uint8_t>(data.hitTargets.size());
if (packet.getReadPos() < packet.getSize()) { if (packet.getReadPos() >= packet.getSize()) {
const uint8_t rawMissCount = packet.readUInt8(); LOG_WARNING("[TBC] Spell go: missing missCount after hit target list");
if (rawMissCount > 128) { packet.setReadPos(startPos);
LOG_WARNING("[TBC] Spell go: missCount capped (requested=", (int)rawMissCount, ")"); return false;
}
const uint8_t rawMissCount = packet.readUInt8();
if (rawMissCount > 128) {
LOG_WARNING("[TBC] Spell go: missCount capped (requested=", (int)rawMissCount, ")");
}
const uint8_t storedMissLimit = std::min<uint8_t>(rawMissCount, 128);
data.missTargets.reserve(storedMissLimit);
for (uint16_t i = 0; i < rawMissCount; ++i) {
if (packet.getReadPos() + 9 > packet.getSize()) {
LOG_WARNING("[TBC] Spell go: truncated miss targets at index ", i,
"/", (int)rawMissCount);
truncatedTargets = true;
break;
} }
const uint8_t storedMissLimit = std::min<uint8_t>(rawMissCount, 128); SpellGoMissEntry m;
data.missTargets.reserve(storedMissLimit); m.targetGuid = packet.readUInt64(); // full GUID in TBC
for (uint16_t i = 0; i < rawMissCount; ++i) { m.missType = packet.readUInt8();
if (packet.getReadPos() + 9 > packet.getSize()) { if (m.missType == 11) {
LOG_WARNING("[TBC] Spell go: truncated miss targets at index ", i, if (packet.getReadPos() + 5 > packet.getSize()) {
LOG_WARNING("[TBC] Spell go: truncated reflect payload at miss index ", i,
"/", (int)rawMissCount); "/", (int)rawMissCount);
truncatedTargets = true; truncatedTargets = true;
break; break;
} }
SpellGoMissEntry m; (void)packet.readUInt32();
m.targetGuid = packet.readUInt64(); // full GUID in TBC (void)packet.readUInt8();
m.missType = packet.readUInt8();
if (m.missType == 11) {
if (packet.getReadPos() + 5 > packet.getSize()) {
LOG_WARNING("[TBC] Spell go: truncated reflect payload at miss index ", i,
"/", (int)rawMissCount);
truncatedTargets = true;
break;
}
(void)packet.readUInt32();
(void)packet.readUInt8();
}
if (i < storedMissLimit) {
data.missTargets.push_back(m);
}
} }
if (truncatedTargets) { if (i < storedMissLimit) {
packet.setReadPos(startPos); data.missTargets.push_back(m);
return false;
} }
data.missCount = static_cast<uint8_t>(data.missTargets.size());
} }
if (truncatedTargets) {
packet.setReadPos(startPos);
return false;
}
data.missCount = static_cast<uint8_t>(data.missTargets.size());
LOG_DEBUG("[TBC] Spell go: spell=", data.spellId, " hits=", (int)data.hitCount, LOG_DEBUG("[TBC] Spell go: spell=", data.spellId, " hits=", (int)data.hitCount,
" misses=", (int)data.missCount); " misses=", (int)data.missCount);

View file

@ -3782,9 +3782,11 @@ bool SpellGoParser::parse(network::Packet& packet, SpellGoData& data) {
} }
data.hitCount = static_cast<uint8_t>(data.hitTargets.size()); data.hitCount = static_cast<uint8_t>(data.hitTargets.size());
// Validate missCount field exists // missCount is mandatory in SMSG_SPELL_GO. Missing byte means truncation.
if (packet.getSize() - packet.getReadPos() < 1) { if (packet.getSize() - packet.getReadPos() < 1) {
return true; // Valid, just no misses LOG_WARNING("Spell go: missing missCount after hit target list");
packet.setReadPos(startPos);
return false;
} }
const uint8_t rawMissCount = packet.readUInt8(); const uint8_t rawMissCount = packet.readUInt8();