Fixed the disaster of packet receive error handling

composer.lock

@@ -295,16 +295,16 @@
         },
         {
             "name": "pocketmine/binaryutils",
-            "version": "0.1.7",
+            "version": "0.1.8",
             "source": {
                 "type": "git",
                 "url": "https://github.com/pmmp/BinaryUtils.git",
-                "reference": "3403751da9d39853b43426085cd242173baadd2b"
+                "reference": "33f511715d22418c03368b49b45a6c25d6b33806"
             },
             "dist": {
                 "type": "zip",
-                "url": "https://api.github.com/repos/pmmp/BinaryUtils/zipball/3403751da9d39853b43426085cd242173baadd2b",
+                "url": "https://api.github.com/repos/pmmp/BinaryUtils/zipball/33f511715d22418c03368b49b45a6c25d6b33806",
-                "reference": "3403751da9d39853b43426085cd242173baadd2b",
+                "reference": "33f511715d22418c03368b49b45a6c25d6b33806",
                 "shasum": ""
             },
             "require": {
@@ -322,10 +322,10 @@
             ],
             "description": "Classes and methods for conveniently handling binary data",
             "support": {
-                "source": "https://github.com/pmmp/BinaryUtils/tree/0.1.7",
+                "source": "https://github.com/pmmp/BinaryUtils/tree/0.1.8",
                 "issues": "https://github.com/pmmp/BinaryUtils/issues"
             },
-            "time": "2019-01-07T15:59:50+00:00"
+            "time": "2019-01-16T17:31:44+00:00"
         },
         {
             "name": "pocketmine/math",

src/pocketmine/network/mcpe/NetworkBinaryStream.php

@@ -33,6 +33,7 @@ use pocketmine\math\Vector3;
 use pocketmine\nbt\LittleEndianNbtSerializer;
 use pocketmine\network\mcpe\protocol\types\CommandOriginData;
 use pocketmine\network\mcpe\protocol\types\EntityLink;
+use pocketmine\utils\BinaryDataException;
 use pocketmine\utils\BinaryStream;
 use pocketmine\utils\UUID;
 use function count;
@@ -100,7 +101,11 @@ class NetworkBinaryStream extends BinaryStream{
 			$this->getString();
 		}
 
-		return ItemFactory::get($id, $data, $cnt, $compound);
+		try{
+			return ItemFactory::get($id, $data, $cnt, $compound);
+		}catch(\InvalidArgumentException $e){
+			throw new BinaryDataException($e->getMessage(), 0, $e);
+		}
 	}
 
 
@@ -179,7 +184,7 @@ class NetworkBinaryStream extends BinaryStream{
 					$value = $this->getVector3();
 					break;
 				default:
-					throw new \UnexpectedValueException("Invalid data type " . $type);
+					throw new BinaryDataException("Invalid data type " . $type);
 			}
 			if($types){
 				$data[$key] = [$type, $value];
@@ -235,7 +240,7 @@ class NetworkBinaryStream extends BinaryStream{
 					$this->putVector3Nullable($d[1]);
 					break;
 				default:
-					throw new \UnexpectedValueException("Invalid data type " . $d[0]);
+					throw new \InvalidArgumentException("Invalid data type " . $d[0]);
 			}
 		}
 	}
@@ -266,7 +271,7 @@ class NetworkBinaryStream extends BinaryStream{
 
 				$list[] = $attr;
 			}else{
-				throw new \UnexpectedValueException("Unknown attribute type \"$id\"");
+				throw new BinaryDataException("Unknown attribute type \"$id\"");
 			}
 		}
 
@@ -450,6 +455,8 @@ class NetworkBinaryStream extends BinaryStream{
 				case 3:
 					$value = $this->getLFloat();
 					break;
+				default:
+					throw new BinaryDataException("Unknown gamerule type $type");
 			}
 
 			$rules[$name] = [$type, $value];
@@ -479,6 +486,8 @@ class NetworkBinaryStream extends BinaryStream{
 				case 3:
 					$this->putLFloat($rule[1]);
 					break;
+				default:
+					throw new \InvalidArgumentException("Invalid gamerule type " . $rule[0]);
 			}
 		}
 	}

src/pocketmine/network/mcpe/NetworkCipher.php

@@ -65,13 +65,13 @@ class NetworkCipher{
 
 	public function decrypt(string $encrypted) : string{
 		if(strlen($encrypted) < 9){
-			throw new \InvalidArgumentException("Payload is too short");
+			throw new \UnexpectedValueException("Payload is too short");
 		}
 		$decrypted = $this->decryptCipher->decryptUpdate($encrypted);
 		$payload = substr($decrypted, 0, -8);
 
 		if(($expected = $this->calculateChecksum($this->decryptCounter++, $payload)) !== ($actual = substr($decrypted, -8))){
-			throw new \InvalidArgumentException("Encrypted payload has invalid checksum (expected " . bin2hex($expected) . ", got " . bin2hex($actual) . ")");
+			throw new \UnexpectedValueException("Encrypted payload has invalid checksum (expected " . bin2hex($expected) . ", got " . bin2hex($actual) . ")");
 		}
 
 		return $payload;

src/pocketmine/network/mcpe/NetworkCompression.php

@@ -35,6 +35,12 @@ final class NetworkCompression{
 
 	}
 
+	/**
+	 * @param string $payload
+	 *
+	 * @return string
+	 * @throws \ErrorException
+	 */
 	public static function decompress(string $payload) : string{
 		return zlib_decode($payload, 1024 * 1024 * 64); //Max 64MB
 	}

src/pocketmine/network/mcpe/NetworkSession.php

@@ -26,6 +26,7 @@ namespace pocketmine\network\mcpe;
 use pocketmine\event\player\PlayerCreationEvent;
 use pocketmine\event\server\DataPacketReceiveEvent;
 use pocketmine\event\server\DataPacketSendEvent;
+use pocketmine\network\BadPacketException;
 use pocketmine\network\mcpe\handler\DeathSessionHandler;
 use pocketmine\network\mcpe\handler\HandshakeSessionHandler;
 use pocketmine\network\mcpe\handler\LoginSessionHandler;
@@ -42,6 +43,7 @@ use pocketmine\network\NetworkInterface;
 use pocketmine\Player;
 use pocketmine\Server;
 use pocketmine\timings\Timings;
+use pocketmine\utils\BinaryDataException;
 use function bin2hex;
 use function strlen;
 use function substr;
@@ -162,6 +164,11 @@ class NetworkSession{
 		$this->handler->setUp();
 	}
 
+	/**
+	 * @param string $payload
+	 *
+	 * @throws BadPacketException
+	 */
 	public function handleEncoded(string $payload) : void{
 		if(!$this->connected){
 			return;
@@ -171,10 +178,9 @@ class NetworkSession{
 			Timings::$playerNetworkReceiveDecryptTimer->startTiming();
 			try{
 				$payload = $this->cipher->decrypt($payload);
-			}catch(\InvalidArgumentException $e){
+			}catch(\UnexpectedValueException $e){
 				$this->server->getLogger()->debug("Encrypted packet from " . $this->ip . " " . $this->port . ": " . bin2hex($payload));
-				$this->disconnect("Packet decryption error: " . $e->getMessage());
+				throw new BadPacketException("Packet decryption error: " . $e->getMessage(), 0, $e);
-				return;
 			}finally{
 				Timings::$playerNetworkReceiveDecryptTimer->stopTiming();
 			}
@@ -185,22 +191,38 @@ class NetworkSession{
 			$stream = new PacketStream(NetworkCompression::decompress($payload));
 		}catch(\ErrorException $e){
 			$this->server->getLogger()->debug("Failed to decompress packet from " . $this->ip . " " . $this->port . ": " . bin2hex($payload));
-			$this->disconnect("Compressed packet batch decode error (incompatible game version?)", false);
+			//TODO: this isn't incompatible game version if we already established protocol version
-			return;
+			throw new BadPacketException("Compressed packet batch decode error (incompatible game version?)", 0, $e);
 		}finally{
 			Timings::$playerNetworkReceiveDecompressTimer->stopTiming();
 		}
 
 		while(!$stream->feof() and $this->connected){
-			$this->handleDataPacket(PacketPool::getPacket($stream->getString()));
+			try{
+				$buf = $stream->getString();
+			}catch(BinaryDataException $e){
+				$this->server->getLogger()->debug("Packet batch from " . $this->ip . " " . $this->port . ": " . bin2hex($stream->getBuffer()));
+				throw new BadPacketException("Packet batch decode error: " . $e->getMessage(), 0, $e);
+			}
+			$this->handleDataPacket(PacketPool::getPacket($buf));
 		}
 	}
 
+	/**
+	 * @param DataPacket $packet
+	 *
+	 * @throws BadPacketException
+	 */
 	public function handleDataPacket(DataPacket $packet) : void{
 		$timings = Timings::getReceiveDataPacketTimings($packet);
 		$timings->startTiming();
 
-		$packet->decode();
+		try{
+			$packet->decode();
+		}catch(BadPacketException $e){
+			$this->server->getLogger()->debug($packet->getName() . " from " . $this->ip . " " . $this->port . ": " . bin2hex($packet->getBuffer()));
+			throw $e;
+		}
 		if(!$packet->feof() and !$packet->mayHaveUnreadBytes()){
 			$remains = substr($packet->getBuffer(), $packet->getOffset());
 			$this->server->getLogger()->debug("Still " . strlen($remains) . " bytes unread in " . $packet->getName() . ": 0x" . bin2hex($remains));

src/pocketmine/network/mcpe/RakLibInterface.php

@@ -25,10 +25,12 @@ namespace pocketmine\network\mcpe;
 
 use pocketmine\GameMode;
 use pocketmine\network\AdvancedNetworkInterface;
+use pocketmine\network\BadPacketException;
 use pocketmine\network\mcpe\protocol\ProtocolInfo;
 use pocketmine\network\Network;
 use pocketmine\Server;
 use pocketmine\snooze\SleeperNotifier;
+use pocketmine\utils\Utils;
 use raklib\protocol\EncapsulatedPacket;
 use raklib\protocol\PacketReliability;
 use raklib\RakLib;
@@ -37,7 +39,6 @@ use raklib\server\ServerHandler;
 use raklib\server\ServerInstance;
 use raklib\utils\InternetAddress;
 use function addcslashes;
-use function bin2hex;
 use function implode;
 use function rtrim;
 use function spl_object_hash;
@@ -142,19 +143,26 @@ class RakLibInterface implements ServerInstance, AdvancedNetworkInterface{
 
 	public function handleEncapsulated(string $identifier, EncapsulatedPacket $packet, int $flags) : void{
 		if(isset($this->sessions[$identifier])){
+			if($packet->buffer === "" or $packet->buffer{0} !== self::MCPE_RAKNET_PACKET_ID){
+				return;
+			}
 			//get this now for blocking in case the player was closed before the exception was raised
 			$session = $this->sessions[$identifier];
 			$address = $session->getIp();
+			$port = $session->getPort();
+			$buf = substr($packet->buffer, 1);
 			try{
-				if($packet->buffer !== "" and $packet->buffer{0} === self::MCPE_RAKNET_PACKET_ID){ //Batch
+				$session->handleEncoded($buf);
-					$session->handleEncoded(substr($packet->buffer, 1));
+			}catch(BadPacketException $e){
-				}
-			}catch(\Throwable $e){
 				$logger = $this->server->getLogger();
-				$logger->debug("EncapsulatedPacket 0x" . bin2hex($packet->buffer));
+				$logger->error("Bad packet from $address $port: " . $e->getMessage());
-				$logger->logException($e);
 
-				$session->disconnect("Internal server error");
+				//intentionally doesn't use logException, we don't want spammy packet error traces to appear in release mode
+				$logger->debug("Origin: " . Utils::cleanPath($e->getFile()) . "(" . $e->getLine() . ")");
+				foreach(Utils::printableTrace($e->getTrace()) as $frame){
+					$logger->debug($frame);
+				}
+				$session->disconnect("Packet processing error");
 				$this->interface->blockAddress($address, 5);
 			}
 		}

src/pocketmine/network/mcpe/handler/SimpleSessionHandler.php

@@ -28,6 +28,7 @@ use pocketmine\inventory\transaction\CraftingTransaction;
 use pocketmine\inventory\transaction\InventoryTransaction;
 use pocketmine\inventory\transaction\TransactionValidationException;
 use pocketmine\math\Vector3;
+use pocketmine\network\BadPacketException;
 use pocketmine\network\mcpe\protocol\AdventureSettingsPacket;
 use pocketmine\network\mcpe\protocol\AnimatePacket;
 use pocketmine\network\mcpe\protocol\BlockEntityDataPacket;
@@ -454,6 +455,7 @@ class SimpleSessionHandler extends SessionHandler{
 	 * @param bool   $assoc
 	 *
 	 * @return mixed
+	 * @throws BadPacketException
 	 */
 	private static function stupid_json_decode(string $json, bool $assoc = false){
 		if(preg_match('/^\[(.+)\]$/s', $json, $matches) > 0){
@@ -468,7 +470,7 @@ class SimpleSessionHandler extends SessionHandler{
 
 			$fixed = "[" . implode(",", $parts) . "]";
 			if(($ret = json_decode($fixed, $assoc)) === null){
-				throw new \InvalidArgumentException("Failed to fix JSON: " . json_last_error_msg() . "(original: $json, modified: $fixed)");
+				throw new BadPacketException("Failed to fix JSON: " . json_last_error_msg() . "(original: $json, modified: $fixed)");
 			}
 
 			return $ret;

src/pocketmine/network/mcpe/protocol/AddEntityPacket.php

@@ -28,6 +28,7 @@ namespace pocketmine\network\mcpe\protocol;
 use pocketmine\entity\Attribute;
 use pocketmine\entity\EntityIds;
 use pocketmine\math\Vector3;
+use pocketmine\network\BadPacketException;
 use pocketmine\network\mcpe\handler\SessionHandler;
 use pocketmine\network\mcpe\protocol\types\EntityLink;
 use function array_search;
@@ -173,7 +174,7 @@ class AddEntityPacket extends DataPacket{
 		$this->entityRuntimeId = $this->getEntityRuntimeId();
 		$this->type = array_search($t = $this->getString(), self::LEGACY_ID_MAP_BC, true);
 		if($this->type === false){
-			throw new \UnexpectedValueException("Can't map ID $t to legacy ID");
+			throw new BadPacketException("Can't map ID $t to legacy ID");
 		}
 		$this->position = $this->getVector3();
 		$this->motion = $this->getVector3();
@@ -195,7 +196,7 @@ class AddEntityPacket extends DataPacket{
 				$attr->setValue($current);
 				$this->attributes[] = $attr;
 			}else{
-				throw new \UnexpectedValueException("Unknown attribute type \"$id\"");
+				throw new BadPacketException("Unknown attribute type \"$id\"");
 			}
 		}
 

src/pocketmine/network/mcpe/protocol/AvailableCommandsPacket.php

@@ -25,6 +25,7 @@ namespace pocketmine\network\mcpe\protocol;
 
 #include <rules/DataPacket.h>
 
+use pocketmine\network\BadPacketException;
 use pocketmine\network\mcpe\handler\SessionHandler;
 use pocketmine\network\mcpe\protocol\types\CommandData;
 use pocketmine\network\mcpe\protocol\types\CommandEnum;
@@ -146,7 +147,7 @@ class AvailableCommandsPacket extends DataPacket{
 		for($i = 0, $count = $this->getUnsignedVarInt(); $i < $count; ++$i){
 			$index = $this->getEnumValueIndex();
 			if(!isset($this->enumValues[$index])){
-				throw new \UnexpectedValueException("Invalid enum value index $index");
+				throw new BadPacketException("Invalid enum value index $index");
 			}
 			//Get the enum value from the initial pile of mess
 			$retval->enumValues[] = $this->enumValues[$index];
@@ -229,16 +230,16 @@ class AvailableCommandsPacket extends DataPacket{
 					$index = ($parameter->paramType & 0xffff);
 					$parameter->enum = $this->enums[$index] ?? null;
 					if($parameter->enum === null){
-						throw new \UnexpectedValueException("expected enum at $index, but got none");
+						throw new BadPacketException("expected enum at $index, but got none");
 					}
 				}elseif($parameter->paramType & self::ARG_FLAG_POSTFIX){
 					$index = ($parameter->paramType & 0xffff);
 					$parameter->postfix = $this->postfixes[$index] ?? null;
 					if($parameter->postfix === null){
-						throw new \UnexpectedValueException("expected postfix at $index, but got none");
+						throw new BadPacketException("expected postfix at $index, but got none");
 					}
 				}elseif(($parameter->paramType & self::ARG_FLAG_VALID) === 0){
-					throw new \UnexpectedValueException("Invalid parameter type 0x" . dechex($parameter->paramType));
+					throw new BadPacketException("Invalid parameter type 0x" . dechex($parameter->paramType));
 				}
 
 				$retval->overloads[$overloadIndex][$paramIndex] = $parameter;

src/pocketmine/network/mcpe/protocol/BookEditPacket.php

@@ -25,6 +25,7 @@ namespace pocketmine\network\mcpe\protocol;
 
 #include <rules/DataPacket.h>
 
+use pocketmine\network\BadPacketException;
 use pocketmine\network\mcpe\handler\SessionHandler;
 
 class BookEditPacket extends DataPacket{
@@ -81,7 +82,7 @@ class BookEditPacket extends DataPacket{
 				$this->xuid = $this->getString();
 				break;
 			default:
-				throw new \UnexpectedValueException("Unknown book edit type $this->type!");
+				throw new BadPacketException("Unknown book edit type $this->type!");
 		}
 	}
 

src/pocketmine/network/mcpe/protocol/ClientboundMapItemDataPacket.php

@@ -27,6 +27,7 @@ namespace pocketmine\network\mcpe\protocol;
 #include <rules/DataPacket.h>
 
 
+use pocketmine\network\BadPacketException;
 use pocketmine\network\mcpe\handler\SessionHandler;
 use pocketmine\network\mcpe\protocol\types\DimensionIds;
 use pocketmine\network\mcpe\protocol\types\MapTrackedObject;
@@ -93,7 +94,7 @@ class ClientboundMapItemDataPacket extends DataPacket{
 				}elseif($object->type === MapTrackedObject::TYPE_ENTITY){
 					$object->entityUniqueId = $this->getEntityUniqueId();
 				}else{
-					throw new \UnexpectedValueException("Unknown map object type $object->type");
+					throw new BadPacketException("Unknown map object type $object->type");
 				}
 				$this->trackedEntities[] = $object;
 			}
@@ -117,7 +118,7 @@ class ClientboundMapItemDataPacket extends DataPacket{
 
 			$count = $this->getUnsignedVarInt();
 			if($count !== $this->width * $this->height){
-				throw new \UnexpectedValueException("Expected colour count of " . ($this->height * $this->width) . " (height $this->height * width $this->width), got $count");
+				throw new BadPacketException("Expected colour count of " . ($this->height * $this->width) . " (height $this->height * width $this->width), got $count");
 			}
 
 			for($y = 0; $y < $this->height; ++$y){

src/pocketmine/network/mcpe/protocol/CraftingDataPacket.php

@@ -30,6 +30,7 @@ use pocketmine\inventory\FurnaceRecipe;
 use pocketmine\inventory\ShapedRecipe;
 use pocketmine\inventory\ShapelessRecipe;
 use pocketmine\item\Item;
+use pocketmine\network\BadPacketException;
 use pocketmine\network\mcpe\handler\SessionHandler;
 use pocketmine\network\mcpe\NetworkBinaryStream;
 use function count;
@@ -107,7 +108,7 @@ class CraftingDataPacket extends DataPacket{
 					$entry["uuid"] = $this->getUUID()->toString();
 					break;
 				default:
-					throw new \UnexpectedValueException("Unhandled recipe type $recipeType!"); //do not continue attempting to decode
+					throw new BadPacketException("Unhandled recipe type $recipeType!"); //do not continue attempting to decode
 			}
 			$this->decodedEntries[] = $entry;
 		}

src/pocketmine/network/mcpe/protocol/DataPacket.php

@@ -25,8 +25,10 @@ namespace pocketmine\network\mcpe\protocol;
 
 #include <rules/DataPacket.h>
 
+use pocketmine\network\BadPacketException;
 use pocketmine\network\mcpe\handler\SessionHandler;
 use pocketmine\network\mcpe\NetworkBinaryStream;
+use pocketmine\utils\BinaryDataException;
 use pocketmine\utils\Utils;
 use function bin2hex;
 use function get_class;
@@ -67,22 +69,26 @@ abstract class DataPacket extends NetworkBinaryStream{
 	}
 
 	/**
-	 * @throws \OutOfBoundsException
+	 * @throws BadPacketException
-	 * @throws \UnexpectedValueException
 	 */
 	final public function decode() : void{
 		$this->rewind();
-		$this->decodeHeader();
+		try{
-		$this->decodePayload();
+			$this->decodeHeader();
+			$this->decodePayload();
+		}catch(BinaryDataException | BadPacketException $e){
+			throw new BadPacketException($this->getName() . ": " . $e->getMessage(), 0, $e);
+		}
 	}
 
 	/**
-	 * @throws \OutOfBoundsException
+	 * @throws BinaryDataException
 	 * @throws \UnexpectedValueException
 	 */
 	protected function decodeHeader() : void{
 		$pid = $this->getUnsignedVarInt();
 		if($pid !== static::NETWORK_ID){
+			//TODO: this means a logical error in the code, but how to prevent it from happening?
 			throw new \UnexpectedValueException("Expected " . static::NETWORK_ID . " for packet ID, got $pid");
 		}
 	}
@@ -90,8 +96,8 @@ abstract class DataPacket extends NetworkBinaryStream{
 	/**
 	 * Decodes the packet body, without the packet ID or other generic header fields.
 	 *
-	 * @throws \OutOfBoundsException
+	 * @throws BadPacketException
-	 * @throws \UnexpectedValueException
+	 * @throws BinaryDataException
 	 */
 	abstract protected function decodePayload() : void;
 
@@ -124,6 +130,7 @@ abstract class DataPacket extends NetworkBinaryStream{
 	 * @param SessionHandler $handler
 	 *
 	 * @return bool true if the packet was handled successfully, false if not.
+	 * @throws BadPacketException if broken data was found in the packet
 	 */
 	abstract public function handle(SessionHandler $handler) : bool;
 

src/pocketmine/network/mcpe/protocol/InventoryTransactionPacket.php

@@ -25,6 +25,7 @@ namespace pocketmine\network\mcpe\protocol;
 
 #include <rules/DataPacket.h>
 
+use pocketmine\network\BadPacketException;
 use pocketmine\network\mcpe\handler\SessionHandler;
 use pocketmine\network\mcpe\protocol\types\MismatchTransactionData;
 use pocketmine\network\mcpe\protocol\types\NormalTransactionData;
@@ -68,7 +69,7 @@ class InventoryTransactionPacket extends DataPacket{
 				$this->trData = new ReleaseItemTransactionData();
 				break;
 			default:
-				throw new \UnexpectedValueException("Unknown transaction type $transactionType");
+				throw new BadPacketException("Unknown transaction type $transactionType");
 		}
 
 		$this->trData->decode($this);

src/pocketmine/network/mcpe/protocol/LoginPacket.php

@@ -28,6 +28,7 @@ namespace pocketmine\network\mcpe\protocol;
 
 use Particle\Validator\Validator;
 use pocketmine\entity\Skin;
+use pocketmine\network\BadPacketException;
 use pocketmine\network\mcpe\handler\SessionHandler;
 use pocketmine\PlayerInfo;
 use pocketmine\utils\BinaryStream;
@@ -110,7 +111,7 @@ class LoginPacket extends DataPacket{
 			foreach($result->getFailures() as $f){
 				$messages[] = $f->format();
 			}
-			throw new \UnexpectedValueException("Failed to validate '$name': " . implode(", ", $messages));
+			throw new BadPacketException("Failed to validate '$name': " . implode(", ", $messages));
 		}
 	}
 
@@ -123,7 +124,7 @@ class LoginPacket extends DataPacket{
 
 		$chainData = json_decode($buffer->get($buffer->getLInt()), true);
 		if(!is_array($chainData)){
-			throw new \UnexpectedValueException("Failed to decode chainData JSON: " . json_last_error_msg());
+			throw new BadPacketException("Failed to decode chainData JSON: " . json_last_error_msg());
 		}
 
 		$vd = new Validator();
@@ -138,10 +139,10 @@ class LoginPacket extends DataPacket{
 			$claims = Utils::getJwtClaims($chain);
 			if(isset($claims["extraData"])){
 				if(!is_array($claims["extraData"])){
-					throw new \UnexpectedValueException("'extraData' key should be an array");
+					throw new BadPacketException("'extraData' key should be an array");
 				}
 				if($this->extraData !== null){
-					throw new \UnexpectedValueException("Found 'extraData' more than once in chainData");
+					throw new BadPacketException("Found 'extraData' more than once in chainData");
 				}
 
 				$extraV = new Validator();
@@ -154,7 +155,7 @@ class LoginPacket extends DataPacket{
 			}
 		}
 		if($this->extraData === null){
-			throw new \UnexpectedValueException("'extraData' not found in chain data");
+			throw new BadPacketException("'extraData' not found in chain data");
 		}
 
 		$this->clientDataJwt = $buffer->get($buffer->getLInt());

src/pocketmine/network/mcpe/protocol/PacketPool.php

@@ -23,7 +23,9 @@ declare(strict_types=1);
 
 namespace pocketmine\network\mcpe\protocol;
 
+use pocketmine\network\BadPacketException;
 use pocketmine\utils\Binary;
+use pocketmine\utils\BinaryDataException;
 
 class PacketPool{
 	/** @var \SplFixedArray<DataPacket> */
@@ -175,10 +177,15 @@ class PacketPool{
 	 * @param string $buffer
 	 *
 	 * @return DataPacket
+	 * @throws BadPacketException
 	 */
 	public static function getPacket(string $buffer) : DataPacket{
 		$offset = 0;
-		$pk = static::getPacketById(Binary::readUnsignedVarInt($buffer, $offset));
+		try{
+			$pk = static::getPacketById(Binary::readUnsignedVarInt($buffer, $offset));
+		}catch(BinaryDataException $e){
+			throw new BadPacketException("Packet is too short");
+		}
 		$pk->setBuffer($buffer, $offset);
 
 		return $pk;

src/pocketmine/network/mcpe/protocol/SetScorePacket.php

@@ -25,6 +25,7 @@ namespace pocketmine\network\mcpe\protocol;
 
 #include <rules/DataPacket.h>
 
+use pocketmine\network\BadPacketException;
 use pocketmine\network\mcpe\handler\SessionHandler;
 use pocketmine\network\mcpe\protocol\types\ScorePacketEntry;
 use function count;
@@ -58,7 +59,7 @@ class SetScorePacket extends DataPacket{
 						$entry->customName = $this->getString();
 						break;
 					default:
-						throw new \UnexpectedValueException("Unknown entry type $entry->type");
+						throw new BadPacketException("Unknown entry type $entry->type");
 				}
 			}
 		}

src/pocketmine/network/mcpe/protocol/types/MismatchTransactionData.php

@@ -23,6 +23,7 @@ declare(strict_types=1);
 
 namespace pocketmine\network\mcpe\protocol\types;
 
+use pocketmine\network\BadPacketException;
 use pocketmine\network\mcpe\NetworkBinaryStream;
 use pocketmine\network\mcpe\protocol\InventoryTransactionPacket;
 use function count;
@@ -35,7 +36,7 @@ class MismatchTransactionData extends TransactionData{
 
 	protected function decodeData(NetworkBinaryStream $stream) : void{
 		if(!empty($this->actions)){
-			throw new \UnexpectedValueException("Mismatch transaction type should not have any actions associated with it, but got " . count($this->actions));
+			throw new BadPacketException("Mismatch transaction type should not have any actions associated with it, but got " . count($this->actions));
 		}
 	}
 

src/pocketmine/network/mcpe/protocol/types/NetworkInventoryAction.php

@@ -28,6 +28,7 @@ use pocketmine\inventory\transaction\action\DropItemAction;
 use pocketmine\inventory\transaction\action\InventoryAction;
 use pocketmine\inventory\transaction\action\SlotChangeAction;
 use pocketmine\item\Item;
+use pocketmine\network\BadPacketException;
 use pocketmine\network\mcpe\NetworkBinaryStream;
 use pocketmine\Player;
 
@@ -115,7 +116,7 @@ class NetworkInventoryAction{
 				$this->windowId = $packet->getVarInt();
 				break;
 			default:
-				throw new \UnexpectedValueException("Unknown inventory action source type $this->sourceType");
+				throw new BadPacketException("Unknown inventory action source type $this->sourceType");
 		}
 
 		$this->inventorySlot = $packet->getUnsignedVarInt();