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.
This commit is contained in:
simoleo89
2026-06-13 15:15:19 +02:00
parent aaad94f954
commit 4330bf5a62
2 changed files with 42 additions and 22 deletions
@@ -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();
}
}
}
@@ -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");
}
}