From 3342b22a76d425e5660360cea214f297d9984a69 Mon Sep 17 00:00:00 2001 From: simoleo89 Date: Tue, 16 Jun 2026 20:57:11 +0200 Subject: [PATCH] fix(rooms): bound room user inputs --- .../rooms/users/RoomUserActionEvent.java | 3 + .../rooms/users/RoomUserBanEvent.java | 4 + .../rooms/users/RoomUserGiveRightsEvent.java | 4 + .../rooms/users/RoomUserInputGuard.java | 17 ++++ .../rooms/users/RoomUserKickEvent.java | 15 +-- .../rooms/users/RoomUserMuteEvent.java | 4 + .../users/RoomUserRemoveRightsEvent.java | 3 + .../rooms/users/UnbanRoomUserEvent.java | 4 + .../users/RoomUserInputGuardContractTest.java | 91 +++++++++++++++++++ 9 files changed, 139 insertions(+), 6 deletions(-) create mode 100644 Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/users/RoomUserInputGuard.java create mode 100644 Emulator/src/test/java/com/eu/habbo/messages/incoming/rooms/users/RoomUserInputGuardContractTest.java diff --git a/Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/users/RoomUserActionEvent.java b/Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/users/RoomUserActionEvent.java index bc32465b..6607652e 100644 --- a/Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/users/RoomUserActionEvent.java +++ b/Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/users/RoomUserActionEvent.java @@ -28,6 +28,9 @@ public class RoomUserActionEvent extends MessageHandler { } int action = this.packet.readInt(); + if (!RoomUserInputGuard.isValidAction(action)) + return; + int wiredAction = 0; if (action == 5) { diff --git a/Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/users/RoomUserBanEvent.java b/Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/users/RoomUserBanEvent.java index df071e0a..b999c87d 100644 --- a/Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/users/RoomUserBanEvent.java +++ b/Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/users/RoomUserBanEvent.java @@ -12,6 +12,10 @@ public class RoomUserBanEvent extends MessageHandler { int roomId = this.packet.readInt(); String banName = this.packet.readString(); + if (!RoomUserInputGuard.isPositiveId(userId) || !RoomUserInputGuard.isPositiveId(roomId)) { + return; + } + Room room = this.client.getHabbo().getHabboInfo().getCurrentRoom(); if (room == null || room.getId() != roomId) { return; diff --git a/Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/users/RoomUserGiveRightsEvent.java b/Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/users/RoomUserGiveRightsEvent.java index 479b0612..8d1ed63a 100644 --- a/Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/users/RoomUserGiveRightsEvent.java +++ b/Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/users/RoomUserGiveRightsEvent.java @@ -13,6 +13,10 @@ public class RoomUserGiveRightsEvent extends MessageHandler { public void handle() throws Exception { int userId = this.packet.readInt(); + if (!RoomUserInputGuard.isPositiveId(userId)) { + return; + } + Room room = this.client.getHabbo().getHabboInfo().getCurrentRoom(); if (room == null) diff --git a/Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/users/RoomUserInputGuard.java b/Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/users/RoomUserInputGuard.java new file mode 100644 index 00000000..317e0897 --- /dev/null +++ b/Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/users/RoomUserInputGuard.java @@ -0,0 +1,17 @@ +package com.eu.habbo.messages.incoming.rooms.users; + +final class RoomUserInputGuard { + static final int MIN_ACTION_ID = 0; + static final int MAX_ACTION_ID = 7; + + private RoomUserInputGuard() { + } + + static boolean isPositiveId(int id) { + return id > 0; + } + + static boolean isValidAction(int action) { + return action >= MIN_ACTION_ID && action <= MAX_ACTION_ID; + } +} diff --git a/Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/users/RoomUserKickEvent.java b/Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/users/RoomUserKickEvent.java index 814b0599..27e6bc01 100644 --- a/Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/users/RoomUserKickEvent.java +++ b/Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/users/RoomUserKickEvent.java @@ -21,6 +21,9 @@ public class RoomUserKickEvent extends MessageHandler { int userId = this.packet.readInt(); + if (!RoomUserInputGuard.isPositiveId(userId)) + return; + Habbo target = room.getHabbo(userId); if (target == null) @@ -35,15 +38,15 @@ public class RoomUserKickEvent extends MessageHandler { return; } - UserKickEvent event = new UserKickEvent(this.client.getHabbo(), target); - Emulator.getPluginManager().fireEvent(event); - - if (event.isCancelled()) - return; - if (room.hasRights(this.client.getHabbo()) || this.client.getHabbo().hasPermission(Permission.ACC_ANYROOMOWNER) || this.client.getHabbo().hasPermission(Permission.ACC_AMBASSADOR)) { if (target.hasPermission(Permission.ACC_UNKICKABLE)) return; + UserKickEvent event = new UserKickEvent(this.client.getHabbo(), target); + Emulator.getPluginManager().fireEvent(event); + + if (event.isCancelled()) + return; + room.kickHabbo(target, true); AchievementManager.progressAchievement(this.client.getHabbo(), Emulator.getGameEnvironment().getAchievementManager().getAchievement("SelfModKickSeen")); } diff --git a/Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/users/RoomUserMuteEvent.java b/Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/users/RoomUserMuteEvent.java index 7a8a55bf..9cc6c373 100644 --- a/Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/users/RoomUserMuteEvent.java +++ b/Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/users/RoomUserMuteEvent.java @@ -18,6 +18,10 @@ public class RoomUserMuteEvent extends MessageHandler { int roomId = this.packet.readInt(); int minutes = this.packet.readInt(); + if (!RoomUserInputGuard.isPositiveId(userId) || !RoomUserInputGuard.isPositiveId(roomId)) { + return; + } + Room room = this.client.getHabbo().getHabboInfo().getCurrentRoom(); if (room == null || room.getId() != roomId) { return; diff --git a/Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/users/RoomUserRemoveRightsEvent.java b/Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/users/RoomUserRemoveRightsEvent.java index 987ec101..f6041563 100644 --- a/Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/users/RoomUserRemoveRightsEvent.java +++ b/Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/users/RoomUserRemoveRightsEvent.java @@ -27,6 +27,9 @@ public class RoomUserRemoveRightsEvent extends MessageHandler { for (int i = 0; i < amount; i++) { int userId = this.packet.readInt(); + if (!RoomUserInputGuard.isPositiveId(userId)) + continue; + room.removeRights(userId); } } diff --git a/Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/users/UnbanRoomUserEvent.java b/Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/users/UnbanRoomUserEvent.java index 662d8d30..369f5bfb 100644 --- a/Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/users/UnbanRoomUserEvent.java +++ b/Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/users/UnbanRoomUserEvent.java @@ -10,6 +10,10 @@ public class UnbanRoomUserEvent extends MessageHandler { int userId = this.packet.readInt(); int roomId = this.packet.readInt(); + if (!RoomUserInputGuard.isPositiveId(userId) || !RoomUserInputGuard.isPositiveId(roomId)) { + return; + } + Room room = this.client.getHabbo().getHabboInfo().getCurrentRoom(); if (room == null || room.getId() != roomId) { return; diff --git a/Emulator/src/test/java/com/eu/habbo/messages/incoming/rooms/users/RoomUserInputGuardContractTest.java b/Emulator/src/test/java/com/eu/habbo/messages/incoming/rooms/users/RoomUserInputGuardContractTest.java new file mode 100644 index 00000000..3f319b88 --- /dev/null +++ b/Emulator/src/test/java/com/eu/habbo/messages/incoming/rooms/users/RoomUserInputGuardContractTest.java @@ -0,0 +1,91 @@ +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.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class RoomUserInputGuardContractTest { + private static String source(String name) throws Exception { + return Files.readString(Path.of("src/main/java/com/eu/habbo/messages/incoming/rooms/users/" + name + ".java")); + } + + @Test + void roomModerationHandlersRejectInvalidUserAndRoomIds() throws Exception { + for (String handler : new String[]{"RoomUserBanEvent", "UnbanRoomUserEvent", "RoomUserMuteEvent"}) { + String source = source(handler); + int userRead = source.indexOf("int userId = this.packet.readInt()"); + int roomRead = source.indexOf("int roomId = this.packet.readInt()", userRead); + int guard = source.indexOf("RoomUserInputGuard.isPositiveId(userId)", roomRead); + int roomLookup = source.indexOf("getCurrentRoom()", guard); + + assertTrue(userRead > -1 && roomRead > userRead, handler + " should read user and room ids"); + assertTrue(guard > roomRead && guard < roomLookup, + handler + " should reject invalid ids before resolving room state"); + } + } + + @Test + void rightsMutationHandlersRejectInvalidUserIds() throws Exception { + String giveRights = source("RoomUserGiveRightsEvent"); + String removeRights = source("RoomUserRemoveRightsEvent"); + + int giveRead = giveRights.indexOf("int userId = this.packet.readInt()"); + int giveGuard = giveRights.indexOf("RoomUserInputGuard.isPositiveId(userId)", giveRead); + int giveTarget = giveRights.indexOf("room.getHabbo(userId)", giveGuard); + + int removeRead = removeRights.indexOf("int userId = this.packet.readInt()"); + int removeGuard = removeRights.indexOf("RoomUserInputGuard.isPositiveId(userId)", removeRead); + int removeCall = removeRights.indexOf("room.removeRights(userId)", removeGuard); + + assertTrue(giveGuard > giveRead && giveGuard < giveTarget, + "give-rights should validate target id before online/friend lookups"); + assertTrue(removeGuard > removeRead && removeGuard < removeCall, + "remove-rights should skip invalid ids before removing rights"); + } + + @Test + void kickPluginEventOnlyFiresAfterPermissionCheck() throws Exception { + String source = source("RoomUserKickEvent"); + + int userRead = source.indexOf("int userId = this.packet.readInt()"); + int idGuard = source.indexOf("RoomUserInputGuard.isPositiveId(userId)", userRead); + int targetLookup = source.indexOf("room.getHabbo(userId)", idGuard); + int permissionCheck = source.indexOf("room.hasRights(this.client.getHabbo())", targetLookup); + int event = source.indexOf("new UserKickEvent", permissionCheck); + int kick = source.indexOf("room.kickHabbo(target, true)", event); + + assertTrue(idGuard > userRead && idGuard < targetLookup, + "kick should validate target id before room lookup"); + assertTrue(permissionCheck > targetLookup && event > permissionCheck && event < kick, + "kick plugin event should only fire once the actor is authorized"); + } + + @Test + void roomActionsRejectUnknownActionIdsBeforeComposersAndWired() throws Exception { + String source = source("RoomUserActionEvent"); + + int actionRead = source.indexOf("int action = this.packet.readInt()"); + int guard = source.indexOf("RoomUserInputGuard.isValidAction(action)", actionRead); + int composer = source.indexOf("new RoomUserActionComposer", guard); + int wired = source.indexOf("WiredManager.triggerUserPerformsAction", guard); + + assertTrue(guard > actionRead && guard < composer, + "room actions should reject unknown ids before composing room state"); + assertTrue(guard < wired, + "room actions should reject unknown ids before wired triggers"); + } + + @Test + void helperBoundsExpectedRanges() { + assertFalse(RoomUserInputGuard.isPositiveId(0)); + assertTrue(RoomUserInputGuard.isPositiveId(1)); + assertFalse(RoomUserInputGuard.isValidAction(-1)); + assertTrue(RoomUserInputGuard.isValidAction(0)); + assertTrue(RoomUserInputGuard.isValidAction(7)); + assertFalse(RoomUserInputGuard.isValidAction(8)); + } +}