Properly handle Query packet errors

src/pocketmine/Server.php

@@ -2118,17 +2118,9 @@ class Server{
 	 * TODO: move this to Network
 	 */
 	public function handlePacket(AdvancedNetworkInterface $interface, string $address, int $port, string $payload){
-		try{
+		if($this->queryHandler === null or !$this->queryHandler->handle($interface, $address, $port, $payload)){
-			if(strlen($payload) > 2 and substr($payload, 0, 2) === "\xfe\xfd" and $this->queryHandler instanceof QueryHandler){
-				$this->queryHandler->handle($interface, $address, $port, $payload);
-			}else{
 			$this->logger->debug("Unhandled raw packet from $address $port: " . bin2hex($payload));
 		}
-		}catch(\Throwable $e){
-			$this->logger->logException($e);
-
-			$this->getNetwork()->blockAddress($address, 600);
-		}
 		//TODO: add raw packet events
 	}
 

src/pocketmine/network/query/QueryHandler.php

@@ -30,11 +30,11 @@ namespace pocketmine\network\query;
 use pocketmine\network\AdvancedNetworkInterface;
 use pocketmine\Server;
 use pocketmine\utils\Binary;
-use function bin2hex;
+use pocketmine\utils\BinaryDataException;
+use pocketmine\utils\BinaryStream;
 use function chr;
 use function hash;
 use function microtime;
-use function ord;
 use function random_bytes;
 use function strlen;
 use function substr;
@@ -88,12 +88,23 @@ class QueryHandler{
 		return Binary::readInt(substr(hash("sha512", $salt . ":" . $token, true), 7, 4));
 	}
 
-	public function handle(AdvancedNetworkInterface $interface, string $address, int $port, string $packet) : void{
+	/**
-		$offset = 2;
+	 * @param AdvancedNetworkInterface $interface
-		$packetType = ord($packet{$offset++});
+	 * @param string                   $address
-		$sessionID = Binary::readInt(substr($packet, $offset, 4));
+	 * @param int                      $port
-		$offset += 4;
+	 * @param string                   $packet
-		$payload = substr($packet, $offset);
+	 *
+	 * @return bool
+	 */
+	public function handle(AdvancedNetworkInterface $interface, string $address, int $port, string $packet) : bool{
+		try{
+			$stream = new BinaryStream($packet);
+			$header = $stream->get(2);
+			if($header !== "\xfe\xfd"){ //TODO: have this filtered by the regex filter we installed above
+				return false;
+			}
+			$packetType = $stream->getByte();
+			$sessionID = $stream->getInt();
 
 			switch($packetType){
 				case self::HANDSHAKE: //Handshake
@@ -102,12 +113,14 @@ class QueryHandler{
 					$reply .= self::getTokenString($this->token, $address) . "\x00";
 
 					$interface->sendRawPacket($address, $port, $reply);
-				break;
+
+					return true;
 				case self::STATISTICS: //Stat
-				$token = Binary::readInt(substr($payload, 0, 4));
+					$token = $stream->getInt();
 					if($token !== ($t1 = self::getTokenString($this->token, $address)) and $token !== ($t2 = self::getTokenString($this->lastToken, $address))){
 						$this->debug("Bad token $token from $address $port, expected $t1 or $t2");
-					break;
+
+						return true;
 					}
 					$reply = chr(self::STATISTICS);
 					$reply .= Binary::writeInt($sessionID);
@@ -116,16 +129,21 @@ class QueryHandler{
 						$this->regenerateInfo();
 					}
 
-				if(strlen($payload) === 8){
+					$remaining = $stream->getRemaining();
+					if(strlen($remaining) === 4){ //TODO: check this! according to the spec, this should always be here and always be FF FF FF 01
 						$reply .= $this->longData;
 					}else{
 						$reply .= $this->shortData;
 					}
 					$interface->sendRawPacket($address, $port, $reply);
-				break;
+
+					return true;
 				default:
-				$this->debug("Unhandled packet from $address $port: 0x" . bin2hex($packet));
+					return false;
-				break;
+			}
+		}catch(BinaryDataException $e){
+			$this->debug("Bad packet from $address $port: " . $e->getMessage());
+			return false;
 		}
 	}
 }