diff --git a/Emulator/src/main/java/com/eu/habbo/habbohotel/guilds/GuildManager.java b/Emulator/src/main/java/com/eu/habbo/habbohotel/guilds/GuildManager.java index 9441fc11..8f1cfbd3 100644 --- a/Emulator/src/main/java/com/eu/habbo/habbohotel/guilds/GuildManager.java +++ b/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()) { diff --git a/Emulator/src/main/java/com/eu/habbo/messages/incoming/guilds/GuildChangeColorsEvent.java b/Emulator/src/main/java/com/eu/habbo/messages/incoming/guilds/GuildChangeColorsEvent.java index d184af1b..aa6246e2 100644 --- a/Emulator/src/main/java/com/eu/habbo/messages/incoming/guilds/GuildChangeColorsEvent.java +++ b/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); diff --git a/Emulator/src/main/java/com/eu/habbo/messages/incoming/guilds/GuildChangeNameDescEvent.java b/Emulator/src/main/java/com/eu/habbo/messages/incoming/guilds/GuildChangeNameDescEvent.java index b1fbefcf..d1adec2c 100644 --- a/Emulator/src/main/java/com/eu/habbo/messages/incoming/guilds/GuildChangeNameDescEvent.java +++ b/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); diff --git a/Emulator/src/main/java/com/eu/habbo/messages/incoming/guilds/GuildChangeSettingsEvent.java b/Emulator/src/main/java/com/eu/habbo/messages/incoming/guilds/GuildChangeSettingsEvent.java index 5ad35f21..84ebbf72 100644 --- a/Emulator/src/main/java/com/eu/habbo/messages/incoming/guilds/GuildChangeSettingsEvent.java +++ b/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); diff --git a/Emulator/src/main/java/com/eu/habbo/messages/incoming/guilds/GuildInputLimits.java b/Emulator/src/main/java/com/eu/habbo/messages/incoming/guilds/GuildInputLimits.java new file mode 100644 index 00000000..c860b7cd --- /dev/null +++ b/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; + } +} diff --git a/Emulator/src/main/java/com/eu/habbo/messages/incoming/guilds/RequestGuildBuyEvent.java b/Emulator/src/main/java/com/eu/habbo/messages/incoming/guilds/RequestGuildBuyEvent.java index 1b7a8c0b..31cd489a 100644 --- a/Emulator/src/main/java/com/eu/habbo/messages/incoming/guilds/RequestGuildBuyEvent.java +++ b/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); diff --git a/Emulator/src/main/java/com/eu/habbo/messages/incoming/guilds/RequestGuildMembersEvent.java b/Emulator/src/main/java/com/eu/habbo/messages/incoming/guilds/RequestGuildMembersEvent.java index e9c218ca..9452d7cc 100644 --- a/Emulator/src/main/java/com/eu/habbo/messages/incoming/guilds/RequestGuildMembersEvent.java +++ b/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); diff --git a/Emulator/src/test/java/com/eu/habbo/messages/incoming/guilds/GuildManagementInputGuardContractTest.java b/Emulator/src/test/java/com/eu/habbo/messages/incoming/guilds/GuildManagementInputGuardContractTest.java new file mode 100644 index 00000000..425f5327 --- /dev/null +++ b/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"); + } +} diff --git a/Emulator/src/test/java/com/eu/habbo/messages/incoming/guilds/GuildMembersInputGuardContractTest.java b/Emulator/src/test/java/com/eu/habbo/messages/incoming/guilds/GuildMembersInputGuardContractTest.java new file mode 100644 index 00000000..31b9c70e --- /dev/null +++ b/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 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"); + } +}