badpackets N, aimmodulo360, timera when leaving a boat

[spring-dependency-management]

Describe the false positive and how to replicate it

leave a boat while you are holding down the movement keys and moving in it, simple as, paper 1.21.4 server, 1.21.4 client

it will flag bad packets n, and sometimes aimmodulo360 and timer A

Grim version

1ce5a53

Server version

This server is running Paper version 1.21.4-DEV-master@71a367e (2024-12-08T13:59:53Z) (Implementing API version 1.21.4-R0.1-SNAPSHOT)
You are 11 version(s) behind
Download the new version at: https://papermc.io/downloads/paper
Previous version: 1.21.3-DEV-c2294d7 (MC: 1.21.3)

Client version

1.21.4

Plugins

Grim

[SamB440]

Feel free to figure out how teleports work in 1.21.4.

[spring-dependency-management]

Feel free to figure out how teleports work in 1.21.4.

I don't have a clear picture as to how mounting/dismounting vehicles works pre 1.21.4 because I never looked into it, but I will describe precisely how it works in 1.21.4 and you can see if there are any discrepancies/new stuff

SERVER:
Leaving boats appears to be server-side, in player rideTick(), the server checks player.wantsToStopRiding() which simply checks

isShiftKeyDown()
(isShiftKeyDown is controlled by the client in 2 ways, when they send ServerboundPlayerCommandPacket with either press shift key or release shift key actions and they can change whether their shift key is down inside of ServerboundInteractPacket isUsingSecondaryAction() flag, grim already has all of this logic covered)

it then triggers stopRiding() which calls

removeVehicle() which removes the entities vehicle reference, and removes the entity from the vehicles passenger list.

then it runs dismountVehicle() for the entity, which finds a free position to place the player nearby and calls teleportTo() with said position, this calls setPosRaw() which as the name suggests sets the entities position values, yaw and pitch, not informing the client in any way.

At this point in time, the client should still be on the vehicle sneaking, at the same position, because the server has yet to tell them anything

remember removeVehicle() from earlier though? the server ticks the entity tracker newTrackerTick() before it ticks entities, so in the tick AFTER stopRiding(), sendChanges() is called on the vehicle entity by the tracker which checks if the passenger list has changed, which it was the previous tick, this causes it to send a ClientboundSetPassengersPacket to the client, and finally it sends a ClientboundPlayerPositionPacket to the player passenger informing them of their current server-side x y and z, here is the logic for sending that ClientboundPlayerPositionPacket packet:

Side note, the x y and z we are about to inform them about always be the same exact position as what we set it to last tick, as packet processing for the last tick happened before rideTick() and packet processing for the current tick happens after the entity tracker tick we are currently in. Just in case you thought the player might be able to move during the tick gap between removing passengers and sending the update packet

teleport() is called which runs some bukkit event logic, unimportant for now, then it calls
internalTeleport() which sets justTeleported flag to true, unimportant,
it will also set a timestamp of the current tick called awaitingTeleportTime,
and it will set the players raw pos again to the teleport position, as it may have been changed by the bukkit teleport event.
It will then set a variable in the player awaitingPositionFromClient to the target teleport location, in this case still the dismount point location.
It will then finally dispatches the ClientboundPlayerPositionPacket to the client.
This ClientboundPlayerPositionPacket contains the position of the player on the server (obviously) but also includes another field which is incremented called awaitingTeleport, basically a transaction ID, this will be important later

Remember that awaitingPositionFromClient field? its logic is important, while the client has this field set they are in "pending teleport" state

• their ServerboundUseItemOnPacket packets are ignored
• their ServerboundMoveVehiclePacket packets are ignored
• their ServerboundMovePlayerPacket packets are ignored

if awaitingTeleportTime exceeds 20 ticks while in pending teleport state, i.e we haven't gotten back a response to the teleport from the client (ServerboundAcceptTeleportationPacket) in over a second, teleport() will be re-ran basically telling them again "this is your position" this logic is disabled on paper however, as for clients with over 1000ms ping, this creates a buggy experience.

lol another side note, issue found in paper while reviewing code, awaitingPositionFromClient redundantly checked twice PaperMC/Paper#11895

CLIENT:
Now we are on the client side, the client receives a ClientboundSetPassengersPacket packet which causes it to stopRiding() for all passengers to sync it with the server state.
ClientboundPlayerPositionPacket when recieved by the client will first check if the player is a passenger of any sorts, which will return false because setpassengerspacket is sent first and it removed the player as a passenger.
it will then call setValuesFromPositionPacket() which sets the clients posraw in the exact same way as the server did earlier, finally we are in sync, the player finally sends a ServerboundAcceptTeleportationPacket with the transaction ID from earlier back to the server and it also sends a ServerboundMovePlayerPacket with the players current client updated position and yaw

SERVER:
If the transaction ID sent back from the client matches the awaitingTeleport, the server will once again set the raw pos to awaitingPositionFromClient, seemingly redundantly though because I don't know how their position could have changed but better safe than sorry I guess, and as for that serverboundmoveplayerpacket the player sends, there is no extra special handling, theoretically the player should appear to have moved 0 distance.

This is all cool in theory, but lets look at the actual packet sequence in flight from the moment a player presses shift to the moment they are fully out of the boat (excluding tick_end/ping pong packets)

0ms C->S BOAT_MOVE [left(boolean):false],[right(boolean):false],

// client: hey, i am sneaking
2ms C->S ENTITY_ACTION [id(int):4664],[action(net.minecraft.network.protocol.game.ServerboundPlayerCommandPacket$Action):PRESS_SHIFT_KEY],[data(int):0],

// (client is still in boat as far as they are concerned, still send vehicle packets)
2ms C->S STEER_VEHICLE [input(net.minecraft.world.entity.player.Input):Input[forward=false, backward=false, left=false, right=false, jump=false, shift=true, sprint=false]],
3ms C->S LOOK [x(double):0.0],[y(double):0.0],[z(double):0.0],[yRot(float):0.0],[xRot(float):37.99643],[onGround(boolean):false],[horizontalCollision(boolean):false],[hasPos(boolean):false],[hasRot(boolean):true],
4ms C->S VEHICLE_MOVE [position(net.minecraft.world.phys.Vec3):(1000039.362070985, 89.0, 1000000.6886316518)],[yRot(float):0.0],[xRot(float):0.0],[onGround(boolean):true],
51ms C->S BOAT_MOVE [left(boolean):false],[right(boolean):false],
52ms C->S LOOK [x(double):0.0],[y(double):0.0],[z(double):0.0],[yRot(float):0.0],[xRot(float):37.99643],[onGround(boolean):false],[horizontalCollision(boolean):false],[hasPos(boolean):false],[hasRot(boolean):true],
53ms C->S VEHICLE_MOVE [position(net.minecraft.world.phys.Vec3):(1000039.362070985, 89.0, 1000000.6886316518)],[yRot(float):0.0],[xRot(float):0.0],[onGround(boolean):true],

// server: hey, you are sneaking now
53ms S->C ENTITY_METADATA [id(int):4664],[packedItems(java.util.List):[DataValue[id=0, serializer=net.minecraft.network.syncher.EntityDataSerializer$$Lambda/0x00007f6d9c856278@6246fd8, value=2]]],

// client: okay, im still in the boat though
101ms C->S BOAT_MOVE [left(boolean):false],[right(boolean):false],
101ms C->S ENTITY_ACTION [id(int):4664],[action(net.minecraft.network.protocol.game.ServerboundPlayerCommandPacket$Action):RELEASE_SHIFT_KEY],[data(int):0],
101ms C->S STEER_VEHICLE [input(net.minecraft.world.entity.player.Input):Input[forward=false, backward=false, left=false, right=false, jump=false, shift=false, sprint=false]],
102ms C->S LOOK [x(double):0.0],[y(double):0.0],[z(double):0.0],[yRot(float):0.0],[xRot(float):37.99643],[onGround(boolean):false],[horizontalCollision(boolean):false],[hasPos(boolean):false],[hasRot(boolean):true],
103ms C->S VEHICLE_MOVE [position(net.minecraft.world.phys.Vec3):(1000039.362070985, 89.0, 1000000.6886316518)],[yRot(float):0.0],[xRot(float):0.0],[onGround(boolean):true],

// server: you aren't sneaking anymore
103ms S->C ENTITY_METADATA [id(int):4664],[packedItems(java.util.List):[DataValue[id=0, serializer=net.minecraft.network.syncher.EntityDataSerializer$$Lambda/0x00007f6d9c856278@6246fd8, value=0]]],

// server: infact, the boat you are in, no longer has any passengers
104ms S->C MOUNT [vehicle(int):4842],[passengers([I):[I@33d62f44],

// server: and you belong here, out of the boat, here is position with transaction id 3
104ms S->C POSITION [id(int):3],[change(net.minecraft.world.entity.PositionMoveRotation):PositionMoveRotation[position=(1000039.362070985, 89.0, 1000001.9609084638), deltaMovement=(0.0, 0.0, 0.0), yRot=0.0, xRot=37.99643]],[relatives(java.util.Set):[]],

// server: just reminding you the boat is at this location also
104ms S->C ENTITY_POSITION_SYNC [id(int):4842],[values(net.minecraft.world.entity.PositionMoveRotation):PositionMoveRotation[position=(1000039.362070985, 89.0, 1000000.6886316518), deltaMovement=(0.0, 0.0, 0.0), yRot=0.0, xRot=0.0]],[onGround(boolean):true],

// client: packet hasn't arrived to me yet, i am still in the boat
155ms C->S BOAT_MOVE [left(boolean):false],[right(boolean):false],

// client: oh, i accept your transaction 3 position sync, but for some reason my next packet isn't the pos rot I should be sending...
197ms C->S TELEPORT_ACCEPT [id(int):3],

// server: heres another useless packet reminding you that the boat has in-fact NOT moved
253ms S->C ENTITY_POSITION_SYNC [id(int):4842],[values(net.minecraft.world.entity.PositionMoveRotation):PositionMoveRotation[position=(1000039.362070985, 89.0, 1000000.6886316518), deltaMovement=(0.0, -0.0, 0.0), yRot=0.0, xRot=0.0]],[onGround(boolean):true],

// ... its another teleport accept for a weird NEGATIVE transaction id sent from grim! ??????????
280ms C->S TELEPORT_ACCEPT [id(int):-1114122193],
// here is that pos rot btw
281ms C->S POSITION_LOOK [x(double):1000039.362070985],[y(double):89.0],[z(double):1000001.9609084638],[yRot(float):0.0],[xRot(float):0.0],[onGround(boolean):false],[horizontalCollision(boolean):false],[hasPos(boolean):true],[hasRot(boolean):true],

// client out of the boat as normal
303ms C->S LOOK [x(double):0.0],[y(double):0.0],[z(double):0.0],[yRot(float):0.0],[xRot(float):0.0],[onGround(boolean):false],[horizontalCollision(boolean):false],[hasPos(boolean):false],[hasRot(boolean):true],
357ms C->S GROUND [x(double):0.0],[y(double):0.0],[z(double):0.0],[yRot(float):0.0],[xRot(float):0.0],[onGround(boolean):true],[horizontalCollision(boolean):false],[hasPos(boolean):false],[hasRot(boolean):false],

// and server continues sending useless boat packets :sob:
403ms S->C ENTITY_POSITION_SYNC [id(int):4842],[values(net.minecraft.world.entity.PositionMoveRotation):PositionMoveRotation[position=(1000039.362070985, 89.0, 1000000.6886316518), deltaMovement=(0.0, -0.0, 0.0), yRot=0.0, xRot=0.0]],[onGround(boolean):true],
553ms S->C ENTITY_POSITION_SYNC [id(int):4842],[values(net.minecraft.world.entity.PositionMoveRotation):PositionMoveRotation[position=(1000039.362070985, 89.0, 1000000.6886316518), deltaMovement=(0.0, -0.0, 0.0), yRot=0.0, xRot=0.0]],[onGround(boolean):true],
703ms S->C ENTITY_POSITION_SYNC [id(int):4842],[values(net.minecraft.world.entity.PositionMoveRotation):PositionMoveRotation[position=(1000039.362070985, 89.0, 1000000.6886316518), deltaMovement=(0.0, -0.0, 0.0), yRot=0.0, xRot=0.0]],[onGround(boolean):true],
852ms S->C UPDATE_TIME [gameTime(long):298286835],[dayTime(long):298286835],[tickDayTime(boolean):true],
853ms S->C ENTITY_POSITION_SYNC [id(int):4842],[values(net.minecraft.world.entity.PositionMoveRotation):PositionMoveRotation[position=(1000039.362070985, 89.0, 1000000.6886316518), deltaMovement=(0.0, -0.0, 0.0), yRot=0.0, xRot=0.0]],[onGround(boolean):true],
1003ms S->C ENTITY_POSITION_SYNC [id(int):4842],[values(net.minecraft.world.entity.PositionMoveRotation):PositionMoveRotation[position=(1000039.362070985, 89.0, 1000000.6886316518), deltaMovement=(0.0, -0.0, 0.0), yRot=0.0, xRot=0.0]],[onGround(boolean):true],

it seems grim is messing with the packet flow somehow, and injecting its own transaction id that is causing the client to ignore the server one? wtf?

Grim/src/main/java/ac/grim/grimac/manager/SetbackTeleportUtil.java

Line 244 in 66f6bd7

// Min value is 10000000000000000000000000000000 in binary, this makes sure the number is always < 0

grim is responsible for sending the negative transactions

[spring-dependency-management]

This begs the question though, why is grim sending this transaction? I have located from where its being sent

Grim/src/main/java/ac/grim/grimac/manager/SetbackTeleportUtil.java

Line 309 in 66f6bd7

sendSetback(requiredSetBack);

so why is player.lastTransactionReceived.get() > teleportPos.getTransaction(), this should never happen? the packet sequence seems fine

   // server: your transaction id is 3
   2606 S->C POSITION [id(int):3],[change(net.minecraft.world.entity.PositionMoveRotation):PositionMoveRotation[position=(1000039.362070985, 89.0, 1000001.9609084638), deltaMovement=(0.0, 0.0, 0.0), yRot=0.0, xRot=41.208416]],[relatives(java.util.Set):[]],
  
  2606 S->C ENTITY_POSITION_SYNC [id(int):113],[values(net.minecraft.world.entity.PositionMoveRotation):PositionMoveRotation[position=(1000039.362070985, 89.0, 1000000.6886316518), deltaMovement=(0.0, 0.0, 0.0), yRot=0.0, xRot=0.0]],[onGround(boolean):true],
   2628 C->S BOAT_MOVE [left(boolean):false],[right(boolean):false],
   2629 C->S ENTITY_ACTION [id(int):112],[action(net.minecraft.network.protocol.game.ServerboundPlayerCommandPacket$Action):RELEASE_SHIFT_KEY],[data(int):0],
   2629 C->S STEER_VEHICLE [input(net.minecraft.world.entity.player.Input):Input[forward=false, backward=false, left=false, right=false, jump=false, shift=false, sprint=false]],
   2631 C->S CLIENT_TICK_END 
   2680 C->S BOAT_MOVE [left(boolean):false],[right(boolean):false],
   2686 C->S CLIENT_TICK_END 
   2694 C->S PONG [id(int):-29766],
   
   // client: my transaction id is 3
   2699 C->S TELEPORT_ACCEPT [id(int):3],
   // grim: WHAAAAT??? NO IT ISNT, YOU ARE BEING SETBACKED!
   // *Grim freaks out here and sends setback with id: -1686282818*

[spring-dependency-management]

oh wtf, this code is transactions, not teleport transactions, am mad confused lol, well ima take a look at this tomorrow maybe, hope I was of some use though so far

[spring-dependency-management]

This also flags aimduplicatelook, I didn't notice before because I had the check disabled