Ad Astra

Ad Astra

23M Downloads

[Bug]: ServerboundLandPacket input validation for dimension.

tacopeland opened this issue ยท 3 comments

commented

Bug Description

It is (probably) possible to send a ServerboundLandPacket containing any arbitrary dimension that counts as a planet and have ServerboundLandPacket.Handler.handle() send a player to this dimension, even if the planet is in another solar system, or if the planet is a space station. If it's a space station, it isn't checked whether a player owns it.

How to Reproduce?

  1. Fly a rocket to space until the PlanetsMenu GUI is open.
  2. Send this packet to the server using a hacked client.
  3. Get teleported to another dimension.

Expected Behavior

Only teleport a player if their rocket can do so (that is, if the rocket's tier is high enough, if they're in the same solar system, and if it's a space station, whether the player owns it or their group owns it).

Version

develop branch (1.15.7)

Mod Loader Version

None (common code)

Mod Loader

N/A

Logs or additional context

No response

Code of Conduct

  • I have searched the issue tracker and confirmed that the issue has not been reported
  • I have checked the FAQ (if one is present) and my issue is not listed
  • I have verified that I am using the latest version of the mod
  • I have verified that I have installed all the required dependencies for the mod
  • I have verified that I do not have Optifine installed. We will close issues if we detect Optifine is in your pack. We cannot fix Optifine issues. Please do not report issues with Optifine present.
commented

That looks good.

The only other things I can see are that setting the flag URL in ServerboundSetFlagUrlPacket is done without a check for the distance between the player and the flag, and there are a few unhandled exceptions:

  • In ServerboundClearFluidTankPacket you can set a negative tank and get an IndexOutOfBoundsException from List.get(). Same with ServerboundSetSideConfigPacket's configIndex.
  • You can probably send some number greater than the number of enum constants in an enum, such as RedstoneControl, and inside the ByteCodec you'll get an ArrayOutOfBoundsException.
commented

Here's what I've done to address this:

  • The tier is now checked serverside to ensure the rocket tier is at or above the target planet tier.
  • ServerboundLandPacket now only allows teleportation to valid planets (excluding orbit dimensions).
  • The rocket now must be above the "Atmosphere Leave" config value (default y600).

for the ServerboundConstructSpaceStationPacket and ServerboundLandOnSpaceStationPacket, these now have the same checks and only allow the target dimension to be an orbit dimension.

These checks are exempted for OP players, creative, and spectator players.

As for landing on space stations, the ServerboundLandOnSpaceStationPacket already checks if the player owns the space station. See:

private static boolean isAllowed(ServerPlayer player, ServerLevel level, ChunkPos targetPos) {
Set<SpaceStation> stations = new HashSet<>(SpaceStationHandler.getOwnedSpaceStations(player, level));

If there are any other checks I'm missing, let me know.

commented

As for the redstone one, I'm pretty sure there are multiple instances where you can do that in vanilla, although I'm not sure if it's caught.