From 4c32788db0a9b760ab7742c16e1a1d59832e1562 Mon Sep 17 00:00:00 2001 From: Spottedleaf Date: Fri, 12 Jun 2020 16:51:39 -0700 Subject: [PATCH] Prevent position desync causing tp exploit Caused the server to revert to the player's overworld coordinates after teleporting into the end. Sidenote: The underlying issue is that the move call can teleport entities and do other things like kill the entity. In the future, to fix all exploits derieved from this usually unexpected behaviour, we need to move all of this dangerous logic outside of the move call and into an appropriate place in the tick method. --- .../ServerGamePacketListenerImpl.java.patch | 109 +++++++++--------- 1 file changed, 56 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 5e3d6711ce..14bbbf5f4b 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 @@ -277,7 +277,7 @@ ServerGamePacketListenerImpl.LOGGER.warn("{} (vehicle of {}) moved too quickly! {},{},{}", new Object[]{entity.getName().getString(), this.player.getName().getString(), d6, d7, d8}); this.send(ClientboundMoveVehiclePacket.fromEntity(entity)); return; -@@ -449,20 +582,73 @@ +@@ -449,19 +582,72 @@ d10 = d6 * d6 + d7 * d7 + d8 * d8; boolean flag2 = false; @@ -296,8 +296,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) { @@ -346,12 +346,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); @@ -499,6 +685,7 @@ this.lastGoodZ = this.awaitingPositionFromClient.z; this.player.hasChangedDimension(); @@ -639,7 +638,7 @@ ServerGamePacketListenerImpl.LOGGER.warn("{} moved too quickly! {},{},{}", new Object[]{this.player.getName().getString(), d6, d7, d8}); this.teleport(this.player.getX(), this.player.getY(), this.player.getZ(), this.player.getYRot(), this.player.getXRot()); return; -@@ -1043,12 +1379,40 @@ +@@ -1043,12 +1379,45 @@ boolean flag1 = d7 > 0.0D; if (this.player.onGround() && !packet.isOnGround() && flag1) { @@ -678,10 +677,15 @@ this.player.move(MoverType.PLAYER, new Vec3(d6, d7, d8)); + this.player.onGround = packet.isOnGround(); // CraftBukkit - SPIGOT-5810, SPIGOT-5835, SPIGOT-6828: reset by this.player.move ++ // Paper start - prevent position desync ++ if (this.awaitingPositionFromClient != null) { ++ return; // ... thanks Mojang for letting move calls teleport across dimensions. ++ } ++ // Paper end - prevent position desync double d11 = d7; d6 = d0 - this.player.getX(); -@@ -1061,15 +1425,81 @@ +@@ -1061,15 +1430,81 @@ d10 = d6 * d6 + d7 * d7 + d8 * d8; boolean flag3 = false; @@ -765,7 +769,7 @@ this.player.absMoveTo(d0, d1, d2, f, f1); boolean flag4 = this.player.isAutoSpinAttack(); -@@ -1119,6 +1549,7 @@ +@@ -1119,6 +1554,7 @@ this.awaitingTeleportTime = this.tickCount; this.teleport(this.awaitingPositionFromClient.x, this.awaitingPositionFromClient.y, this.awaitingPositionFromClient.z, this.player.getYRot(), this.player.getXRot()); } @@ -773,7 +777,7 @@ return true; } else { -@@ -1147,23 +1578,90 @@ +@@ -1147,23 +1583,90 @@ } public void teleport(double x, double y, double z, float yaw, float pitch) { @@ -867,7 +871,7 @@ if (this.player.hasClientLoaded()) { BlockPos blockposition = packet.getPos(); -@@ -1175,14 +1673,46 @@ +@@ -1175,14 +1678,46 @@ if (!this.player.isSpectator()) { ItemStack itemstack = this.player.getItemInHand(InteractionHand.OFF_HAND); @@ -916,7 +920,7 @@ this.player.drop(false); } -@@ -1199,6 +1729,12 @@ +@@ -1199,6 +1734,12 @@ case START_DESTROY_BLOCK: case ABORT_DESTROY_BLOCK: case STOP_DESTROY_BLOCK: @@ -929,7 +933,7 @@ this.player.gameMode.handleBlockBreakAction(blockposition, packetplayinblockdig_enumplayerdigtype, packet.getDirection(), this.player.level().getMaxY(), packet.getSequence()); this.player.connection.ackBlockChangesUpTo(packet.getSequence()); return; -@@ -1218,9 +1754,31 @@ +@@ -1218,9 +1759,31 @@ } } @@ -961,7 +965,7 @@ if (this.player.hasClientLoaded()) { this.player.connection.ackBlockChangesUpTo(packet.getSequence()); ServerLevel worldserver = this.player.serverLevel(); -@@ -1244,6 +1802,7 @@ +@@ -1244,6 +1807,7 @@ if (blockposition.getY() <= i) { if (this.awaitingPositionFromClient == null && worldserver.mayInteract(this.player, blockposition)) { @@ -969,7 +973,7 @@ InteractionResult enuminteractionresult = this.player.gameMode.useItemOn(this.player, worldserver, itemstack, enumhand, movingobjectpositionblock); if (enuminteractionresult.consumesAction()) { -@@ -1281,6 +1840,8 @@ +@@ -1281,6 +1845,8 @@ @Override public void handleUseItem(ServerboundUseItemPacket packet) { PacketUtils.ensureRunningOnSameThread(packet, this, this.player.serverLevel()); @@ -978,7 +982,7 @@ if (this.player.hasClientLoaded()) { this.ackBlockChangesUpTo(packet.getSequence()); ServerLevel worldserver = this.player.serverLevel(); -@@ -1296,6 +1857,47 @@ +@@ -1296,6 +1862,47 @@ this.player.absRotateTo(f, f1); } @@ -1026,7 +1030,7 @@ InteractionResult enuminteractionresult = this.player.gameMode.useItem(this.player, worldserver, itemstack, enumhand); if (enuminteractionresult instanceof InteractionResult.Success) { -@@ -1321,7 +1923,7 @@ +@@ -1321,7 +1928,7 @@ Entity entity = packet.getEntity(worldserver); if (entity != null) { @@ -1035,7 +1039,7 @@ return; } } -@@ -1342,6 +1944,13 @@ +@@ -1342,6 +1949,13 @@ @Override public void onDisconnect(DisconnectionDetails info) { @@ -1049,7 +1053,7 @@ ServerGamePacketListenerImpl.LOGGER.info("{} lost connection: {}", this.player.getName().getString(), info.reason().getString()); this.removePlayerFromWorld(); super.onDisconnect(info); -@@ -1349,10 +1958,20 @@ +@@ -1349,10 +1963,20 @@ private void removePlayerFromWorld() { this.chatMessageChain.close(); @@ -1072,7 +1076,7 @@ this.player.getTextFilter().leave(); } -@@ -1367,7 +1986,16 @@ +@@ -1367,7 +1991,16 @@ @Override public void handleSetCarriedItem(ServerboundSetCarriedItemPacket packet) { PacketUtils.ensureRunningOnSameThread(packet, this, this.player.serverLevel()); @@ -1089,7 +1093,7 @@ if (this.player.getInventory().selected != packet.getSlot() && this.player.getUsedItemHand() == InteractionHand.MAIN_HAND) { this.player.stopUsingItem(); } -@@ -1376,11 +2004,18 @@ +@@ -1376,11 +2009,18 @@ this.player.resetLastActionTime(); } else { ServerGamePacketListenerImpl.LOGGER.warn("{} tried to set an invalid carried item", this.player.getName().getString()); @@ -1108,7 +1112,7 @@ Optional optional = this.unpackAndApplyLastSeen(packet.lastSeenMessages()); if (!optional.isEmpty()) { -@@ -1394,27 +2029,44 @@ +@@ -1394,27 +2034,44 @@ return; } @@ -1160,7 +1164,7 @@ ParseResults parseresults = this.parseCommand(command); if (this.server.enforceSecureProfile() && SignableCommand.hasSignableArguments(parseresults)) { -@@ -1431,19 +2083,37 @@ +@@ -1431,19 +2088,37 @@ if (!optional.isEmpty()) { this.tryHandleChat(packet.command(), () -> { @@ -1202,7 +1206,7 @@ } catch (SignedMessageChain.DecodeException signedmessagechain_a) { this.handleMessageDecodeFailure(signedmessagechain_a); return; -@@ -1451,10 +2121,10 @@ +@@ -1451,10 +2126,10 @@ CommandSigningContext.SignedArguments commandsigningcontext_a = new CommandSigningContext.SignedArguments(map); @@ -1215,7 +1219,7 @@ } private void handleMessageDecodeFailure(SignedMessageChain.DecodeException exception) { -@@ -1530,14 +2200,20 @@ +@@ -1530,14 +2205,20 @@ return com_mojang_brigadier_commanddispatcher.parse(command, this.player.createCommandSourceStack()); } @@ -1240,7 +1244,7 @@ } } -@@ -1566,6 +2242,127 @@ +@@ -1566,6 +2247,127 @@ return false; } @@ -1368,7 +1372,7 @@ private PlayerChatMessage getSignedMessage(ServerboundChatPacket packet, LastSeenMessages lastSeenMessages) throws SignedMessageChain.DecodeException { SignedMessageBody signedmessagebody = new SignedMessageBody(packet.message(), packet.timeStamp(), packet.salt(), lastSeenMessages); -@@ -1573,13 +2370,42 @@ +@@ -1573,13 +2375,42 @@ } private void broadcastChatMessage(PlayerChatMessage message) { @@ -1416,7 +1420,7 @@ this.disconnect((Component) Component.translatable("disconnect.spam")); } -@@ -1601,7 +2427,33 @@ +@@ -1601,7 +2432,33 @@ @Override public void handleAnimate(ServerboundSwingPacket packet) { PacketUtils.ensureRunningOnSameThread(packet, this, this.player.serverLevel()); @@ -1450,7 +1454,7 @@ this.player.swing(packet.getHand()); } -@@ -1609,6 +2461,29 @@ +@@ -1609,6 +2466,29 @@ public void handlePlayerCommand(ServerboundPlayerCommandPacket packet) { PacketUtils.ensureRunningOnSameThread(packet, this, this.player.serverLevel()); if (this.player.hasClientLoaded()) { @@ -1480,7 +1484,7 @@ this.player.resetLastActionTime(); Entity entity; PlayerRideableJumping ijumpable; -@@ -1616,6 +2491,11 @@ +@@ -1616,6 +2496,11 @@ switch (packet.getAction()) { case PRESS_SHIFT_KEY: this.player.setShiftKeyDown(true); @@ -1492,7 +1496,7 @@ break; case RELEASE_SHIFT_KEY: this.player.setShiftKeyDown(false); -@@ -1691,6 +2571,12 @@ +@@ -1691,6 +2576,12 @@ } public void sendPlayerChatMessage(PlayerChatMessage message, ChatType.Bound params) { @@ -1505,7 +1509,7 @@ this.send(new ClientboundPlayerChatPacket(message.link().sender(), message.link().index(), message.signature(), message.signedBody().pack(this.messageSignatureCache), message.unsignedContent(), message.filterMask(), params)); this.addPendingMessage(message); } -@@ -1703,6 +2589,13 @@ +@@ -1703,6 +2594,13 @@ return this.connection.getRemoteAddress(); } @@ -1519,7 +1523,7 @@ public void switchToConfig() { this.waitingForSwitchToConfig = true; this.removePlayerFromWorld(); -@@ -1718,9 +2611,17 @@ +@@ -1718,9 +2616,17 @@ @Override public void handleInteract(ServerboundInteractPacket packet) { PacketUtils.ensureRunningOnSameThread(packet, this, this.player.serverLevel()); @@ -1537,7 +1541,7 @@ this.player.resetLastActionTime(); this.player.setShiftKeyDown(packet.isUsingSecondaryAction()); -@@ -1733,20 +2634,58 @@ +@@ -1733,20 +2639,58 @@ if (this.player.canInteractWithEntity(axisalignedbb, 3.0D)) { packet.dispatch(new ServerboundInteractPacket.Handler() { @@ -1600,7 +1604,7 @@ } } -@@ -1755,19 +2694,20 @@ +@@ -1755,19 +2699,20 @@ @Override public void onInteraction(InteractionHand hand) { @@ -1624,7 +1628,7 @@ label23: { if (entity instanceof AbstractArrow) { -@@ -1785,6 +2725,11 @@ +@@ -1785,6 +2730,11 @@ } ServerGamePacketListenerImpl.this.player.attack(entity); @@ -1636,7 +1640,7 @@ return; } } -@@ -1795,7 +2740,26 @@ +@@ -1795,7 +2745,26 @@ }); } } @@ -1663,7 +1667,7 @@ } } -@@ -1809,7 +2773,7 @@ +@@ -1809,7 +2778,7 @@ case PERFORM_RESPAWN: if (this.player.wonGame) { this.player.wonGame = false; @@ -1672,7 +1676,7 @@ this.resetPosition(); CriteriaTriggers.CHANGED_DIMENSION.trigger(this.player, Level.END, Level.OVERWORLD); } else { -@@ -1817,11 +2781,11 @@ +@@ -1817,11 +2786,11 @@ return; } @@ -1686,7 +1690,7 @@ } } break; -@@ -1833,16 +2797,27 @@ +@@ -1833,16 +2802,27 @@ @Override public void handleContainerClose(ServerboundContainerClosePacket packet) { @@ -1716,7 +1720,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 +2830,284 @@ +@@ -1855,7 +2835,284 @@ boolean flag = packet.getStateId() != this.player.containerMenu.getStateId(); this.player.containerMenu.suppressRemoteUpdates(); @@ -2002,7 +2006,7 @@ ObjectIterator objectiterator = Int2ObjectMaps.fastIterable(packet.getChangedSlots()).iterator(); while (objectiterator.hasNext()) { -@@ -1901,8 +3153,22 @@ +@@ -1901,8 +3158,22 @@ return; } @@ -2026,7 +2030,7 @@ if (containerrecipebook_a == RecipeBookMenu.PostPlaceAction.PLACE_GHOST_RECIPE) { this.player.connection.send(new ClientboundPlaceGhostRecipePacket(this.player.containerMenu.containerId, craftingmanager_d.display().display())); } -@@ -1917,6 +3183,7 @@ +@@ -1917,6 +3188,7 @@ @Override public void handleContainerButtonClick(ServerboundContainerButtonClickPacket packet) { PacketUtils.ensureRunningOnSameThread(packet, this, this.player.serverLevel()); @@ -2034,7 +2038,7 @@ this.player.resetLastActionTime(); if (this.player.containerMenu.containerId == packet.containerId() && !this.player.isSpectator()) { if (!this.player.containerMenu.stillValid(this.player)) { -@@ -1945,7 +3212,44 @@ +@@ -1945,6 +3217,43 @@ boolean flag1 = packet.slotNum() >= 1 && packet.slotNum() <= 45; boolean flag2 = itemstack.isEmpty() || itemstack.getCount() <= itemstack.getMaxStackSize(); @@ -2042,7 +2046,7 @@ + // CraftBukkit start - Call click event + InventoryView inventory = this.player.inventoryMenu.getBukkitView(); + org.bukkit.inventory.ItemStack item = CraftItemStack.asBukkitCopy(packet.itemStack()); - ++ + SlotType type = SlotType.QUICKBAR; + if (flag) { + type = SlotType.OUTSIDE; @@ -2075,11 +2079,10 @@ + } + } + // CraftBukkit end -+ + if (flag1 && flag2) { this.player.inventoryMenu.getSlot(packet.slotNum()).setByPlayer(itemstack); - this.player.inventoryMenu.setRemoteSlot(packet.slotNum(), itemstack); -@@ -1964,7 +3268,19 @@ +@@ -1964,7 +3273,19 @@ @Override public void handleSignUpdate(ServerboundSignUpdatePacket packet) { @@ -2100,7 +2103,7 @@ this.filterTextPacket(list).thenAcceptAsync((list1) -> { this.updateSignText(packet, list1); -@@ -1972,6 +3288,7 @@ +@@ -1972,6 +3293,7 @@ } private void updateSignText(ServerboundSignUpdatePacket packet, List signText) { @@ -2108,7 +2111,7 @@ this.player.resetLastActionTime(); ServerLevel worldserver = this.player.serverLevel(); BlockPos blockposition = packet.getPos(); -@@ -1993,7 +3310,17 @@ +@@ -1993,7 +3315,17 @@ @Override public void handlePlayerAbilities(ServerboundPlayerAbilitiesPacket packet) { PacketUtils.ensureRunningOnSameThread(packet, this, this.player.serverLevel()); @@ -2127,7 +2130,7 @@ } @Override -@@ -2002,6 +3329,7 @@ +@@ -2002,6 +3334,7 @@ boolean flag = this.player.isModelPartShown(PlayerModelPart.HAT); this.player.updateOptions(packet.information()); @@ -2135,7 +2138,7 @@ if (this.player.isModelPartShown(PlayerModelPart.HAT) != flag) { this.server.getPlayerList().broadcastAll(new ClientboundPlayerInfoUpdatePacket(ClientboundPlayerInfoUpdatePacket.Action.UPDATE_HAT, this.player)); } -@@ -2058,7 +3386,7 @@ +@@ -2058,7 +3391,7 @@ if (!this.waitingForSwitchToConfig) { throw new IllegalStateException("Client acknowledged config, but none was requested"); } else { @@ -2144,7 +2147,7 @@ } } -@@ -2083,8 +3411,10 @@ +@@ -2083,8 +3416,10 @@ }); }