From 3849ad75ce8ebf050e794abce692c8098fffdad9 Mon Sep 17 00:00:00 2001
From: Kelsi <kelsihates2fa@gmail.com>
Date: Wed, 11 Mar 2026 14:42:09 -0700
Subject: [PATCH] Harden GuildRosterParser against unbounded memory allocation

Cap numMembers to 1000 and rankCount to 20 to prevent OOM attacks.
Add per-field bounds checking for member data with graceful truncation.
---
 src/game/world_packets.cpp | 103 +++++++++++++++++++++++++++++++++----
 1 file changed, 92 insertions(+), 11 deletions(-)

diff --git a/src/game/world_packets.cpp b/src/game/world_packets.cpp
index 2c7b4791..473eeb98 100644
--- a/src/game/world_packets.cpp
+++ b/src/game/world_packets.cpp
@@ -2109,16 +2109,49 @@ bool GuildRosterParser::parse(network::Packet& packet, GuildRosterData& data) {
         return false;
     }
     uint32_t numMembers = packet.readUInt32();
+
+    // Cap members and ranks to prevent unbounded memory allocation
+    const uint32_t MAX_GUILD_MEMBERS = 1000;
+    if (numMembers > MAX_GUILD_MEMBERS) {
+        LOG_WARNING("GuildRosterParser: numMembers capped (requested=", numMembers, ")");
+        numMembers = MAX_GUILD_MEMBERS;
+    }
+
     data.motd = packet.readString();
     data.guildInfo = packet.readString();
 
+    if (packet.getReadPos() + 4 > packet.getSize()) {
+        LOG_WARNING("GuildRosterParser: truncated before rankCount");
+        data.ranks.clear();
+        data.members.clear();
+        return true;
+    }
+
     uint32_t rankCount = packet.readUInt32();
+
+    // Cap rank count to prevent unbounded allocation
+    const uint32_t MAX_GUILD_RANKS = 20;
+    if (rankCount > MAX_GUILD_RANKS) {
+        LOG_WARNING("GuildRosterParser: rankCount capped (requested=", rankCount, ")");
+        rankCount = MAX_GUILD_RANKS;
+    }
+
     data.ranks.resize(rankCount);
     for (uint32_t i = 0; i < rankCount; ++i) {
+        // Validate 4 bytes before each rank rights read
+        if (packet.getReadPos() + 4 > packet.getSize()) {
+            LOG_WARNING("GuildRosterParser: truncated rank at index ", i);
+            break;
+        }
         data.ranks[i].rights = packet.readUInt32();
-        data.ranks[i].goldLimit = packet.readUInt32();
+        if (packet.getReadPos() + 4 > packet.getSize()) {
+            data.ranks[i].goldLimit = 0;
+        } else {
+            data.ranks[i].goldLimit = packet.readUInt32();
+        }
         // 6 bank tab flags + 6 bank tab items per day
         for (int t = 0; t < 6; ++t) {
+            if (packet.getReadPos() + 8 > packet.getSize()) break;
             packet.readUInt32(); // tabFlags
             packet.readUInt32(); // tabItemsPerDay
         }
@@ -2126,20 +2159,68 @@ bool GuildRosterParser::parse(network::Packet& packet, GuildRosterData& data) {
 
     data.members.resize(numMembers);
     for (uint32_t i = 0; i < numMembers; ++i) {
+        // Validate minimum bytes before reading member (guid+online+name at minimum is 9+ bytes)
+        if (packet.getReadPos() + 9 > packet.getSize()) {
+            LOG_WARNING("GuildRosterParser: truncated member at index ", i);
+            break;
+        }
         auto& m = data.members[i];
         m.guid = packet.readUInt64();
         m.online = (packet.readUInt8() != 0);
-        m.name = packet.readString();
-        m.rankIndex = packet.readUInt32();
-        m.level = packet.readUInt8();
-        m.classId = packet.readUInt8();
-        m.gender = packet.readUInt8();
-        m.zoneId = packet.readUInt32();
-        if (!m.online) {
-            m.lastOnline = packet.readFloat();
+
+        // Validate before reading name string
+        if (packet.getReadPos() >= packet.getSize()) {
+            m.name.clear();
+        } else {
+            m.name = packet.readString();
+        }
+
+        // Validate before reading rank/level/class/gender/zone
+        if (packet.getReadPos() + 1 > packet.getSize()) {
+            m.rankIndex = 0;
+            m.level = 1;
+            m.classId = 0;
+            m.gender = 0;
+            m.zoneId = 0;
+        } else {
+            m.rankIndex = packet.readUInt32();
+            if (packet.getReadPos() + 3 > packet.getSize()) {
+                m.level = 1;
+                m.classId = 0;
+                m.gender = 0;
+            } else {
+                m.level = packet.readUInt8();
+                m.classId = packet.readUInt8();
+                m.gender = packet.readUInt8();
+            }
+            if (packet.getReadPos() + 4 > packet.getSize()) {
+                m.zoneId = 0;
+            } else {
+                m.zoneId = packet.readUInt32();
+            }
+        }
+
+        // Online status affects next fields
+        if (!m.online) {
+            if (packet.getReadPos() + 4 > packet.getSize()) {
+                m.lastOnline = 0.0f;
+            } else {
+                m.lastOnline = packet.readFloat();
+            }
+        }
+
+        // Read notes
+        if (packet.getReadPos() >= packet.getSize()) {
+            m.publicNote.clear();
+            m.officerNote.clear();
+        } else {
+            m.publicNote = packet.readString();
+            if (packet.getReadPos() >= packet.getSize()) {
+                m.officerNote.clear();
+            } else {
+                m.officerNote = packet.readString();
+            }
         }
-        m.publicNote = packet.readString();
-        m.officerNote = packet.readString();
     }
     LOG_INFO("Parsed SMSG_GUILD_ROSTER: ", numMembers, " members, motd=", data.motd);
     return true;