Merge pull request #222 from simoleo89/fix/polls-answer-guards

fix(polls): bound answer payloads

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

@@ -16,6 +16,9 @@ import java.sql.SQLException;
 
 public class AnswerPollEvent extends MessageHandler {
     private static final Logger LOGGER = LoggerFactory.getLogger(AnswerPollEvent.class);
+    private static final int MAX_ANSWER_COUNT = 20;
+    private static final int MAX_ANSWER_PART_LENGTH = 255;
+    private static final int MAX_COMBINED_ANSWER_LENGTH = 2048;
 
     @Override
     public void handle() throws Exception {
@@ -23,13 +26,19 @@ public class AnswerPollEvent extends MessageHandler {
         int questionId = this.packet.readInt();
         int count = this.packet.readInt();
         String answers = this.packet.readString();
+        if (count <= 0 || count > MAX_ANSWER_COUNT || answers == null || answers.length() > MAX_ANSWER_PART_LENGTH) {
+            return;
+        }
         
         StringBuilder answer = new StringBuilder();
         for (int i = 0; i < count; i++) {
             answer.append(":").append(answers);
+            if (answer.length() > MAX_COMBINED_ANSWER_LENGTH) {
+                return;
+            }
         }
 
-        if(answer.length() <= 0) return;
+        if (answer.length() <= 0) return;
 
         if (pollId == 0 && questionId <= 0) {
             Room room = this.client.getHabbo().getHabboInfo().getCurrentRoom();

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

@@ -0,0 +1,53 @@
+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 PollAnswerInputGuardContractTest {
+    private static String source() throws Exception {
+        return Files.readString(Path.of("src/main/java/com/eu/habbo/messages/incoming/polls/AnswerPollEvent.java"));
+    }
+
+    @Test
+    void pollAnswerCountAndPartLengthAreBoundedBeforeBuildingCombinedAnswer() throws Exception {
+        String source = source();
+
+        int count = source.indexOf("int count = this.packet.readInt()");
+        int answers = source.indexOf("String answers = this.packet.readString()", count);
+        int guard = source.indexOf("count <= 0 || count > MAX_ANSWER_COUNT", answers);
+        int builder = source.indexOf("StringBuilder answer = new StringBuilder()", guard);
+        int loop = source.indexOf("for (int i = 0; i < count; i++)", builder);
+
+        assertTrue(source.contains("MAX_ANSWER_COUNT = 20"),
+                "Poll answers should have a bounded answer count");
+        assertTrue(source.contains("MAX_ANSWER_PART_LENGTH = 255"),
+                "Poll answer fragments should have a bounded length");
+        assertTrue(source.contains("MAX_COMBINED_ANSWER_LENGTH = 2048"),
+                "Poll combined answer should have a bounded final length");
+        assertTrue(count > -1 && answers > count, "Poll handler must read count and answer string");
+        assertTrue(guard > answers, "Poll handler must validate count and answer string after reading them");
+        assertTrue(guard < builder && builder < loop,
+                "Poll handler must validate inputs before building the repeated answer string");
+    }
+
+    @Test
+    void combinedAnswerLengthIsCheckedBeforeWordQuizOrDatabaseWrite() throws Exception {
+        String source = source();
+
+        int append = source.indexOf("answer.append(\":\").append(answers)");
+        int combinedGuard = source.indexOf("answer.length() > MAX_COMBINED_ANSWER_LENGTH", append);
+        int wordQuiz = source.indexOf("handleWordQuiz", combinedGuard);
+        int dbWrite = source.indexOf("INSERT INTO polls_answers", combinedGuard);
+
+        assertTrue(combinedGuard > append,
+                "Poll handler must check combined answer length while building it");
+        assertTrue(combinedGuard < wordQuiz,
+                "Poll handler must bound word quiz answers before dispatching them");
+        assertTrue(combinedGuard < dbWrite,
+                "Poll handler must bound poll answers before persisting them");
+    }
+}