Merge pull request #218 from simoleo89/fix/incoming-packet-guards

fix(friends): bound messenger inputs

Emulator/src/main/java/com/eu/habbo/messages/incoming/friends/AcceptFriendRequestEvent.java

@@ -25,14 +25,16 @@ public class AcceptFriendRequestEvent extends MessageHandler {
 
     @Override
     public void handle() throws Exception {
-        int count = Math.min(this.packet.readInt(), 100);
+        int count = this.packet.readInt();
+        if (count <= 0 || count > 100) return;
+
         int userId;
 
         for (int i = 0; i < count; i++) {
             userId = this.packet.readInt();
 
-            if (userId == 0)
-                return;
+            if (userId <= 0)
+                continue;
 
             if (this.client.getHabbo().getMessenger().getFriends().containsKey(userId)) {
                 this.client.getHabbo().getMessenger().deleteFriendRequests(userId, this.client.getHabbo().getHabboInfo().getId());

Emulator/src/main/java/com/eu/habbo/messages/incoming/friends/ChangeRelationEvent.java

@@ -1,5 +1,6 @@
 package com.eu.habbo.messages.incoming.friends;
 
+import com.eu.habbo.Emulator;
 import com.eu.habbo.habbohotel.messenger.MessengerBuddy;
 import com.eu.habbo.messages.incoming.MessageHandler;
 import com.eu.habbo.messages.outgoing.friends.UpdateFriendComposer;
@@ -12,12 +13,16 @@ public class ChangeRelationEvent extends MessageHandler {
         int relationId = this.packet.readInt();
 
         MessengerBuddy buddy = this.client.getHabbo().getMessenger().getFriends().get(userId);
-        if (buddy != null && relationId >= 0 && relationId <= 3) {
+        if (buddy != null && FriendInputGuard.isValidRelation(relationId)) {
             UserRelationShipEvent event = new UserRelationShipEvent(this.client.getHabbo(), buddy, relationId);
-            if (!event.isCancelled()) {
+            if (Emulator.getPluginManager().fireEvent(event).isCancelled())
+                return;
+
+            if (!FriendInputGuard.isValidRelation(event.relationShip))
+                return;
+
             buddy.setRelation(event.relationShip);
             this.client.sendResponse(new UpdateFriendComposer(this.client.getHabbo(), buddy, 0));
         }
     }
-    }
 }

Emulator/src/main/java/com/eu/habbo/messages/incoming/friends/DeclineFriendRequestEvent.java

@@ -3,6 +3,8 @@ package com.eu.habbo.messages.incoming.friends;
 import com.eu.habbo.messages.incoming.MessageHandler;
 
 public class DeclineFriendRequestEvent extends MessageHandler {
+    private static final int MAX_BATCH_SIZE = 100;
+
     @Override
     public void handle() throws Exception {
         boolean all = this.packet.readBoolean();
@@ -11,6 +13,7 @@ public class DeclineFriendRequestEvent extends MessageHandler {
             this.client.getHabbo().getMessenger().deleteAllFriendRequests(this.client.getHabbo().getHabboInfo().getId());
         } else {
             int count = this.packet.readInt();
+            if (count <= 0 || count > MAX_BATCH_SIZE) return;
 
             for (int i = 0; i < count; i++) {
                 this.client.getHabbo().getMessenger().deleteFriendRequests(this.packet.readInt(), this.client.getHabbo().getHabboInfo().getId());

Emulator/src/main/java/com/eu/habbo/messages/incoming/friends/FriendInputGuard.java

@@ -0,0 +1,31 @@
+package com.eu.habbo.messages.incoming.friends;
+
+final class FriendInputGuard {
+    static final int MAX_USERNAME_LENGTH = 15;
+    static final int MAX_MESSAGE_LENGTH = 255;
+    static final int MAX_RELATION_ID = 3;
+
+    private FriendInputGuard() {
+    }
+
+    static String normalizeUsername(String username) {
+        return username == null ? "" : username.trim();
+    }
+
+    static boolean isValidUsername(String username) {
+        return username != null && !username.isBlank() && username.length() <= MAX_USERNAME_LENGTH;
+    }
+
+    static String normalizeMessage(String message) {
+        if (message == null) {
+            return "";
+        }
+
+        String normalized = message.trim();
+        return normalized.length() > MAX_MESSAGE_LENGTH ? normalized.substring(0, MAX_MESSAGE_LENGTH) : normalized;
+    }
+
+    static boolean isValidRelation(int relationId) {
+        return relationId >= 0 && relationId <= MAX_RELATION_ID;
+    }
+}

Emulator/src/main/java/com/eu/habbo/messages/incoming/friends/FriendPrivateMessageEvent.java

@@ -9,7 +9,11 @@ public class FriendPrivateMessageEvent extends MessageHandler {
     @Override
     public void handle() throws Exception {
         int userId = this.packet.readInt();
-        String message = this.packet.readString();
+        String message = FriendInputGuard.normalizeMessage(this.packet.readString());
+
+        if (message.isEmpty()) {
+            return;
+        }
 
         if (!this.client.getHabbo().getHabboStats().allowTalk()) {
             return;
@@ -25,8 +29,6 @@ public class FriendPrivateMessageEvent extends MessageHandler {
         if (buddy == null)
             return;
 
-        if (message.length() > 255) message = message.substring(0, 255);
-
         UserFriendChatEvent event = new UserFriendChatEvent(this.client.getHabbo(), buddy, message);
         if (Emulator.getPluginManager().fireEvent(event).isCancelled())
             return;

Emulator/src/main/java/com/eu/habbo/messages/incoming/friends/FriendRequestEvent.java

@@ -26,9 +26,9 @@ public class FriendRequestEvent extends MessageHandler {
 
     @Override
     public void handle() throws Exception {
-        String username = this.packet.readString();
+        String username = FriendInputGuard.normalizeUsername(this.packet.readString());
 
-        if (this.client == null || username == null || username.isEmpty())
+        if (this.client == null || !FriendInputGuard.isValidUsername(username))
             return;
 
         // TargetHabbo can be null if the Habbo is not online or when the Habbo doesn't exist
@@ -62,6 +62,12 @@ public class FriendRequestEvent extends MessageHandler {
         if (targetId == this.client.getHabbo().getHabboInfo().getId())
             return;
 
+        if (this.client.getHabbo().getMessenger().getFriends().containsKey(targetId))
+            return;
+
+        if (Messenger.friendRequested(targetId, this.client.getHabbo().getHabboInfo().getId()) || Messenger.friendRequested(this.client.getHabbo().getHabboInfo().getId(), targetId))
+            return;
+
         // Target Habbo exists
         // Check if Habbo is accepting friend requests
         if (targetBlocksFriendRequests) {

Emulator/src/main/java/com/eu/habbo/messages/incoming/friends/InviteFriendsEvent.java

@@ -23,9 +23,14 @@ public class InviteFriendsEvent extends MessageHandler {
                 userIds[i] = this.packet.readInt();
             }
 
-            String message = this.packet.readString();
+            String message = FriendInputGuard.normalizeMessage(this.packet.readString());
+
+            if (message.isEmpty()) {
+                return;
+            }
 
             message = Emulator.getGameEnvironment().getWordFilter().filter(message, this.client.getHabbo());
+            message = FriendInputGuard.normalizeMessage(message);
 
             for (int i : userIds) {
                 if (i == 0)

Emulator/src/main/java/com/eu/habbo/messages/incoming/friends/RemoveFriendEvent.java

@@ -8,6 +8,7 @@ import com.eu.habbo.messages.outgoing.friends.RemoveFriendComposer;
 import gnu.trove.list.array.TIntArrayList;
 
 public class RemoveFriendEvent extends MessageHandler {
+    private static final int MAX_BATCH_SIZE = 100;
 
     private final TIntArrayList removedFriends;
 
@@ -18,8 +19,12 @@ public class RemoveFriendEvent extends MessageHandler {
     @Override
     public void handle() throws Exception {
         int count = this.packet.readInt();
+        if (count <= 0 || count > MAX_BATCH_SIZE) return;
+
         for (int i = 0; i < count; i++) {
             int habboId = this.packet.readInt();
+            if (habboId <= 0) continue;
+
             this.removedFriends.add(habboId);
 
             Messenger.unfriend(this.client.getHabbo().getHabboInfo().getId(), habboId);

Emulator/src/test/java/com/eu/habbo/messages/incoming/friends/FriendBatchGuardContractTest.java

@@ -0,0 +1,107 @@
+package com.eu.habbo.messages.incoming.friends;
+
+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 FriendBatchGuardContractTest {
+    private static String source(String name) throws Exception {
+        return Files.readString(Path.of("src/main/java/com/eu/habbo/messages/incoming/friends/" + name + ".java"));
+    }
+
+    @Test
+    void declineFriendRequestsBoundsClientSuppliedBatchCount() throws Exception {
+        String source = source("DeclineFriendRequestEvent");
+
+        int count = source.indexOf("int count = this.packet.readInt()");
+        int guard = source.indexOf("count <= 0 || count > MAX_BATCH_SIZE", count);
+        int loop = source.indexOf("for (int i = 0; i < count; i++)", count);
+        int delete = source.indexOf("deleteFriendRequests", loop);
+
+        assertTrue(source.contains("MAX_BATCH_SIZE = 100"),
+                "Friend request decline batches should have a conservative cap");
+        assertTrue(count > -1, "DeclineFriendRequestEvent must read the client supplied count");
+        assertTrue(guard > count, "DeclineFriendRequestEvent must validate the count after reading it");
+        assertTrue(guard < loop, "DeclineFriendRequestEvent must validate the count before looping");
+        assertTrue(loop < delete, "DeclineFriendRequestEvent should only mutate after the bounded loop starts");
+    }
+
+    @Test
+    void removeFriendsBoundsClientSuppliedBatchCountBeforeMutations() throws Exception {
+        String source = source("RemoveFriendEvent");
+
+        int count = source.indexOf("int count = this.packet.readInt()");
+        int guard = source.indexOf("count <= 0 || count > MAX_BATCH_SIZE", count);
+        int loop = source.indexOf("for (int i = 0; i < count; i++)", count);
+        int idGuard = source.indexOf("habboId <= 0", loop);
+        int unfriend = source.indexOf("Messenger.unfriend", loop);
+
+        assertTrue(source.contains("MAX_BATCH_SIZE = 100"),
+                "Friend removal batches should have a conservative cap");
+        assertTrue(count > -1, "RemoveFriendEvent must read the client supplied count");
+        assertTrue(guard > count, "RemoveFriendEvent must validate the count after reading it");
+        assertTrue(guard < loop, "RemoveFriendEvent must validate the count before looping");
+        assertTrue(idGuard > loop && idGuard < unfriend,
+                "RemoveFriendEvent must skip invalid ids before mutating friendships");
+    }
+
+    @Test
+    void acceptFriendRequestsBoundsClientSuppliedBatchCountBeforeLoadingTargets() throws Exception {
+        String source = source("AcceptFriendRequestEvent");
+
+        int count = source.indexOf("int count = this.packet.readInt()");
+        int guard = source.indexOf("count <= 0 || count > 100", count);
+        int loop = source.indexOf("for (int i = 0; i < count; i++)", count);
+        int idGuard = source.indexOf("userId <= 0", loop);
+        int loadTarget = source.indexOf("getHabbo(userId)", loop);
+
+        assertTrue(count > -1, "AcceptFriendRequestEvent must read the client supplied count");
+        assertTrue(guard > count && guard < loop,
+                "AcceptFriendRequestEvent must validate the count before looping");
+        assertTrue(idGuard > loop && idGuard < loadTarget,
+                "AcceptFriendRequestEvent must skip invalid ids before loading targets");
+    }
+
+    @Test
+    void friendRequestAndMessagesUseSharedInputGuards() throws Exception {
+        String guard = source("FriendInputGuard");
+        String request = source("FriendRequestEvent");
+        String privateMessage = source("FriendPrivateMessageEvent");
+        String invite = source("InviteFriendsEvent");
+
+        assertTrue(guard.contains("MAX_USERNAME_LENGTH = 15"),
+                "Friend request usernames should keep the Habbo username length bound");
+        assertTrue(guard.contains("MAX_MESSAGE_LENGTH = 255"),
+                "Messenger payloads should keep the client message length bound");
+        assertTrue(request.contains("FriendInputGuard.normalizeUsername"),
+                "Friend requests should normalize usernames before lookup");
+        assertTrue(request.contains("FriendInputGuard.isValidUsername"),
+                "Friend requests should reject empty or oversized usernames before DB lookup");
+        assertTrue(request.contains("Messenger.friendRequested(targetId, this.client.getHabbo().getHabboInfo().getId())"),
+                "Friend requests should reject duplicate outgoing requests");
+        assertTrue(privateMessage.contains("FriendInputGuard.normalizeMessage"),
+                "Private messages should be normalized and capped before plugin dispatch");
+        assertTrue(invite.contains("FriendInputGuard.normalizeMessage"),
+                "Room invites should be normalized and capped before fan-out");
+    }
+
+    @Test
+    void relationshipChangesFirePluginEventAndValidatePluginMutation() throws Exception {
+        String source = source("ChangeRelationEvent");
+
+        int event = source.indexOf("new UserRelationShipEvent");
+        int fire = source.indexOf("Emulator.getPluginManager().fireEvent(event)", event);
+        int pluginGuard = source.indexOf("FriendInputGuard.isValidRelation(event.relationShip)", fire);
+        int setRelation = source.indexOf("buddy.setRelation(event.relationShip)", pluginGuard);
+
+        assertTrue(source.contains("FriendInputGuard.isValidRelation(relationId)"),
+                "Relationship changes should reject invalid client relation ids");
+        assertTrue(event > -1 && fire > event,
+                "Relationship changes should dispatch the plugin event before applying changes");
+        assertTrue(pluginGuard > fire && pluginGuard < setRelation,
+                "Relationship changes should reject invalid plugin-mutated relation ids");
+    }
+}