fix: SMSG_RANDOM_ROLL parsed fields in wrong order — garbled /roll output

WotLK format is min(4)+max(4)+result(4)+guid(8)=20 bytes. The parser
read guid(8) first (treating min|max as a uint64), then targetGuid(8)
(non-existent field), then the actual values at wrong offsets. Every
/roll message showed garbled numbers and a bogus roller identity.

Also adds a hasRemaining guard for the 64 bytes of damage/armor/resist
fields in the item query parser — previously read past end with silent
zero-fill on truncated packets.
This commit is contained in:
Kelsi 2026-03-29 19:51:07 -07:00
parent 0ee57f4257
commit c4d2b1709e

View file

@ -2666,17 +2666,19 @@ network::Packet RandomRollPacket::build(uint32_t minRoll, uint32_t maxRoll) {
}
bool RandomRollParser::parse(network::Packet& packet, RandomRollData& data) {
// Validate minimum packet size: rollerGuid(8) + targetGuid(8) + minRoll(4) + maxRoll(4) + result(4)
if (!packet.hasRemaining(28)) {
// WotLK 3.3.5a format: min(4) + max(4) + result(4) + rollerGuid(8) = 20 bytes.
// Previously read guid first (treating min|max as a uint64 GUID), producing
// garbled roller identity and random numbers in /roll chat messages.
if (!packet.hasRemaining(20)) {
LOG_WARNING("SMSG_RANDOM_ROLL: packet too small (", packet.getSize(), " bytes)");
return false;
}
data.rollerGuid = packet.readUInt64();
data.targetGuid = packet.readUInt64();
data.minRoll = packet.readUInt32();
data.maxRoll = packet.readUInt32();
data.result = packet.readUInt32();
data.rollerGuid = packet.readUInt64();
data.targetGuid = 0; // not present in protocol; kept for struct compatibility
LOG_DEBUG("Parsed SMSG_RANDOM_ROLL: roller=0x", std::hex, data.rollerGuid, std::dec,
" result=", data.result, " (", data.minRoll, "-", data.maxRoll, ")");
return true;
@ -3066,6 +3068,14 @@ bool ItemQueryResponseParser::parse(network::Packet& packet, ItemQueryResponseDa
packet.readUInt32(); // ScalingStatDistribution
packet.readUInt32(); // ScalingStatValue
// WotLK 3.3.5a: 2 damage entries (12 bytes each) + armor + 6 resists + delay + ammoType + rangedModRange
// = 24 + 36 + 4 = 64 bytes minimum. Guard here because the section above
// returns early on truncation, and every other section has its own guard.
if (!packet.hasRemaining(64)) {
LOG_WARNING("SMSG_ITEM_QUERY_SINGLE_RESPONSE: truncated before damage/armor (entry=", data.entry, ")");
return true;
}
// WotLK 3.3.5a: MAX_ITEM_PROTO_DAMAGES = 2
bool haveWeaponDamage = false;
for (int i = 0; i < 2; i++) {