duckietm/Arcturus-Morningstar-Extended
1
0
Fork 0
mirror of https://github.com/duckietm/Arcturus-Morningstar-Extended.git synced 2026-06-19 06:56:19 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: deep-analysis pass — self-review regressions + pre-existing logic bugs

Browse Source 
Regressions found by an adversarial review of this branch's own diff:
- RoomCycleManager: stop holding the currentBots/currentPets monitor across the
  whole bot/pet tick — snapshot under the lock then cycle off-lock. The previous
  fix blocked place/pickup and room dispose for the full tick and inverted lock
  order vs roomUnitLock->currentBots (latent deadlock for any future cycle code
  touching roomUnitLock).
- HabboInfo: complete the currencyLock invariant — getCurrencies() now returns a
  snapshot under the lock (UserInfoCommand iterated the live Trove map off-lock,
  the exact rehash corruption the lock guards); canBuy() uses the lock-guarded
  getCredits()/getCurrencyAmount(); run() reads credits under the lock for save.
- RoomSpecialTypes: synchronize the by-id pet getters (getNest/getPetDrink/
  getPetFood/getPetToy/getPetTree) to match their now-synchronized mutators.
- AuthHttpUtil.isTrustedProxy: exact-match trusted IPs; only treat an entry as a
  range when it ends with "."/":" so "10.0.0.1" can't also trust "10.0.0.12".

Pre-existing logic bugs found by the deep subsystem analysis:
- RoomUsersComposer: the bulk (room-entry) branch wrote the guild id twice;
  the second field must be the 1/-1 group-membership flag (matches the single
  branch) — every user showed a wrong group indicator on room entry.
- BotManager.pickUpBot: room owners (and ACC_PLACEFURNI) couldn't pick up bots
  placed in their own room — added the room-owner clause that placeBot has.
- PetPickupEvent: compared user id to pet.getId() instead of pet.getUserId(), so
  a pet owner who isn't the room owner couldn't pick up their own pet.
- RoomRightsManager.refreshRightsForHabbo: in guild rooms, explicit room_rights
  were stripped (overwritten by guild level NONE); now takes the stronger of
  explicit rights and guild level.
- RoomRequestBannedUsersEvent: `!hasRights || !ACC_ANYROOMOWNER` required BOTH,
  denying legitimate owners the banned-users list — corrected to `&&`.
- InteractionPetBreedingNest.breed: a crafted packet on a not-full nest deleted
  the nest furni then NPE'd (furni loss); guard petOne/petTwo/room before the
  destructive delete; ConfirmPetBreedingEvent null-checks the room.
- WiredEffectTeleport/UserFurniBase: appended item id instead of sprite id in the
  incompatible-triggers list (cosmetic wired-dialog mismatch) — matched the ~10
  other effects' getBaseItem().getSpriteId() convention.
This commit is contained in:
simoleo89
2026-06-09 15:42:22 +00:00
parent 4eb1484daf
commit a0910d822c
13 changed files with 106 additions and 58 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Emulator/src/main/java/com/eu/habbo/habbohotel/bots/BotManager.java
+5 -1
View File
@@ -188,7 +188,11 @@ public class BotManager {
if (pickedUpEvent.isCancelled())
return;
if (habbo == null || (bot.getOwnerId() == habbo.getHabboInfo().getId() || habbo.hasPermission(Permission.ACC_ANYROOMOWNER))) {
Room currentRoom = habbo != null ? habbo.getHabboInfo().getCurrentRoom() : null;
if (habbo == null
|| bot.getOwnerId() == habbo.getHabboInfo().getId()
|| habbo.hasPermission(Permission.ACC_ANYROOMOWNER)
|| (currentRoom != null && (currentRoom.getOwnerId() == habbo.getHabboInfo().getId() || habbo.hasPermission(Permission.ACC_PLACEFURNI)))) {
if (habbo != null && !habbo.hasPermission(Permission.ACC_UNLIMITED_BOTS) && habbo.getInventory().getBotsComponent().getBots().size() >= BotManager.MAXIMUM_BOT_INVENTORY_SIZE) {
habbo.alert(Emulator.getTexts().getValue("error.bots.max.inventory").replace("%amount%", BotManager.MAXIMUM_BOT_INVENTORY_SIZE + ""));
return;
Emulator/src/main/java/com/eu/habbo/habbohotel/items/interactions/pets/InteractionPetBreedingNest.java
+8
View File
@@ -190,6 +190,14 @@ public class InteractionPetBreedingNest extends HabboItem {
}
public void breed(Habbo habbo, String name, int petOneId, int petTwoId) {
// Guard before the destructive delete below: a crafted packet can call
// this on a nest that isn't full, which would delete the nest furni and
// then NPE on petOne/petTwo in the async runnable (losing the furni).
if (habbo == null || this.petOne == null || this.petTwo == null
|| habbo.getHabboInfo().getCurrentRoom() == null) {
return;
}
Emulator.getThreading().run(new QueryDeleteHabboItem(this.getId()));
this.setExtradata("2");
Emulator/src/main/java/com/eu/habbo/habbohotel/items/interactions/wired/effects/WiredEffectTeleport.java
+1 -1
View File
@@ -151,7 +151,7 @@ public class WiredEffectTeleport extends InteractionWiredEffect {
@Override
public boolean execute(InteractionWiredTrigger object) {
if (!object.isTriggeredByRoomUnit()) {
invalidTriggers.add(object.getId());
invalidTriggers.add(object.getBaseItem().getSpriteId());
}
return true;
}
Emulator/src/main/java/com/eu/habbo/habbohotel/items/interactions/wired/effects/WiredEffectUserFurniBase.java
+1 -1
View File
@@ -252,7 +252,7 @@ public abstract class WiredEffectUserFurniBase extends InteractionWiredEffect {
@Override
public boolean execute(InteractionWiredTrigger object) {
if (!object.isTriggeredByRoomUnit()) {
invalidTriggers.add(object.getId());
invalidTriggers.add(object.getBaseItem().getSpriteId());
}
return true;
}
Emulator/src/main/java/com/eu/habbo/habbohotel/rooms/RoomCycleManager.java
+38 -38
View File
@@ -300,37 +300,35 @@ public class RoomCycleManager {
return;
}
// currentBots is a TCollections.synchronizedMap; the iterator is not
// safe against concurrent put/remove from IO threads (bot place/pickup),
// so we must hold the map's monitor for the whole traversal.
// Snapshot under the map monitor (currentBots is a synchronizedMap whose
// iterator isn't concurrency-safe), then cycle OFF-lock. Holding the
// monitor across the whole tick would block bot place/pickup and room
// dispose for the tick duration AND invert the lock order vs
// roomUnitLock -> currentBots taken by RoomUnitManager.addBot/clear.
final ArrayList<Bot> bots;
synchronized (currentBots) {
TIntObjectIterator<Bot> botIterator = currentBots.iterator();
for (int i = currentBots.size(); i-- > 0; ) {
try {
final Bot bot;
try {
botIterator.advance();
bot = botIterator.value();
} catch (Exception e) {
break;
}
bots = new ArrayList<>(currentBots.valueCollection());
}
if (!this.room.isAllowBotsWalk() && bot.getRoomUnit().isWalking()) {
bot.getRoomUnit().stopWalking();
updatedUnit.add(bot.getRoomUnit());
continue;
}
bot.cycle(this.room.isAllowBotsWalk());
if (this.cycleRoomUnit(bot.getRoomUnit(), RoomUnitType.BOT)) {
updatedUnit.add(bot.getRoomUnit());
}
} catch (NoSuchElementException e) {
LOGGER.error("Caught exception", e);
break;
for (Bot bot : bots) {
try {
if (bot == null || bot.getRoomUnit() == null) {
continue;
}
if (!this.room.isAllowBotsWalk() && bot.getRoomUnit().isWalking()) {
bot.getRoomUnit().stopWalking();
updatedUnit.add(bot.getRoomUnit());
continue;
}
bot.cycle(this.room.isAllowBotsWalk());
if (this.cycleRoomUnit(bot.getRoomUnit(), RoomUnitType.BOT)) {
updatedUnit.add(bot.getRoomUnit());
}
} catch (Exception e) {
LOGGER.error("Caught exception", e);
}
}
}
@@ -344,19 +342,19 @@ public class RoomCycleManager {
return;
}
// currentPets is a TCollections.synchronizedMap; hold its monitor for the
// whole traversal to stay safe against concurrent pet place/pickup.
// Snapshot under the monitor, then cycle off-lock (see processBots): avoids
// holding currentPets for the whole tick and the roomUnitLock inversion.
final ArrayList<Pet> pets;
synchronized (currentPets) {
TIntObjectIterator<Pet> petIterator = currentPets.iterator();
for (int i = currentPets.size(); i-- > 0; ) {
try {
petIterator.advance();
} catch (NoSuchElementException e) {
LOGGER.error("Caught exception", e);
break;
pets = new ArrayList<>(currentPets.valueCollection());
}
for (Pet pet : pets) {
try {
if (pet == null || pet.getRoomUnit() == null) {
continue;
}
Pet pet = petIterator.value();
if (this.cycleRoomUnit(pet.getRoomUnit(), RoomUnitType.PET)) {
updatedUnit.add(pet.getRoomUnit());
}
@@ -373,6 +371,8 @@ public class RoomCycleManager {
pet.getRoomUnit().removeStatus(RoomUnitStatus.GESTURE);
updatedUnit.add(pet.getRoomUnit());
}
} catch (Exception e) {
LOGGER.error("Caught exception", e);
}
}
}
Emulator/src/main/java/com/eu/habbo/habbohotel/rooms/RoomRightsManager.java
+9 -3
View File
@@ -272,10 +272,16 @@ public class RoomRightsManager {
} else if (this.isOwner(habbo)) {
habbo.getClient().sendResponse(new RoomOwnerComposer());
flatCtrl = RoomRightLevels.MODERATOR;
} else if (this.hasRights(habbo) && !this.room.hasGuild()) {
flatCtrl = RoomRightLevels.RIGHTS;
} else if (this.room.hasGuild()) {
flatCtrl = this.getGuildRightLevel(habbo);
// Explicit room rights must still be honoured in guild rooms (the old
// `&& !hasGuild()` guard stripped them for non-guild members) — take
// whichever of the two is stronger.
RoomRightLevels guildLevel = this.getGuildRightLevel(habbo);
flatCtrl = (this.hasRights(habbo) && RoomRightLevels.RIGHTS.isEqualOrGreaterThan(guildLevel))
? RoomRightLevels.RIGHTS
: guildLevel;
} else if (this.hasRights(habbo)) {
flatCtrl = RoomRightLevels.RIGHTS;
}
habbo.getClient().sendResponse(new RoomRightsComposer(flatCtrl));
Emulator/src/main/java/com/eu/habbo/habbohotel/rooms/RoomSpecialTypes.java
+15 -5
View File
@@ -152,7 +152,9 @@ public class RoomSpecialTypes {
public InteractionNest getNest(int itemId) {
return this.nests.get(itemId);
synchronized (this.nests) {
return this.nests.get(itemId);
}
}
public void addNest(InteractionNest item) {
@@ -180,7 +182,9 @@ public class RoomSpecialTypes {
public InteractionPetDrink getPetDrink(int itemId) {
return this.petDrinks.get(itemId);
synchronized (this.petDrinks) {
return this.petDrinks.get(itemId);
}
}
public void addPetDrink(InteractionPetDrink item) {
@@ -208,7 +212,9 @@ public class RoomSpecialTypes {
public InteractionPetFood getPetFood(int itemId) {
return this.petFoods.get(itemId);
synchronized (this.petFoods) {
return this.petFoods.get(itemId);
}
}
public void addPetFood(InteractionPetFood item) {
@@ -236,7 +242,9 @@ public class RoomSpecialTypes {
public InteractionPetToy getPetToy(int itemId) {
return this.petToys.get(itemId);
synchronized (this.petToys) {
return this.petToys.get(itemId);
}
}
public void addPetToy(InteractionPetToy item) {
@@ -264,7 +272,9 @@ public class RoomSpecialTypes {
public InteractionPetTree getPetTree(int itemId) {
return this.petTrees.get(itemId);
synchronized (this.petTrees) {
return this.petTrees.get(itemId);
}
}
public void addPetTree(InteractionPetTree item) {
Emulator/src/main/java/com/eu/habbo/habbohotel/users/HabboInfo.java
+14 -3
View File
@@ -254,7 +254,11 @@ public class HabboInfo implements Runnable {
}
public TIntIntHashMap getCurrencies() {
return this.currencies;
// Return a snapshot under the lock: callers iterate this map, which would
// otherwise corrupt during a concurrent adjustOrPutValue rehash.
synchronized (this.currencyLock) {
return new TIntIntHashMap(this.currencies);
}
}
public void addCurrencyAmount(int type, int amount) {
@@ -396,7 +400,7 @@ public class HabboInfo implements Runnable {
}
public boolean canBuy(CatalogItem item) {
return this.credits >= item.getCredits() && this.getCurrencies().get(item.getPointsType()) >= item.getPoints();
return this.getCredits() >= item.getCredits() && this.getCurrencyAmount(item.getPointsType()) >= item.getPoints();
}
public int getCredits() {
@@ -622,6 +626,13 @@ public class HabboInfo implements Runnable {
public void run() {
this.saveCurrencies();
// Read credits under the lock so the persisted value is consistent with
// concurrent addCredits/setCredits (matches the currencyLock invariant).
final int creditsForSave;
synchronized (this.currencyLock) {
creditsForSave = this.credits;
}
try {
SqlQueries.update(
"UPDATE users SET motto = ?, online = ?, look = ?, gender = ?, credits = ?, last_login = ?, last_online = ?, home_room = ?, ip_current = ?, `rank` = ?, machine_id = ?, username = ?, background_id = ?, background_stand_id = ?, background_overlay_id = ?, background_card_id = ?, background_border_id = ? WHERE id = ?",
@@ -629,7 +640,7 @@ public class HabboInfo implements Runnable {
this.online ? "1" : "0",
this.look,
this.gender.name(),
this.credits,
creditsForSave,
Emulator.getIntUnixTimestamp(),
this.lastOnline,
this.homeRoom,
Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/RoomRequestBannedUsersEvent.java
+1 -1
View File
@@ -14,7 +14,7 @@ public class RoomRequestBannedUsersEvent extends MessageHandler {
Room room = Emulator.getGameEnvironment().getRoomManager().getRoom(roomId);
if (room == null) return;
if (!room.hasRights(this.client.getHabbo()) || !this.client.getHabbo().hasPermission(Permission.ACC_ANYROOMOWNER)) return;
if (!room.hasRights(this.client.getHabbo()) && !this.client.getHabbo().hasPermission(Permission.ACC_ANYROOMOWNER)) return;
this.client.sendResponse(new RoomBannedUsersComposer(room));
Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/pets/ConfirmPetBreedingEvent.java
+5 -1
View File
@@ -1,6 +1,7 @@
package com.eu.habbo.messages.incoming.rooms.pets;
import com.eu.habbo.habbohotel.items.interactions.pets.InteractionPetBreedingNest;
import com.eu.habbo.habbohotel.rooms.Room;
import com.eu.habbo.habbohotel.users.HabboItem;
import com.eu.habbo.messages.incoming.MessageHandler;
@@ -13,7 +14,10 @@ public class ConfirmPetBreedingEvent extends MessageHandler {
int petOneId = this.packet.readInt();
int petTwoId = this.packet.readInt();
HabboItem item = this.client.getHabbo().getHabboInfo().getCurrentRoom().getHabboItem(itemId);
Room room = this.client.getHabbo().getHabboInfo().getCurrentRoom();
if (room == null) return;
HabboItem item = room.getHabboItem(itemId);
if (item instanceof InteractionPetBreedingNest) {
((InteractionPetBreedingNest) item).breed(this.client.getHabbo(), name, petOneId, petTwoId);
Emulator/src/main/java/com/eu/habbo/messages/incoming/rooms/pets/PetPickupEvent.java
+1 -1
View File
@@ -23,7 +23,7 @@ public class PetPickupEvent extends MessageHandler {
Pet pet = room.getPet(petId);
if (pet != null) {
if (this.client.getHabbo().getHabboInfo().getId() == pet.getId() || room.getOwnerId() == this.client.getHabbo().getHabboInfo().getId() || this.client.getHabbo().hasPermission(Permission.ACC_ANYROOMOWNER)) {
if (this.client.getHabbo().getHabboInfo().getId() == pet.getUserId() || room.getOwnerId() == this.client.getHabbo().getHabboInfo().getId() || this.client.getHabbo().hasPermission(Permission.ACC_ANYROOMOWNER)) {
if (!this.client.getHabbo().hasPermission(Permission.ACC_UNLIMITED_PETS) && this.client.getHabbo().getInventory().getPetsComponent().getPets().size() >= PetManager.MAXIMUM_PET_INVENTORY_SIZE) {
this.client.getHabbo().alert(Emulator.getTexts().getValue("error.pets.max.inventory").replace("%amount%", PetManager.MAXIMUM_PET_INVENTORY_SIZE + ""));
return;
Emulator/src/main/java/com/eu/habbo/messages/outgoing/rooms/users/RoomUsersComposer.java
+1 -1
View File
@@ -99,7 +99,7 @@ public class RoomUsersComposer extends MessageComposer {
this.response.appendInt(1);
this.response.appendString(habbo.getHabboInfo().getGender().name().toUpperCase());
this.response.appendInt(habbo.getHabboStats().guild != 0 ? habbo.getHabboStats().guild : -1);
this.response.appendInt(habbo.getHabboStats().guild != 0 ? habbo.getHabboStats().guild : -1);
this.response.appendInt(habbo.getHabboStats().guild != 0 ? 1 : -1);
String name = "";
if (habbo.getHabboStats().guild != 0) {
Guild g = Emulator.getGameEnvironment().getGuildManager().getGuild(habbo.getHabboStats().guild);
Emulator/src/main/java/com/eu/habbo/networking/gameserver/auth/AuthHttpUtil.java
+7 -2
View File
@@ -169,8 +169,13 @@ public final class AuthHttpUtil {
: "";
if (trusted.isEmpty()) return false;
for (String entry : trusted.split(",")) {
String prefix = entry.trim();
if (!prefix.isEmpty() && (peerIp.equals(prefix) || peerIp.startsWith(prefix))) {
String t = entry.trim();
if (t.isEmpty()) continue;
// Exact IP match, or a dotted/colon prefix range (e.g. "10.0.0." or
// "2001:db8:") — never a bare-IP prefix, so "10.0.0.1" can't also
// trust "10.0.0.12".
boolean isRange = t.endsWith(".") || t.endsWith(":");
if (peerIp.equals(t) || (isRange && peerIp.startsWith(t))) {
return true;
}
}