Release 4.12.7

Bumps [phpstan/phpstan](https://github.com/phpstan/phpstan) from 1.9.6 to 1.9.7.
- [Release notes](https://github.com/phpstan/phpstan/releases)
- [Changelog](https://github.com/phpstan/phpstan/blob/1.9.x/CHANGELOG.md)
- [Commits](https://github.com/phpstan/phpstan/compare/1.9.6...1.9.7)

---
updated-dependencies:
- dependency-name: phpstan/phpstan
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

.github/dependabot.yml

@@ -6,6 +6,12 @@ updates:
     interval: daily
     time: "10:00"
   open-pull-requests-limit: 10
+  ignore:
+    #only allow patch updates for locale-data - this has to be updated manually due to codegen
+    - dependency-name: pocketmine/locale-data
+      update-types:
+        - "version-update:semver-major"
+        - "version-update:semver-minor"
 
 - package-ecosystem: gitsubmodule
   directory: "/"

.github/workflows/discord-release-notify.yml

@@ -13,7 +13,7 @@ jobs:
       - uses: actions/checkout@v3
 
       - name: Setup PHP and tools
-        uses: shivammathur/setup-php@2.22.0
+        uses: shivammathur/setup-php@2.23.0
         with:
           php-version: 8.0
 

.github/workflows/draft-release.yml

@@ -18,7 +18,7 @@ jobs:
           submodules: true
 
       - name: Setup PHP
-        uses: shivammathur/setup-php@2.22.0
+        uses: shivammathur/setup-php@2.23.0
         with:
           php-version: 8.0
 

.github/workflows/main.yml

@@ -195,7 +195,7 @@ jobs:
       - uses: actions/checkout@v3
 
       - name: Setup PHP and tools
-        uses: shivammathur/setup-php@2.22.0
+        uses: shivammathur/setup-php@2.23.0
         with:
           php-version: 8.0
           tools: php-cs-fixer:3.11

changelogs/4.12.md

@@ -37,4 +37,29 @@ Released 28th December 2022.
 ## Fixes
 - Fixed unauthenticated connections taking up player count slots, preventing players from joining.
 - Fixed a possible crash in `World->tickChunk()` when plugins unload chunks during some events.
-- `/gamemode` will now report a failure to change game mode if the player is already in the requested game mode.
+- `/gamemode` will now report a failure to change game mode if the player is already in the requested game mode.
+
+# 4.12.4
+Released 3rd January 2023.
+
+## Fixes
+- Added workarounds for an active exploit being used to deny service to servers.
+
+# 4.12.5
+Released 6th January 2023.
+
+## Fixes
+- Removed a workaround for an old client bug in custom form responses. The code contained a denial-of-service vulnerability.
+
+# 4.12.6
+Released 7th January 2023.
+
+## Changes
+- Added a new security measure to `NetworkSession` to detect and ban players who flood the server with packets.
+
+# 4.12.7
+Released 8th January 2023.
+
+## Fixes
+- Fixed players getting kicked when the server lags for too long.
+- Fixed players getting kicked when a debugging session is active and a breakpoint is hit.

composer.json

@@ -54,7 +54,7 @@
       "webmozart/path-util": "^2.3"
    },
    "require-dev": {
-      "phpstan/phpstan": "1.9.4",
+      "phpstan/phpstan": "1.9.7",
       "phpstan/phpstan-phpunit": "^1.1.0",
       "phpstan/phpstan-strict-rules": "^1.2.0",
       "phpunit/phpunit": "^9.2"

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": "393c7921d03d080d3ef3b836f90b4415",
+    "content-hash": "76c6b5521d8f88d9070e8dec1c0ae144",
     "packages": [
         {
             "name": "adhocore/json-comment",
@@ -1821,16 +1821,16 @@
         },
         {
             "name": "phpstan/phpstan",
-            "version": "1.9.4",
+            "version": "1.9.7",
             "source": {
                 "type": "git",
                 "url": "https://github.com/phpstan/phpstan.git",
-                "reference": "d03bccee595e2146b7c9d174486b84f4dc61b0f2"
+                "reference": "0501435cd342eac7664bd62155b1ef907fc60b6f"
             },
             "dist": {
                 "type": "zip",
-                "url": "https://api.github.com/repos/phpstan/phpstan/zipball/d03bccee595e2146b7c9d174486b84f4dc61b0f2",
-                "reference": "d03bccee595e2146b7c9d174486b84f4dc61b0f2",
+                "url": "https://api.github.com/repos/phpstan/phpstan/zipball/0501435cd342eac7664bd62155b1ef907fc60b6f",
+                "reference": "0501435cd342eac7664bd62155b1ef907fc60b6f",
                 "shasum": ""
             },
             "require": {
@@ -1860,7 +1860,7 @@
             ],
             "support": {
                 "issues": "https://github.com/phpstan/phpstan/issues",
-                "source": "https://github.com/phpstan/phpstan/tree/1.9.4"
+                "source": "https://github.com/phpstan/phpstan/tree/1.9.7"
             },
             "funding": [
                 {
@@ -1876,7 +1876,7 @@
                     "type": "tidelift"
                 }
             ],
-            "time": "2022-12-17T13:33:52+00:00"
+            "time": "2023-01-04T21:59:57+00:00"
         },
         {
             "name": "phpstan/phpstan-phpunit",

src/VersionInfo.php

@@ -31,7 +31,7 @@ use function str_repeat;
 
 final class VersionInfo{
 	public const NAME = "PocketMine-MP";
-	public const BASE_VERSION = "4.12.3";
+	public const BASE_VERSION = "4.12.7";
 	public const IS_DEVELOPMENT_BUILD = false;
 	public const BUILD_CHANNEL = "stable";
 

src/crafting/CraftingManager.php

@@ -34,9 +34,15 @@ use function usort;
 class CraftingManager{
 	use DestructorCallbackTrait;
 
-	/** @var ShapedRecipe[][] */
+	/**
+	 * @var ShapedRecipe[][]
+	 * @phpstan-var array<string, list<ShapedRecipe>>
+	 */
 	protected $shapedRecipes = [];
-	/** @var ShapelessRecipe[][] */
+	/**
+	 * @var ShapelessRecipe[][]
+	 * @phpstan-var array<string, list<ShapelessRecipe>>
+	 */
 	protected $shapelessRecipes = [];
 
 	/**
@@ -133,6 +139,7 @@ class CraftingManager{
 
 	/**
 	 * @return ShapelessRecipe[][]
+	 * @phpstan-return array<string, list<ShapelessRecipe>>
 	 */
 	public function getShapelessRecipes() : array{
 		return $this->shapelessRecipes;
@@ -140,6 +147,7 @@ class CraftingManager{
 
 	/**
 	 * @return ShapedRecipe[][]
+	 * @phpstan-return array<string, list<ShapedRecipe>>
 	 */
 	public function getShapedRecipes() : array{
 		return $this->shapedRecipes;

src/network/mcpe/NetworkSession.php

@@ -128,20 +128,38 @@ use function array_values;
 use function base64_encode;
 use function bin2hex;
 use function count;
+use function function_exists;
 use function get_class;
+use function hrtime;
 use function in_array;
+use function intdiv;
 use function json_encode;
 use function ksort;
+use function min;
 use function strcasecmp;
 use function strlen;
 use function strtolower;
 use function substr;
 use function time;
 use function ucfirst;
+use function xdebug_is_debugger_active;
 use const JSON_THROW_ON_ERROR;
 use const SORT_NUMERIC;
 
 class NetworkSession{
+	private const INCOMING_PACKET_BATCH_PER_TICK = 2; //we should normally see only 1 per tick - 2 allows recovery of the budget after a lag spike
+	private const INCOMING_PACKET_BATCH_MAX_BUDGET = 100; //enough to account for a 5-second lag spike
+
+	/**
+	 * At most this many more packets can be received. If this reaches zero, any additional packets received will cause
+	 * the player to be kicked from the server.
+	 * This number is increased every tick up to a maximum limit.
+	 *
+	 * @see self::INCOMING_PACKET_BATCH_PER_TICK
+	 * @see self::INCOMING_PACKET_BATCH_MAX_BUDGET
+	 */
+	private int $incomingPacketBatchBudget = self::INCOMING_PACKET_BATCH_MAX_BUDGET;
+
 	private \PrefixedLogger $logger;
 	private ?Player $player = null;
 	private ?PlayerInfo $info = null;
@@ -179,6 +197,8 @@ class NetworkSession{
 	 */
 	private ObjectSet $disposeHooks;
 
+	private int $lastUpdateTimeNs;
+
 	public function __construct(
 		private Server $server,
 		private NetworkSessionManager $manager,
@@ -199,6 +219,7 @@ class NetworkSession{
 		$this->disposeHooks = new ObjectSet();
 
 		$this->connectTime = time();
+		$this->lastUpdateTimeNs = hrtime(true);
 
 		$this->setHandler(new SessionStartPacketHandler(
 			$this->server,
@@ -339,6 +360,17 @@ class NetworkSession{
 			return;
 		}
 
+		if($this->incomingPacketBatchBudget <= 0){
+			if(!function_exists('xdebug_is_debugger_active') || !xdebug_is_debugger_active()){
+				throw new PacketHandlingException("Receiving packets too fast");
+			}else{
+				//when a debugging session is active, the server may halt at any point for an indefinite length of time,
+				//in which time the client will continue to send packets
+				$this->incomingPacketBatchBudget = self::INCOMING_PACKET_BATCH_MAX_BUDGET;
+			}
+		}
+		$this->incomingPacketBatchBudget--;
+
 		if($this->cipher !== null){
 			Timings::$playerNetworkReceiveDecrypt->startTiming();
 			try{
@@ -366,7 +398,7 @@ class NetworkSession{
 		}
 
 		try{
-			foreach((new PacketBatch($decompressed))->getPackets($this->packetPool, $this->packetSerializerContext, 500) as [$packet, $buffer]){
+			foreach((new PacketBatch($decompressed))->getPackets($this->packetPool, $this->packetSerializerContext, 1300) as [$packet, $buffer]){
 				if($packet === null){
 					$this->logger->debug("Unknown packet: " . base64_encode($buffer));
 					throw new PacketHandlingException("Unknown packet received");
@@ -1108,6 +1140,10 @@ class NetworkSession{
 	}
 
 	public function tick() : void{
+		$nowNs = hrtime(true);
+		$timeSinceLastUpdateNs = $nowNs - $this->lastUpdateTimeNs;
+		$this->lastUpdateTimeNs = $nowNs;
+
 		if($this->info === null){
 			if(time() >= $this->connectTime + 10){
 				$this->disconnect("Login timeout");
@@ -1129,5 +1165,15 @@ class NetworkSession{
 		}
 
 		$this->flushSendBuffer();
+		$ticksSinceLastUpdate = intdiv($timeSinceLastUpdateNs, 50_000_000);
+		if($ticksSinceLastUpdate > 0){
+			if($ticksSinceLastUpdate > 1){
+				$this->logger->debug("Adding packet budget for $ticksSinceLastUpdate ticks (current budget is $this->incomingPacketBatchBudget)");
+			}
+			$this->incomingPacketBatchBudget = min(
+				$this->incomingPacketBatchBudget + (self::INCOMING_PACKET_BATCH_PER_TICK * $ticksSinceLastUpdate),
+				self::INCOMING_PACKET_BATCH_MAX_BUDGET
+			);
+		}
 	}
 }

src/network/mcpe/convert/TypeConverter.php

@@ -175,6 +175,7 @@ class TypeConverter{
 					$nbt = new CompoundTag();
 				}
 				$nbt->setInt(self::DAMAGE_TAG, $itemStack->getDamage());
+				$meta = 0;
 			}elseif($isBlockItem && $itemStack->getMeta() !== 0){
 				//TODO HACK: This foul-smelling code ensures that we can correctly deserialize an item when the
 				//client sends it back to us, because as of 1.16.220, blockitems quietly discard their metadata
@@ -183,6 +184,7 @@ class TypeConverter{
 					$nbt = new CompoundTag();
 				}
 				$nbt->setInt(self::PM_META_TAG, $itemStack->getMeta());
+				$meta = 0;
 			}
 		}
 

src/network/mcpe/handler/InGamePacketHandler.php

@@ -112,7 +112,6 @@ use function array_push;
 use function base64_encode;
 use function count;
 use function fmod;
-use function implode;
 use function in_array;
 use function is_bool;
 use function is_infinite;
@@ -122,12 +121,9 @@ use function json_encode;
 use function max;
 use function mb_strlen;
 use function microtime;
-use function preg_match;
 use function sprintf;
 use function strlen;
 use function strpos;
-use function substr;
-use function trim;
 use const JSON_THROW_ON_ERROR;
 
 /**
@@ -254,6 +250,10 @@ class InGamePacketHandler extends PacketHandler{
 
 		$useItemTransaction = $packet->getItemInteractionData();
 		if($useItemTransaction !== null){
+			if(count($useItemTransaction->getTransactionData()->getActions()) > 100){
+				throw new PacketHandlingException("Too many actions in item use transaction");
+			}
+			$this->inventoryManager->addPredictedSlotChanges($useItemTransaction->getTransactionData()->getActions());
 			if(!$this->handleUseItemTransaction($useItemTransaction->getTransactionData())){
 				$packetHandled = false;
 				$this->session->getLogger()->debug("Unhandled transaction in PlayerAuthInputPacket (type " . $useItemTransaction->getTransactionData()->getActionType() . ")");
@@ -264,6 +264,9 @@ class InGamePacketHandler extends PacketHandler{
 
 		$blockActions = $packet->getBlockActions();
 		if($blockActions !== null){
+			if(count($blockActions) > 100){
+				throw new PacketHandlingException("Too many block actions in PlayerAuthInputPacket");
+			}
 			foreach($blockActions as $k => $blockAction){
 				$actionHandled = false;
 				if($blockAction instanceof PlayerBlockActionStopBreak){
@@ -310,6 +313,10 @@ class InGamePacketHandler extends PacketHandler{
 	public function handleInventoryTransaction(InventoryTransactionPacket $packet) : bool{
 		$result = true;
 
+		if(count($packet->trData->getActions()) > 100){
+			throw new PacketHandlingException("Too many actions in inventory transaction");
+		}
+
 		$this->inventoryManager->addPredictedSlotChanges($packet->trData->getActions());
 
 		if($packet->trData instanceof NormalTransactionData){
@@ -864,60 +871,17 @@ class InGamePacketHandler extends PacketHandler{
 			//TODO: make APIs for this to allow plugins to use this information
 			return $this->player->onFormSubmit($packet->formId, null);
 		}elseif($packet->formData !== null){
-			return $this->player->onFormSubmit($packet->formId, self::stupid_json_decode($packet->formData, true));
+			try{
+				$responseData = json_decode($packet->formData, true, self::MAX_FORM_RESPONSE_DEPTH, JSON_THROW_ON_ERROR);
+			}catch(\JsonException $e){
+				throw PacketHandlingException::wrap($e, "Failed to decode form response data");
+			}
+			return $this->player->onFormSubmit($packet->formId, $responseData);
 		}else{
 			throw new PacketHandlingException("Expected either formData or cancelReason to be set in ModalFormResponsePacket");
 		}
 	}
 
-	/**
-	 * Hack to work around a stupid bug in Minecraft W10 which causes empty strings to be sent unquoted in form responses.
-	 *
-	 * @return mixed
-	 * @throws PacketHandlingException
-	 */
-	private static function stupid_json_decode(string $json, bool $assoc = false){
-		if(preg_match('/^\[(.+)\]$/s', $json, $matches) > 0){
-			$raw = $matches[1];
-			$lastComma = -1;
-			$newParts = [];
-			$inQuotes = false;
-			for($i = 0, $len = strlen($raw); $i <= $len; ++$i){
-				if($i === $len || ($raw[$i] === "," && !$inQuotes)){
-					$part = substr($raw, $lastComma + 1, $i - ($lastComma + 1));
-					if(trim($part) === ""){ //regular parts will have quotes or something else that makes them non-empty
-						$part = '""';
-					}
-					$newParts[] = $part;
-					$lastComma = $i;
-				}elseif($raw[$i] === '"'){
-					if(!$inQuotes){
-						$inQuotes = true;
-					}else{
-						$backslashes = 0;
-						for(; $backslashes < $i && $raw[$i - $backslashes - 1] === "\\"; ++$backslashes){}
-						if(($backslashes % 2) === 0){ //unescaped quote
-							$inQuotes = false;
-						}
-					}
-				}
-			}
-
-			$fixed = "[" . implode(",", $newParts) . "]";
-			try{
-				return json_decode($fixed, $assoc, self::MAX_FORM_RESPONSE_DEPTH, JSON_THROW_ON_ERROR);
-			}catch(\JsonException $e){
-				throw PacketHandlingException::wrap($e, "Failed to fix JSON (original: $json, modified: $fixed)");
-			}
-		}
-
-		try{
-			return json_decode($json, $assoc, self::MAX_FORM_RESPONSE_DEPTH, JSON_THROW_ON_ERROR);
-		}catch(\JsonException $e){
-			throw PacketHandlingException::wrap($e);
-		}
-	}
-
 	public function handleServerSettingsRequest(ServerSettingsRequestPacket $packet) : bool{
 		return false; //TODO: GUI stuff
 	}

src/player/Player.php

@@ -2448,7 +2448,7 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{
 		$this->cursorInventory = new PlayerCursorInventory($this);
 		$this->craftingGrid = new PlayerCraftingInventory($this);
 
-		$this->addPermanentInventories($this->inventory, $this->armorInventory, $this->cursorInventory, $this->offHandInventory);
+		$this->addPermanentInventories($this->inventory, $this->armorInventory, $this->cursorInventory, $this->offHandInventory, $this->craftingGrid);
 
 		//TODO: more windows
 	}

src/world/World.php

@@ -1321,7 +1321,9 @@ class World implements ChunkManager{
 
 	/**
 	 * Notify the blocks at and around the position that the block at the position may have changed.
-	 * This will cause onNeighbourBlockUpdate() to be called for these blocks.
+	 * This will cause onNearbyBlockChange() to be called for these blocks.
+	 *
+	 * @see Block::onNearbyBlockChange()
 	 */
 	public function notifyNeighbourBlockUpdate(Vector3 $pos) : void{
 		$this->tryAddToNeighbourUpdateQueue($pos);

tests/phpunit/network/mcpe/handler/StupidJsonDecodeTest.php

@@ -1,68 +0,0 @@
-<?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 PHPUnit\Framework\TestCase;
-
-class StupidJsonDecodeTest extends TestCase{
-	/**
-	 * @var \Closure
-	 * @phpstan-var \Closure(string $json, bool $assoc=) : mixed
-	 */
-	private $stupidJsonDecodeFunc;
-
-	public function setUp() : void{
-		$this->stupidJsonDecodeFunc = (new \ReflectionMethod(InGamePacketHandler::class, 'stupid_json_decode'))->getClosure();
-	}
-
-	/**
-	 * @return mixed[][]
-	 * @phpstan-return list<array{string,mixed}>
-	 */
-	public function stupidJsonDecodeProvider() : array{
-		return [
-			["[\n   \"a\",\"b,c,d,e\\\"   \",,0,1,2, false, 0.001]", ['a', 'b,c,d,e"   ', '', 0, 1, 2, false, 0.001]],
-			["0", 0],
-			["false", false],
-			["null", null],
-			['["\",,\"word","a\",,\"word2",]', ['",,"word', 'a",,"word2', '']],
-			['["\",,\"word","a\",,\"word2",""]', ['",,"word', 'a",,"word2', '']],
-			['["Hello,, PocketMine"]', ['Hello,, PocketMine']],
-			['[,]', ['', '']],
-			['[]', []]
-		];
-	}
-
-	/**
-	 * @dataProvider stupidJsonDecodeProvider
-	 *
-	 * @param mixed $expect
-	 *
-	 * @throws \ReflectionException
-	 */
-	public function testStupidJsonDecode(string $brokenJson, $expect) : void{
-		$decoded = ($this->stupidJsonDecodeFunc)($brokenJson, true);
-		self::assertEquals($expect, $decoded);
-	}
-}