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.
This commit is contained in:
Spottedleaf 2020-06-12 16:51:39 -07:00
parent c637868aef
commit 4c32788db0

View file

@ -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<LastSeenMessages> optional = this.unpackAndApplyLastSeen(packet.lastSeenMessages());
if (!optional.isEmpty()) {
@@ -1394,27 +2029,44 @@
@@ -1394,27 +2034,44 @@
return;
}
@ -1160,7 +1164,7 @@
ParseResults<CommandSourceStack> 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<FilteredText> 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 @@
});
}