Updated Moving-check:

- fixed potential exploit to partially bypass nofly protection in
certain cases
- removed some legacy workarounds that are probably no longer needed
- teleport players after violations closer to the ground, if the
original target location is high above ground
- fixed bug occuring while reading config file if String parameters
are missing in the file

plugin.yml

@@ -3,7 +3,7 @@ name: NoCheat
 author: Evenprime
 
 main: cc.co.evenprime.bukkit.nocheat.NoCheat
-version: 1.00b
+version: 1.01
 
 commands:
   nocheat:

src/cc/co/evenprime/bukkit/nocheat/checks/MovingCheck.java

@@ -94,29 +94,24 @@ public class MovingCheck extends Check {
 
 		// Get the two locations of the event
 		final Location to = event.getTo();
-		Location from = event.getFrom();
 
-		// The use of event.getFrom() is intentional
+		// the from location of the event may be different from the location the player 
+		// actually was - choose appropriately
+		Location from = data.teleportTo != null ? data.teleportTo : event.getFrom();
+		data.teleportTo = null;
+		
 		if(shouldBeIgnored(player, data, from, to)) {
 			statisticElapsedTimeNano += System.nanoTime() - startTime;
 			statisticTotalEvents++;
 			return;
 		}
 
-		// WORKAROUND for changed PLAYER_MOVE logic
-		if(data.teleportTo != null) {
-			from = data.teleportTo;
-			data.teleportTo = null;
-		}
-
-
 		// First check the distance the player has moved horizontally
 		final double xDistance = from.getX()-to.getX();
 		final double zDistance = from.getZ()-to.getZ();
 
 		double combined = Math.sqrt((xDistance*xDistance + zDistance*zDistance));
 
-
 		// If the target is a bed and distance not too big, allow it
 		// Bukkit prevents using blocks behind walls already, so I don't have to check for that
 		if(to.getWorld().getBlockTypeIdAt(to) == Material.BED_BLOCK.getId() && combined < 8.0D) {
@@ -125,23 +120,7 @@ public class MovingCheck extends Check {
 			return;
 		}
 
-
-		final boolean onGroundFrom = playerIsOnGround(from, 0.0D);		
-
-		final boolean canFly;
-		if(allowFlying || plugin.hasPermission(player, PermissionData.PERMISSION_FLYING)) {
-			canFly = true;
-			data.jumpPhase = 0;
-		}
-		else
-			canFly = false;
-
-		final boolean canFakeSneak;
-		if(allowFakeSneak || plugin.hasPermission(player, PermissionData.PERMISSION_FAKESNEAK)) {
-			canFakeSneak = true;
-		}
-		else
-			canFakeSneak = false;
+		final boolean canFakeSneak = allowFakeSneak || plugin.hasPermission(player, PermissionData.PERMISSION_FAKESNEAK);
 
 		/**** Horizontal movement check START ****/
 
@@ -183,6 +162,8 @@ public class MovingCheck extends Check {
 
 		// The location we'd use as a new setback if there are no violations
 		Location newSetBack = null;
+		
+		final boolean onGroundFrom = playerIsOnGround(from, 0.0D);
 
 		double limit = calculateVerticalLimit(data, onGroundFrom);
 
@@ -208,6 +189,8 @@ public class MovingCheck extends Check {
 		else
 		{
 			final Location l;
+			
+			final boolean canFly = allowFlying || plugin.hasPermission(player, PermissionData.PERMISSION_FLYING);
 
 			if(data.setBackPoint == null || canFly)
 				l = from;
@@ -326,17 +309,16 @@ public class MovingCheck extends Check {
 		}
 		// Something or someone moved the player without causing a move event - Can't do much with that
 		else if(!(x == l.getX() && z == l.getZ() && y == l.getY())){
-			resetData(data, to);
 			return true;
 		}
-		// Player was respawned just before, this causes all kinds of weirdness - better ignore it
+		// Player respawned just before, this causes all kinds of weirdness - better ignore it
 		else if(data.respawned) {
 			data.respawned = false;
 			return true;
 		}
-		// Player changed the world before, which makes any location information basically useless
-		else if(data.worldChanged) {
-			data.worldChanged = false;
+		// Player respawned just before, this causes all kinds of weirdness - better ignore it
+		else if(data.worldChanged > 0) {
+			data.worldChanged--;
 			return true;
 		}
 		// Player is inside a vehicle, this causes all kinds of weirdness - better ignore it
@@ -379,8 +361,8 @@ public class MovingCheck extends Check {
 	}
 
 	/**
-	 * Call this when a player got successfully teleported with the corresponding event to set new "setback" points
-	 * and reset data (if necessary)
+	 * Call this when a player got successfully teleported with the corresponding event to adjust stored
+	 * data to the new situation
 	 * 
 	 * @param event
 	 */
@@ -388,27 +370,15 @@ public class MovingCheck extends Check {
 
 		MovingData data = MovingData.get(event.getPlayer());
 
-		if(event.getTo().equals(data.teleportInitializedByMe)) { // My plugin requested this teleport while handling another event
-
-			// DANGEROUS, but I have no real choice on that one thanks to Essentials jail simply blocking ALL kinds of teleports
-			// even the respawn teleport, the player moved wrongly teleport, the "get player out of the void" teleport", ...
-
-			// TODO: Make this optional OR detect Essentials and make this dependent on essential
-			event.setCancelled(false);
-			data.teleportInitializedByMe = null;
-			//data.movingTeleportTo = event.getTo();
-		}
-		else if(!event.isCancelled()) {
-			// If it wasn't our plugin that ordered the teleport, forget (almost) all our information and start from scratch
-			resetData(data, event.getTo());
-
-			if(event.getFrom().getWorld().equals(event.getTo().getWorld())) {
-				// WORKAROUND for changed PLAYER_MOVE logic - I need to remember the "to" location of teleports and use it as a from-Location
-				// for the move event that comes next
-				data.teleportTo = event.getTo();
+		if(!event.isCancelled()) {
+			data.lastLocation = event.getTo();
+			data.teleportTo = event.getTo();
+			data.jumpPhase = 0;
+			data.setBackPoint = event.getTo();
+			
+			if(!event.getFrom().getWorld().getName().equals(event.getTo().getWorld().getName())) {
+				data.worldChanged = 2; // ignore two events, because teleporting through nether portals is really weird -.-
 			}
-			else
-				data.worldChanged = true;
 		}
 	}
 
@@ -418,7 +388,6 @@ public class MovingCheck extends Check {
 	 */
 	public void respawned(PlayerRespawnEvent event) {
 		MovingData data = MovingData.get(event.getPlayer());
-
 		data.respawned = true;
 
 	}
@@ -466,7 +435,7 @@ public class MovingCheck extends Check {
 						if(data.highestLogLevel.intValue() < ((LogAction)a).level.intValue()) data.highestLogLevel = ((LogAction)a).level;
 					}
 					else if(!cancelled && a instanceof CancelAction) {
-						resetPlayer(event, from);
+						resetPlayer(event);
 						cancelled = true;
 					}
 					else if(a instanceof CustomAction)
@@ -495,30 +464,26 @@ public class MovingCheck extends Check {
 	}
 
 	/** 
-	 * Return the player to a stored location or if that is not available,
-	 * the previous location.
-	 * @param data
+	 * Return the player to the stored setBackPoint location
 	 * @param event
 	 */
-	private void resetPlayer(PlayerMoveEvent event, Location from) {
+	private void resetPlayer(PlayerMoveEvent event) {
 
 		MovingData data = MovingData.get(event.getPlayer());
 
-		// Reset the jumpphase. We choose the setback-point such that it should be
-		// on solid ground, but in case it isn't (maybe the ground is gone now) we
-		// still have to allow the player some freedom with vertical movement due
-		// to lost vertical momentum to prevent him from getting stuck
-
-		if(data.setBackPoint == null) data.setBackPoint = from;
-
-		// Set a flag that gets used while handling teleport events (to determine if
-		// it was my teleport or someone else'
-		Location t = data.setBackPoint;
-
-		t = new Location(t.getWorld(), t.getX(),  t.getY(), t.getZ(), event.getTo().getYaw(), event.getTo().getPitch());
-		data.teleportInitializedByMe = t;
-
-		resetData(data, t);
+		// Make a modified copy of the setBackPoint to prevent other plugins from accidentally modifying it
+		// and keep the current pitch and yaw (setbacks "feel" better that way).
+		
+		double y = data.setBackPoint.getY();
+		// search for the first solid block up to 5 blocks below the setbackpoint and teleport the player there
+		for(int i = 0; i < 20; i++) {
+			if(playerIsOnGround(data.setBackPoint, -0.5*i)) {
+				y -= 0.5*i;
+				break;
+			}
+		}	
+		
+		Location t = new Location(data.setBackPoint.getWorld(), data.setBackPoint.getX(), y, data.setBackPoint.getZ(), event.getTo().getYaw(), event.getTo().getPitch());
 
 		// Only reset player and cancel event if teleport is successful
 		if(event.getPlayer().teleport(t)) {
@@ -528,7 +493,6 @@ public class MovingCheck extends Check {
 			event.setTo(t);
 
 			event.setCancelled(true);
-
 		}
 	}
 
@@ -636,18 +600,6 @@ public class MovingCheck extends Check {
 		return (int) (floor - d4);
 	}
 
-	/**
-	 * Reset all temporary information of this check
-	 * @param data
-	 * @param l
-	 */
-	private void resetData(MovingData data, Location l) {
-
-		data.setBackPoint = l;
-		data.jumpPhase = 0;
-		data.teleportTo = null;
-	}
-
 	@Override
 	public void configure(NoCheatConfiguration config) {
 

src/cc/co/evenprime/bukkit/nocheat/data/MovingData.java

@@ -14,14 +14,18 @@ public class MovingData {
 	public double horizFreedom = 0.0D;
 	public double vertFreedom = 0.0D;
 	public int vertFreedomCounter = 0;
+	
+	// setbackpoint is a recommendation - try to teleport to first solid block below it
+	// for better effect
 	public Location setBackPoint = null;
+	
 	public int summaryTask = -1;
 	public Level highestLogLevel = null;
 	public double maxYVelocity = 0.0D;
 	public int sneakingFreedomCounter = 10;
 	public double sneakingLastDistance = 0.0D;
 
-	public boolean worldChanged = false;
+	public int worldChanged = 0;
 	public boolean respawned = false;
 
 	// WORKAROUND for changed PLAYER_MOVE logic

src/cc/co/evenprime/bukkit/nocheat/listeners/MovingPlayerMonitor.java

@@ -39,7 +39,7 @@ public class MovingPlayerMonitor extends PlayerListener {
 
 	@Override
 	public void onPlayerMove(PlayerMoveEvent event) {
-		if(!event.getPlayer().isInsideVehicle()) {
+		if(!event.isCancelled() && !event.getPlayer().isInsideVehicle()) {
 			MovingData data = MovingData.get(event.getPlayer());
 			data.lastLocation = event.getTo();
 

src/cc/co/evenprime/bukkit/nocheat/yaml/SimpleYaml.java

@@ -129,7 +129,9 @@ public class SimpleYaml {
 
 	public static String getString(String path, String defaultValue, Map<String, Object> node) {
 		try {
-			return (String) getProperty(path, node);
+			String result = (String) getProperty(path, node);
+			if(result == null) return defaultValue;
+			return result;
 		}
 		catch(Exception e) {
 			return defaultValue;