Merge pull request #212 from simoleo89/fix/room-item-safety

Harden room item packet guards

Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/items/RoomPickupChooserEvent.java

@@ -1,15 +1,21 @@
 package com.eu.habbo.messages.incoming.rooms.items;
 
+import com.eu.habbo.habbohotel.items.interactions.InteractionPostIt;
 import com.eu.habbo.habbohotel.permissions.Permission;
 import com.eu.habbo.habbohotel.rooms.Room;
 import com.eu.habbo.habbohotel.users.HabboItem;
 import com.eu.habbo.messages.incoming.MessageHandler;
 
 public class RoomPickupChooserEvent extends MessageHandler {
+    private static final int MAX_PICKUP_CHOOSER_ITEMS = 100;
+
     @Override
     public void handle() throws Exception {
         int count = this.packet.readInt();
 
+        if (count <= 0 || count > MAX_PICKUP_CHOOSER_ITEMS)
+            return;
+
         Room room = this.client.getHabbo().getHabboInfo().getCurrentRoom();
 
         if (room == null)
@@ -20,6 +26,9 @@ public class RoomPickupChooserEvent extends MessageHandler {
             HabboItem item = room.getHabboItem(itemId);
 
             if (item != null) {
+                if (item instanceof InteractionPostIt)
+                    continue;
+
                 if (item.getUserId() == this.client.getHabbo().getHabboInfo().getId()) {
                     room.pickUpItem(item, this.client.getHabbo());
                 } else {
@@ -36,4 +45,4 @@ public class RoomPickupChooserEvent extends MessageHandler {
             }
         }
     }
-}
+}

Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/items/SavePostItStickyPoleEvent.java

@@ -33,16 +33,22 @@ public class SavePostItStickyPoleEvent extends MessageHandler {
                 LOGGER.info("Scripter Alert! {} | {}", this.client.getHabbo().getHabboInfo().getUsername(), this.packet.readString());
             }
         } else {
-            String text = this.packet.readString();
+            String text = Emulator.getGameEnvironment().getWordFilter().filter(this.packet.readString().replace(((char) 9) + "", ""), this.client.getHabbo());
+
+            if (text.length() > Emulator.getConfig().getInt("postit.charlimit"))
+                return;
 
             Room room = this.client.getHabbo().getHabboInfo().getCurrentRoom();
+            if (room == null)
+                return;
+
             HabboItem sticky = room.getHabboItem(itemId);
 
-            if (sticky != null) {
+            if (sticky instanceof InteractionPostIt) {
                 if (sticky.getUserId() == this.client.getHabbo().getHabboInfo().getId()) {
                     sticky.setUserId(room.getOwnerId());
 
-                    if (color.equalsIgnoreCase(PostItColor.YELLOW.hexColor)) {
+                    if (PostItColor.isCustomColor(color) || color.equalsIgnoreCase(PostItColor.YELLOW.hexColor)) {
                         color = PostItColor.randomColorNotYellow().hexColor;
                     }
                     if (!InteractionPostIt.STICKYPOLE_PREFIX_TEXT.isEmpty()) {

Emulator/src/test/java/com/eu/habbo/messages/incoming/rooms/items/RoomPickupChooserContractTest.java

@@ -0,0 +1,37 @@
+package com.eu.habbo.messages.incoming.rooms.items;
+
+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 RoomPickupChooserContractTest {
+
+    private static String source() throws Exception {
+        return Files.readString(Path.of("src/main/java/com/eu/habbo/messages/incoming/rooms/items/RoomPickupChooserEvent.java"));
+    }
+
+    @Test
+    void rejectsInvalidBatchSizesBeforeReadingItemIds() throws Exception {
+        String source = source();
+
+        int countRead = source.indexOf("int count = this.packet.readInt()");
+        int guard = source.indexOf("count <= 0 || count > MAX_PICKUP_CHOOSER_ITEMS", countRead);
+        int loop = source.indexOf("for (int i = 0; i < count; i++)", countRead);
+
+        assertTrue(countRead > -1, "Pickup chooser must read the client-provided count");
+        assertTrue(guard > countRead, "Pickup chooser must reject invalid batch sizes");
+        assertTrue(guard < loop, "Batch size validation must happen before consuming item ids");
+    }
+
+    @Test
+    void chooserSkipsPostItsLikeSinglePickup() throws Exception {
+        String source = source();
+
+        assertTrue(source.contains("import com.eu.habbo.habbohotel.items.interactions.InteractionPostIt;"));
+        assertTrue(source.contains("item instanceof InteractionPostIt"));
+        assertTrue(source.contains("continue;"));
+    }
+}

Emulator/src/test/java/com/eu/habbo/messages/incoming/rooms/items/SavePostItStickyPoleContractTest.java

@@ -0,0 +1,44 @@
+package com.eu.habbo.messages.incoming.rooms.items;
+
+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 SavePostItStickyPoleContractTest {
+
+    private static String source() throws Exception {
+        return Files.readString(Path.of("src/main/java/com/eu/habbo/messages/incoming/rooms/items/SavePostItStickyPoleEvent.java"));
+    }
+
+    @Test
+    void stickyPoleTextIsFilteredAndBoundedBeforeMutation() throws Exception {
+        String source = source();
+
+        int textRead = source.indexOf("WordFilter().filter");
+        int charLimit = source.indexOf("text.length() > Emulator.getConfig().getInt(\"postit.charlimit\")", textRead);
+        int update = source.indexOf("sticky.setExtradata", textRead);
+
+        assertTrue(textRead > -1, "Sticky pole text should pass through the word filter");
+        assertTrue(charLimit > textRead, "Sticky pole text must enforce the configured post-it char limit");
+        assertTrue(charLimit < update, "Sticky pole text must be bounded before item mutation");
+    }
+
+    @Test
+    void stickyPoleOnlyMutatesPostItItemsInARoom() throws Exception {
+        String source = source();
+
+        assertTrue(source.contains("if (room == null)"));
+        assertTrue(source.contains("sticky instanceof InteractionPostIt"));
+    }
+
+    @Test
+    void stickyPoleNormalizesCustomOrYellowColors() throws Exception {
+        String source = source();
+
+        assertTrue(source.contains("PostItColor.isCustomColor(color) || color.equalsIgnoreCase(PostItColor.YELLOW.hexColor)"));
+        assertTrue(source.contains("PostItColor.randomColorNotYellow().hexColor"));
+    }
+}