From 01c17c051198dece426f16680c89e95363e84821 Mon Sep 17 00:00:00 2001 From: simoleo89 Date: Tue, 9 Jun 2026 11:11:56 +0000 Subject: [PATCH] fix: wired double-fire guard, RoomUnit path race, roomItems iteration, Netty CVE MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Continuation of the concurrency hardening from the audit: - InteractionWired/WiredHandler (E4): add an atomic per-box processing guard so one trigger box is handled by a single thread at a time, making the cooldown check-and-set effectively atomic; mark `cooldown` volatile. Prevents a packet thread and the room cycle thread from double-firing the same wired stack (double teleport/reward). - RoomUnit (C1): the walk path is now a volatile ConcurrentLinkedDeque instead of a plain LinkedList, so the room cycle popping steps can't corrupt it while a walk packet rebuilds it via findPath/setPath. - RoomItemManager (C2): iterate roomItems under its own monitor in getFloorItems/ getWallItems/getPostItNotes/getUserUniqueFurniCount/getItemsAt, matching the existing put/remove sync sites — stops place/pickup from corrupting the traversal into a silently-incomplete item set. - pom.xml (S4): bump netty-all 4.1.115 -> 4.1.118.Final (CVE-2025-24970 SslHandler pre-auth DoS, CVE-2025-25193). --- Emulator/pom.xml | 2 +- .../items/interactions/InteractionWired.java | 16 +- .../habbohotel/rooms/RoomItemManager.java | 138 ++++++++++-------- .../eu/habbo/habbohotel/rooms/RoomUnit.java | 10 +- .../habbo/habbohotel/wired/WiredHandler.java | 12 ++ 5 files changed, 112 insertions(+), 66 deletions(-) diff --git a/Emulator/pom.xml b/Emulator/pom.xml index 022cec42..a1ab3749 100644 --- a/Emulator/pom.xml +++ b/Emulator/pom.xml @@ -83,7 +83,7 @@ io.netty netty-all - 4.1.115.Final + 4.1.118.Final diff --git a/Emulator/src/main/java/com/eu/habbo/habbohotel/items/interactions/InteractionWired.java b/Emulator/src/main/java/com/eu/habbo/habbohotel/items/interactions/InteractionWired.java index 6054066e..ffbf6996 100644 --- a/Emulator/src/main/java/com/eu/habbo/habbohotel/items/interactions/InteractionWired.java +++ b/Emulator/src/main/java/com/eu/habbo/habbohotel/items/interactions/InteractionWired.java @@ -18,6 +18,7 @@ import java.sql.SQLException; import java.util.Iterator; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicBoolean; /** * Base abstract class for all wired furniture items (triggers, effects, conditions, extras). @@ -61,7 +62,11 @@ public abstract class InteractionWired extends InteractionDefault { */ private static final long CACHE_EXPIRY_MS = 5 * 60 * 1000; - private long cooldown; + private volatile long cooldown; + // Ensures one box is processed by a single thread at a time, so the + // cooldown check-and-set in WiredHandler can't double-fire when a packet + // thread and the room cycle thread trigger the same box concurrently. + private final AtomicBoolean processing = new AtomicBoolean(false); private final ConcurrentHashMap userExecutionCache = new ConcurrentHashMap<>(); InteractionWired(ResultSet set, Item baseItem) throws SQLException { @@ -149,6 +154,15 @@ public abstract class InteractionWired extends InteractionDefault { this.cooldown = newMillis; } + /** Claims exclusive processing of this box; returns false if another thread is already in it. */ + public boolean tryBeginProcessing() { + return this.processing.compareAndSet(false, true); + } + + public void endProcessing() { + this.processing.set(false); + } + @Override public boolean allowWiredResetState() { return false; diff --git a/Emulator/src/main/java/com/eu/habbo/habbohotel/rooms/RoomItemManager.java b/Emulator/src/main/java/com/eu/habbo/habbohotel/rooms/RoomItemManager.java index d823d8ea..b307d989 100644 --- a/Emulator/src/main/java/com/eu/habbo/habbohotel/rooms/RoomItemManager.java +++ b/Emulator/src/main/java/com/eu/habbo/habbohotel/rooms/RoomItemManager.java @@ -167,17 +167,22 @@ public class RoomItemManager { */ public THashSet getFloorItems() { THashSet items = new THashSet<>(); - TIntObjectIterator iterator = this.roomItems.iterator(); + // roomItems is a TCollections.synchronizedMap; its iterator is not safe + // against concurrent put/remove (item place/pickup), so hold the map + // monitor for the whole traversal, matching the mutation sites. + synchronized (this.roomItems) { + TIntObjectIterator iterator = this.roomItems.iterator(); - for (int i = this.roomItems.size(); i-- > 0; ) { - try { - iterator.advance(); - } catch (Exception e) { - break; - } + for (int i = this.roomItems.size(); i-- > 0; ) { + try { + iterator.advance(); + } catch (Exception e) { + break; + } - if (iterator.value().getBaseItem().getType() == FurnitureType.FLOOR) { - items.add(iterator.value()); + if (iterator.value().getBaseItem().getType() == FurnitureType.FLOOR) { + items.add(iterator.value()); + } } } @@ -189,17 +194,19 @@ public class RoomItemManager { */ public THashSet getWallItems() { THashSet items = new THashSet<>(); - TIntObjectIterator iterator = this.roomItems.iterator(); + synchronized (this.roomItems) { + TIntObjectIterator iterator = this.roomItems.iterator(); - for (int i = this.roomItems.size(); i-- > 0; ) { - try { - iterator.advance(); - } catch (Exception e) { - break; - } + for (int i = this.roomItems.size(); i-- > 0; ) { + try { + iterator.advance(); + } catch (Exception e) { + break; + } - if (iterator.value().getBaseItem().getType() == FurnitureType.WALL) { - items.add(iterator.value()); + if (iterator.value().getBaseItem().getType() == FurnitureType.WALL) { + items.add(iterator.value()); + } } } @@ -211,18 +218,20 @@ public class RoomItemManager { */ public THashSet getPostItNotes() { THashSet items = new THashSet<>(); - TIntObjectIterator iterator = this.roomItems.iterator(); + synchronized (this.roomItems) { + TIntObjectIterator iterator = this.roomItems.iterator(); - for (int i = this.roomItems.size(); i-- > 0; ) { - try { - iterator.advance(); - } catch (Exception e) { - break; - } + for (int i = this.roomItems.size(); i-- > 0; ) { + try { + iterator.advance(); + } catch (Exception e) { + break; + } - if (iterator.value().getBaseItem().getInteractionType().getType() - == InteractionPostIt.class) { - items.add(iterator.value()); + if (iterator.value().getBaseItem().getInteractionType().getType() + == InteractionPostIt.class) { + items.add(iterator.value()); + } } } @@ -276,44 +285,49 @@ public class RoomItemManager { } } - TIntObjectIterator iterator = this.roomItems.iterator(); + // Cache miss: iterate roomItems under its monitor so a concurrent + // place/pickup can't rehash the map mid-traversal (which the per-advance + // try/catch would otherwise silently swallow into an incomplete result). + synchronized (this.roomItems) { + TIntObjectIterator iterator = this.roomItems.iterator(); - for (int i = this.roomItems.size(); i-- > 0; ) { - HabboItem item; - try { - iterator.advance(); - item = iterator.value(); - } catch (Exception e) { - break; - } + for (int i = this.roomItems.size(); i-- > 0; ) { + HabboItem item; + try { + iterator.advance(); + item = iterator.value(); + } catch (Exception e) { + break; + } - if (item == null) { - continue; - } + if (item == null) { + continue; + } - if (item.getBaseItem().getType() != FurnitureType.FLOOR) { - continue; - } + if (item.getBaseItem().getType() != FurnitureType.FLOOR) { + continue; + } - int width, length; + int width, length; - if (item.getRotation() != 2 && item.getRotation() != 6) { - width = item.getBaseItem().getWidth() > 0 ? item.getBaseItem().getWidth() : 1; - length = item.getBaseItem().getLength() > 0 ? item.getBaseItem().getLength() : 1; - } else { - width = item.getBaseItem().getLength() > 0 ? item.getBaseItem().getLength() : 1; - length = item.getBaseItem().getWidth() > 0 ? item.getBaseItem().getWidth() : 1; - } + if (item.getRotation() != 2 && item.getRotation() != 6) { + width = item.getBaseItem().getWidth() > 0 ? item.getBaseItem().getWidth() : 1; + length = item.getBaseItem().getLength() > 0 ? item.getBaseItem().getLength() : 1; + } else { + width = item.getBaseItem().getLength() > 0 ? item.getBaseItem().getLength() : 1; + length = item.getBaseItem().getWidth() > 0 ? item.getBaseItem().getWidth() : 1; + } - if (!(tile.x >= item.getX() && tile.x <= item.getX() + width - 1 && tile.y >= item.getY() - && tile.y <= item.getY() + length - 1)) { - continue; - } + if (!(tile.x >= item.getX() && tile.x <= item.getX() + width - 1 && tile.y >= item.getY() + && tile.y <= item.getY() + length - 1)) { + continue; + } - items.add(item); + items.add(item); - if (returnOnFirst) { - return items; + if (returnOnFirst) { + return items; + } } } @@ -956,9 +970,11 @@ public class RoomItemManager { public int getUserUniqueFurniCount(int userId) { THashSet items = new THashSet<>(); - for (HabboItem item : this.roomItems.valueCollection()) { - if (!items.contains(item.getBaseItem()) && item.getUserId() == userId) { - items.add(item.getBaseItem()); + synchronized (this.roomItems) { + for (HabboItem item : this.roomItems.valueCollection()) { + if (!items.contains(item.getBaseItem()) && item.getUserId() == userId) { + items.add(item.getBaseItem()); + } } } diff --git a/Emulator/src/main/java/com/eu/habbo/habbohotel/rooms/RoomUnit.java b/Emulator/src/main/java/com/eu/habbo/habbohotel/rooms/RoomUnit.java index 7b9326e0..af4987f9 100644 --- a/Emulator/src/main/java/com/eu/habbo/habbohotel/rooms/RoomUnit.java +++ b/Emulator/src/main/java/com/eu/habbo/habbohotel/rooms/RoomUnit.java @@ -31,6 +31,7 @@ import org.slf4j.LoggerFactory; import java.util.*; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentLinkedDeque; import java.util.concurrent.ScheduledFuture; import java.util.stream.Collectors; @@ -71,7 +72,10 @@ public class RoomUnit { private RoomUserRotation headRotation = RoomUserRotation.NORTH; private DanceType danceType; private RoomUnitType roomUnitType; - private Deque path = new LinkedList<>(); + // Concurrent + volatile: the room cycle thread polls/clears this path while a + // walk packet thread rebuilds it via findPath/setPath. A plain LinkedList would + // corrupt under the concurrent structural modification. + private volatile Deque path = new ConcurrentLinkedDeque<>(); private int handItem; private long handItemTimestamp; private long lastRollerTime; @@ -587,7 +591,7 @@ public class RoomUnit { Deque newPath = this.room.getLayout().getPathfinder() .findPath(this.currentLocation, this.goalLocation, this.goalLocation, this); if (newPath != null && !newPath.isEmpty()) { - this.path = newPath; + this.path = new ConcurrentLinkedDeque<>(newPath); } } @@ -765,7 +769,7 @@ public class RoomUnit { } public void setPath(Deque path) { - this.path = path; + this.path = (path == null) ? new ConcurrentLinkedDeque<>() : new ConcurrentLinkedDeque<>(path); } public RoomRightLevels getRightsLevel() { diff --git a/Emulator/src/main/java/com/eu/habbo/habbohotel/wired/WiredHandler.java b/Emulator/src/main/java/com/eu/habbo/habbohotel/wired/WiredHandler.java index 9ee3631a..119e1745 100644 --- a/Emulator/src/main/java/com/eu/habbo/habbohotel/wired/WiredHandler.java +++ b/Emulator/src/main/java/com/eu/habbo/habbohotel/wired/WiredHandler.java @@ -178,6 +178,15 @@ public class WiredHandler { private static boolean handle(InteractionWiredTrigger trigger, final RoomUnit roomUnit, final Room room, final Object[] stuff, final LegacyExecutionPlan executionPlan) { long millis = System.currentTimeMillis(); int roomUnitId = roomUnit != null ? roomUnit.getId() : -1; + + // Only one thread may process a given trigger box at a time, so the + // cooldown check (below) and setCooldown (further down) act as one + // atomic claim — preventing a concurrent packet/cycle double-fire. + if (!trigger.tryBeginProcessing()) { + return false; + } + + try { if (Emulator.isReady && ((Emulator.getConfig().getBoolean("wired.custom.enabled", false) && (trigger.canExecute(millis) || roomUnitId > -1) && trigger.userCanExecute(roomUnitId, millis)) || (!Emulator.getConfig().getBoolean("wired.custom.enabled", false) && trigger.canExecute(millis))) && trigger.execute(roomUnit, room, stuff)) { THashSet conditions = room.getRoomSpecialTypes().getConditions(trigger.getX(), trigger.getY()); THashSet effects = room.getRoomSpecialTypes().getEffects(trigger.getX(), trigger.getY()); @@ -272,6 +281,9 @@ public class WiredHandler { } return false; + } finally { + trigger.endProcessing(); + } } private static boolean evaluateConditions(THashSet conditions, RoomUnit roomUnit, Room room, Object[] stuff, int evaluationMode, int evaluationValue) {