From 0d9a097a59a8087d976c5ffac7ff0553bac7f36f Mon Sep 17 00:00:00 2001
From: Joo200 <github@joo200.de>
Date: Thu, 12 Oct 2023 16:51:19 +0200
Subject: [PATCH] Fix unsigned commands detected as signed (#1082)

* fix: commands flagged as signed without signed arguments

* feat: improve error message for illegal protocol state.
---
 .../packet/chat/LastSeenMessages.java         |  8 ++++++
 .../chat/keyed/KeyedCommandHandler.java       |  6 ++---
 .../chat/session/SessionCommandHandler.java   |  6 ++---
 .../chat/session/SessionPlayerCommand.java    | 27 ++++++++++++++++++-
 4 files changed, 40 insertions(+), 7 deletions(-)

diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/LastSeenMessages.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/LastSeenMessages.java
index fa3113af..a6f79123 100644
--- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/LastSeenMessages.java
+++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/LastSeenMessages.java
@@ -49,4 +49,12 @@ public class LastSeenMessages {
   public boolean isEmpty() {
     return acknowledged.isEmpty();
   }
+
+  @Override
+  public String toString() {
+    return "LastSeenMessages{" +
+            "offset=" + offset +
+            ", acknowledged=" + acknowledged +
+            '}';
+  }
 }
diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/keyed/KeyedCommandHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/keyed/KeyedCommandHandler.java
index 8aa1051a..193371aa 100644
--- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/keyed/KeyedCommandHandler.java
+++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/keyed/KeyedCommandHandler.java
@@ -52,7 +52,7 @@ public class KeyedCommandHandler implements CommandHandler<KeyedPlayerCommand> {
               && playerKey.getKeyRevision().compareTo(IdentifiedKey.Revision.LINKED_V2) >= 0) {
             logger.fatal("A plugin tried to deny a command with signable component(s). "
                 + "This is not supported. "
-                + "Disconnecting player " + player.getUsername());
+                + "Disconnecting player " + player.getUsername() + ". Command packet: " + packet);
             player.disconnect(Component.text(
                 "A proxy plugin caused an illegal protocol state. "
                     + "Contact your network administrator."));
@@ -75,7 +75,7 @@ public class KeyedCommandHandler implements CommandHandler<KeyedPlayerCommand> {
               && playerKey.getKeyRevision().compareTo(IdentifiedKey.Revision.LINKED_V2) >= 0) {
             logger.fatal("A plugin tried to change a command with signed component(s). "
                 + "This is not supported. "
-                + "Disconnecting player " + player.getUsername());
+                + "Disconnecting player " + player.getUsername() + ". Command packet: " + packet);
             player.disconnect(Component.text(
                 "A proxy plugin caused an illegal protocol state. "
                     + "Contact your network administrator."));
@@ -95,7 +95,7 @@ public class KeyedCommandHandler implements CommandHandler<KeyedPlayerCommand> {
               && playerKey.getKeyRevision().compareTo(IdentifiedKey.Revision.LINKED_V2) >= 0) {
             logger.fatal("A plugin tried to change a command with signed component(s). "
                 + "This is not supported. "
-                + "Disconnecting player " + player.getUsername());
+                + "Disconnecting player " + player.getUsername() + ". Command packet: " + packet);
             player.disconnect(Component.text(
                 "A proxy plugin caused an illegal protocol state. "
                     + "Contact your network administrator."));
diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/session/SessionCommandHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/session/SessionCommandHandler.java
index 782c7835..b963ac2e 100644
--- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/session/SessionCommandHandler.java
+++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/session/SessionCommandHandler.java
@@ -47,7 +47,7 @@ public class SessionCommandHandler implements CommandHandler<SessionPlayerComman
         if (packet.isSigned()) {
           logger.fatal("A plugin tried to deny a command with signable component(s). "
               + "This is not supported. "
-              + "Disconnecting player " + player.getUsername());
+              + "Disconnecting player " + player.getUsername() + ". Command packet: " + packet);
           player.disconnect(Component.text(
               "A proxy plugin caused an illegal protocol state. "
                   + "Contact your network administrator."));
@@ -63,7 +63,7 @@ public class SessionCommandHandler implements CommandHandler<SessionPlayerComman
           if (packet.isSigned()) {
             logger.fatal("A plugin tried to change a command with signed component(s). "
                 + "This is not supported. "
-                + "Disconnecting player " + player.getUsername());
+                + "Disconnecting player " + player.getUsername() + ". Command packet: " + packet);
             player.disconnect(Component.text(
                 "A proxy plugin caused an illegal protocol state. "
                     + "Contact your network administrator."));
@@ -87,7 +87,7 @@ public class SessionCommandHandler implements CommandHandler<SessionPlayerComman
             if (packet.isSigned()) {
               logger.fatal("A plugin tried to change a command with signed component(s). "
                   + "This is not supported. "
-                  + "Disconnecting player " + player.getUsername());
+                  + "Disconnecting player " + player.getUsername() + ". Command packet: " + packet);
               player.disconnect(Component.text(
                   "A proxy plugin caused an illegal protocol state. "
                       + "Contact your network administrator."));
diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/session/SessionPlayerCommand.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/session/SessionPlayerCommand.java
index 5954530e..5ef83fa4 100644
--- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/session/SessionPlayerCommand.java
+++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/session/SessionPlayerCommand.java
@@ -65,7 +65,8 @@ public class SessionPlayerCommand implements MinecraftPacket {
   }
 
   public boolean isSigned() {
-    return salt != 0 || !lastSeenMessages.isEmpty() || !argumentSignatures.isEmpty();
+    if (salt == 0) return false;
+    return !lastSeenMessages.isEmpty() || !argumentSignatures.isEmpty();
   }
 
   @Override
@@ -73,6 +74,17 @@ public class SessionPlayerCommand implements MinecraftPacket {
     return handler.handle(this);
   }
 
+  @Override
+  public String toString() {
+    return "SessionPlayerCommand{" +
+            "command='" + command + '\'' +
+            ", timeStamp=" + timeStamp +
+            ", salt=" + salt +
+            ", argumentSignatures=" + argumentSignatures +
+            ", lastSeenMessages=" + lastSeenMessages +
+            '}';
+  }
+
   public static class ArgumentSignatures {
 
     private final List<ArgumentSignature> entries;
@@ -104,6 +116,12 @@ public class SessionPlayerCommand implements MinecraftPacket {
         entry.encode(buf);
       }
     }
+    @Override
+    public String toString() {
+      return "ArgumentSignatures{" +
+              "entries=" + entries +
+              '}';
+    }
   }
 
   public static class ArgumentSignature {
@@ -120,5 +138,12 @@ public class SessionPlayerCommand implements MinecraftPacket {
       ProtocolUtils.writeString(buf, name);
       buf.writeBytes(signature);
     }
+
+    @Override
+    public String toString() {
+      return "ArgumentSignature{" +
+              "name='" + name + '\'' +
+              '}';
+    }
   }
 }