Merge pull request #223 from simoleo89/fix/guild-badge-guards

fix(guilds): bound guild management inputs

Emulator/src/main/java/com/eu/habbo/habbohotel/guilds/GuildManager.java

@@ -441,7 +441,7 @@ public class GuildManager {
     public int getGuildMembersCount(Guild guild, int page, int levelId, String query) {
         try (Connection connection = Emulator.getDatabase().getDataSource().getConnection(); PreparedStatement statement = connection.prepareStatement("SELECT COUNT(*) FROM guilds_members INNER JOIN users ON guilds_members.user_id = users.id WHERE guilds_members.guild_id = ?  " + (rankQuery(levelId)) + " AND users.username LIKE ? ORDER BY level_id, member_since ASC")) {
             statement.setInt(1, guild.getId());
-            statement.setString(2, "%" + query + "%");
+            statement.setString(2, "%" + com.eu.habbo.util.SqlLikeEscaper.escape(query) + "%");
 
             try (ResultSet set = statement.executeQuery()) {
                 while (set.next()) {

Emulator/src/main/java/com/eu/habbo/messages/incoming/guilds/GuildChangeColorsEvent.java

@@ -21,12 +21,21 @@ public class GuildChangeColorsEvent extends MessageHandler {
 
         if (guild != null) {
             if (guild.getOwnerId() == this.client.getHabbo().getHabboInfo().getId() || this.client.getHabbo().hasPermission(Permission.ACC_GUILD_ADMIN)) {
-                GuildChangedColorsEvent colorsEvent = new GuildChangedColorsEvent(guild, this.packet.readInt(), this.packet.readInt());
+                int colorOne = this.packet.readInt();
+                int colorTwo = this.packet.readInt();
+
+                if (!Emulator.getGameEnvironment().getGuildManager().symbolColor(colorOne) || !Emulator.getGameEnvironment().getGuildManager().backgroundColor(colorTwo))
+                    return;
+
+                GuildChangedColorsEvent colorsEvent = new GuildChangedColorsEvent(guild, colorOne, colorTwo);
                 Emulator.getPluginManager().fireEvent(colorsEvent);
 
                 if (colorsEvent.isCancelled())
                     return;
 
+                if (!Emulator.getGameEnvironment().getGuildManager().symbolColor(colorsEvent.colorOne) || !Emulator.getGameEnvironment().getGuildManager().backgroundColor(colorsEvent.colorTwo))
+                    return;
+
                 if (guild.getColorOne() != colorsEvent.colorOne || guild.getColorTwo() != colorsEvent.colorTwo) {
                     guild.setColorOne(colorsEvent.colorOne);
                     guild.setColorTwo(colorsEvent.colorTwo);

Emulator/src/main/java/com/eu/habbo/messages/incoming/guilds/GuildChangeNameDescEvent.java

@@ -23,6 +23,10 @@ public class GuildChangeNameDescEvent extends MessageHandler {
             if (guild.getOwnerId() == this.client.getHabbo().getHabboInfo().getId() || this.client.getHabbo().hasPermission(Permission.ACC_GUILD_ADMIN)) {
                 String newName = Emulator.getGameEnvironment().getWordFilter().filter(this.packet.readString(), this.client.getHabbo());
                 String newDesc = Emulator.getGameEnvironment().getWordFilter().filter(this.packet.readString(), this.client.getHabbo());
+
+                if (!GuildInputLimits.isValidGuildName(newName) || !GuildInputLimits.isValidGuildDescription(newDesc))
+                    return;
+
                 GuildChangedNameEvent nameEvent = new GuildChangedNameEvent(guild, newName, newDesc);
                 Emulator.getPluginManager().fireEvent(nameEvent);
 
@@ -32,7 +36,7 @@ public class GuildChangeNameDescEvent extends MessageHandler {
                 if (guild.getName().equals(nameEvent.name) && guild.getDescription().equals(nameEvent.description))
                     return;
 
-                if(nameEvent.name.length() > 29 || nameEvent.description.length() > 254)
+                if (!GuildInputLimits.isValidGuildName(nameEvent.name) || !GuildInputLimits.isValidGuildDescription(nameEvent.description))
                     return;
 
                 guild.setName(nameEvent.name);

Emulator/src/main/java/com/eu/habbo/messages/incoming/guilds/GuildChangeSettingsEvent.java

@@ -40,12 +40,20 @@ public class GuildChangeSettingsEvent extends MessageHandler {
 
         if (guild != null) {
             if (guild.getOwnerId() == this.client.getHabbo().getHabboInfo().getId() || this.client.getHabbo().hasPermission(Permission.ACC_GUILD_ADMIN)) {
-                GuildChangedSettingsEvent settingsEvent = new GuildChangedSettingsEvent(guild, this.packet.readInt(), this.packet.readInt() == 0);
+                int state = this.packet.readInt();
+
+                if (state < 0 || state >= GuildState.values().length)
+                    return;
+
+                GuildChangedSettingsEvent settingsEvent = new GuildChangedSettingsEvent(guild, state, this.packet.readInt() == 0);
                 Emulator.getPluginManager().fireEvent(settingsEvent);
 
                 if (settingsEvent.isCancelled())
                     return;
 
+                if (settingsEvent.state < 0 || settingsEvent.state >= GuildState.values().length)
+                    return;
+
                 guild.setState(GuildState.valueOf(settingsEvent.state));
                 guild.setRights(settingsEvent.rights);
 

Emulator/src/main/java/com/eu/habbo/messages/incoming/guilds/GuildInputLimits.java

@@ -0,0 +1,17 @@
+package com.eu.habbo.messages.incoming.guilds;
+
+final class GuildInputLimits {
+    static final int MAX_GUILD_NAME_LENGTH = 29;
+    static final int MAX_GUILD_DESCRIPTION_LENGTH = 254;
+
+    private GuildInputLimits() {
+    }
+
+    static boolean isValidGuildName(String name) {
+        return name != null && !name.isBlank() && name.length() <= MAX_GUILD_NAME_LENGTH;
+    }
+
+    static boolean isValidGuildDescription(String description) {
+        return description != null && description.length() <= MAX_GUILD_DESCRIPTION_LENGTH;
+    }
+}

Emulator/src/main/java/com/eu/habbo/messages/incoming/guilds/RequestGuildBuyEvent.java

@@ -31,11 +31,11 @@ public class RequestGuildBuyEvent extends MessageHandler {
         final String name = Emulator.getGameEnvironment().getWordFilter().filter(this.packet.readString(), this.client.getHabbo());
         final String description = Emulator.getGameEnvironment().getWordFilter().filter(this.packet.readString(), this.client.getHabbo());
 
-        if (name.length() == 0 || name.length() > 29) {
+        if (!GuildInputLimits.isValidGuildName(name)) {
             this.client.sendResponse(new GuildEditFailComposer(GuildEditFailComposer.INVALID_GUILD_NAME));
             return;
         }
-        if (description.length() > 254) {
+        if (!GuildInputLimits.isValidGuildDescription(description)) {
             return;
         }
 
@@ -68,6 +68,11 @@ public class RequestGuildBuyEvent extends MessageHandler {
         int colorOne = this.packet.readInt();
         int colorTwo = this.packet.readInt();
 
+        if (!Emulator.getGameEnvironment().getGuildManager().symbolColor(colorOne) || !Emulator.getGameEnvironment().getGuildManager().backgroundColor(colorTwo)) {
+            this.client.sendResponse(new GuildEditFailComposer(GuildEditFailComposer.INVALID_GUILD_NAME));
+            return;
+        }
+
         int count = this.packet.readInt();
 
         String badge = GuildBadgeBuilder.readBadge(this.packet, count);

Emulator/src/main/java/com/eu/habbo/messages/incoming/guilds/RequestGuildMembersEvent.java

@@ -9,6 +9,10 @@ import com.eu.habbo.messages.incoming.MessageHandler;
 import com.eu.habbo.messages.outgoing.guilds.GuildMembersComposer;
 
 public class RequestGuildMembersEvent extends MessageHandler {
+    private static final int MAX_PAGE_ID = 1000;
+    private static final int MAX_QUERY_LENGTH = 32;
+    private static final int MAX_LEVEL_ID = 2;
+
     @Override
     public int getRatelimit() {
         return 500;
@@ -20,6 +24,9 @@ public class RequestGuildMembersEvent extends MessageHandler {
         int pageId = this.packet.readInt();
         String query = this.packet.readString();
         int levelId = this.packet.readInt();
+        if (pageId < 0 || pageId > MAX_PAGE_ID || levelId < 0 || levelId > MAX_LEVEL_ID || query == null || query.length() > MAX_QUERY_LENGTH) {
+            return;
+        }
 
         Guild g = Emulator.getGameEnvironment().getGuildManager().getGuild(groupId);
 

Emulator/src/test/java/com/eu/habbo/messages/incoming/guilds/GuildManagementInputGuardContractTest.java

@@ -0,0 +1,59 @@
+package com.eu.habbo.messages.incoming.guilds;
+
+import org.junit.jupiter.api.Test;
+
+import java.nio.file.Files;
+import java.nio.file.Path;
+
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+class GuildManagementInputGuardContractTest {
+    private static String source(String file) throws Exception {
+        return Files.readString(Path.of("src/main/java/com/eu/habbo/messages/incoming/guilds/" + file));
+    }
+
+    @Test
+    void guildCreateAndRenameShareNameAndDescriptionBounds() throws Exception {
+        String limits = source("GuildInputLimits.java");
+        String buy = source("RequestGuildBuyEvent.java");
+        String rename = source("GuildChangeNameDescEvent.java");
+
+        assertTrue(limits.contains("MAX_GUILD_NAME_LENGTH = 29"),
+                "Guild names should keep the existing 29 character protocol bound");
+        assertTrue(limits.contains("MAX_GUILD_DESCRIPTION_LENGTH = 254"),
+                "Guild descriptions should keep the existing database/protocol bound");
+        assertTrue(limits.contains("!name.isBlank()"),
+                "Guild names must not be empty or whitespace-only");
+        assertTrue(buy.contains("GuildInputLimits.isValidGuildName(name)"),
+                "Guild purchase should use the shared name guard");
+        assertTrue(buy.contains("GuildInputLimits.isValidGuildDescription(description)"),
+                "Guild purchase should use the shared description guard");
+        assertTrue(rename.contains("GuildInputLimits.isValidGuildName(newName)"),
+                "Guild rename should reject invalid client names before plugin events");
+        assertTrue(rename.contains("GuildInputLimits.isValidGuildName(nameEvent.name)"),
+                "Guild rename should reject invalid plugin-mutated names before persistence");
+    }
+
+    @Test
+    void guildColorInputsAreCheckedAgainstLoadedPalette() throws Exception {
+        String buy = source("RequestGuildBuyEvent.java");
+        String colors = source("GuildChangeColorsEvent.java");
+
+        assertTrue(buy.contains("symbolColor(colorOne)") && buy.contains("backgroundColor(colorTwo)"),
+                "Guild purchase should reject color ids that are not in the loaded guild palette");
+        assertTrue(colors.contains("symbolColor(colorOne)") && colors.contains("backgroundColor(colorTwo)"),
+                "Guild color changes should reject invalid client color ids");
+        assertTrue(colors.contains("symbolColor(colorsEvent.colorOne)") && colors.contains("backgroundColor(colorsEvent.colorTwo)"),
+                "Guild color changes should reject invalid plugin-mutated color ids");
+    }
+
+    @Test
+    void guildStateInputsStayInsideKnownEnumRange() throws Exception {
+        String settings = source("GuildChangeSettingsEvent.java");
+
+        assertTrue(settings.contains("state < 0 || state >= GuildState.values().length"),
+                "Guild settings should reject invalid client state ids before event dispatch");
+        assertTrue(settings.contains("settingsEvent.state < 0 || settingsEvent.state >= GuildState.values().length"),
+                "Guild settings should reject invalid plugin-mutated state ids before applying them");
+    }
+}

Emulator/src/test/java/com/eu/habbo/messages/incoming/guilds/GuildMembersInputGuardContractTest.java

@@ -0,0 +1,55 @@
+package com.eu.habbo.messages.incoming.guilds;
+
+import org.junit.jupiter.api.Test;
+
+import java.nio.file.Files;
+import java.nio.file.Path;
+
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+class GuildMembersInputGuardContractTest {
+    private static String eventSource() throws Exception {
+        return Files.readString(Path.of("src/main/java/com/eu/habbo/messages/incoming/guilds/RequestGuildMembersEvent.java"));
+    }
+
+    private static String managerSource() throws Exception {
+        return Files.readString(Path.of("src/main/java/com/eu/habbo/habbohotel/guilds/GuildManager.java"));
+    }
+
+    @Test
+    void guildMemberListInputsAreBoundedBeforeManagerQueries() throws Exception {
+        String source = eventSource();
+
+        int pageRead = source.indexOf("int pageId = this.packet.readInt()");
+        int queryRead = source.indexOf("String query = this.packet.readString()", pageRead);
+        int levelRead = source.indexOf("int levelId = this.packet.readInt()", queryRead);
+        int guard = source.indexOf("pageId < 0 || pageId > MAX_PAGE_ID", levelRead);
+        int managerCall = source.indexOf("getGuildMembers(g, pageId, levelId, query)", guard);
+
+        assertTrue(source.contains("MAX_PAGE_ID = 1000"),
+                "Guild member pagination should have a server-side upper bound");
+        assertTrue(source.contains("MAX_QUERY_LENGTH = 32"),
+                "Guild member search query should have a server-side length bound");
+        assertTrue(source.contains("MAX_LEVEL_ID = 2"),
+                "Guild member rank filter should be bounded to known levels");
+        assertTrue(pageRead > -1 && queryRead > pageRead && levelRead > queryRead,
+                "Guild member handler must read page/query/level from the packet");
+        assertTrue(guard > levelRead && guard < managerCall,
+                "Guild member handler must validate inputs before querying the manager");
+    }
+
+    @Test
+    void guildMemberCountEscapesLikeQuerySameAsListQuery() throws Exception {
+        String source = managerSource();
+
+        int listMethod = source.indexOf("public ArrayList<GuildMember> getGuildMembers(Guild guild, int page, int levelId, String query)");
+        int listEscape = source.indexOf("SqlLikeEscaper.escape(query)", listMethod);
+        int countMethod = source.indexOf("public int getGuildMembersCount(Guild guild, int page, int levelId, String query)");
+        int countEscape = source.indexOf("SqlLikeEscaper.escape(query)", countMethod);
+
+        assertTrue(listEscape > listMethod,
+                "Guild member list query should escape SQL LIKE wildcards");
+        assertTrue(countEscape > countMethod,
+                "Guild member count query should escape SQL LIKE wildcards too");
+    }
+}