fix: guard double gift-open and harden client string reads

- InteractionGift/OpenRecycleBoxEvent: add an atomic open-once guard so two
  near-simultaneous OpenRecycleBox packets can't both schedule the async,
  delayed OpenGift before the wrapper is removed (redundant double-process).
- ClientMessage.readString: treat the length prefix as unsigned (mask 0xFFFF)
  and clamp to the buffered bytes, so a bogus/oversized length no longer
  throws mid-read and desyncs the remaining fields of the packet.
This commit is contained in:
simoleo89
2026-06-09 10:35:23 +00:00
parent da1fd01074
commit c98d3a3205
3 changed files with 24 additions and 1 deletions
@@ -13,11 +13,13 @@ import org.slf4j.LoggerFactory;
import java.sql.ResultSet; import java.sql.ResultSet;
import java.sql.SQLException; import java.sql.SQLException;
import java.util.concurrent.atomic.AtomicBoolean;
public class InteractionGift extends HabboItem { public class InteractionGift extends HabboItem {
private static final Logger LOGGER = LoggerFactory.getLogger(InteractionGift.class); private static final Logger LOGGER = LoggerFactory.getLogger(InteractionGift.class);
public boolean explode = false; public boolean explode = false;
private final AtomicBoolean opening = new AtomicBoolean(false);
private int[] itemId; private int[] itemId;
private int colorId = 0; private int colorId = 0;
private int ribbonId = 0; private int ribbonId = 0;
@@ -46,6 +48,15 @@ public class InteractionGift extends HabboItem {
} }
} }
/**
* Claims the right to open this gift, returning true exactly once. Guards
* against two near-simultaneous OpenRecycleBox packets both scheduling an
* (async, delayed) OpenGift before the wrapper is removed from the room.
*/
public boolean tryStartOpening() {
return this.opening.compareAndSet(false, true);
}
@Override @Override
public void serializeExtradata(ServerMessage serverMessage) { public void serializeExtradata(ServerMessage serverMessage) {
//serverMessage.appendInt(this.colorId * 1000 + this.ribbonId); //serverMessage.appendInt(this.colorId * 1000 + this.ribbonId);
@@ -61,7 +61,14 @@ public class ClientMessage {
public String readString() { public String readString() {
try { try {
int length = this.readShort(); // Length is an unsigned short in the protocol; mask to avoid a
// negative array size, and clamp to what's actually buffered so a
// bogus length can't throw mid-read and desync the remaining fields.
int length = this.readShort() & 0xFFFF;
int available = this.buffer.readableBytes();
if (length > available) {
length = available;
}
byte[] data = new byte[length]; byte[] data = new byte[length];
this.buffer.readBytes(data); this.buffer.readBytes(data);
return new String(data); return new String(data);
@@ -29,6 +29,11 @@ public class OpenRecycleBoxEvent extends MessageHandler {
if (item.getUserId() != this.client.getHabbo().getHabboInfo().getId()) return; if (item.getUserId() != this.client.getHabbo().getHabboInfo().getId()) return;
if (item instanceof InteractionGift) { if (item instanceof InteractionGift) {
// The actual unwrap (OpenGift) runs async/delayed and only then
// removes the wrapper, so a second packet would otherwise pass
// the room/owner checks and double-process the gift. Claim it once.
if (!((InteractionGift) item).tryStartOpening()) return;
if (item.getBaseItem().getName().contains("present_wrap")) { if (item.getBaseItem().getName().contains("present_wrap")) {
((InteractionGift) item).explode = true; ((InteractionGift) item).explode = true;
room.updateItem(item); room.updateItem(item);