From 81688d28d24fe622d01de253963f37b622e64c5f Mon Sep 17 00:00:00 2001 From: Minecrell Date: Fri, 13 Jul 2018 09:43:56 +0200 Subject: [PATCH] Avoid Netty buffer leak in LegacyPingHandler. Fixes #1200 (#1201) The extra buffer used to decode the strings sent by the client in the legacy ping protocol was never released. However, creating an extra copy of the buffer just to decode it to a string isn't actually necessary: We can just call toString() directly on the original buffer. Additionally, free the buffer in handlerRemoved() to handle cases where the client never sends enough bytes to form a valid legacy ping request. --- ...e-legacy-ping-handler-more-reliable.patch} | 52 ++++++++++++------- ...ServerListPingEvent-for-legacy-pings.patch | 6 +-- 2 files changed, 37 insertions(+), 21 deletions(-) rename Spigot-Server-Patches/{0284-Make-the-legacy-ping-handler-more-reliable.patch => 0284-Make-legacy-ping-handler-more-reliable.patch} (79%) diff --git a/Spigot-Server-Patches/0284-Make-the-legacy-ping-handler-more-reliable.patch b/Spigot-Server-Patches/0284-Make-legacy-ping-handler-more-reliable.patch similarity index 79% rename from Spigot-Server-Patches/0284-Make-the-legacy-ping-handler-more-reliable.patch rename to Spigot-Server-Patches/0284-Make-legacy-ping-handler-more-reliable.patch index 7d4aef268..de76d59c9 100644 --- a/Spigot-Server-Patches/0284-Make-the-legacy-ping-handler-more-reliable.patch +++ b/Spigot-Server-Patches/0284-Make-legacy-ping-handler-more-reliable.patch @@ -1,7 +1,7 @@ -From e24f4a9cce13be40d11613acdfa591bcaa64fdd2 Mon Sep 17 00:00:00 2001 +From 342fd0abbc9127651a9aafabe10a7b2afa2bf98d Mon Sep 17 00:00:00 2001 From: Minecrell Date: Wed, 11 Oct 2017 18:22:50 +0200 -Subject: [PATCH] Make the legacy ping handler more reliable +Subject: [PATCH] Make legacy ping handler more reliable The Minecraft server often fails to respond to old ("legacy") pings from old Minecraft versions using the protocol used before the switch @@ -28,7 +28,7 @@ respond to the request. [2]: https://netty.io/wiki/user-guide-for-4.x.html#wiki-h4-13 diff --git a/src/main/java/net/minecraft/server/LegacyPingHandler.java b/src/main/java/net/minecraft/server/LegacyPingHandler.java -index 4c1a0181a..f084a653a 100644 +index 4c1a0181a..a89a86e6d 100644 --- a/src/main/java/net/minecraft/server/LegacyPingHandler.java +++ b/src/main/java/net/minecraft/server/LegacyPingHandler.java @@ -14,6 +14,7 @@ public class LegacyPingHandler extends ChannelInboundHandlerAdapter { @@ -77,11 +77,22 @@ index 4c1a0181a..f084a653a 100644 } bytebuf.release(); -@@ -92,6 +108,74 @@ public class LegacyPingHandler extends ChannelInboundHandlerAdapter { +@@ -92,6 +108,90 @@ public class LegacyPingHandler extends ChannelInboundHandlerAdapter { } + // Paper start ++ private static String readLegacyString(ByteBuf buf) { ++ int size = buf.readShort() * Character.BYTES; ++ if (!buf.isReadable(size)) { ++ return null; ++ } ++ ++ String result = buf.toString(buf.readerIndex(), size, StandardCharsets.UTF_16BE); ++ buf.skipBytes(size); // toString doesn't increase readerIndex automatically ++ return result; ++ } ++ + private void readLegacy1_6(ChannelHandlerContext ctx, ByteBuf part) { + ByteBuf buf = this.buf; + @@ -94,34 +105,31 @@ index 4c1a0181a..f084a653a 100644 + + buf.writeBytes(part); + -+ // Short + Short + Byte + Short + Int -+ if (!buf.isReadable(2 + 2 + 1 + 2 + 4)) { ++ if (!buf.isReadable(Short.BYTES + Short.BYTES + Byte.BYTES + Short.BYTES + Integer.BYTES)) { + return; + } + -+ short length = buf.readShort(); -+ if (!buf.isReadable(length * 2)) { ++ String s = readLegacyString(buf); ++ if (s == null) { + return; + } + -+ if (!buf.readBytes(length * 2).toString(StandardCharsets.UTF_16BE).equals("MC|PingHost")) { ++ if (!s.equals("MC|PingHost")) { + removeHandler(ctx); + return; + } + -+ if (!buf.isReadable(2)) { -+ return; -+ } -+ -+ length = buf.readShort(); -+ if (!buf.isReadable(length)) { ++ if (!buf.isReadable(Short.BYTES) || !buf.isReadable(buf.readShort())) { + return; + } + + MinecraftServer server = this.b.d(); + int protocolVersion = buf.readByte(); -+ length = buf.readShort(); -+ String host = buf.readBytes(length * 2).toString(StandardCharsets.UTF_16BE); ++ String host = readLegacyString(buf); ++ if (host == null) { ++ removeHandler(ctx); ++ return; ++ } + int port = buf.readInt(); + + if (buf.isReadable()) { @@ -144,9 +152,17 @@ index 4c1a0181a..f084a653a 100644 + this.buf = null; + + buf.resetReaderIndex(); -+ ctx.pipeline().remove("legacy_query"); ++ ctx.pipeline().remove(this); + ctx.fireChannelRead(buf); + } ++ ++ @Override ++ public void handlerRemoved(ChannelHandlerContext ctx) { ++ if (this.buf != null) { ++ this.buf.release(); ++ this.buf = null; ++ } ++ } + // Paper end + private void a(ChannelHandlerContext channelhandlercontext, ByteBuf bytebuf) { diff --git a/Spigot-Server-Patches/0285-Call-PaperServerListPingEvent-for-legacy-pings.patch b/Spigot-Server-Patches/0285-Call-PaperServerListPingEvent-for-legacy-pings.patch index d95b3a260..5eb99a20f 100644 --- a/Spigot-Server-Patches/0285-Call-PaperServerListPingEvent-for-legacy-pings.patch +++ b/Spigot-Server-Patches/0285-Call-PaperServerListPingEvent-for-legacy-pings.patch @@ -1,4 +1,4 @@ -From 93caa0bf416f61fe9f92a45a4631900b78e939dc Mon Sep 17 00:00:00 2001 +From fdff041cd1df94bc3a0173545b6e171c3cdf4579 Mon Sep 17 00:00:00 2001 From: Minecrell Date: Wed, 11 Oct 2017 19:30:51 +0200 Subject: [PATCH] Call PaperServerListPingEvent for legacy pings @@ -84,7 +84,7 @@ index 000000000..74c012fd4 + +} diff --git a/src/main/java/net/minecraft/server/LegacyPingHandler.java b/src/main/java/net/minecraft/server/LegacyPingHandler.java -index f084a653a..39d19e91b 100644 +index a89a86e6d..2762bcc2e 100644 --- a/src/main/java/net/minecraft/server/LegacyPingHandler.java +++ b/src/main/java/net/minecraft/server/LegacyPingHandler.java @@ -9,6 +9,7 @@ import java.net.InetSocketAddress; @@ -133,7 +133,7 @@ index f084a653a..39d19e91b 100644 this.a(channelhandlercontext, this.a(s)); break; -@@ -161,8 +178,16 @@ public class LegacyPingHandler extends ChannelInboundHandlerAdapter { +@@ -169,8 +186,16 @@ public class LegacyPingHandler extends ChannelInboundHandlerAdapter { a.debug("Ping: (1.6) from {}", ctx.channel().remoteAddress());