LoginPacket: Cater for more error cases

This now doesn't crash unexpectedly at the first sign of broken data.

composer.json

@@ -34,7 +34,8 @@
       "pocketmine/math": "dev-master",
       "pocketmine/snooze": "^0.1.0",
       "daverandom/callback-validator": "dev-master",
-      "adhocore/json-comment": "^0.0.7"
+      "adhocore/json-comment": "^0.0.7",
+      "particle/validator": "^2.3"
    },
    "autoload": {
       "psr-4": {

composer.lock

@@ -4,7 +4,7 @@
         "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies",
         "This file is @generated automatically"
     ],
-    "content-hash": "cdf1ae08bd2f3e13b0a766a835ed8cb8",
+    "content-hash": "a011d12545207848fb8fe0fccb1cf18c",
     "packages": [
         {
             "name": "adhocore/json-comment",
@@ -231,6 +231,68 @@
             ],
             "time": "2018-12-03T18:17:01+00:00"
         },
+        {
+            "name": "particle/validator",
+            "version": "v2.3.3",
+            "source": {
+                "type": "git",
+                "url": "https://github.com/particle-php/Validator.git",
+                "reference": "becaa89160fe220ebd9e9cd10addc62cf2adf3f0"
+            },
+            "dist": {
+                "type": "zip",
+                "url": "https://api.github.com/repos/particle-php/Validator/zipball/becaa89160fe220ebd9e9cd10addc62cf2adf3f0",
+                "reference": "becaa89160fe220ebd9e9cd10addc62cf2adf3f0",
+                "shasum": ""
+            },
+            "require": {
+                "php": ">=5.4"
+            },
+            "require-dev": {
+                "byrokrat/checkdigit": "^1.0",
+                "giggsey/libphonenumber-for-php": "^7.2",
+                "phpunit/phpunit": "~4.0",
+                "squizlabs/php_codesniffer": "2.*"
+            },
+            "suggest": {
+                "byrokrat/checkdigit": "If you want to use CreditCard validation rule, this library must be installed.",
+                "giggsey/libphonenumber-for-php": "If you want to use Phone validation rule, this library must be installed."
+            },
+            "type": "library",
+            "extra": {
+                "branch-alias": {
+                    "dev-master": "2.0-dev"
+                }
+            },
+            "autoload": {
+                "psr-4": {
+                    "Particle\\Validator\\": "src/"
+                }
+            },
+            "notification-url": "https://packagist.org/downloads/",
+            "license": [
+                "BSD-3-Clause"
+            ],
+            "authors": [
+                {
+                    "name": "Berry Langerak",
+                    "email": "berry@berryllium.nl",
+                    "role": "developer"
+                },
+                {
+                    "name": "Rick van der Staaij",
+                    "homepage": "http://rickvanderstaaij.nl",
+                    "role": "Developer"
+                }
+            ],
+            "description": "Flexible and highly usable validation library with no dependencies.",
+            "homepage": "http://github.com/particle-php/validator",
+            "keywords": [
+                "validation",
+                "validator"
+            ],
+            "time": "2018-09-12T08:03:23+00:00"
+        },
         {
             "name": "pocketmine/binaryutils",
             "version": "0.1.5",

src/pocketmine/network/mcpe/ProcessLoginTask.php

@@ -138,7 +138,7 @@ class ProcessLoginTask extends AsyncTask{
 		$currentKey = null;
 		$first = true;
 
-		foreach($packet->chainData["chain"] as $jwt){
+		foreach($packet->chainDataJwt as $jwt){
 			$this->validateToken($jwt, $currentKey, $first);
 			if($first){
 				$first = false;

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

@@ -26,19 +26,34 @@ namespace pocketmine\network\mcpe\protocol;
 #include <rules/DataPacket.h>
 
 
+use Particle\Validator\Validator;
 use pocketmine\entity\Skin;
 use pocketmine\network\mcpe\handler\SessionHandler;
 use pocketmine\utils\BinaryStream;
 use pocketmine\utils\Utils;
+use function array_filter;
 use function base64_decode;
-use function get_class;
+use function count;
+use function implode;
+use function is_array;
 use function json_decode;
+use function json_last_error_msg;
 
 class LoginPacket extends DataPacket{
 	public const NETWORK_ID = ProtocolInfo::LOGIN_PACKET;
 
 	public const EDITION_POCKET = 0;
 
+	public const I_CLIENT_RANDOM_ID = 'ClientRandomId';
+	public const I_SERVER_ADDRESS = 'ServerAddress';
+	public const I_LANGUAGE_CODE = 'LanguageCode';
+
+	public const I_SKIN_ID = 'SkinId';
+	public const I_SKIN_DATA = 'SkinData';
+	public const I_CAPE_DATA = 'CapeData';
+	public const I_GEOMETRY_NAME = 'SkinGeometryName';
+	public const I_GEOMETRY_DATA = 'SkinGeometry';
+
 	/** @var string */
 	public $username;
 	/** @var int */
@@ -58,8 +73,8 @@ class LoginPacket extends DataPacket{
 	/** @var Skin|null */
 	public $skin;
 
-	/** @var array (the "chain" index contains one or more JWTs) */
+	/** @var string[] array of encoded JWT */
-	public $chainData = [];
+	public $chainDataJwt = [];
 	/** @var string */
 	public $clientDataJwt;
 	/** @var array decoded payload of the clientData JWT */
@@ -83,65 +98,99 @@ class LoginPacket extends DataPacket{
 
 	protected function decodePayload() : void{
 		$this->protocol = $this->getInt();
+		$this->decodeConnectionRequest();
+	}
 
-		try{
+	/**
-			$this->decodeConnectionRequest();
+	 * @param Validator $v
-		}catch(\Throwable $e){
+	 * @param string    $name
-			if($this->protocol === ProtocolInfo::CURRENT_PROTOCOL){
+	 * @param           $data
-				throw $e;
+	 *
-			}
+	 * @throws \UnexpectedValueException
-
+	 */
-			$logger = \GlobalLogger::get();
+	private static function validate(Validator $v, string $name, $data) : void{
-			$logger->debug(get_class($e) . " was thrown while decoding connection request in login (protocol version " . ($this->protocol ?? "unknown") . "): " . $e->getMessage());
+		$result = $v->validate($data);
-			foreach(Utils::printableTrace($e->getTrace()) as $line){
+		if($result->isNotValid()){
-				$logger->debug($line);
+			$messages = [];
+			foreach($result->getFailures() as $f){
+				$messages[] = $f->format();
 			}
+			throw new \UnexpectedValueException("Failed to validate '$name': " . implode(", ", $messages));
 		}
 	}
 
+	/**
+	 * @throws \OutOfBoundsException
+	 * @throws \UnexpectedValueException
+	 */
 	protected function decodeConnectionRequest() : void{
 		$buffer = new BinaryStream($this->getString());
 
-		$this->chainData = json_decode($buffer->get($buffer->getLInt()), true);
+		$chainData = json_decode($buffer->get($buffer->getLInt()), true);
+		if(!is_array($chainData)){
+			throw new \UnexpectedValueException("Failed to decode chainData JSON: " . json_last_error_msg());
+		}
+
+		$vd = new Validator();
+		$vd->required('chain')->isArray()->callback(function(array $data) : bool{
+			return count($data) === 3 and count(array_filter($data, '\is_string')) === count($data);
+		});
+		self::validate($vd, "chainData", $chainData);
+
+		$this->chainDataJwt = $chainData['chain'];
 
 		$hasExtraData = false;
-		foreach($this->chainData["chain"] as $chain){
+		foreach($this->chainDataJwt as $k => $chain){
-			$webtoken = Utils::decodeJWT($chain);
+			//validate every chain element
-			if(isset($webtoken["extraData"])){
+			$claims = Utils::getJwtClaims($chain);
+			if(isset($claims["extraData"])){
+				if(!is_array($claims["extraData"])){
+					throw new \UnexpectedValueException("'extraData' key should be an array");
+				}
 				if($hasExtraData){
-					throw new \RuntimeException("Found 'extraData' multiple times in key chain");
+					throw new \UnexpectedValueException("Found 'extraData' more than once in chainData");
 				}
 				$hasExtraData = true;
-				if(isset($webtoken["extraData"]["displayName"])){
-					$this->username = $webtoken["extraData"]["displayName"];
-				}
-				if(isset($webtoken["extraData"]["identity"])){
-					$this->clientUUID = $webtoken["extraData"]["identity"];
-				}
-				if(isset($webtoken["extraData"]["XUID"])){
-					$this->xuid = $webtoken["extraData"]["XUID"];
-				}
-			}
 
-			if(isset($webtoken["identityPublicKey"])){
+				$extraV = new Validator();
-				$this->identityPublicKey = $webtoken["identityPublicKey"];
+				$extraV->required('displayName')->string();
+				$extraV->required('identity')->uuid();
+				$extraV->required('XUID')->string()->digits();
+				self::validate($extraV, "chain.$k.extraData", $claims['extraData']);
+
+				$this->username = $claims["extraData"]["displayName"];
+				$this->clientUUID = $claims["extraData"]["identity"];
+				$this->xuid = $claims["extraData"]["XUID"];
 			}
 		}
 
 		$this->clientDataJwt = $buffer->get($buffer->getLInt());
-		$this->clientData = Utils::decodeJWT($this->clientDataJwt);
+		$clientData = Utils::getJwtClaims($this->clientDataJwt);
 
-		$this->clientId = $this->clientData["ClientRandomId"] ?? null;
+		$v = new Validator();
-		$this->serverAddress = $this->clientData["ServerAddress"] ?? null;
+		$v->required(self::I_CLIENT_RANDOM_ID)->integer();
+		$v->required(self::I_SERVER_ADDRESS)->string();
+		$v->required(self::I_LANGUAGE_CODE)->string();
 
-		$this->locale = $this->clientData["LanguageCode"] ?? "en_US";
+		$v->required(self::I_SKIN_ID)->string();
+		$v->required(self::I_SKIN_DATA)->string();
+		$v->required(self::I_CAPE_DATA, null, true)->string();
+		$v->required(self::I_GEOMETRY_NAME)->string();
+		$v->required(self::I_GEOMETRY_DATA, null, true)->string();
+		self::validate($v, 'clientData', $clientData);
+
+		$this->clientData = $clientData;
+
+		$this->clientId = $this->clientData[self::I_CLIENT_RANDOM_ID];
+		$this->serverAddress = $this->clientData[self::I_SERVER_ADDRESS];
+		$this->locale = $this->clientData[self::I_LANGUAGE_CODE];
 
 		$this->skin = new Skin(
-			$this->clientData["SkinId"] ?? "",
+			$this->clientData[self::I_SKIN_ID],
-			base64_decode($this->clientData["SkinData"] ?? ""),
+			base64_decode($this->clientData[self::I_SKIN_DATA]),
-			base64_decode($this->clientData["CapeData"] ?? ""),
+			base64_decode($this->clientData[self::I_CAPE_DATA]),
-			$this->clientData["SkinGeometryName"] ?? "",
+			$this->clientData[self::I_GEOMETRY_NAME],
-			base64_decode($this->clientData["SkinGeometry"] ?? "")
+			base64_decode($this->clientData[self::I_GEOMETRY_DATA])
 		);
 	}
 

src/pocketmine/utils/Utils.php

@@ -59,6 +59,7 @@ use function is_object;
 use function is_readable;
 use function is_string;
 use function json_decode;
+use function json_last_error_msg;
 use function memory_get_usage;
 use function ob_end_clean;
 use function ob_get_contents;
@@ -480,10 +481,29 @@ class Utils{
 		return proc_close($process);
 	}
 
-	public static function decodeJWT(string $token) : array{
+	/**
-		list($headB64, $payloadB64, $sigB64) = explode(".", $token);
+	 * @param string $token
+	 *
+	 * @return array of claims
+	 *
+	 * @throws \UnexpectedValueException
+	 */
+	public static function getJwtClaims(string $token) : array{
+		$v = explode(".", $token);
+		if(count($v) !== 3){
+			throw new \UnexpectedValueException("Expected exactly 3 JWT parts, got " . count($v));
+		}
+		$payloadB64 = $v[1];
+		$payloadJSON = base64_decode(strtr($payloadB64, '-_', '+/'), true);
+		if($payloadJSON === false){
+			throw new \UnexpectedValueException("Invalid base64 JWT payload");
+		}
+		$result = json_decode($payloadJSON, true);
+		if(!is_array($result)){
+			throw new \UnexpectedValueException("Failed to decode JWT payload JSON: " . json_last_error_msg());
+		}
 
-		return json_decode(base64_decode(strtr($payloadB64, '-_', '+/'), true), true);
+		return $result;
 	}
 
 	public static function kill($pid) : void{