You've already forked Arcturus-Morningstar-Extended
mirror of
https://github.com/duckietm/Arcturus-Morningstar-Extended.git
synced 2026-06-19 15:06:19 +00:00
Merge pull request #211 from simoleo89/fix/catalog-inventory-safety
Harden catalog inventory safety guards
This commit is contained in:
+21
-4
@@ -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<MarketPlaceOffer> getOffers(int minPrice, int maxPrice, String search, int sort) {
|
||||
List<MarketPlaceOffer> 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;
|
||||
}
|
||||
}
|
||||
|
||||
+9
-1
@@ -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"));
|
||||
|
||||
+11
-1
@@ -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"));
|
||||
|
||||
+1
-1
@@ -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);
|
||||
|
||||
+10
-4
@@ -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<HabboItem> toRemove = new TIntObjectHashMap();
|
||||
for (int i = 0; i < Math.abs(amount); i++) {
|
||||
TIntObjectHashMap<HabboItem> 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;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+55
@@ -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");
|
||||
}
|
||||
}
|
||||
+25
@@ -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: "));
|
||||
}
|
||||
}
|
||||
|
||||
+45
@@ -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");
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user