phaneron/Kelsidavis-WoWee
1
0
Fork 0
mirror of https://github.com/Kelsidavis/WoWee.git synced 2026-03-23 07:40:14 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 26

Add packet size validation to character enum and movement parsing

Browse source 
Improve parser robustness by adding defensive size checks to prevent reading
beyond packet boundaries. Specifically:

- parseCharEnum (Classic/TBC): Add packet size validation and character count cap
  (max 32 chars) to prevent truncated packets from silently parsing garbage data
- parseMovementBlock (Classic/TBC): Add early validation for minimum packet size
  before reading updateFlags to catch empty packets early
- All changes are backward compatible and log warnings on truncation

This is part of Tier 1/2 work to improve multi-expansion packet parsing robustness
and prevent undefined behavior from malformed or truncated server packets.
This commit is contained in:
Kelsi 2026-03-11 13:55:20 -07:00
parent fe2987dae1
commit d14f82cb7c
2 changed files with 64 additions and 0 deletions
Download patch file Download diff file Expand all files Collapse all files

32
src/game/packet_parsers_tbc.cpp
View file

@ -30,6 +30,12 @@ namespace TbcMoveFlags {
// - Flag 0x08 (HIGH_GUID) reads 2 u32s (Classic: 1 u32)
// ============================================================================
bool TbcPacketParsers::parseMovementBlock(network::Packet& packet, UpdateBlock& block) {
// Validate minimum packet size for updateFlags byte
if (packet.getReadPos() >= packet.getSize()) {
LOG_WARNING("[TBC] Movement block packet too small (need at least 1 byte for updateFlags)");
return false;
}
// TBC 2.4.3: UpdateFlags is uint8 (1 byte)
uint8_t updateFlags = packet.readUInt8();
block.updateFlags = static_cast<uint16_t>(updateFlags);
@ -297,14 +303,40 @@ network::Packet TbcPacketParsers::buildMovementPacket(LogicalOpcode opcode,
// - Equipment: 20 items (not 23)
// ============================================================================
bool TbcPacketParsers::parseCharEnum(network::Packet& packet, CharEnumResponse& response) {
// Validate minimum packet size for count byte
if (packet.getSize() < 1) {
LOG_ERROR("[TBC] SMSG_CHAR_ENUM packet too small: ", packet.getSize(), " bytes");
return false;
}
uint8_t count = packet.readUInt8();
// Cap count to prevent excessive memory allocation
constexpr uint8_t kMaxCharacters = 32;
if (count > kMaxCharacters) {
LOG_WARNING("[TBC] Character count ", (int)count, " exceeds max ", (int)kMaxCharacters,
", capping");
count = kMaxCharacters;
}
LOG_INFO("[TBC] Parsing SMSG_CHAR_ENUM: ", (int)count, " characters");
response.characters.clear();
response.characters.reserve(count);
for (uint8_t i = 0; i < count; ++i) {
// Sanity check: ensure we have at least minimal data before reading next character
// Minimum: guid(8) + name(1) + race(1) + class(1) + gender(1) + appearance(4)
// + facialFeatures(1) + level(1) + zone(4) + map(4) + pos(12) + guild(4)
// + flags(4) + firstLogin(1) + pet(12) + equipment(20*9)
constexpr size_t kMinCharacterSize = 8 + 1 + 1 + 1 + 1 + 4 + 1 + 1 + 4 + 4 + 12 + 4 + 4 + 1 + 12 + 180;
if (packet.getReadPos() + kMinCharacterSize > packet.getSize()) {
LOG_WARNING("[TBC] Character enum packet truncated at character ", (int)(i + 1),
", pos=", packet.getReadPos(), " needed=", kMinCharacterSize,
" size=", packet.getSize());
break;
}
Character character;
// GUID (8 bytes)