From 5688b5cf504f4a709d8b48d40992d0140d984c42 Mon Sep 17 00:00:00 2001 From: Nassim Jahnke <nassim@njahnke.dev> Date: Mon, 11 Sep 2023 12:01:57 +1000 Subject: [PATCH] Add slot sanity checks in container clicks --- .../ServerGamePacketListenerImpl.java.patch | 79 ++++++++++--------- .../AbstractContainerMenu.java.patch | 47 +++++++---- 2 files changed, 73 insertions(+), 53 deletions(-) diff --git a/paper-server/patches/sources/net/minecraft/server/network/ServerGamePacketListenerImpl.java.patch b/paper-server/patches/sources/net/minecraft/server/network/ServerGamePacketListenerImpl.java.patch index 2e0eb6eb0a..1a71686581 100644 --- a/paper-server/patches/sources/net/minecraft/server/network/ServerGamePacketListenerImpl.java.patch +++ b/paper-server/patches/sources/net/minecraft/server/network/ServerGamePacketListenerImpl.java.patch @@ -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); } diff --git a/paper-server/patches/sources/net/minecraft/world/inventory/AbstractContainerMenu.java.patch b/paper-server/patches/sources/net/minecraft/world/inventory/AbstractContainerMenu.java.patch index 7b3d185a8f..ca24079af3 100644 --- a/paper-server/patches/sources/net/minecraft/world/inventory/AbstractContainerMenu.java.patch +++ b/paper-server/patches/sources/net/minecraft/world/inventory/AbstractContainerMenu.java.patch @@ -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() {