From 4330bf5a6292043995b5fedf57492f906302dc49 Mon Sep 17 00:00:00 2001 From: simoleo89 Date: Sat, 13 Jun 2026 15:15:19 +0200 Subject: [PATCH] fix(rcon): always release inbound buffers RCONServerHandler released the inbound ByteBuf only after successfully parsing, writing, flushing, and closing the response. Any exception before the tail release could leak Netty buffers and let malformed RCON traffic consume memory over time. Guard non-ByteBuf messages, release accepted buffers from a finally block, and add a contract test for the release invariant. --- .../rconserver/RCONServerHandler.java | 50 +++++++++++-------- .../RCONServerHandlerContractTest.java | 14 ++++++ 2 files changed, 42 insertions(+), 22 deletions(-) diff --git a/Emulator/src/main/java/com/eu/habbo/networking/rconserver/RCONServerHandler.java b/Emulator/src/main/java/com/eu/habbo/networking/rconserver/RCONServerHandler.java index 26226413..014ba0ec 100644 --- a/Emulator/src/main/java/com/eu/habbo/networking/rconserver/RCONServerHandler.java +++ b/Emulator/src/main/java/com/eu/habbo/networking/rconserver/RCONServerHandler.java @@ -37,29 +37,35 @@ public class RCONServerHandler extends ChannelInboundHandlerAdapter { @Override public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception { - ByteBuf data = (ByteBuf) msg; - - byte[] d = new byte[data.readableBytes()]; - data.getBytes(0, d); - String message = new String(d, java.nio.charset.StandardCharsets.UTF_8); - Gson gson = GSON; - String response = "ERROR"; - String key = ""; - try { - JsonObject object = gson.fromJson(message, JsonObject.class); - key = object.get("key").getAsString(); - response = Emulator.getRconServer().handle(ctx, key, object.get("data").toString()); - } catch (ArrayIndexOutOfBoundsException e) { - LOGGER.error("Unknown RCON Message: {}", key); - } catch (Exception e) { - LOGGER.error("Invalid RCON Message: {}", message); - e.printStackTrace(); + if (!(msg instanceof ByteBuf data)) { + ctx.fireChannelRead(msg); + return; } - ChannelFuture f = ctx.channel().write(Unpooled.copiedBuffer(response.getBytes(java.nio.charset.StandardCharsets.UTF_8)), ctx.channel().voidPromise()); - ctx.channel().flush(); - ctx.flush(); - f.channel().close(); - data.release(); + try { + byte[] d = new byte[data.readableBytes()]; + data.getBytes(0, d); + String message = new String(d, java.nio.charset.StandardCharsets.UTF_8); + Gson gson = GSON; + String response = "ERROR"; + String key = ""; + try { + JsonObject object = gson.fromJson(message, JsonObject.class); + key = object.get("key").getAsString(); + response = Emulator.getRconServer().handle(ctx, key, object.get("data").toString()); + } catch (ArrayIndexOutOfBoundsException e) { + LOGGER.error("Unknown RCON Message: {}", key); + } catch (Exception e) { + LOGGER.error("Invalid RCON Message: {}", message); + e.printStackTrace(); + } + + ChannelFuture f = ctx.channel().write(Unpooled.copiedBuffer(response.getBytes(java.nio.charset.StandardCharsets.UTF_8)), ctx.channel().voidPromise()); + ctx.channel().flush(); + ctx.flush(); + f.channel().close(); + } finally { + data.release(); + } } } diff --git a/Emulator/src/test/java/com/eu/habbo/networking/rconserver/RCONServerHandlerContractTest.java b/Emulator/src/test/java/com/eu/habbo/networking/rconserver/RCONServerHandlerContractTest.java index 7888b0d0..893d556b 100644 --- a/Emulator/src/test/java/com/eu/habbo/networking/rconserver/RCONServerHandlerContractTest.java +++ b/Emulator/src/test/java/com/eu/habbo/networking/rconserver/RCONServerHandlerContractTest.java @@ -12,6 +12,10 @@ class RCONServerHandlerContractTest { return Files.readString(Path.of("src/main/java/com/eu/habbo/networking/rconserver/RCONServer.java")); } + private static String handlerSource() throws Exception { + return Files.readString(Path.of("src/main/java/com/eu/habbo/networking/rconserver/RCONServerHandler.java")); + } + private static String emulatorSource() throws Exception { return Files.readString(Path.of("src/main/java/com/eu/habbo/Emulator.java")); } @@ -45,4 +49,14 @@ class RCONServerHandlerContractTest { "RCON rate limit must reject immediately by default instead of blocking event loops"); assertTrue(registerIndex < serverIndex, "RCON rate limit defaults must be registered before RCONServer is constructed"); } + + @Test + void inboundByteBufIsReleasedFromFinallyBlock() throws Exception { + String source = handlerSource(); + int finallyIndex = source.indexOf("finally"); + int releaseIndex = source.indexOf("data.release()"); + + assertTrue(finallyIndex >= 0, "RCON channelRead must release inbound ByteBufs from a finally block"); + assertTrue(releaseIndex > finallyIndex, "RCON channelRead must release the inbound ByteBuf after finally starts"); + } }