From 3bb53658daf73f2f19f31bfd16377cbf3e6b968f Mon Sep 17 00:00:00 2001
From: "Dylan K. Taylor" <odigiman@gmail.com>
Date: Wed, 20 May 2020 11:08:49 +0100
Subject: [PATCH] Do not allow remote clients to spawn themselves before we're
 ready this would have allowed clients to send
 SetLocalPlayerAsInitializedPacket at any time during the spawn sequence,
 which would have caused undefined behaviour around spawning logic.

---
 src/network/mcpe/NetworkSession.php           |  9 +++-
 .../mcpe/handler/PreSpawnPacketHandler.php    | 12 -----
 .../handler/SpawnResponsePacketHandler.php    | 47 +++++++++++++++++++
 3 files changed, 55 insertions(+), 13 deletions(-)
 create mode 100644 src/network/mcpe/handler/SpawnResponsePacketHandler.php

diff --git a/src/network/mcpe/NetworkSession.php b/src/network/mcpe/NetworkSession.php
index d5439a0a3..1a962a252 100644
--- a/src/network/mcpe/NetworkSession.php
+++ b/src/network/mcpe/NetworkSession.php
@@ -51,6 +51,7 @@ use pocketmine\network\mcpe\handler\LoginPacketHandler;
 use pocketmine\network\mcpe\handler\PacketHandler;
 use pocketmine\network\mcpe\handler\PreSpawnPacketHandler;
 use pocketmine\network\mcpe\handler\ResourcePacksPacketHandler;
+use pocketmine\network\mcpe\handler\SpawnResponsePacketHandler;
 use pocketmine\network\mcpe\protocol\AdventureSettingsPacket;
 use pocketmine\network\mcpe\protocol\AvailableCommandsPacket;
 use pocketmine\network\mcpe\protocol\ChunkRadiusUpdatedPacket;
@@ -619,16 +620,22 @@ class NetworkSession{
 		$this->createPlayer();
 
 		$this->setHandler(new PreSpawnPacketHandler($this->server, $this->player, $this));
+		$this->player->setImmobile(); //TODO: HACK: fix client-side falling pre-spawn
+
 		$this->logger->debug("Waiting for spawn chunks");
 	}
 
 	public function onTerrainReady() : void{
 		$this->logger->debug("Sending spawn notification, waiting for spawn response");
 		$this->sendDataPacket(PlayStatusPacket::create(PlayStatusPacket::PLAYER_SPAWN));
+		$this->setHandler(new SpawnResponsePacketHandler(function() : void{
+			$this->onSpawn();
+		}));
 	}
 
-	public function onSpawn() : void{
+	private function onSpawn() : void{
 		$this->logger->debug("Received spawn response, entering in-game phase");
+		$this->player->setImmobile(false); //TODO: HACK: we set this during the spawn sequence to prevent the client sending junk movements
 		$this->player->doFirstSpawn();
 		$this->setHandler(new InGamePacketHandler($this->player, $this));
 	}
diff --git a/src/network/mcpe/handler/PreSpawnPacketHandler.php b/src/network/mcpe/handler/PreSpawnPacketHandler.php
index d3ddb516f..e71e755b7 100644
--- a/src/network/mcpe/handler/PreSpawnPacketHandler.php
+++ b/src/network/mcpe/handler/PreSpawnPacketHandler.php
@@ -27,7 +27,6 @@ use pocketmine\data\bedrock\LegacyItemIdToStringIdMap;
 use pocketmine\network\mcpe\convert\RuntimeBlockMapping;
 use pocketmine\network\mcpe\NetworkSession;
 use pocketmine\network\mcpe\protocol\RequestChunkRadiusPacket;
-use pocketmine\network\mcpe\protocol\SetLocalPlayerAsInitializedPacket;
 use pocketmine\network\mcpe\protocol\StartGamePacket;
 use pocketmine\network\mcpe\protocol\types\DimensionIds;
 use pocketmine\network\mcpe\StaticPacketCache;
@@ -84,9 +83,6 @@ class PreSpawnPacketHandler extends PacketHandler{
 
 		$this->session->sendDataPacket(StaticPacketCache::getInstance()->getAvailableActorIdentifiers());
 		$this->session->sendDataPacket(StaticPacketCache::getInstance()->getBiomeDefs());
-
-		$this->player->setImmobile(); //HACK: fix client-side falling pre-spawn
-
 		$this->session->syncAttributes($this->player, $this->player->getAttributeMap()->getAll());
 		$this->session->syncAvailableCommands();
 		$this->session->syncAdventureSettings($this->player);
@@ -108,12 +104,4 @@ class PreSpawnPacketHandler extends PacketHandler{
 
 		return true;
 	}
-
-	public function handleSetLocalPlayerAsInitialized(SetLocalPlayerAsInitializedPacket $packet) : bool{
-		$this->player->setImmobile(false); //HACK: this is set to prevent client-side falling before spawn
-
-		$this->session->onSpawn();
-
-		return true;
-	}
 }
diff --git a/src/network/mcpe/handler/SpawnResponsePacketHandler.php b/src/network/mcpe/handler/SpawnResponsePacketHandler.php
new file mode 100644
index 000000000..cc33a6908
--- /dev/null
+++ b/src/network/mcpe/handler/SpawnResponsePacketHandler.php
@@ -0,0 +1,47 @@
+<?php
+
+/*
+ *
+ *  ____            _        _   __  __ _                  __  __ ____
+ * |  _ \ ___   ___| | _____| |_|  \/  (_)_ __   ___      |  \/  |  _ \
+ * | |_) / _ \ / __| |/ / _ \ __| |\/| | | '_ \ / _ \_____| |\/| | |_) |
+ * |  __/ (_) | (__|   <  __/ |_| |  | | | | | |  __/_____| |  | |  __/
+ * |_|   \___/ \___|_|\_\___|\__|_|  |_|_|_| |_|\___|     |_|  |_|_|
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * @author PocketMine Team
+ * @link http://www.pocketmine.net/
+ *
+ *
+*/
+
+declare(strict_types=1);
+
+namespace pocketmine\network\mcpe\handler;
+
+use pocketmine\network\mcpe\protocol\SetLocalPlayerAsInitializedPacket;
+
+final class SpawnResponsePacketHandler extends PacketHandler{
+
+	/**
+	 * @var \Closure
+	 * @phpstan-var \Closure() : void
+	 */
+	private $responseCallback;
+
+	/**
+	 * @phpstan-param \Closure() : void $responseCallback
+	 */
+	public function __construct(\Closure $responseCallback){
+		$this->responseCallback = $responseCallback;
+	}
+
+	public function handleSetLocalPlayerAsInitialized(SetLocalPlayerAsInitializedPacket $packet) : bool{
+		($this->responseCallback)();
+		return true;
+	}
+}