diff --git a/Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/RoomSettingsSaveEvent.java b/Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/RoomSettingsSaveEvent.java index 8200b5f8..51fad249 100644 --- a/Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/RoomSettingsSaveEvent.java +++ b/Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/RoomSettingsSaveEvent.java @@ -15,6 +15,15 @@ import java.util.Set; public class RoomSettingsSaveEvent extends MessageHandler { private static final Logger LOGGER = LoggerFactory.getLogger(RoomSettingsSaveEvent.class); + private static final int MAX_ROOM_PASSWORD_LENGTH = 64; + private static final int MAX_TAGS = 2; + private static final int MIN_USERS_MAX = 1; + private static final int MAX_USERS_MAX = 200; + private static final int MIN_THICKNESS = -2; + private static final int MAX_THICKNESS = 1; + private static final int MAX_OPTION_LEVEL = 2; + private static final int MIN_CHAT_DISTANCE = 1; + private static final int MAX_CHAT_DISTANCE = 99; @Override public void handle() throws Exception { @@ -47,19 +56,33 @@ public class RoomSettingsSaveEvent extends MessageHandler { return; } - RoomState state = RoomState.values()[this.packet.readInt() % RoomState.values().length]; + int stateId = this.packet.readInt(); + if (stateId < 0 || stateId >= RoomState.values().length) { + return; + } + RoomState state = RoomState.values()[stateId]; String password = this.packet.readString(); + if (password.length() > MAX_ROOM_PASSWORD_LENGTH) { + return; + } if (state == RoomState.PASSWORD && password.isEmpty() && (room.getPassword() == null || room.getPassword().isEmpty())) { this.client.sendResponse(new RoomEditSettingsErrorComposer(room.getId(), RoomEditSettingsErrorComposer.PASSWORD_REQUIRED, "")); return; } int usersMax = this.packet.readInt(); + if (usersMax < MIN_USERS_MAX || usersMax > MAX_USERS_MAX) { + return; + } + int categoryId = this.packet.readInt(); StringBuilder tags = new StringBuilder(); Set uniqueTags = new HashSet<>(); - int count = Math.min(this.packet.readInt(), 2); + int count = this.packet.readInt(); + if (count < 0 || count > MAX_TAGS) { + return; + } for (int i = 0; i < count; i++) { String tag = this.packet.readString(); @@ -113,22 +136,52 @@ public class RoomSettingsSaveEvent extends MessageHandler { } + int tradeMode = this.packet.readInt(); + boolean allowPets = this.packet.readBoolean(); + boolean allowPetsEat = this.packet.readBoolean(); + boolean allowWalkthrough = this.packet.readBoolean(); + boolean hideWall = this.packet.readBoolean(); + int wallSize = this.packet.readInt(); + int floorSize = this.packet.readInt(); + int muteOption = this.packet.readInt(); + int kickOption = this.packet.readInt(); + int banOption = this.packet.readInt(); + int chatMode = this.packet.readInt(); + int chatWeight = this.packet.readInt(); + int chatSpeed = this.packet.readInt(); + int chatDistance = this.packet.readInt(); + int chatProtection = this.packet.readInt(); + + if (!isInRange(tradeMode, 0, MAX_OPTION_LEVEL) + || !isInRange(wallSize, MIN_THICKNESS, MAX_THICKNESS) + || !isInRange(floorSize, MIN_THICKNESS, MAX_THICKNESS) + || !isInRange(muteOption, 0, MAX_OPTION_LEVEL) + || !isInRange(kickOption, 0, MAX_OPTION_LEVEL) + || !isInRange(banOption, 0, MAX_OPTION_LEVEL) + || !isInRange(chatMode, 0, MAX_OPTION_LEVEL) + || !isInRange(chatWeight, 0, MAX_OPTION_LEVEL) + || !isInRange(chatSpeed, 0, MAX_OPTION_LEVEL) + || !isInRange(chatDistance, MIN_CHAT_DISTANCE, MAX_CHAT_DISTANCE) + || !isInRange(chatProtection, 0, MAX_OPTION_LEVEL)) { + return; + } + room.setTags(tags.toString()); - room.setTradeMode(this.packet.readInt()); - room.setAllowPets(this.packet.readBoolean()); - room.setAllowPetsEat(this.packet.readBoolean()); - room.setAllowWalkthrough(this.packet.readBoolean()); - room.setHideWall(this.packet.readBoolean()); - room.setWallSize(this.packet.readInt()); - room.setFloorSize(this.packet.readInt()); - room.setMuteOption(this.packet.readInt()); - room.setKickOption(this.packet.readInt()); - room.setBanOption(this.packet.readInt()); - room.setChatMode(this.packet.readInt()); - room.setChatWeight(this.packet.readInt()); - room.setChatSpeed(this.packet.readInt()); - room.setChatDistance(Math.abs(this.packet.readInt())); - room.setChatProtection(this.packet.readInt()); + room.setTradeMode(tradeMode); + room.setAllowPets(allowPets); + room.setAllowPetsEat(allowPetsEat); + room.setAllowWalkthrough(allowWalkthrough); + room.setHideWall(hideWall); + room.setWallSize(wallSize); + room.setFloorSize(floorSize); + room.setMuteOption(muteOption); + room.setKickOption(kickOption); + room.setBanOption(banOption); + room.setChatMode(chatMode); + room.setChatWeight(chatWeight); + room.setChatSpeed(chatSpeed); + room.setChatDistance(chatDistance); + room.setChatProtection(chatProtection); if (this.packet.bytesAvailable() > 0) { room.setAllowUnderpass(this.packet.readBoolean()); @@ -144,4 +197,8 @@ public class RoomSettingsSaveEvent extends MessageHandler { } } } + + private static boolean isInRange(int value, int min, int max) { + return value >= min && value <= max; + } } diff --git a/Emulator/src/test/java/com/eu/habbo/messages/incoming/rooms/RoomSettingsInputGuardContractTest.java b/Emulator/src/test/java/com/eu/habbo/messages/incoming/rooms/RoomSettingsInputGuardContractTest.java new file mode 100644 index 00000000..ae93f7ef --- /dev/null +++ b/Emulator/src/test/java/com/eu/habbo/messages/incoming/rooms/RoomSettingsInputGuardContractTest.java @@ -0,0 +1,60 @@ +package com.eu.habbo.messages.incoming.rooms; + +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 RoomSettingsInputGuardContractTest { + private static String source() throws Exception { + return Files.readString(Path.of("src/main/java/com/eu/habbo/messages/incoming/rooms/RoomSettingsSaveEvent.java")); + } + + @Test + void roomStateAndTagCountAreValidatedWithoutModuloOrTruncation() throws Exception { + String source = source(); + + int stateRead = source.indexOf("int stateId = this.packet.readInt()"); + int stateGuard = source.indexOf("stateId < 0 || stateId >= RoomState.values().length", stateRead); + int stateAssign = source.indexOf("RoomState state = RoomState.values()[stateId]", stateGuard); + int tagCount = source.indexOf("int count = this.packet.readInt()", stateAssign); + int tagGuard = source.indexOf("count < 0 || count > MAX_TAGS", tagCount); + int tagLoop = source.indexOf("for (int i = 0; i < count; i++)", tagGuard); + + assertTrue(source.contains("MAX_TAGS = 2"), "Room settings tag count should have an explicit cap"); + assertTrue(!source.contains("% RoomState.values().length"), + "Room state must not use modulo because negative values can crash or remap input"); + assertTrue(!source.contains("Math.min(this.packet.readInt(), 2)"), + "Room settings must reject oversized tag counts instead of truncating and desynchronizing the packet"); + assertTrue(stateRead > -1 && stateGuard > stateRead && stateAssign > stateGuard, + "Room state must be range-checked before indexing RoomState.values()"); + assertTrue(tagCount > -1 && tagGuard > tagCount && tagLoop > tagGuard, + "Tag count must be range-checked before reading tag strings"); + } + + @Test + void roomSettingsOptionsAreValidatedBeforeMutatingRoom() throws Exception { + String source = source(); + + int tradeMode = source.indexOf("int tradeMode = this.packet.readInt()"); + int validation = source.indexOf("!isInRange(tradeMode, 0, MAX_OPTION_LEVEL)", tradeMode); + int setTags = source.indexOf("room.setTags(tags.toString())", validation); + int setChatDistance = source.indexOf("room.setChatDistance(chatDistance)", setTags); + + assertTrue(source.contains("MAX_ROOM_PASSWORD_LENGTH = 64"), + "Room password should have a bounded server-side length"); + assertTrue(source.contains("MAX_USERS_MAX = 200"), + "Room capacity should have a bounded server-side maximum"); + assertTrue(source.contains("MIN_CHAT_DISTANCE = 1") && source.contains("MAX_CHAT_DISTANCE = 99"), + "Room chat distance should be explicitly bounded"); + assertTrue(!source.contains("Math.abs(this.packet.readInt())"), + "Room settings must reject invalid chat distance instead of converting negative values"); + assertTrue(validation > tradeMode, "Room options must be validated after reading them"); + assertTrue(validation < setTags, "Room options must be validated before mutating room fields"); + assertTrue(setChatDistance > setTags, "Validated chat distance should be applied after the guard block"); + assertTrue(source.contains("private static boolean isInRange"), + "Room settings should use one clear range helper for numeric option guards"); + } +}