Merge pull request #209 from simoleo89/fix/forum-input-guards

fix(forums): validate guild forum inputs

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

@@ -0,0 +1,37 @@
+package com.eu.habbo.messages.incoming.guilds.forums;
+
+final class GuildForumInputGuard {
+    static final int MAX_PAGE_LIMIT = 50;
+    static final int MAX_MARK_READ_BATCH = 50;
+
+    private GuildForumInputGuard() {
+    }
+
+    static String normalize(String value) {
+        return value == null ? "" : value.trim();
+    }
+
+    static boolean isPositiveId(int id) {
+        return id > 0;
+    }
+
+    static boolean isValidPage(int index, int limit) {
+        return index >= 0 && limit > 0 && limit <= MAX_PAGE_LIMIT;
+    }
+
+    static boolean isValidMarkReadBatch(int count) {
+        return count > 0 && count <= MAX_MARK_READ_BATCH;
+    }
+
+    static boolean isSettingsState(int state) {
+        return state >= 0 && state <= 3;
+    }
+
+    static boolean isThreadModerationState(int state) {
+        return state == 1 || state == 10 || state == 20;
+    }
+
+    static boolean isMessageModerationState(int state) {
+        return state == 1 || state == 10 || state == 20;
+    }
+}

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

@@ -24,11 +24,19 @@ public class GuildForumMarkAsReadEvent extends MessageHandler {
         int userId = this.client.getHabbo().getHabboInfo().getId();
         int timestamp = Emulator.getIntUnixTimestamp();
 
+        if (!GuildForumInputGuard.isValidMarkReadBatch(count)) {
+            return;
+        }
+
         for (int i = 0; i < count; i++) {
             int guildId = this.packet.readInt();
             this.packet.readInt(); // messageId (not used, we track by timestamp)
             this.packet.readBoolean(); // isRead
 
+            if (!GuildForumInputGuard.isPositiveId(guildId)) {
+                continue;
+            }
+
             try (Connection connection = Emulator.getDatabase().getDataSource().getConnection(); PreparedStatement statement = connection.prepareStatement(
                     "INSERT INTO `guild_forum_views` (`user_id`, `guild_id`, `timestamp`) VALUES (?, ?, ?) " +
                             "ON DUPLICATE KEY UPDATE `timestamp` = ?"

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

@@ -28,6 +28,14 @@ public class GuildForumModerateMessageEvent extends MessageHandler {
         int messageId = packet.readInt();
         int state = packet.readInt();
 
+        if (!GuildForumInputGuard.isPositiveId(guildId) ||
+                !GuildForumInputGuard.isPositiveId(threadId) ||
+                !GuildForumInputGuard.isPositiveId(messageId) ||
+                !GuildForumInputGuard.isMessageModerationState(state)) {
+            this.client.sendResponse(new ConnectionErrorComposer(400));
+            return;
+        }
+
         Guild guild = Emulator.getGameEnvironment().getGuildManager().getGuild(guildId);
         ForumThread thread = ForumThread.getById(threadId);
 
@@ -85,4 +93,4 @@ public class GuildForumModerateMessageEvent extends MessageHandler {
         }
 
     }
-}
+}

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

@@ -36,6 +36,13 @@ public class GuildForumModerateThreadEvent extends MessageHandler {
         int threadId = packet.readInt();
         int state = packet.readInt();
 
+        if (!GuildForumInputGuard.isPositiveId(guildId) ||
+                !GuildForumInputGuard.isPositiveId(threadId) ||
+                !GuildForumInputGuard.isThreadModerationState(state)) {
+            this.client.sendResponse(new ConnectionErrorComposer(400));
+            return;
+        }
+
         Guild guild = Emulator.getGameEnvironment().getGuildManager().getGuild(guildId);
         ForumThread thread = ForumThread.getById(threadId);
 
@@ -108,4 +115,4 @@ public class GuildForumModerateThreadEvent extends MessageHandler {
             LOGGER.error("Failed to delete thread " + threadId, e);
         }
     }
-}
+}

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

@@ -25,8 +25,13 @@ public class GuildForumPostThreadEvent extends MessageHandler {
     public void handle() throws Exception {
         int guildId = this.packet.readInt();
         int threadId = this.packet.readInt();
-        String subject = Emulator.getGameEnvironment().getWordFilter().filter(this.packet.readString(), this.client.getHabbo());
-        String message = Emulator.getGameEnvironment().getWordFilter().filter(this.packet.readString(), this.client.getHabbo());
+        String subject = Emulator.getGameEnvironment().getWordFilter().filter(GuildForumInputGuard.normalize(this.packet.readString()), this.client.getHabbo());
+        String message = Emulator.getGameEnvironment().getWordFilter().filter(GuildForumInputGuard.normalize(this.packet.readString()), this.client.getHabbo());
+
+        if (!GuildForumInputGuard.isPositiveId(guildId) || threadId < 0) {
+            this.client.sendResponse(new ConnectionErrorComposer(400));
+            return;
+        }
 
         Guild guild = Emulator.getGameEnvironment().getGuildManager().getGuild(guildId);
 
@@ -108,4 +113,4 @@ public class GuildForumPostThreadEvent extends MessageHandler {
             this.client.sendResponse(new ConnectionErrorComposer(500));
         }
     }
-}
+}

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

@@ -27,6 +27,11 @@ public class GuildForumThreadUpdateEvent extends MessageHandler {
         boolean isPinned = this.packet.readBoolean();
         boolean isLocked = this.packet.readBoolean();
 
+        if (!GuildForumInputGuard.isPositiveId(guildId) || !GuildForumInputGuard.isPositiveId(threadId)) {
+            this.client.sendResponse(new ConnectionErrorComposer(400));
+            return;
+        }
+
         Guild guild = Emulator.getGameEnvironment().getGuildManager().getGuild(guildId);
         ForumThread thread = ForumThread.getById(threadId);
 
@@ -71,4 +76,4 @@ public class GuildForumThreadUpdateEvent extends MessageHandler {
             this.client.sendResponse(new GuildForumThreadsComposer(guild, 0));
         }
     }
-}
+}

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

@@ -22,6 +22,11 @@ public class GuildForumThreadsEvent extends MessageHandler {
         int guildId = packet.readInt();
         int index = packet.readInt();
 
+        if (!GuildForumInputGuard.isPositiveId(guildId) || index < 0) {
+            this.client.sendResponse(new ConnectionErrorComposer(400));
+            return;
+        }
+
         Guild guild = Emulator.getGameEnvironment().getGuildManager().getGuild(guildId);
 
         if (guild == null) {

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

@@ -29,6 +29,12 @@ public class GuildForumThreadsMessagesEvent extends MessageHandler {
         int index = packet.readInt(); // 40
         int limit = packet.readInt(); // 20
 
+        if (!GuildForumInputGuard.isPositiveId(guildId) ||
+                !GuildForumInputGuard.isPositiveId(threadId) ||
+                !GuildForumInputGuard.isValidPage(index, limit)) {
+            this.client.sendResponse(new ConnectionErrorComposer(400));
+            return;
+        }
 
         Guild guild = Emulator.getGameEnvironment().getGuildManager().getGuild(guildId);
         ForumThread thread = ForumThread.getById(threadId);
@@ -59,4 +65,4 @@ public class GuildForumThreadsMessagesEvent extends MessageHandler {
             this.client.sendResponse(new BubbleAlertComposer(BubbleAlertKeys.FORUMS_ACCESS_DENIED.key).compose());
         }
     }
-}
+}

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

@@ -23,6 +23,15 @@ public class GuildForumUpdateSettingsEvent extends MessageHandler {
         int postThreads = packet.readInt();
         int modForum = packet.readInt();
 
+        if (!GuildForumInputGuard.isPositiveId(guildId) ||
+                !GuildForumInputGuard.isSettingsState(canRead) ||
+                !GuildForumInputGuard.isSettingsState(postMessages) ||
+                !GuildForumInputGuard.isSettingsState(postThreads) ||
+                !GuildForumInputGuard.isSettingsState(modForum)) {
+            this.client.sendResponse(new ConnectionErrorComposer(400));
+            return;
+        }
+
         Guild guild = Emulator.getGameEnvironment().getGuildManager().getGuild(guildId);
 
         if (guild == null) {
@@ -48,4 +57,4 @@ public class GuildForumUpdateSettingsEvent extends MessageHandler {
 
         this.client.sendResponse(new GuildForumDataComposer(guild, this.client.getHabbo()));
     }
-}
+}

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

@@ -0,0 +1,62 @@
+package com.eu.habbo.messages.incoming.guilds.forums;
+
+import org.junit.jupiter.api.Test;
+
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.List;
+
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+class GuildForumInputGuardContractTest {
+    @Test
+    void forumHandlersValidateClientProvidedIds() throws Exception {
+        Path base = Path.of("src/main/java/com/eu/habbo/messages/incoming/guilds/forums");
+
+        for (String handler : List.of(
+                "GuildForumPostThreadEvent.java",
+                "GuildForumModerateMessageEvent.java",
+                "GuildForumModerateThreadEvent.java",
+                "GuildForumThreadUpdateEvent.java",
+                "GuildForumThreadsEvent.java",
+                "GuildForumThreadsMessagesEvent.java",
+                "GuildForumMarkAsReadEvent.java",
+                "GuildForumUpdateSettingsEvent.java"
+        )) {
+            String source = Files.readString(base.resolve(handler));
+
+            assertTrue(source.contains("GuildForumInputGuard.isPositiveId"),
+                    handler + " must reject zero or negative client-provided ids");
+        }
+    }
+
+    @Test
+    void forumHandlersBoundExpensiveClientInputs() throws Exception {
+        Path base = Path.of("src/main/java/com/eu/habbo/messages/incoming/guilds/forums");
+
+        String messages = Files.readString(base.resolve("GuildForumThreadsMessagesEvent.java"));
+        String markRead = Files.readString(base.resolve("GuildForumMarkAsReadEvent.java"));
+        String settings = Files.readString(base.resolve("GuildForumUpdateSettingsEvent.java"));
+        String moderateThread = Files.readString(base.resolve("GuildForumModerateThreadEvent.java"));
+        String moderateMessage = Files.readString(base.resolve("GuildForumModerateMessageEvent.java"));
+
+        assertTrue(messages.contains("GuildForumInputGuard.isValidPage(index, limit)"),
+                "thread message reads must bound index/limit before fetching comments");
+        assertTrue(markRead.contains("GuildForumInputGuard.isValidMarkReadBatch(count)"),
+                "mark-as-read must bound the client-provided batch count before DB writes");
+        assertTrue(settings.contains("GuildForumInputGuard.isSettingsState"),
+                "forum settings must reject unknown SettingsState values");
+        assertTrue(moderateThread.contains("GuildForumInputGuard.isThreadModerationState(state)"),
+                "thread moderation must reject unknown ForumThreadState values");
+        assertTrue(moderateMessage.contains("GuildForumInputGuard.isMessageModerationState(state)"),
+                "message moderation must reject unknown ForumThreadState values");
+    }
+
+    @Test
+    void forumPostsNormalizeTextBeforeFiltering() throws Exception {
+        String source = Files.readString(Path.of("src/main/java/com/eu/habbo/messages/incoming/guilds/forums/GuildForumPostThreadEvent.java"));
+
+        assertTrue(source.contains("GuildForumInputGuard.normalize(this.packet.readString())"),
+                "forum post subject and body should be normalized before word filtering and length checks");
+    }
+}

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

@@ -0,0 +1,41 @@
+package com.eu.habbo.messages.incoming.guilds.forums;
+
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+class GuildForumInputGuardTest {
+    @Test
+    void normalizesNullableText() {
+        assertEquals("", GuildForumInputGuard.normalize(null));
+        assertEquals("hello", GuildForumInputGuard.normalize("  hello  "));
+    }
+
+    @Test
+    void validatesIdsAndPaging() {
+        assertFalse(GuildForumInputGuard.isPositiveId(0));
+        assertTrue(GuildForumInputGuard.isPositiveId(1));
+        assertFalse(GuildForumInputGuard.isValidPage(-1, 20));
+        assertFalse(GuildForumInputGuard.isValidPage(0, 0));
+        assertTrue(GuildForumInputGuard.isValidPage(0, GuildForumInputGuard.MAX_PAGE_LIMIT));
+        assertFalse(GuildForumInputGuard.isValidPage(0, GuildForumInputGuard.MAX_PAGE_LIMIT + 1));
+    }
+
+    @Test
+    void validatesBatchAndStates() {
+        assertFalse(GuildForumInputGuard.isValidMarkReadBatch(0));
+        assertTrue(GuildForumInputGuard.isValidMarkReadBatch(GuildForumInputGuard.MAX_MARK_READ_BATCH));
+        assertFalse(GuildForumInputGuard.isValidMarkReadBatch(GuildForumInputGuard.MAX_MARK_READ_BATCH + 1));
+
+        assertTrue(GuildForumInputGuard.isSettingsState(0));
+        assertTrue(GuildForumInputGuard.isSettingsState(3));
+        assertFalse(GuildForumInputGuard.isSettingsState(4));
+
+        assertTrue(GuildForumInputGuard.isThreadModerationState(20));
+        assertFalse(GuildForumInputGuard.isThreadModerationState(999));
+        assertTrue(GuildForumInputGuard.isMessageModerationState(10));
+        assertFalse(GuildForumInputGuard.isMessageModerationState(0));
+    }
+}