fix(combatlog): fail classic and tbc spell go parse on truncation

This commit is contained in:
Kelsi 2026-03-14 11:10:08 -07:00
parent e0ac81450d
commit 6b290009aa
2 changed files with 55 additions and 28 deletions

View file

@ -397,6 +397,7 @@ bool ClassicPacketParsers::parseSpellStart(network::Packet& packet, SpellStartDa
// ============================================================================ // ============================================================================
bool ClassicPacketParsers::parseSpellGo(network::Packet& packet, SpellGoData& data) { bool ClassicPacketParsers::parseSpellGo(network::Packet& packet, SpellGoData& data) {
auto rem = [&]() { return packet.getSize() - packet.getReadPos(); }; auto rem = [&]() { return packet.getSize() - packet.getReadPos(); };
const size_t startPos = packet.getReadPos();
if (rem() < 2) return false; if (rem() < 2) return false;
if (!hasFullPackedGuid(packet)) return false; if (!hasFullPackedGuid(packet)) return false;
@ -418,23 +419,24 @@ bool ClassicPacketParsers::parseSpellGo(network::Packet& packet, SpellGoData& da
} }
const uint8_t storedHitLimit = std::min<uint8_t>(rawHitCount, 128); const uint8_t storedHitLimit = std::min<uint8_t>(rawHitCount, 128);
data.hitTargets.reserve(storedHitLimit); data.hitTargets.reserve(storedHitLimit);
uint16_t parsedHitCount = 0; bool truncatedTargets = false;
for (uint16_t i = 0; i < rawHitCount; ++i) { for (uint16_t i = 0; i < rawHitCount; ++i) {
if (!hasFullPackedGuid(packet)) { if (!hasFullPackedGuid(packet)) {
LOG_WARNING("[Classic] Spell go: truncated hit targets at index ", i,
"/", (int)rawHitCount);
truncatedTargets = true;
break; break;
} }
const uint64_t targetGuid = UpdateObjectParser::readPackedGuid(packet); const uint64_t targetGuid = UpdateObjectParser::readPackedGuid(packet);
++parsedHitCount;
if (i < storedHitLimit) { if (i < storedHitLimit) {
data.hitTargets.push_back(targetGuid); data.hitTargets.push_back(targetGuid);
} }
} }
data.hitCount = static_cast<uint8_t>(data.hitTargets.size()); if (truncatedTargets) {
// Check if we read all expected hits packet.setReadPos(startPos);
if (parsedHitCount < rawHitCount) { return false;
LOG_WARNING("[Classic] Spell go: truncated hit targets at index ", (int)parsedHitCount,
"/", (int)rawHitCount);
} }
data.hitCount = static_cast<uint8_t>(data.hitTargets.size());
// Miss targets // Miss targets
if (rem() < 1) return true; if (rem() < 1) return true;
@ -444,31 +446,41 @@ bool ClassicPacketParsers::parseSpellGo(network::Packet& packet, SpellGoData& da
} }
const uint8_t storedMissLimit = std::min<uint8_t>(rawMissCount, 128); const uint8_t storedMissLimit = std::min<uint8_t>(rawMissCount, 128);
data.missTargets.reserve(storedMissLimit); data.missTargets.reserve(storedMissLimit);
uint16_t parsedMissCount = 0;
for (uint16_t i = 0; i < rawMissCount; ++i) { for (uint16_t i = 0; i < rawMissCount; ++i) {
if (!hasFullPackedGuid(packet)) { if (!hasFullPackedGuid(packet)) {
LOG_WARNING("[Classic] Spell go: truncated miss targets at index ", i,
"/", (int)rawMissCount);
truncatedTargets = true;
break; break;
} }
SpellGoMissEntry m; SpellGoMissEntry m;
m.targetGuid = UpdateObjectParser::readPackedGuid(packet); m.targetGuid = UpdateObjectParser::readPackedGuid(packet);
if (rem() < 1) break; if (rem() < 1) {
LOG_WARNING("[Classic] Spell go: missing missType at miss index ", i,
"/", (int)rawMissCount);
truncatedTargets = true;
break;
}
m.missType = packet.readUInt8(); m.missType = packet.readUInt8();
if (m.missType == 11) { if (m.missType == 11) {
if (rem() < 5) break; if (rem() < 5) {
LOG_WARNING("[Classic] Spell go: truncated reflect payload at miss index ", i,
"/", (int)rawMissCount);
truncatedTargets = true;
break;
}
(void)packet.readUInt32(); (void)packet.readUInt32();
(void)packet.readUInt8(); (void)packet.readUInt8();
} }
++parsedMissCount;
if (i < storedMissLimit) { if (i < storedMissLimit) {
data.missTargets.push_back(m); data.missTargets.push_back(m);
} }
} }
data.missCount = static_cast<uint8_t>(data.missTargets.size()); if (truncatedTargets) {
// Check if we read all expected misses packet.setReadPos(startPos);
if (parsedMissCount < rawMissCount) { return false;
LOG_WARNING("[Classic] Spell go: truncated miss targets at index ", (int)parsedMissCount,
"/", (int)rawMissCount);
} }
data.missCount = static_cast<uint8_t>(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,
" misses=", (int)data.missCount); " misses=", (int)data.missCount);

View file

@ -1261,6 +1261,7 @@ bool TbcPacketParsers::parseSpellStart(network::Packet& packet, SpellStartData&
// WotLK uses packed GUIDs and adds a timestamp (u32) after castFlags. // WotLK uses packed GUIDs and adds a timestamp (u32) after castFlags.
// ============================================================================ // ============================================================================
bool TbcPacketParsers::parseSpellGo(network::Packet& packet, SpellGoData& data) { bool TbcPacketParsers::parseSpellGo(network::Packet& packet, SpellGoData& data) {
const size_t startPos = packet.getReadPos();
// Fixed header before hit/miss lists: // Fixed header before hit/miss lists:
// casterGuid(u64) + casterUnit(u64) + castCount(u8) + spellId(u32) + castFlags(u32) // casterGuid(u64) + casterUnit(u64) + castCount(u8) + spellId(u32) + castFlags(u32)
if (packet.getSize() - packet.getReadPos() < 25) return false; if (packet.getSize() - packet.getReadPos() < 25) return false;
@ -1283,18 +1284,24 @@ bool TbcPacketParsers::parseSpellGo(network::Packet& packet, SpellGoData& data)
} }
const uint8_t storedHitLimit = std::min<uint8_t>(rawHitCount, 128); const uint8_t storedHitLimit = std::min<uint8_t>(rawHitCount, 128);
data.hitTargets.reserve(storedHitLimit); data.hitTargets.reserve(storedHitLimit);
for (uint16_t i = 0; i < rawHitCount && packet.getReadPos() + 8 <= packet.getSize(); ++i) { bool truncatedTargets = false;
for (uint16_t i = 0; i < rawHitCount; ++i) {
if (packet.getReadPos() + 8 > packet.getSize()) {
LOG_WARNING("[TBC] Spell go: truncated hit targets at index ", i,
"/", (int)rawHitCount);
truncatedTargets = true;
break;
}
const uint64_t targetGuid = packet.readUInt64(); // full GUID in TBC const uint64_t targetGuid = packet.readUInt64(); // full GUID in TBC
if (i < storedHitLimit) { if (i < storedHitLimit) {
data.hitTargets.push_back(targetGuid); data.hitTargets.push_back(targetGuid);
} }
} }
data.hitCount = static_cast<uint8_t>(data.hitTargets.size()); if (truncatedTargets) {
// Check if we read all expected hits packet.setReadPos(startPos);
if (data.hitTargets.size() < rawHitCount) { return false;
LOG_WARNING("[TBC] Spell go: truncated hit targets at index ", (int)data.hitTargets.size(),
"/", (int)rawHitCount);
} }
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(); const uint8_t rawMissCount = packet.readUInt8();
@ -1303,12 +1310,21 @@ bool TbcPacketParsers::parseSpellGo(network::Packet& packet, SpellGoData& data)
} }
const uint8_t storedMissLimit = std::min<uint8_t>(rawMissCount, 128); const uint8_t storedMissLimit = std::min<uint8_t>(rawMissCount, 128);
data.missTargets.reserve(storedMissLimit); data.missTargets.reserve(storedMissLimit);
for (uint16_t i = 0; i < rawMissCount && packet.getReadPos() + 9 <= packet.getSize(); ++i) { 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;
}
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();
if (m.missType == 11) { if (m.missType == 11) {
if (packet.getReadPos() + 5 > packet.getSize()) { if (packet.getReadPos() + 5 > packet.getSize()) {
LOG_WARNING("[TBC] Spell go: truncated reflect payload at miss index ", i,
"/", (int)rawMissCount);
truncatedTargets = true;
break; break;
} }
(void)packet.readUInt32(); (void)packet.readUInt32();
@ -1318,12 +1334,11 @@ bool TbcPacketParsers::parseSpellGo(network::Packet& packet, SpellGoData& data)
data.missTargets.push_back(m); data.missTargets.push_back(m);
} }
} }
data.missCount = static_cast<uint8_t>(data.missTargets.size()); if (truncatedTargets) {
// Check if we read all expected misses packet.setReadPos(startPos);
if (data.missTargets.size() < rawMissCount) { return false;
LOG_WARNING("[TBC] Spell go: truncated miss targets at index ", (int)data.missTargets.size(),
"/", (int)rawMissCount);
} }
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,