From 484f1a8c054b181539a122bb8be5d872db5f1105 Mon Sep 17 00:00:00 2001 From: laforetbrut Date: Thu, 26 Mar 2026 18:33:00 +0100 Subject: [PATCH] Final audit: fix ghost-online, SQL injection, resource leak, NPE CRITICAL-1/2: Remove duplicate online=1 writes from doPlayerJoin. The synchronous onPlayerLoggedInKickCheck already sets online=1. The background thread writes raced with logout's online=0, permanently locking players as "online" after crash-disconnect during join. HIGH-1: Startup SQL uses PreparedStatement for server_id (was string concat). HIGH-2: update() method now uses try-with-resources for PreparedStatement. HIGH-3: NPE guard in RS2 data file logging when getRS2DataFile returns null. Vyrriox --- src/main/java/vip/fubuki/playersync/PlayerSync.java | 2 +- .../java/vip/fubuki/playersync/sync/VanillaSync.java | 10 +++++----- .../vip/fubuki/playersync/sync/addons/ModsSupport.java | 2 +- .../java/vip/fubuki/playersync/util/JDBCsetUp.java | 4 ++-- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/main/java/vip/fubuki/playersync/PlayerSync.java b/src/main/java/vip/fubuki/playersync/PlayerSync.java index 5727974..c91c019 100644 --- a/src/main/java/vip/fubuki/playersync/PlayerSync.java +++ b/src/main/java/vip/fubuki/playersync/PlayerSync.java @@ -204,7 +204,7 @@ public class PlayerSync { ); try { - JDBCsetUp.executeUpdate("UPDATE player_data SET online=0 WHERE last_server=" + JdbcConfig.SERVER_ID.get() +" AND online=1 LIMIT 1000"); + JDBCsetUp.executePreparedUpdate("UPDATE player_data SET online=0 WHERE last_server=? AND online=1 LIMIT 1000", JdbcConfig.SERVER_ID.get()); } catch (Exception e) { LOGGER.error("An exception occurred while trying change wrong player-status\n" + e.getMessage()); } diff --git a/src/main/java/vip/fubuki/playersync/sync/VanillaSync.java b/src/main/java/vip/fubuki/playersync/sync/VanillaSync.java index 6fad305..551796b 100644 --- a/src/main/java/vip/fubuki/playersync/sync/VanillaSync.java +++ b/src/main/java/vip/fubuki/playersync/sync/VanillaSync.java @@ -269,12 +269,13 @@ public class VanillaSync { } if (!playerExists) { + // FIX CRITICAL-1/2: online=1 is already set by onPlayerLoggedInKickCheck (synchronous). + // Do NOT write online=1 again from background/queued threads - if the player disconnects + // quickly, the background write races with logout's online=0 and permanently locks the player. server.execute(() -> { try { new ModsSupport().doCuriosRestore(serverPlayer); - store(serverPlayer, true); - JDBCsetUp.executePreparedUpdate("UPDATE server_info SET last_update=? WHERE id=?", System.currentTimeMillis(), JdbcConfig.SERVER_ID.get()); - JDBCsetUp.executePreparedUpdate("UPDATE player_data SET online=1, last_server=? WHERE uuid=?", JdbcConfig.SERVER_ID.get(), player_uuid); + store(serverPlayer, true); // INSERT with online=1 handled by store() init path serverPlayer.addTag("player_synced"); } catch (Exception e) { PlayerSync.LOGGER.error("Error initializing new player {}", player_uuid, e); @@ -285,8 +286,7 @@ public class VanillaSync { return; } - JDBCsetUp.executePreparedUpdate("UPDATE server_info SET last_update=? WHERE id=?", System.currentTimeMillis(), JdbcConfig.SERVER_ID.get()); - JDBCsetUp.executePreparedUpdate("UPDATE player_data SET online=1, last_server=? WHERE uuid=?", JdbcConfig.SERVER_ID.get(), player_uuid); + // online=1 already set by onPlayerLoggedInKickCheck - no duplicate write here // Read all DB data into local variables (background thread - safe) final int health, foodLevel, xp, score; diff --git a/src/main/java/vip/fubuki/playersync/sync/addons/ModsSupport.java b/src/main/java/vip/fubuki/playersync/sync/addons/ModsSupport.java index 82f3bf1..49e5193 100644 --- a/src/main/java/vip/fubuki/playersync/sync/addons/ModsSupport.java +++ b/src/main/java/vip/fubuki/playersync/sync/addons/ModsSupport.java @@ -462,7 +462,7 @@ public class ModsSupport { // Read the .dat file directly (getDataFile is private, use reflection) java.io.File datFile = getRS2DataFile(sp); if (datFile == null || !datFile.exists()) { - PlayerSync.LOGGER.warn("RS2 storage data file not found: {}", datFile.getAbsolutePath()); + PlayerSync.LOGGER.warn("RS2 storage data file not found: {}", datFile != null ? datFile.getAbsolutePath() : ""); return; } diff --git a/src/main/java/vip/fubuki/playersync/util/JDBCsetUp.java b/src/main/java/vip/fubuki/playersync/util/JDBCsetUp.java index ec17b64..26d6bc6 100644 --- a/src/main/java/vip/fubuki/playersync/util/JDBCsetUp.java +++ b/src/main/java/vip/fubuki/playersync/util/JDBCsetUp.java @@ -90,8 +90,8 @@ public class JDBCsetUp { */ public static void update(String sql, String... argument) throws SQLException { LOGGER.trace(sql); - try (Connection connection = getConnection()) { // With database selected - PreparedStatement updateStatement = connection.prepareStatement(sql); + try (Connection connection = getConnection(); + PreparedStatement updateStatement = connection.prepareStatement(sql)) { for (int i = 0; i < argument.length; i++) { updateStatement.setString(i + 1, argument[i]); }