Add slot sanity checks in container clicks

This commit is contained in:
Nassim Jahnke 2023-09-11 12:01:57 +10:00
parent a951373bae
commit 5688b5cf50
2 changed files with 73 additions and 53 deletions

View File

@ -48,7 +48,7 @@
import net.minecraft.world.level.GameRules;
import net.minecraft.world.level.GameType;
import net.minecraft.world.level.Level;
@@ -192,11 +196,72 @@
@@ -192,12 +196,73 @@
import net.minecraft.world.level.block.state.BlockState;
import net.minecraft.world.phys.AABB;
import net.minecraft.world.phys.BlockHitResult;
@ -59,7 +59,7 @@
import net.minecraft.world.phys.shapes.VoxelShape;
+import org.bukkit.NamespacedKey;
import org.slf4j.Logger;
+
+// CraftBukkit start
+import io.papermc.paper.adventure.ChatProcessor; // Paper
+import io.papermc.paper.adventure.PaperAdventure; // Paper
@ -118,9 +118,10 @@
+import org.bukkit.inventory.InventoryView;
+import org.bukkit.inventory.SmithingInventory;
+// CraftBukkit end
+
public class ServerGamePacketListenerImpl extends ServerCommonPacketListenerImpl implements ServerGamePacketListener, ServerPlayerConnection, TickablePacketListener {
static final Logger LOGGER = LogUtils.getLogger();
@@ -212,7 +277,9 @@
private int tickCount;
private int ackBlockChangesUpTo = -1;
@ -338,7 +339,7 @@
boolean flag1 = entity.verticalCollisionBelow;
if (entity instanceof LivingEntity) {
@@ -449,20 +607,73 @@
@@ -449,19 +607,72 @@
d10 = d6 * d6 + d7 * d7 + d8 * d8;
boolean flag2 = false;
@ -357,8 +358,8 @@
+ this.player.absMoveTo(d0, d1, d2, this.player.getYRot(), this.player.getXRot()); // CraftBukkit
this.send(ClientboundMoveVehiclePacket.fromEntity(entity));
return;
}
+ }
+
+ // CraftBukkit start - fire PlayerMoveEvent
+ Player player = this.getCraftPlayer();
+ if (!this.hasMoved) {
@ -407,12 +408,11 @@
+ this.justTeleported = false;
+ return;
+ }
+ }
}
+ // CraftBukkit end
+
this.player.serverLevel().getChunkSource().move(this.player);
entity.recordMovementThroughBlocks(new Vec3(d0, d1, d2), entity.position());
Vec3 vec3d = new Vec3(entity.getX() - d0, entity.getY() - d1, entity.getZ() - d2);
@@ -489,16 +700,17 @@
PacketUtils.ensureRunningOnSameThread(packet, this, this.player.serverLevel());
if (packet.getId() == this.awaitingTeleport) {
@ -1169,10 +1169,13 @@
return;
default:
throw new IllegalArgumentException("Invalid player action");
@@ -1218,9 +1871,31 @@
}
}
@@ -1215,12 +1868,34 @@
Item item = stack.getItem();
return (item instanceof BlockItem || item instanceof BucketItem) && !player.getCooldowns().isOnCooldown(stack);
+ }
+ }
+
+ // Spigot start - limit place/interactions
+ private int limitedPackets;
+ private long lastLimitedPacket = -1;
@ -1181,7 +1184,7 @@
+ private boolean checkLimit(long timestamp) {
+ if (this.lastLimitedPacket != -1 && timestamp - this.lastLimitedPacket < getSpamThreshold() && this.limitedPackets++ >= 8) { // Paper - Configurable threshold; raise packet limit to 8
+ return false;
+ }
}
+
+ if (this.lastLimitedPacket == -1 || timestamp - this.lastLimitedPacket >= getSpamThreshold()) { // Paper - Configurable threshold
+ this.lastLimitedPacket = timestamp;
@ -1190,9 +1193,9 @@
+ }
+
+ return true;
+ }
}
+ // Spigot end
+
@Override
public void handleUseItemOn(ServerboundUseItemOnPacket packet) {
PacketUtils.ensureRunningOnSameThread(packet, this, this.player.serverLevel());
@ -1543,11 +1546,10 @@
}
return optional;
@@ -1564,7 +2380,128 @@
}
@@ -1565,6 +2381,127 @@
return false;
+ }
}
+
+ // CraftBukkit start - add method
+ public void chat(String s, PlayerChatMessage original, boolean async) {
@ -1667,7 +1669,7 @@
+ return;
+ } finally {
+ }
}
+ }
+ // CraftBukkit end
private PlayerChatMessage getSignedMessage(ServerboundChatPacket packet, LastSeenMessages lastSeenMessages) throws SignedMessageChain.DecodeException {
@ -2058,7 +2060,7 @@
this.player.containerMenu.sendAllDataToRemote();
} else if (!this.player.containerMenu.stillValid(this.player)) {
ServerGamePacketListenerImpl.LOGGER.debug("Player {} interacted with invalid menu {}", this.player, this.player.containerMenu);
@@ -1855,7 +2983,284 @@
@@ -1855,7 +2983,290 @@
boolean flag = packet.getStateId() != this.player.containerMenu.getStateId();
this.player.containerMenu.suppressRemoteUpdates();
@ -2156,6 +2158,12 @@
+ break;
+ case SWAP:
+ if ((packet.getButtonNum() >= 0 && packet.getButtonNum() < 9) || packet.getButtonNum() == 40) {
+ // Paper start - Add slot sanity checks to container clicks
+ if (packet.getSlotNum() < 0) {
+ action = InventoryAction.NOTHING;
+ break;
+ }
+ // Paper end - Add slot sanity checks to container clicks
+ click = (packet.getButtonNum() == 40) ? ClickType.SWAP_OFFHAND : ClickType.NUMBER_KEY;
+ Slot clickedSlot = this.player.containerMenu.getSlot(packet.getSlotNum());
+ if (clickedSlot.mayPickup(this.player)) {
@ -2344,7 +2352,7 @@
ObjectIterator objectiterator = Int2ObjectMaps.fastIterable(packet.getChangedSlots()).iterator();
while (objectiterator.hasNext()) {
@@ -1879,6 +3284,14 @@
@@ -1879,6 +3290,14 @@
@Override
public void handlePlaceRecipe(ServerboundPlaceRecipePacket packet) {
@ -2359,7 +2367,7 @@
PacketUtils.ensureRunningOnSameThread(packet, this, this.player.serverLevel());
this.player.resetLastActionTime();
if (!this.player.isSpectator() && this.player.containerMenu.containerId == packet.containerId()) {
@@ -1900,9 +3313,43 @@
@@ -1900,8 +3319,42 @@
ServerGamePacketListenerImpl.LOGGER.debug("Player {} tried to place impossible recipe {}", this.player, recipeholder.id().location());
return;
}
@ -2392,7 +2400,7 @@
+ return;
+ }
+ // Paper end - Add PlayerRecipeBookClickEvent - forward to legacy event
+
+ // Cast to keyed should be safe as the recipe will never be a MerchantRecipe.
+ recipeholder = this.server.getRecipeManager().byKey(net.minecraft.resources.ResourceKey.create(net.minecraft.core.registries.Registries.RECIPE, org.bukkit.craftbukkit.util.CraftNamespacedKey.toMinecraft(recipeName))).orElse(null); // Paper - Add PlayerRecipeBookClickEvent - forward to legacy event
+ if (recipeholder == null) {
@ -2400,11 +2408,10 @@
+ }
+ RecipeBookMenu.PostPlaceAction containerrecipebook_a = containerrecipebook.handlePlacement(makeAll, this.player.isCreative(), recipeholder, this.player.serverLevel(), this.player.getInventory());
+ // CraftBukkit end
+
if (containerrecipebook_a == RecipeBookMenu.PostPlaceAction.PLACE_GHOST_RECIPE) {
this.player.connection.send(new ClientboundPlaceGhostRecipePacket(this.player.containerMenu.containerId, craftingmanager_d.display().display()));
}
@@ -1917,6 +3364,7 @@
@@ -1917,6 +3370,7 @@
@Override
public void handleContainerButtonClick(ServerboundContainerButtonClickPacket packet) {
PacketUtils.ensureRunningOnSameThread(packet, this, this.player.serverLevel());
@ -2412,7 +2419,7 @@
this.player.resetLastActionTime();
if (this.player.containerMenu.containerId == packet.containerId() && !this.player.isSpectator()) {
if (!this.player.containerMenu.stillValid(this.player)) {
@@ -1945,7 +3393,44 @@
@@ -1945,7 +3399,44 @@
boolean flag1 = packet.slotNum() >= 1 && packet.slotNum() <= 45;
boolean flag2 = itemstack.isEmpty() || itemstack.getCount() <= itemstack.getMaxStackSize();
@ -2457,7 +2464,7 @@
if (flag1 && flag2) {
this.player.inventoryMenu.getSlot(packet.slotNum()).setByPlayer(itemstack);
this.player.inventoryMenu.setRemoteSlot(packet.slotNum(), itemstack);
@@ -1964,7 +3449,19 @@
@@ -1964,7 +3455,19 @@
@Override
public void handleSignUpdate(ServerboundSignUpdatePacket packet) {
@ -2478,7 +2485,7 @@
this.filterTextPacket(list).thenAcceptAsync((list1) -> {
this.updateSignText(packet, list1);
@@ -1972,6 +3469,7 @@
@@ -1972,6 +3475,7 @@
}
private void updateSignText(ServerboundSignUpdatePacket packet, List<FilteredText> signText) {
@ -2486,7 +2493,7 @@
this.player.resetLastActionTime();
ServerLevel worldserver = this.player.serverLevel();
BlockPos blockposition = packet.getPos();
@@ -1993,15 +3491,33 @@
@@ -1993,15 +3497,33 @@
@Override
public void handlePlayerAbilities(ServerboundPlayerAbilitiesPacket packet) {
PacketUtils.ensureRunningOnSameThread(packet, this, this.player.serverLevel());
@ -2521,7 +2528,7 @@
if (this.player.isModelPartShown(PlayerModelPart.HAT) != flag) {
this.server.getPlayerList().broadcastAll(new ClientboundPlayerInfoUpdatePacket(ClientboundPlayerInfoUpdatePacket.Action.UPDATE_HAT, this.player));
}
@@ -2012,7 +3528,7 @@
@@ -2012,7 +3534,7 @@
public void handleChangeDifficulty(ServerboundChangeDifficultyPacket packet) {
PacketUtils.ensureRunningOnSameThread(packet, this, this.player.serverLevel());
if (this.player.hasPermissions(2) || this.isSingleplayerOwner()) {
@ -2530,7 +2537,7 @@
}
}
@@ -2033,7 +3549,7 @@
@@ -2033,7 +3555,7 @@
if (!Objects.equals(profilepublickey_a, profilepublickey_a1)) {
if (profilepublickey_a != null && profilepublickey_a1.expiresAt().isBefore(profilepublickey_a.expiresAt())) {
@ -2539,7 +2546,7 @@
} else {
try {
SignatureValidator signaturevalidator = this.server.getProfileKeySignatureValidator();
@@ -2045,8 +3561,8 @@
@@ -2045,8 +3567,8 @@
this.resetPlayerChatState(remotechatsession_a.validate(this.player.getGameProfile(), signaturevalidator));
} catch (ProfilePublicKey.ValidationException profilepublickey_b) {
@ -2550,7 +2557,7 @@
}
}
@@ -2058,7 +3574,7 @@
@@ -2058,7 +3580,7 @@
if (!this.waitingForSwitchToConfig) {
throw new IllegalStateException("Client acknowledged config, but none was requested");
} else {
@ -2559,7 +2566,7 @@
}
}
@@ -2076,15 +3592,18 @@
@@ -2076,15 +3598,18 @@
private void resetPlayerChatState(RemoteChatSession session) {
this.chatSession = session;
@ -2581,7 +2588,7 @@
@Override
public void handleClientTickEnd(ServerboundClientTickEndPacket packet) {
@@ -2115,4 +3634,17 @@
@@ -2115,4 +3640,17 @@
InteractionResult run(ServerPlayer player, Entity entity, InteractionHand hand);
}

View File

@ -28,11 +28,10 @@
public abstract class AbstractContainerMenu {
private static final Logger LOGGER = LogUtils.getLogger();
@@ -66,6 +80,32 @@
@Nullable
@@ -67,6 +81,32 @@
private ContainerSynchronizer synchronizer;
private boolean suppressRemoteUpdates;
+
+ // CraftBukkit start
+ public boolean checkReachable = true;
+ public abstract InventoryView getBukkitView();
@ -58,30 +57,30 @@
+ this.title = title;
+ }
+ // CraftBukkit end
+
protected AbstractContainerMenu(@Nullable MenuType<?> type, int syncId) {
this.carried = ItemStack.EMPTY;
@@ -188,10 +228,20 @@
this.remoteSlots = NonNullList.create();
@@ -188,9 +228,19 @@
if (this.synchronizer != null) {
this.synchronizer.sendInitialData(this, this.remoteSlots, this.remoteCarried, this.remoteDataSlots.toIntArray());
+ this.synchronizer.sendOffHandSlotChange(); // Paper - Sync offhand slot in menus; update player's offhand since the offhand slot is not added to the slots for menus but can be changed by swapping from a menu slot
}
}
+ }
+
+ // CraftBukkit start
+ public void broadcastCarriedItem() {
+ this.remoteCarried = this.getCarried().copy();
+ if (this.synchronizer != null) {
+ this.synchronizer.sendCarriedChange(this, this.remoteCarried);
+ }
+ }
}
+ // CraftBukkit end
+
public void removeSlotListener(ContainerListener listener) {
this.containerListeners.remove(listener);
}
@@ -281,7 +331,7 @@
while (iterator.hasNext()) {
ContainerListener icrafting = (ContainerListener) iterator.next();
@ -91,7 +90,15 @@
}
}
@@ -417,7 +467,7 @@
@@ -410,6 +460,7 @@
this.resetQuickCraft();
}
} else if (this.quickcraftStatus == 1) {
+ if (slotIndex < 0) return; // Paper - Add slot sanity checks to container clicks
slot = (Slot) this.slots.get(slotIndex);
itemstack = this.getCarried();
if (AbstractContainerMenu.canItemQuickReplace(slot, itemstack, true) && slot.mayPlace(itemstack) && (this.quickcraftType == 2 || itemstack.getCount() > this.quickcraftSlots.size()) && this.canDragTo(slot)) {
@@ -417,7 +468,7 @@
}
} else if (this.quickcraftStatus == 2) {
if (!this.quickcraftSlots.isEmpty()) {
@ -100,7 +107,7 @@
k = ((Slot) this.quickcraftSlots.iterator().next()).index;
this.resetQuickCraft();
this.doClick(k, this.quickcraftType, ClickType.PICKUP, player);
@@ -433,6 +483,7 @@
@@ -433,6 +484,7 @@
l = this.getCarried().getCount();
Iterator iterator = this.quickcraftSlots.iterator();
@ -108,7 +115,7 @@
while (iterator.hasNext()) {
Slot slot1 = (Slot) iterator.next();
ItemStack itemstack2 = this.getCarried();
@@ -443,12 +494,48 @@
@@ -443,12 +495,48 @@
int l1 = Math.min(AbstractContainerMenu.getQuickCraftPlaceCount(this.quickcraftSlots, this.quickcraftType, itemstack1) + j1, k1);
l -= l1 - j1;
@ -160,7 +167,7 @@
}
this.resetQuickCraft();
@@ -466,8 +553,11 @@
@@ -466,8 +554,11 @@
if (slotIndex == -999) {
if (!this.getCarried().isEmpty()) {
if (clickaction == ClickAction.PRIMARY) {
@ -173,7 +180,7 @@
} else {
player.drop(this.getCarried().split(1), true);
}
@@ -530,6 +620,15 @@
@@ -530,11 +621,21 @@
}
slot.setChanged();
@ -189,7 +196,13 @@
}
} else {
int j2;
@@ -662,8 +761,9 @@
if (actionType == ClickType.SWAP && (button >= 0 && button < 9 || button == 40)) {
+ if (slotIndex < 0) return; // Paper - Add slot sanity checks to container clicks
ItemStack itemstack4 = playerinventory.getItem(button);
slot = (Slot) this.slots.get(slotIndex);
@@ -662,8 +763,9 @@
ItemStack itemstack = this.getCarried();
if (!itemstack.isEmpty()) {
@ -200,7 +213,7 @@
}
}
@@ -893,6 +993,11 @@
@@ -893,6 +995,11 @@
}
public ItemStack getCarried() {