From 8c7d6db1354ae81c2f7fe8f28f064b5d6451d067 Mon Sep 17 00:00:00 2001 From: simoleo89 Date: Mon, 15 Jun 2026 22:01:38 +0200 Subject: [PATCH] fix(catalog): harden marketplace and inventory mutations --- .../catalog/marketplace/MarketPlace.java | 25 +++++++-- .../CatalogAdminDeleteOfferEvent.java | 10 +++- .../CatalogAdminMoveOfferEvent.java | 12 +++- .../catalog/marketplace/SellItemEvent.java | 2 +- .../RequestInventoryItemsDelete.java | 14 +++-- .../MarketPlaceListingPriceContractTest.java | 55 +++++++++++++++++++ ...CatalogAdminOfferMutationContractTest.java | 25 +++++++++ ...questInventoryItemsDeleteContractTest.java | 45 +++++++++++++++ 8 files changed, 177 insertions(+), 11 deletions(-) create mode 100644 Emulator/src/test/java/com/eu/habbo/habbohotel/catalog/marketplace/MarketPlaceListingPriceContractTest.java create mode 100644 Emulator/src/test/java/com/eu/habbo/messages/incoming/inventory/RequestInventoryItemsDeleteContractTest.java diff --git a/Emulator/src/main/java/com/eu/habbo/habbohotel/catalog/marketplace/MarketPlace.java b/Emulator/src/main/java/com/eu/habbo/habbohotel/catalog/marketplace/MarketPlace.java index b0646439..b9d0b7ab 100644 --- a/Emulator/src/main/java/com/eu/habbo/habbohotel/catalog/marketplace/MarketPlace.java +++ b/Emulator/src/main/java/com/eu/habbo/habbohotel/catalog/marketplace/MarketPlace.java @@ -28,6 +28,8 @@ import java.util.List; public class MarketPlace { private static final Logger LOGGER = LoggerFactory.getLogger(MarketPlace.class); + public static final int MINIMUM_LISTING_PRICE = 1; + public static final int MAXIMUM_LISTING_PRICE = 1_000_000_000; //Configuration. Loaded from database & updated accordingly. public static boolean MARKETPLACE_ENABLED = true; @@ -119,7 +121,7 @@ public class MarketPlace { public static List getOffers(int minPrice, int maxPrice, String search, int sort) { List offers = new ArrayList<>(10); - String query = "SELECT B.* FROM marketplace_items a INNER JOIN (SELECT b.item_id AS base_item_id, b.limited_data AS ltd_data, marketplace_items.*, AVG(price) as avg, MIN(marketplace_items.price) as minPrice, MAX(marketplace_items.price) as maxPrice, COUNT(*) as number, (SELECT COUNT(*) FROM marketplace_items c INNER JOIN items as items_b ON c.item_id = items_b.id WHERE state = 2 AND items_b.item_id = base_item_id AND DATE(from_unixtime(sold_timestamp)) = CURDATE()) as sold_count_today FROM marketplace_items INNER JOIN items b ON marketplace_items.item_id = b.id INNER JOIN items_base bi ON b.item_id = bi.id INNER JOIN catalog_items ci ON bi.id = ci.item_ids WHERE price = (SELECT MIN(e.price) FROM marketplace_items e, items d WHERE e.item_id = d.id AND d.item_id = b.item_id AND e.state = 1 AND e.timestamp > ? GROUP BY d.item_id) AND state = 1 AND timestamp > ?"; + String query = "SELECT B.* FROM marketplace_items a INNER JOIN (SELECT b.item_id AS base_item_id, b.limited_data AS ltd_data, marketplace_items.*, AVG(price) as avg, MIN(marketplace_items.price) as minPrice, MAX(marketplace_items.price) as maxPrice, COUNT(*) as number, (SELECT COUNT(*) FROM marketplace_items c INNER JOIN items as items_b ON c.item_id = items_b.id WHERE state = 2 AND items_b.item_id = base_item_id AND DATE(from_unixtime(sold_timestamp)) = CURDATE()) as sold_count_today FROM marketplace_items INNER JOIN items b ON marketplace_items.item_id = b.id INNER JOIN items_base bi ON b.item_id = bi.id INNER JOIN catalog_items ci ON bi.id = ci.item_ids WHERE price = (SELECT MIN(e.price) FROM marketplace_items e, items d WHERE e.item_id = d.id AND d.item_id = b.item_id AND e.state = 1 AND e.timestamp > ? AND e.price BETWEEN ? AND ? GROUP BY d.item_id) AND state = 1 AND timestamp > ? AND marketplace_items.price BETWEEN ? AND ?"; if (minPrice > 0) { query += " AND CEIL(price + (price / 100)) >= ?"; } @@ -163,7 +165,11 @@ public class MarketPlace { try (Connection connection = Emulator.getDatabase().getDataSource().getConnection(); PreparedStatement statement = connection.prepareStatement(query)) { int paramIndex = 1; statement.setInt(paramIndex++, Emulator.getIntUnixTimestamp() - 172800); + statement.setInt(paramIndex++, MINIMUM_LISTING_PRICE); + statement.setInt(paramIndex++, MAXIMUM_LISTING_PRICE); statement.setInt(paramIndex++, Emulator.getIntUnixTimestamp() - 172800); + statement.setInt(paramIndex++, MINIMUM_LISTING_PRICE); + statement.setInt(paramIndex++, MAXIMUM_LISTING_PRICE); if (minPrice > 0) { statement.setInt(paramIndex++, minPrice); } @@ -265,7 +271,13 @@ public class MarketPlace { itemSet.first(); if (itemSet.getRow() > 0) { - int price = MarketPlace.calculateCommision(set.getInt("price")); + int rawPrice = set.getInt("price"); + if (!isValidListingPrice(rawPrice)) { + sendErrorMessage(client, set.getInt("item_id"), offerId); + return; + } + + int price = MarketPlace.calculateCommision(rawPrice); if (set.getInt("state") != 1) { sendErrorMessage(client, set.getInt("item_id"), offerId); } else if ((MARKETPLACE_CURRENCY == 0 && price > client.getHabbo().getHabboInfo().getCredits()) || (MARKETPLACE_CURRENCY > 0 && price > client.getHabbo().getHabboInfo().getCurrencyAmount(MARKETPLACE_CURRENCY))) { @@ -358,7 +370,7 @@ public class MarketPlace { if (item == null || client == null) return false; - if (!item.getBaseItem().allowMarketplace() || price < 0) + if (!item.getBaseItem().allowMarketplace() || !isValidListingPrice(price)) return false; MarketPlaceItemOfferedEvent event = new MarketPlaceItemOfferedEvent(client.getHabbo(), item, price); @@ -430,6 +442,11 @@ public class MarketPlace { public static int calculateCommision(int price) { - return price + (int) Math.ceil(price / 100.0); + long commission = price + (long) Math.ceil(price / 100.0); + return commission > Integer.MAX_VALUE ? Integer.MAX_VALUE : (int) commission; + } + + public static boolean isValidListingPrice(int price) { + return price >= MINIMUM_LISTING_PRICE && price <= MAXIMUM_LISTING_PRICE; } } diff --git a/Emulator/src/main/java/com/eu/habbo/messages/incoming/catalog/catalogadmin/CatalogAdminDeleteOfferEvent.java b/Emulator/src/main/java/com/eu/habbo/messages/incoming/catalog/catalogadmin/CatalogAdminDeleteOfferEvent.java index 1363af8c..64c972e6 100644 --- a/Emulator/src/main/java/com/eu/habbo/messages/incoming/catalog/catalogadmin/CatalogAdminDeleteOfferEvent.java +++ b/Emulator/src/main/java/com/eu/habbo/messages/incoming/catalog/catalogadmin/CatalogAdminDeleteOfferEvent.java @@ -21,10 +21,18 @@ public class CatalogAdminDeleteOfferEvent extends MessageHandler { int offerId = this.packet.readInt(); CatalogPageType pageType = CatalogPageType.fromString(this.packet.readString()); + if (offerId <= 0) { + this.client.sendResponse(new CatalogAdminResultComposer(false, "Invalid offer id")); + return; + } + try (Connection connection = Emulator.getDatabase().getDataSource().getConnection(); PreparedStatement statement = connection.prepareStatement((pageType == CatalogPageType.BUILDER) ? "DELETE FROM catalog_items_bc WHERE id = ?" : "DELETE FROM catalog_items WHERE id = ?")) { statement.setInt(1, offerId); - statement.execute(); + if (statement.executeUpdate() == 0) { + this.client.sendResponse(new CatalogAdminResultComposer(false, "Offer not found: " + offerId)); + return; + } } this.client.sendResponse(new CatalogAdminResultComposer(true, "Offer deleted")); diff --git a/Emulator/src/main/java/com/eu/habbo/messages/incoming/catalog/catalogadmin/CatalogAdminMoveOfferEvent.java b/Emulator/src/main/java/com/eu/habbo/messages/incoming/catalog/catalogadmin/CatalogAdminMoveOfferEvent.java index f6be0130..3c4a71e6 100644 --- a/Emulator/src/main/java/com/eu/habbo/messages/incoming/catalog/catalogadmin/CatalogAdminMoveOfferEvent.java +++ b/Emulator/src/main/java/com/eu/habbo/messages/incoming/catalog/catalogadmin/CatalogAdminMoveOfferEvent.java @@ -22,11 +22,21 @@ public class CatalogAdminMoveOfferEvent extends MessageHandler { int orderNumber = this.packet.readInt(); CatalogPageType pageType = CatalogPageType.fromString(this.packet.readString()); + if (offerId <= 0) { + this.client.sendResponse(new CatalogAdminResultComposer(false, "Invalid offer id")); + return; + } + + if (orderNumber < 0) orderNumber = 0; + try (Connection connection = Emulator.getDatabase().getDataSource().getConnection(); PreparedStatement statement = connection.prepareStatement((pageType == CatalogPageType.BUILDER) ? "UPDATE catalog_items_bc SET order_number = ? WHERE id = ?" : "UPDATE catalog_items SET order_number = ? WHERE id = ?")) { statement.setInt(1, orderNumber); statement.setInt(2, offerId); - statement.execute(); + if (statement.executeUpdate() == 0) { + this.client.sendResponse(new CatalogAdminResultComposer(false, "Offer not found: " + offerId)); + return; + } } this.client.sendResponse(new CatalogAdminResultComposer(true, "Offer reordered")); diff --git a/Emulator/src/main/java/com/eu/habbo/messages/incoming/catalog/marketplace/SellItemEvent.java b/Emulator/src/main/java/com/eu/habbo/messages/incoming/catalog/marketplace/SellItemEvent.java index 37eb6325..65ce7d6f 100644 --- a/Emulator/src/main/java/com/eu/habbo/messages/incoming/catalog/marketplace/SellItemEvent.java +++ b/Emulator/src/main/java/com/eu/habbo/messages/incoming/catalog/marketplace/SellItemEvent.java @@ -41,7 +41,7 @@ public class SellItemEvent extends MessageHandler { return; } - if (credits < 0) { + if (!MarketPlace.isValidListingPrice(credits)) { String message = Emulator.getTexts().getValue("scripter.warning.marketplace.negative").replace("%username%", this.client.getHabbo().getHabboInfo().getUsername()).replace("%itemname%", item.getBaseItem().getName()).replace("%credits%", credits + ""); ScripterManager.scripterDetected(this.client, message); LOGGER.info(message); diff --git a/Emulator/src/main/java/com/eu/habbo/messages/incoming/inventory/RequestInventoryItemsDelete.java b/Emulator/src/main/java/com/eu/habbo/messages/incoming/inventory/RequestInventoryItemsDelete.java index 6306121c..98d54324 100644 --- a/Emulator/src/main/java/com/eu/habbo/messages/incoming/inventory/RequestInventoryItemsDelete.java +++ b/Emulator/src/main/java/com/eu/habbo/messages/incoming/inventory/RequestInventoryItemsDelete.java @@ -12,6 +12,8 @@ import gnu.trove.iterator.hash.TObjectHashIterator; import gnu.trove.map.hash.TIntObjectHashMap; public class RequestInventoryItemsDelete extends MessageHandler { + private static final int MAX_DELETE_AMOUNT = 1000; + public int getRatelimit() { return 500; } @@ -19,19 +21,23 @@ public class RequestInventoryItemsDelete extends MessageHandler { public void handle() { int itemId = this.packet.readInt(); int amount = this.packet.readInt(); + + if (amount <= 0 || amount > MAX_DELETE_AMOUNT) + return; + HabboItem habboItem = this.client.getHabbo().getInventory().getItemsComponent().getHabboItem(itemId); if (habboItem == null) return; Item item = habboItem.getBaseItem(); if (item == null) return; - if (!hasFurnitureInInventory(this.client.getHabbo(), item, Math.abs(amount))) + if (!hasFurnitureInInventory(this.client.getHabbo(), item, amount)) return; final Habbo habbo = this.client.getHabbo(); if (habbo == null) return; - TIntObjectHashMap toRemove = new TIntObjectHashMap(); - for (int i = 0; i < Math.abs(amount); i++) { + TIntObjectHashMap toRemove = new TIntObjectHashMap<>(); + for (int i = 0; i < amount; i++) { HabboItem habboInventoryItem = habbo.getInventory().getItemsComponent().getAndRemoveHabboItem(item); if (habboInventoryItem != null) toRemove.put(habboInventoryItem.getId(), habboInventoryItem); @@ -53,4 +59,4 @@ public class RequestInventoryItemsDelete extends MessageHandler { } return count >= amount; } -} \ No newline at end of file +} diff --git a/Emulator/src/test/java/com/eu/habbo/habbohotel/catalog/marketplace/MarketPlaceListingPriceContractTest.java b/Emulator/src/test/java/com/eu/habbo/habbohotel/catalog/marketplace/MarketPlaceListingPriceContractTest.java new file mode 100644 index 00000000..669e8476 --- /dev/null +++ b/Emulator/src/test/java/com/eu/habbo/habbohotel/catalog/marketplace/MarketPlaceListingPriceContractTest.java @@ -0,0 +1,55 @@ +package com.eu.habbo.habbohotel.catalog.marketplace; + +import org.junit.jupiter.api.Test; + +import java.nio.file.Files; +import java.nio.file.Path; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class MarketPlaceListingPriceContractTest { + + @Test + void listingPriceMustBePositiveAndBounded() { + assertFalse(MarketPlace.isValidListingPrice(Integer.MIN_VALUE)); + assertFalse(MarketPlace.isValidListingPrice(-1)); + assertFalse(MarketPlace.isValidListingPrice(0)); + assertTrue(MarketPlace.isValidListingPrice(1)); + assertTrue(MarketPlace.isValidListingPrice(MarketPlace.MAXIMUM_LISTING_PRICE)); + assertFalse(MarketPlace.isValidListingPrice(MarketPlace.MAXIMUM_LISTING_PRICE + 1)); + } + + @Test + void commissionCalculationDoesNotOverflow() { + assertEquals(101, MarketPlace.calculateCommision(100)); + assertEquals(Integer.MAX_VALUE, MarketPlace.calculateCommision(Integer.MAX_VALUE)); + } + + @Test + void marketplaceCoreRejectsInvalidListingPrices() throws Exception { + String source = Files.readString(Path.of("src/main/java/com/eu/habbo/habbohotel/catalog/marketplace/MarketPlace.java")); + int sellItemStart = source.indexOf("public static boolean sellItem"); + int eventCreation = source.indexOf("new MarketPlaceItemOfferedEvent", sellItemStart); + int validation = source.indexOf("!isValidListingPrice(price)", sellItemStart); + + assertTrue(sellItemStart > -1, "MarketPlace.sellItem must exist"); + assertTrue(validation > sellItemStart, "Marketplace listings must validate price before persisting"); + assertTrue(validation < eventCreation, "Invalid prices must be rejected before plugin events and DB writes"); + } + + @Test + void marketplaceBuyIgnoresInvalidPersistedPrices() throws Exception { + String source = Files.readString(Path.of("src/main/java/com/eu/habbo/habbohotel/catalog/marketplace/MarketPlace.java")); + int buyItemStart = source.indexOf("public static void buyItem"); + int rawPrice = source.indexOf("int rawPrice = set.getInt(\"price\")", buyItemStart); + int validation = source.indexOf("!isValidListingPrice(rawPrice)", rawPrice); + int commission = source.indexOf("MarketPlace.calculateCommision(rawPrice)", rawPrice); + + assertTrue(buyItemStart > -1, "MarketPlace.buyItem must exist"); + assertTrue(rawPrice > buyItemStart, "Marketplace buy path should read the persisted raw price"); + assertTrue(validation > rawPrice, "Persisted marketplace prices must be validated before charging buyers"); + assertTrue(validation < commission, "Invalid persisted prices must not reach commission calculation"); + } +} diff --git a/Emulator/src/test/java/com/eu/habbo/messages/incoming/catalog/catalogadmin/CatalogAdminOfferMutationContractTest.java b/Emulator/src/test/java/com/eu/habbo/messages/incoming/catalog/catalogadmin/CatalogAdminOfferMutationContractTest.java index 901922ca..933cc051 100644 --- a/Emulator/src/test/java/com/eu/habbo/messages/incoming/catalog/catalogadmin/CatalogAdminOfferMutationContractTest.java +++ b/Emulator/src/test/java/com/eu/habbo/messages/incoming/catalog/catalogadmin/CatalogAdminOfferMutationContractTest.java @@ -13,6 +13,10 @@ class CatalogAdminOfferMutationContractTest { "src/main/java/com/eu/habbo/messages/incoming/catalog/catalogadmin/CatalogAdminCreateOfferEvent.java"); private static final Path SAVE_SOURCE = Path.of( "src/main/java/com/eu/habbo/messages/incoming/catalog/catalogadmin/CatalogAdminSaveOfferEvent.java"); + private static final Path DELETE_SOURCE = Path.of( + "src/main/java/com/eu/habbo/messages/incoming/catalog/catalogadmin/CatalogAdminDeleteOfferEvent.java"); + private static final Path MOVE_SOURCE = Path.of( + "src/main/java/com/eu/habbo/messages/incoming/catalog/catalogadmin/CatalogAdminMoveOfferEvent.java"); @Test void createAndSaveValidatePayloadAndTargetPageBeforeWriting() throws IOException { @@ -40,4 +44,25 @@ class CatalogAdminOfferMutationContractTest { assertTrue(save.contains("statement.executeUpdate() == 0")); assertTrue(save.contains("Offer not found: ")); } + + @Test + void deleteOfferRejectsInvalidIdsAndReportsMissingRows() throws IOException { + String delete = Files.readString(DELETE_SOURCE); + + assertTrue(delete.contains("offerId <= 0")); + assertTrue(delete.contains("Invalid offer id")); + assertTrue(delete.contains("statement.executeUpdate() == 0")); + assertTrue(delete.contains("Offer not found: ")); + } + + @Test + void moveOfferRejectsInvalidIdsClampsOrderAndReportsMissingRows() throws IOException { + String move = Files.readString(MOVE_SOURCE); + + assertTrue(move.contains("offerId <= 0")); + assertTrue(move.contains("Invalid offer id")); + assertTrue(move.contains("if (orderNumber < 0) orderNumber = 0;")); + assertTrue(move.contains("statement.executeUpdate() == 0")); + assertTrue(move.contains("Offer not found: ")); + } } diff --git a/Emulator/src/test/java/com/eu/habbo/messages/incoming/inventory/RequestInventoryItemsDeleteContractTest.java b/Emulator/src/test/java/com/eu/habbo/messages/incoming/inventory/RequestInventoryItemsDeleteContractTest.java new file mode 100644 index 00000000..fdca3fac --- /dev/null +++ b/Emulator/src/test/java/com/eu/habbo/messages/incoming/inventory/RequestInventoryItemsDeleteContractTest.java @@ -0,0 +1,45 @@ +package com.eu.habbo.messages.incoming.inventory; + +import org.junit.jupiter.api.Test; + +import java.nio.file.Files; +import java.nio.file.Path; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class RequestInventoryItemsDeleteContractTest { + + private static String source() throws Exception { + return Files.readString(Path.of("src/main/java/com/eu/habbo/messages/incoming/inventory/RequestInventoryItemsDelete.java")); + } + + @Test + void rejectsInvalidDeleteAmountsBeforeMutatingInventory() throws Exception { + String source = source(); + int amountRead = source.indexOf("int amount = this.packet.readInt()"); + int amountGuard = source.indexOf("amount <= 0 || amount > MAX_DELETE_AMOUNT", amountRead); + int firstInventoryLookup = source.indexOf("getItemsComponent().getHabboItem", amountRead); + + assertTrue(amountRead > -1, "Delete handler must read the client-provided amount"); + assertTrue(amountGuard > amountRead, "Delete handler must reject non-positive and oversized amounts"); + assertTrue(amountGuard < firstInventoryLookup, + "Amount validation must happen before inventory lookup or mutation work starts"); + } + + @Test + void doesNotUseAbsoluteValueForClientProvidedAmount() throws Exception { + String source = source(); + + assertFalse(source.contains("Math.abs(amount)"), + "Delete amount must not use Math.abs because Integer.MIN_VALUE remains negative"); + } + + @Test + void deleteLoopUsesValidatedAmountDirectly() throws Exception { + String source = source(); + + assertTrue(source.contains("for (int i = 0; i < amount; i++)"), + "Delete loop should use the already validated positive amount"); + } +}