Merge pull request #179 from simoleo89/fix/rooms-self-moderation-scope

fix(rooms): scope room actions and bound rights removal

Emulator/src/main/java/com/eu/habbo/messages/incoming/polls/AnswerPollEvent.java

@@ -2,6 +2,7 @@ package com.eu.habbo.messages.incoming.polls;
 
 import com.eu.habbo.Emulator;
 import com.eu.habbo.habbohotel.polls.Poll;
+import com.eu.habbo.habbohotel.rooms.Room;
 import com.eu.habbo.habbohotel.users.HabboBadge;
 import com.eu.habbo.messages.incoming.MessageHandler;
 import com.eu.habbo.messages.outgoing.users.AddUserBadgeComposer;
@@ -31,12 +32,20 @@ public class AnswerPollEvent extends MessageHandler {
         if(answer.length() <= 0) return;
 
         if (pollId == 0 && questionId <= 0) {
-            this.client.getHabbo().getHabboInfo().getCurrentRoom().handleWordQuiz(this.client.getHabbo(), answer.toString());
+            Room room = this.client.getHabbo().getHabboInfo().getCurrentRoom();
+            if (room != null) {
+                room.handleWordQuiz(this.client.getHabbo(), answer.toString());
+            }
             return;
         }
 
         answer = new StringBuilder(answer.substring(1));
 
+        Room room = this.client.getHabbo().getHabboInfo().getCurrentRoom();
+        if (room == null || room.getPollId() != pollId) {
+            return;
+        }
+
         Poll poll = Emulator.getGameEnvironment().getPollManager().getPoll(pollId);
 
         if (poll != null) {

Emulator/src/main/java/com/eu/habbo/messages/incoming/polls/CancelPollEvent.java

@@ -2,6 +2,7 @@ package com.eu.habbo.messages.incoming.polls;
 
 import com.eu.habbo.Emulator;
 import com.eu.habbo.habbohotel.polls.Poll;
+import com.eu.habbo.habbohotel.rooms.Room;
 import com.eu.habbo.messages.incoming.MessageHandler;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -17,6 +18,10 @@ public class CancelPollEvent extends MessageHandler {
     public void handle() throws Exception {
         int pollId = this.packet.readInt();
 
+        Room room = this.client.getHabbo().getHabboInfo().getCurrentRoom();
+        if (room == null || room.getPollId() != pollId) {
+            return;
+        }
 
         Poll poll = Emulator.getGameEnvironment().getPollManager().getPoll(pollId);
 

Emulator/src/main/java/com/eu/habbo/messages/incoming/polls/GetPollDataEvent.java

@@ -2,6 +2,7 @@ package com.eu.habbo.messages.incoming.polls;
 
 import com.eu.habbo.Emulator;
 import com.eu.habbo.habbohotel.polls.Poll;
+import com.eu.habbo.habbohotel.rooms.Room;
 import com.eu.habbo.messages.incoming.MessageHandler;
 import com.eu.habbo.messages.outgoing.polls.PollQuestionsComposer;
 
@@ -10,6 +11,11 @@ public class GetPollDataEvent extends MessageHandler {
     public void handle() throws Exception {
         int pollId = this.packet.readInt();
 
+        Room room = this.client.getHabbo().getHabboInfo().getCurrentRoom();
+        if (room == null || room.getPollId() != pollId) {
+            return;
+        }
+
         Poll poll = Emulator.getGameEnvironment().getPollManager().getPoll(pollId);
 
         if (poll != null) {

Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/users/RoomUserBanEvent.java

@@ -2,6 +2,7 @@ package com.eu.habbo.messages.incoming.rooms.users;
 
 import com.eu.habbo.Emulator;
 import com.eu.habbo.habbohotel.rooms.RoomManager;
+import com.eu.habbo.habbohotel.rooms.Room;
 import com.eu.habbo.messages.incoming.MessageHandler;
 
 public class RoomUserBanEvent extends MessageHandler {
@@ -11,6 +12,18 @@ public class RoomUserBanEvent extends MessageHandler {
         int roomId = this.packet.readInt();
         String banName = this.packet.readString();
 
-        Emulator.getGameEnvironment().getRoomManager().banUserFromRoom(this.client.getHabbo(), userId, roomId, RoomManager.RoomBanTypes.valueOf(banName));
+        Room room = this.client.getHabbo().getHabboInfo().getCurrentRoom();
+        if (room == null || room.getId() != roomId) {
+            return;
+        }
+
+        RoomManager.RoomBanTypes banType;
+        try {
+            banType = RoomManager.RoomBanTypes.valueOf(banName);
+        } catch (IllegalArgumentException e) {
+            return;
+        }
+
+        Emulator.getGameEnvironment().getRoomManager().banUserFromRoom(this.client.getHabbo(), userId, roomId, banType);
     }
-}
+}

Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/users/RoomUserMuteEvent.java

@@ -15,17 +15,18 @@ public class RoomUserMuteEvent extends MessageHandler {
         int roomId = this.packet.readInt();
         int minutes = this.packet.readInt();
 
-        Room room = Emulator.getGameEnvironment().getRoomManager().getRoom(roomId);
+        Room room = this.client.getHabbo().getHabboInfo().getCurrentRoom();
+        if (room == null || room.getId() != roomId) {
+            return;
+        }
 
-        if (room != null) {
-            if (room.hasRights(this.client.getHabbo()) || this.client.getHabbo().hasPermission("cmd_mute") || this.client.getHabbo().hasPermission(Permission.ACC_AMBASSADOR)) {
-                Habbo habbo = room.getHabbo(userId);
+        if (room.hasRights(this.client.getHabbo()) || this.client.getHabbo().hasPermission("cmd_mute") || this.client.getHabbo().hasPermission(Permission.ACC_AMBASSADOR)) {
+            Habbo habbo = room.getHabbo(userId);
 
-                if (habbo != null) {
-                    room.muteHabbo(habbo, minutes);
-                    habbo.getClient().sendResponse(new MutedWhisperComposer(minutes * 60));
-                    AchievementManager.progressAchievement(this.client.getHabbo(), Emulator.getGameEnvironment().getAchievementManager().getAchievement("SelfModMuteSeen"));
-                }
+            if (habbo != null) {
+                room.muteHabbo(habbo, minutes);
+                habbo.getClient().sendResponse(new MutedWhisperComposer(minutes * 60));
+                AchievementManager.progressAchievement(this.client.getHabbo(), Emulator.getGameEnvironment().getAchievementManager().getAchievement("SelfModMuteSeen"));
             }
         }
     }

Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/users/RoomUserRemoveRightsEvent.java

@@ -3,8 +3,12 @@ package com.eu.habbo.messages.incoming.rooms.users;
 import com.eu.habbo.habbohotel.permissions.Permission;
 import com.eu.habbo.habbohotel.rooms.Room;
 import com.eu.habbo.messages.incoming.MessageHandler;
+import com.eu.habbo.util.PacketGuard;
 
 public class RoomUserRemoveRightsEvent extends MessageHandler {
+    private static final int MAX_RIGHTS_REMOVALS = 100;
+    private static final int BYTES_PER_USER_ID = 4;
+
     @Override
     public void handle() throws Exception {
         int amount = this.packet.readInt();
@@ -15,6 +19,11 @@ public class RoomUserRemoveRightsEvent extends MessageHandler {
             return;
 
         if (room.getOwnerId() == this.client.getHabbo().getHabboInfo().getId() || this.client.getHabbo().hasPermission(Permission.ACC_ANYROOMOWNER)) {
+            if (!PacketGuard.isCountInRange(amount, 1, MAX_RIGHTS_REMOVALS)
+                    || !PacketGuard.hasFixedWidthEntries(this.packet, amount, BYTES_PER_USER_ID)) {
+                return;
+            }
+
             for (int i = 0; i < amount; i++) {
                 int userId = this.packet.readInt();
 

Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/users/UnbanRoomUserEvent.java

@@ -10,13 +10,13 @@ public class UnbanRoomUserEvent extends MessageHandler {
         int userId = this.packet.readInt();
         int roomId = this.packet.readInt();
 
-        Room room = Emulator.getGameEnvironment().getRoomManager().getRoom(roomId);
-
-        if (room != null) {
-            if (room.isOwner(this.client.getHabbo())) {
-                room.unbanHabbo(userId);
-            }
+        Room room = this.client.getHabbo().getHabboInfo().getCurrentRoom();
+        if (room == null || room.getId() != roomId) {
+            return;
         }
 
+        if (room.isOwner(this.client.getHabbo())) {
+            room.unbanHabbo(userId);
+        }
     }
 }

Emulator/src/test/java/com/eu/habbo/messages/incoming/polls/PollRoomScopeContractTest.java

@@ -0,0 +1,33 @@
+package com.eu.habbo.messages.incoming.polls;
+
+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 PollRoomScopeContractTest {
+    @Test
+    void pollHandlersRequireMatchingCurrentRoomPoll() throws Exception {
+        assertRequiresMatchingRoomPoll("AnswerPollEvent.java");
+        assertRequiresMatchingRoomPoll("CancelPollEvent.java");
+        assertRequiresMatchingRoomPoll("GetPollDataEvent.java");
+    }
+
+    private void assertRequiresMatchingRoomPoll(String fileName) throws Exception {
+        String source = Files.readString(Path.of("src/main/java/com/eu/habbo/messages/incoming/polls/" + fileName));
+        int packetPollId = source.indexOf("int pollId = this.packet.readInt();");
+        int pollLookup = source.indexOf("getPoll(pollId)");
+
+        assertTrue(packetPollId >= 0, fileName + " must read the poll id from the packet");
+        assertTrue(pollLookup >= 0, fileName + " must look up the requested poll explicitly");
+
+        String guardedSection = source.substring(packetPollId, pollLookup);
+
+        assertTrue(guardedSection.contains("getCurrentRoom()"),
+                fileName + " must bind poll actions to the caller's current room");
+        assertTrue(guardedSection.contains("room == null || room.getPollId() != pollId"),
+                fileName + " must reject poll ids that are not active in the current room");
+    }
+}

Emulator/src/test/java/com/eu/habbo/messages/incoming/rooms/users/RoomModerationScopeContractTest.java

@@ -0,0 +1,24 @@
+package com.eu.habbo.messages.incoming.rooms.users;
+
+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 RoomModerationScopeContractTest {
+    @Test
+    void roomUserBanAndMuteAreScopedToCurrentRoom() throws Exception {
+        Path base = Path.of("src/main/java/com/eu/habbo/messages/incoming/rooms/users");
+
+        for (String handler : new String[]{"RoomUserBanEvent.java", "RoomUserMuteEvent.java", "UnbanRoomUserEvent.java"}) {
+            String source = Files.readString(base.resolve(handler));
+
+            assertTrue(source.contains("getCurrentRoom()"),
+                    handler + " must authorize room moderation against the user's current room");
+            assertTrue(source.contains("room.getId() != roomId"),
+                    handler + " must reject client-supplied room ids that do not match the current room");
+        }
+    }
+}

Emulator/src/test/java/com/eu/habbo/messages/incoming/rooms/users/RoomUserRemoveRightsContractTest.java

@@ -0,0 +1,32 @@
+package com.eu.habbo.messages.incoming.rooms.users;
+
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+
+import org.junit.jupiter.api.Test;
+
+class RoomUserRemoveRightsContractTest {
+    private static final Path SOURCE = Path.of(
+            "src/main/java/com/eu/habbo/messages/incoming/rooms/users/RoomUserRemoveRightsEvent.java");
+
+    @Test
+    void removeRightsBatchIsBoundedAndRequiresCompletePayload() throws IOException {
+        String source = Files.readString(SOURCE);
+
+        assertTrue(source.contains("private static final int MAX_RIGHTS_REMOVALS = 100;"));
+        assertTrue(source.contains("PacketGuard.isCountInRange(amount, 1, MAX_RIGHTS_REMOVALS)"));
+        assertTrue(source.contains("PacketGuard.hasFixedWidthEntries(this.packet, amount, BYTES_PER_USER_ID)"));
+
+        int guardIndex = source.indexOf("PacketGuard.isCountInRange(amount, 1, MAX_RIGHTS_REMOVALS)");
+        int payloadIndex = source.indexOf("PacketGuard.hasFixedWidthEntries(this.packet, amount, BYTES_PER_USER_ID)");
+        int readIndex = source.indexOf("int userId = this.packet.readInt();");
+        int removeIndex = source.indexOf("room.removeRights(userId);");
+
+        assertTrue(guardIndex < readIndex, "batch size should be validated before reading user ids");
+        assertTrue(payloadIndex < readIndex, "payload length should be validated before reading user ids");
+        assertTrue(readIndex < removeIndex, "rights should only be removed after reading a validated user id");
+    }
+}