Strip extra Sign data to/from client - #1876

modified clients can send abnormally large data from the client
to the server and it would get stored on the sign as sent.

the client can barely render around 16 characters as-is, but formatting
codes can get it to be more than 16 actual length.

Set a limit of 80 which should give an average of 16 characters 2
sets of legacy formatting codes which should be plenty for all uses.

This does not strip any existing data from the NBT as plugins
may use this for storing data out of the rendered area.

it only impacts data sent to and from the client.

Set -DPaper.maxSignLength=XX to change limit or -1 to disable

Also fix sign edit memory leak

2 files changed, 124 insertions, 0 deletions

diff --git a/Spigot-Server-Patches/0374-Strip-extra-Sign-data-to-from-client.patch b/Spigot-Server-Patches/0374-Strip-extra-Sign-data-to-from-client.patch
new file mode 100644
index 0000000000..22ea1f5134
--- /dev/null
+++ b/Spigot-Server-Patches/0374-Strip-extra-Sign-data-to-from-client.patch
@@ -0,0 +1,77 @@
+From 2b5bbee0eb86a2486367181c384d7040b8c52656 Mon Sep 17 00:00:00 2001
+From: Aikar <[email protected]>
+Date: Wed, 27 Feb 2019 22:18:40 -0500
+Subject: [PATCH] Strip extra Sign data to/from client
+
+modified clients can send abnormally large data from the client
+to the server and it would get stored on the sign as sent.
+
+the client can barely render around 16 characters as-is, but formatting
+codes can get it to be more than 16 actual length.
+
+Set a limit of 80 which should give an average of 16 characters 2
+sets of legacy formatting codes which should be plenty for all uses.
+
+This does not strip any existing data from the NBT as plugins
+may use this for storing data out of the rendered area.
+
+it only impacts data sent to and from the client.
+
+Set -DPaper.maxSignLength=XX to change limit or -1 to disable
+
+diff --git a/src/main/java/net/minecraft/server/PlayerConnection.java b/src/main/java/net/minecraft/server/PlayerConnection.java
+index de62c3b7..cb47e54d 100644
+--- a/src/main/java/net/minecraft/server/PlayerConnection.java
++++ b/src/main/java/net/minecraft/server/PlayerConnection.java
+@@ -2227,6 +2227,11 @@ public class PlayerConnection implements PacketListenerPlayIn, ITickable {
+             String[] lines = new String[4];
+ 
+             for (int i = 0; i < astring.length; ++i) {
++                // Paper start - cap line length - modified clients can send longer data than normal
++                if (astring[i].length() > TileEntitySign.MAX_SIGN_LINE_LENGTH && TileEntitySign.MAX_SIGN_LINE_LENGTH > 0) {
++                    astring[i] = astring[i].substring(0, TileEntitySign.MAX_SIGN_LINE_LENGTH);
++                }
++                // Paper end
+                 lines[i] = SharedConstants.a(astring[i]); //Paper - Replaced with anvil color stripping method to stop exploits that allow colored signs to be created.
+             }
+             SignChangeEvent event = new SignChangeEvent((org.bukkit.craftbukkit.block.CraftBlock) player.getWorld().getBlockAt(x, y, z), this.server.getPlayer(this.player), lines);
+diff --git a/src/main/java/net/minecraft/server/TileEntitySign.java b/src/main/java/net/minecraft/server/TileEntitySign.java
+index 67bd3bcb..786b2984 100644
+--- a/src/main/java/net/minecraft/server/TileEntitySign.java
++++ b/src/main/java/net/minecraft/server/TileEntitySign.java
+@@ -9,16 +9,22 @@ public class TileEntitySign extends TileEntity {
+     public boolean isEditable = true;
+     private EntityHuman h;
+     private final CommandObjectiveExecutor i = new CommandObjectiveExecutor();
++    public static final int MAX_SIGN_LINE_LENGTH = Integer.getInteger("Paper.maxSignLength", 80); // Paper
+ 
+     public TileEntitySign() {}
+ 
++    // Paper start
+     public NBTTagCompound save(NBTTagCompound nbttagcompound) {
++        return save(nbttagcompound, false);
++    }
++    public NBTTagCompound save(NBTTagCompound nbttagcompound, boolean filterLines) {
++        // Paper end
+         super.save(nbttagcompound);
+ 
+         for (int i = 0; i < 4; ++i) {
+             String s = IChatBaseComponent.ChatSerializer.a(this.lines[i]);
+ 
+-            nbttagcompound.setString("Text" + (i + 1), s);
++            nbttagcompound.setString("Text" + (i + 1), filterLines && MAX_SIGN_LINE_LENGTH > 0 && s.length() > MAX_SIGN_LINE_LENGTH ? s.substring(0, MAX_SIGN_LINE_LENGTH): s); // Paper
+         }
+ 
+         // CraftBukkit start
+@@ -105,7 +111,7 @@ public class TileEntitySign extends TileEntity {
+     }
+ 
+     public NBTTagCompound d() {
+-        return this.save(new NBTTagCompound());
++        return this.save(new NBTTagCompound(), true); // Paper - filter lines
+     }
+ 
+     public boolean isFilteredNBT() {
+-- 
+2.20.1
+
diff --git a/Spigot-Server-Patches/0375-Fix-sign-edit-memory-leak.patch b/Spigot-Server-Patches/0375-Fix-sign-edit-memory-leak.patch
new file mode 100644
index 0000000000..dc299737ba
--- /dev/null
+++ b/Spigot-Server-Patches/0375-Fix-sign-edit-memory-leak.patch
@@ -0,0 +1,47 @@
+From 5a1e1396b62a291dc2f6a2b3ee92cce18b8ebd74 Mon Sep 17 00:00:00 2001
+From: Aikar <[email protected]>
+Date: Thu, 28 Feb 2019 00:15:28 -0500
+Subject: [PATCH] Fix sign edit memory leak
+
+when a player edits a sign, a reference to their Entity is never cleand up.
+
+diff --git a/src/main/java/net/minecraft/server/PlayerConnection.java b/src/main/java/net/minecraft/server/PlayerConnection.java
+index cb47e54d..68728d43 100644
+--- a/src/main/java/net/minecraft/server/PlayerConnection.java
++++ b/src/main/java/net/minecraft/server/PlayerConnection.java
+@@ -2211,7 +2211,7 @@ public class PlayerConnection implements PacketListenerPlayIn, ITickable {
+ 
+             TileEntitySign tileentitysign = (TileEntitySign) tileentity;
+ 
+-            if (!tileentitysign.a() || tileentitysign.e() != this.player) {
++            if (!tileentitysign.a() || tileentitysign.signEditor == null || !tileentitysign.signEditor.equals(this.player.getUniqueID())) { // Paper
+                 this.minecraftServer.warning("Player " + this.player.getName() + " just tried to change non-editable sign");
+                 this.sendPacket(tileentity.getUpdatePacket()); // CraftBukkit
+                 return;
+diff --git a/src/main/java/net/minecraft/server/TileEntitySign.java b/src/main/java/net/minecraft/server/TileEntitySign.java
+index 786b2984..e7d9d811 100644
+--- a/src/main/java/net/minecraft/server/TileEntitySign.java
++++ b/src/main/java/net/minecraft/server/TileEntitySign.java
+@@ -10,6 +10,7 @@ public class TileEntitySign extends TileEntity {
+     private EntityHuman h;
+     private final CommandObjectiveExecutor i = new CommandObjectiveExecutor();
+     public static final int MAX_SIGN_LINE_LENGTH = Integer.getInteger("Paper.maxSignLength", 80); // Paper
++    public java.util.UUID signEditor; // Paper
+ 
+     public TileEntitySign() {}
+ 
+@@ -123,7 +124,10 @@ public class TileEntitySign extends TileEntity {
+     }
+ 
+     public void a(EntityHuman entityhuman) {
+-        this.h = entityhuman;
++        // Paper start
++        //this.h = entityhuman;
++        signEditor = entityhuman != null ? entityhuman.getUniqueID() : null;
++        // Paper end
+     }
+ 
+     public EntityHuman e() {
+-- 
+2.20.1
+