game: fix Classic/TBC SMSG_GROUP_LIST parsing - missing roles byte

WotLK 3.3.5a added a group-level and per-member roles byte (tank/healer/dps)
for the Dungeon Finder system. Classic 1.12 and TBC 2.4.3 do not send this byte.

The previous GroupListParser always read the roles byte, causing a one-byte
misalignment in Classic/TBC group lists that corrupted member GUID reads and
all subsequent fields (loot method, leader GUID, etc.).

GroupListParser::parse now takes a hasRoles parameter (default true for
backward compatibility). handleGroupList passes hasRoles=isActiveExpansion("wotlk").
Also adds range-checking throughout to prevent out-of-bounds reads on
malformed or unexpectedly short group list packets.
This commit is contained in:
Kelsi 2026-03-10 00:58:56 -07:00
parent 04f22376ce
commit a0979b9cd8
3 changed files with 66 additions and 26 deletions

View file

@ -1855,7 +1855,9 @@ public:
/** SMSG_GROUP_LIST parser */ /** SMSG_GROUP_LIST parser */
class GroupListParser { class GroupListParser {
public: public:
static bool parse(network::Packet& packet, GroupListData& data); // hasRoles: WotLK 3.3.5a added a roles byte at group level and per-member for LFD.
// Classic 1.12 and TBC 2.4.3 do not send this byte.
static bool parse(network::Packet& packet, GroupListData& data, bool hasRoles = true);
}; };
/** SMSG_PARTY_COMMAND_RESULT data */ /** SMSG_PARTY_COMMAND_RESULT data */

View file

@ -13260,7 +13260,10 @@ void GameHandler::handleGroupDecline(network::Packet& packet) {
} }
void GameHandler::handleGroupList(network::Packet& packet) { void GameHandler::handleGroupList(network::Packet& packet) {
if (!GroupListParser::parse(packet, partyData)) return; // WotLK 3.3.5a added a roles byte (group level + per-member) for the dungeon finder.
// Classic 1.12 and TBC 2.4.3 do not send the roles byte.
const bool hasRoles = isActiveExpansion("wotlk");
if (!GroupListParser::parse(packet, partyData, hasRoles)) return;
if (partyData.isEmpty()) { if (partyData.isEmpty()) {
LOG_INFO("No longer in a group"); LOG_INFO("No longer in a group");

View file

@ -3090,47 +3090,82 @@ network::Packet GroupDeclinePacket::build() {
return packet; return packet;
} }
bool GroupListParser::parse(network::Packet& packet, GroupListData& data) { bool GroupListParser::parse(network::Packet& packet, GroupListData& data, bool hasRoles) {
auto rem = [&]() { return packet.getSize() - packet.getReadPos(); };
if (rem() < 3) return false;
data.groupType = packet.readUInt8(); data.groupType = packet.readUInt8();
data.subGroup = packet.readUInt8(); data.subGroup = packet.readUInt8();
data.flags = packet.readUInt8(); data.flags = packet.readUInt8();
data.roles = packet.readUInt8();
// Skip LFG data if present // WotLK 3.3.5a added a roles byte (tank/healer/dps) for the dungeon finder.
if (data.groupType & 0x04) { // Classic 1.12 and TBC 2.4.3 do not have this byte.
packet.readUInt8(); // lfg state if (hasRoles) {
packet.readUInt32(); // lfg entry if (rem() < 1) return false;
packet.readUInt8(); // lfg flags (3.3.5a may not have this) data.roles = packet.readUInt8();
} else {
data.roles = 0;
} }
packet.readUInt64(); // group GUID // WotLK: LFG data gated by groupType bit 0x04 (LFD group type)
packet.readUInt32(); // counter if (hasRoles && (data.groupType & 0x04)) {
if (rem() < 5) return false;
packet.readUInt8(); // lfg state
packet.readUInt32(); // lfg entry
// WotLK 3.3.5a may or may not send the lfg flags byte — read it only if present
if (rem() >= 13) { // enough for lfgFlags(1)+groupGuid(8)+counter(4)
packet.readUInt8(); // lfg flags
}
}
if (rem() < 12) return false;
packet.readUInt64(); // group GUID
packet.readUInt32(); // update counter
if (rem() < 4) return false;
data.memberCount = packet.readUInt32(); data.memberCount = packet.readUInt32();
if (data.memberCount > 40) {
LOG_WARNING("GroupListParser: implausible memberCount=", data.memberCount, ", clamping");
data.memberCount = 40;
}
data.members.reserve(data.memberCount); data.members.reserve(data.memberCount);
for (uint32_t i = 0; i < data.memberCount; ++i) { for (uint32_t i = 0; i < data.memberCount; ++i) {
if (rem() == 0) break;
GroupMember member; GroupMember member;
member.name = packet.readString(); member.name = packet.readString();
if (rem() < 8) break;
member.guid = packet.readUInt64(); member.guid = packet.readUInt64();
if (rem() < 3) break;
member.isOnline = packet.readUInt8(); member.isOnline = packet.readUInt8();
member.subGroup = packet.readUInt8(); member.subGroup = packet.readUInt8();
member.flags = packet.readUInt8(); member.flags = packet.readUInt8();
// WotLK added per-member roles byte; Classic/TBC do not have it.
if (hasRoles) {
if (rem() < 1) break;
member.roles = packet.readUInt8(); member.roles = packet.readUInt8();
} else {
member.roles = 0;
}
data.members.push_back(member); data.members.push_back(member);
} }
if (rem() < 8) {
LOG_INFO("Group list: ", data.memberCount, " members (no leader GUID in packet)");
return true;
}
data.leaderGuid = packet.readUInt64(); data.leaderGuid = packet.readUInt64();
if (data.memberCount > 0 && packet.getReadPos() < packet.getSize()) { if (data.memberCount > 0 && rem() >= 10) {
data.lootMethod = packet.readUInt8(); data.lootMethod = packet.readUInt8();
data.looterGuid = packet.readUInt64(); data.looterGuid = packet.readUInt64();
data.lootThreshold = packet.readUInt8(); data.lootThreshold = packet.readUInt8();
data.difficultyId = packet.readUInt8(); // Dungeon difficulty (heroic/normal) — Classic doesn't send this; TBC/WotLK do
data.raidDifficultyId = packet.readUInt8(); if (rem() >= 1) data.difficultyId = packet.readUInt8();
if (packet.getReadPos() < packet.getSize()) { // Raid difficulty — WotLK only
packet.readUInt8(); // unknown byte if (rem() >= 1) data.raidDifficultyId = packet.readUInt8();
} // Extra byte in some 3.3.5a builds
if (hasRoles && rem() >= 1) packet.readUInt8();
} }
LOG_INFO("Group list: ", data.memberCount, " members, leader=0x", LOG_INFO("Group list: ", data.memberCount, " members, leader=0x",