From 7cf83cf15863526d1031b0867ee4ac1a4969476a Mon Sep 17 00:00:00 2001 From: simoleo89 Date: Wed, 17 Jun 2026 21:26:04 +0200 Subject: [PATCH] fix(trade): verify ownership transfers --- .../eu/habbo/habbohotel/rooms/RoomTrade.java | 25 +++++++++++++++-- .../rooms/RoomTradeSafetyContractTest.java | 27 +++++++++++++++++-- 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/Emulator/src/main/java/com/eu/habbo/habbohotel/rooms/RoomTrade.java b/Emulator/src/main/java/com/eu/habbo/habbohotel/rooms/RoomTrade.java index 13a4deba..36647e21 100644 --- a/Emulator/src/main/java/com/eu/habbo/habbohotel/rooms/RoomTrade.java +++ b/Emulator/src/main/java/com/eu/habbo/habbohotel/rooms/RoomTrade.java @@ -204,11 +204,12 @@ public class RoomTrade { int userOneId = userOne.getHabbo().getHabboInfo().getId(); int userTwoId = userTwo.getHabbo().getHabboInfo().getId(); - try (PreparedStatement statement = connection.prepareStatement("UPDATE items SET user_id = ? WHERE id = ? LIMIT 1")) { + try (PreparedStatement statement = connection.prepareStatement("UPDATE items SET user_id = ? WHERE id = ? AND user_id = ? LIMIT 1")) { try (PreparedStatement stmt = connection.prepareStatement("INSERT INTO room_trade_log_items (id, item_id, user_id) VALUES (?, ?, ?)")) { for (HabboItem item : userOne.getItems()) { statement.setInt(1, userTwoId); statement.setInt(2, item.getId()); + statement.setInt(3, userOneId); statement.addBatch(); if (logTrades) { @@ -222,6 +223,7 @@ public class RoomTrade { for (HabboItem item : userTwo.getItems()) { statement.setInt(1, userOneId); statement.setInt(2, item.getId()); + statement.setInt(3, userTwoId); statement.addBatch(); if (logTrades) { @@ -237,7 +239,12 @@ public class RoomTrade { } } - statement.executeBatch(); + int expectedUpdates = userOne.getItems().size() + userTwo.getItems().size(); + int[] updateCounts = statement.executeBatch(); + if (!RoomTrade.allOwnershipUpdatesSucceeded(updateCounts, expectedUpdates)) { + this.sendMessageToUsers(new TradeClosedComposer(userOne.getHabbo().getRoomUnit().getId(), TradeClosedComposer.ITEMS_NOT_FOUND)); + return false; + } } } catch (SQLException e) { LOGGER.error("Caught SQL exception", e); @@ -364,6 +371,20 @@ public class RoomTrade { return this.users; } + static boolean allOwnershipUpdatesSucceeded(int[] updateCounts, int expectedUpdates) { + if (updateCounts == null || updateCounts.length != expectedUpdates) { + return false; + } + + for (int updateCount : updateCounts) { + if (updateCount == Statement.EXECUTE_FAILED || updateCount == 0) { + return false; + } + } + + return true; + } + public static int getCreditsByItem(HabboItem item) { if (!Emulator.getConfig().getBoolean("redeem.currency.trade")) return 0; diff --git a/Emulator/src/test/java/com/eu/habbo/habbohotel/rooms/RoomTradeSafetyContractTest.java b/Emulator/src/test/java/com/eu/habbo/habbohotel/rooms/RoomTradeSafetyContractTest.java index d1614e84..65152393 100644 --- a/Emulator/src/test/java/com/eu/habbo/habbohotel/rooms/RoomTradeSafetyContractTest.java +++ b/Emulator/src/test/java/com/eu/habbo/habbohotel/rooms/RoomTradeSafetyContractTest.java @@ -2,6 +2,7 @@ package com.eu.habbo.habbohotel.rooms; import org.junit.jupiter.api.Test; +import java.sql.Statement; import java.nio.file.Files; import java.nio.file.Path; @@ -28,11 +29,33 @@ class RoomTradeSafetyContractTest { void itemOwnersChangeOnlyAfterDatabaseBatchSucceeds() throws Exception { String source = roomTradeSource(); int firstOwnerMutation = source.indexOf("item.setUserId("); - int batchExecution = source.indexOf("statement.executeBatch();"); + int batchExecution = source.indexOf("int[] updateCounts = statement.executeBatch();"); + int batchGuard = source.indexOf("allOwnershipUpdatesSucceeded(updateCounts, expectedUpdates)", batchExecution); assertTrue(firstOwnerMutation > -1, "RoomTrade should update in-memory item owners after commit"); assertTrue(batchExecution > -1, "RoomTrade should persist item owner changes with a batch update"); - assertTrue(firstOwnerMutation > batchExecution, + assertTrue(batchGuard > batchExecution, "RoomTrade must validate every ownership update before mutating memory"); + assertTrue(firstOwnerMutation > batchGuard, "In-memory item owners must not change until the database batch has succeeded"); } + + @Test + void ownershipUpdatesRequireExpectedDatabaseOwner() throws Exception { + String source = roomTradeSource(); + + assertTrue(source.contains("UPDATE items SET user_id = ? WHERE id = ? AND user_id = ? LIMIT 1"), + "RoomTrade ownership transfer should only update items still owned by the offering user"); + assertTrue(source.contains("statement.setInt(3, userOneId)"), + "User one offered items must require user one as the current database owner"); + assertTrue(source.contains("statement.setInt(3, userTwoId)"), + "User two offered items must require user two as the current database owner"); + } + + @Test + void zeroBatchUpdatesAbortTheTrade() { + assertTrue(RoomTrade.allOwnershipUpdatesSucceeded(new int[]{1, Statement.SUCCESS_NO_INFO}, 2)); + assertTrue(!RoomTrade.allOwnershipUpdatesSucceeded(new int[]{1, 0}, 2)); + assertTrue(!RoomTrade.allOwnershipUpdatesSucceeded(new int[]{1}, 2)); + assertTrue(!RoomTrade.allOwnershipUpdatesSucceeded(new int[]{Statement.EXECUTE_FAILED}, 1)); + } }