diff --git a/README.md b/README.md index 402558916..4d376751c 100644 --- a/README.md +++ b/README.md @@ -4,13 +4,13 @@

- CI - GitHub release (latest SemVer) + CI + GitHub release (latest SemVer) Docker image version (latest semver) Discord
- GitHub all releases - GitHub release (latest by SemVer) + GitHub all releases + GitHub release (latest by SemVer)

## Getting started diff --git a/changelogs/3.26.md b/changelogs/3.26.md index 35b840a98..102e68aad 100644 --- a/changelogs/3.26.md +++ b/changelogs/3.26.md @@ -26,3 +26,7 @@ Plugin developers should **only** update their required API to this version if y - Fixed skins appearing black when using RTX resource packs. - Fixed chunks containing furnaces in old worlds (pre-2017) being discarded as corrupted. - This was caused by a strict corruption check detecting bad data created by a bug in PocketMine-MP that was fixed in 2017. + +# 3.26.5 +- Fixed several denial-of-service attack vectors related to writable book text length and encoding. +- Fixed several denial-of-service attack vectors related to skin data field lengths. diff --git a/changelogs/4.0.md b/changelogs/4.0.md index 211b825e8..a12c11cd4 100644 --- a/changelogs/4.0.md +++ b/changelogs/4.0.md @@ -1590,4 +1590,13 @@ Released 1st January 2022. - Fixed message length limit for chat (now 512 instead of 255, and accounts for UTF-8). - Fixed incorrect message being displayed when trying to sleep in a bed which is too far away. - Fixed missing space between `Kicked by admin.` and `Reason` when using `/kick` to kick a player. -- Fixed client-side performance issue of entities with very large scale. \ No newline at end of file +- Fixed client-side performance issue of entities with very large scale. + +# 4.0.5 +Released 4th January 2022. + +## Fixes +- Fixed several denial-of-service attack vectors related to writable book text length and encoding. +- Fixed several denial-of-service attack vectors related to skin data field lengths. +- Fixed food bar desync when cancelling `PlayerItemConsumeEvent` in a plugin. +- Fixed compass needles not updating when the world spawn is changed. diff --git a/composer.json b/composer.json index f3f680f7c..22410324f 100644 --- a/composer.json +++ b/composer.json @@ -53,7 +53,7 @@ "webmozart/path-util": "^2.3" }, "require-dev": { - "phpstan/phpstan": "1.2.0", + "phpstan/phpstan": "1.3.1", "phpstan/phpstan-phpunit": "^1.0.0", "phpstan/phpstan-strict-rules": "^1.0.0", "phpunit/phpunit": "^9.2" diff --git a/composer.lock b/composer.lock index 5786279f3..3b631974a 100644 --- a/composer.lock +++ b/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": "5d3bf1dc144f66c995d677cae7de8b63", + "content-hash": "8b6fac2b9bbfe685d654ed417713d894", "packages": [ { "name": "adhocore/json-comment", @@ -1900,16 +1900,16 @@ }, { "name": "phpstan/phpstan", - "version": "1.2.0", + "version": "1.3.1", "source": { "type": "git", "url": "https://github.com/phpstan/phpstan.git", - "reference": "cbe085f9fdead5b6d62e4c022ca52dc9427a10ee" + "reference": "c3e7a5837829b3cd5907b895da73a4da084a9f8f" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/phpstan/phpstan/zipball/cbe085f9fdead5b6d62e4c022ca52dc9427a10ee", - "reference": "cbe085f9fdead5b6d62e4c022ca52dc9427a10ee", + "url": "https://api.github.com/repos/phpstan/phpstan/zipball/c3e7a5837829b3cd5907b895da73a4da084a9f8f", + "reference": "c3e7a5837829b3cd5907b895da73a4da084a9f8f", "shasum": "" }, "require": { @@ -1925,7 +1925,7 @@ "type": "library", "extra": { "branch-alias": { - "dev-master": "1.2-dev" + "dev-master": "1.3-dev" } }, "autoload": { @@ -1940,7 +1940,7 @@ "description": "PHPStan - PHP Static Analysis Tool", "support": { "issues": "https://github.com/phpstan/phpstan/issues", - "source": "https://github.com/phpstan/phpstan/tree/1.2.0" + "source": "https://github.com/phpstan/phpstan/tree/1.3.1" }, "funding": [ { @@ -1960,7 +1960,7 @@ "type": "tidelift" } ], - "time": "2021-11-18T14:09:01+00:00" + "time": "2022-01-04T17:12:37+00:00" }, { "name": "phpstan/phpstan-phpunit", diff --git a/src/entity/Skin.php b/src/entity/Skin.php index d9489772f..604d6f6a2 100644 --- a/src/entity/Skin.php +++ b/src/entity/Skin.php @@ -24,6 +24,7 @@ declare(strict_types=1); namespace pocketmine\entity; use Ahc\Json\Comment as CommentedJsonDecoder; +use pocketmine\utils\Limits; use function implode; use function in_array; use function json_encode; @@ -49,7 +50,17 @@ final class Skin{ /** @var string */ private $geometryData; + private static function checkLength(string $string, string $name, int $maxLength) : void{ + if(strlen($string) > $maxLength){ + throw new InvalidSkinException("$name must be at most $maxLength bytes, but have " . strlen($string) . " bytes"); + } + } + public function __construct(string $skinId, string $skinData, string $capeData = "", string $geometryName = "", string $geometryData = ""){ + self::checkLength($skinId, "Skin ID", Limits::INT16_MAX); + self::checkLength($geometryName, "Geometry name", Limits::INT16_MAX); + self::checkLength($geometryData, "Geometry data", Limits::INT32_MAX); + if($skinId === ""){ throw new InvalidSkinException("Skin ID must not be empty"); } diff --git a/src/item/WritableBookPage.php b/src/item/WritableBookPage.php index 62ebbcf50..95d5a6258 100644 --- a/src/item/WritableBookPage.php +++ b/src/item/WritableBookPage.php @@ -23,17 +23,32 @@ declare(strict_types=1); namespace pocketmine\item; +use pocketmine\utils\Limits; use pocketmine\utils\Utils; +use function sprintf; +use function strlen; class WritableBookPage{ + public const PAGE_LENGTH_HARD_LIMIT_BYTES = Limits::INT16_MAX; + public const PHOTO_NAME_LENGTH_HARD_LIMIT_BYTES = Limits::INT16_MAX; /** @var string */ private $text; /** @var string */ private $photoName; + /** + * @throws \InvalidArgumentException + */ + private static function checkLength(string $string, string $name, int $maxLength) : void{ + if(strlen($string) > $maxLength){ + throw new \InvalidArgumentException(sprintf("$name must be at most %d bytes, but have %d bytes", $maxLength, strlen($string))); + } + } + public function __construct(string $text, string $photoName = ""){ - //TODO: data validation + self::checkLength($text, "Text", self::PAGE_LENGTH_HARD_LIMIT_BYTES); + self::checkLength($photoName, "Photo name", self::PHOTO_NAME_LENGTH_HARD_LIMIT_BYTES); Utils::checkUTF8($text); $this->text = $text; $this->photoName = $photoName; diff --git a/src/item/WrittenBook.php b/src/item/WrittenBook.php index f356b1cd6..88352cdd1 100644 --- a/src/item/WrittenBook.php +++ b/src/item/WrittenBook.php @@ -24,7 +24,10 @@ declare(strict_types=1); namespace pocketmine\item; use pocketmine\nbt\tag\CompoundTag; +use pocketmine\utils\Limits; use pocketmine\utils\Utils; +use function sprintf; +use function strlen; class WrittenBook extends WritableBookBase{ @@ -85,6 +88,9 @@ class WrittenBook extends WritableBookBase{ * @return $this */ public function setAuthor(string $authorName) : self{ + if(strlen($authorName) > Limits::INT16_MAX){ + throw new \InvalidArgumentException(sprintf("Author must be at most %d bytes, but have %d bytes", Limits::INT16_MAX, strlen($authorName))); + } Utils::checkUTF8($authorName); $this->author = $authorName; return $this; @@ -103,6 +109,9 @@ class WrittenBook extends WritableBookBase{ * @return $this */ public function setTitle(string $title) : self{ + if(strlen($title) > Limits::INT16_MAX){ + throw new \InvalidArgumentException(sprintf("Title must be at most %d bytes, but have %d bytes", Limits::INT16_MAX, strlen($title))); + } Utils::checkUTF8($title); $this->title = $title; return $this; diff --git a/src/network/mcpe/handler/InGamePacketHandler.php b/src/network/mcpe/handler/InGamePacketHandler.php index 94db1817f..eb3c2b6c6 100644 --- a/src/network/mcpe/handler/InGamePacketHandler.php +++ b/src/network/mcpe/handler/InGamePacketHandler.php @@ -37,6 +37,7 @@ use pocketmine\inventory\transaction\TransactionException; use pocketmine\inventory\transaction\TransactionValidationException; use pocketmine\item\VanillaItems; use pocketmine\item\WritableBook; +use pocketmine\item\WritableBookPage; use pocketmine\item\WrittenBook; use pocketmine\math\Facing; use pocketmine\math\Vector3; @@ -101,6 +102,8 @@ use pocketmine\network\mcpe\protocol\types\PlayerBlockActionWithBlockInfo; use pocketmine\network\PacketHandlingException; use pocketmine\player\Player; use pocketmine\utils\AssumptionFailedError; +use pocketmine\utils\Limits; +use pocketmine\utils\TextFormat; use function array_push; use function base64_encode; use function count; @@ -112,8 +115,10 @@ use function json_decode; use function json_encode; use function json_last_error_msg; 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; @@ -741,6 +746,24 @@ class InGamePacketHandler extends PacketHandler{ return false; //TODO } + /** + * @throws PacketHandlingException + */ + private function checkBookText(string $string, string $fieldName, int $softLimit, int $hardLimit, bool &$cancel) : string{ + if(strlen($string) > $hardLimit){ + throw new PacketHandlingException(sprintf("Book %s must be at most %d bytes, but have %d bytes", $fieldName, $hardLimit, strlen($string))); + } + + $result = TextFormat::clean($string, false); + //strlen() is O(1), mb_strlen() is O(n) + if(strlen($result) > $softLimit * 4 || mb_strlen($result, 'UTF-8') > $softLimit){ + $cancel = true; + $this->session->getLogger()->debug("Cancelled book edit due to $fieldName exceeded soft limit of $softLimit chars"); + } + + return $result; + } + public function handleBookEdit(BookEditPacket $packet) : bool{ //TODO: break this up into book API things $oldBook = $this->player->getInventory()->getItem($packet->inventorySlot); @@ -750,10 +773,11 @@ class InGamePacketHandler extends PacketHandler{ $newBook = clone $oldBook; $modifiedPages = []; - + $cancel = false; switch($packet->type){ case BookEditPacket::TYPE_REPLACE_PAGE: - $newBook->setPageText($packet->pageNumber, $packet->text); + $text = self::checkBookText($packet->text, "page text", 256, WritableBookPage::PAGE_LENGTH_HARD_LIMIT_BYTES, $cancel); + $newBook->setPageText($packet->pageNumber, $text); $modifiedPages[] = $packet->pageNumber; break; case BookEditPacket::TYPE_ADD_PAGE: @@ -762,7 +786,8 @@ class InGamePacketHandler extends PacketHandler{ //TODO: the client can send insert-before actions on trailing client-side pages which cause odd behaviour on the server return false; } - $newBook->insertPage($packet->pageNumber, $packet->text); + $text = self::checkBookText($packet->text, "page text", 256, WritableBookPage::PAGE_LENGTH_HARD_LIMIT_BYTES, $cancel); + $newBook->insertPage($packet->pageNumber, $text); $modifiedPages[] = $packet->pageNumber; break; case BookEditPacket::TYPE_DELETE_PAGE: @@ -781,11 +806,14 @@ class InGamePacketHandler extends PacketHandler{ $modifiedPages = [$packet->pageNumber, $packet->secondaryPageNumber]; break; case BookEditPacket::TYPE_SIGN_BOOK: - /** @var WrittenBook $newBook */ + $title = self::checkBookText($packet->title, "title", 16, Limits::INT16_MAX, $cancel); + //this one doesn't have a limit in vanilla, so we have to improvise + $author = self::checkBookText($packet->author, "author", 256, Limits::INT16_MAX, $cancel); + $newBook = VanillaItems::WRITTEN_BOOK() ->setPages($oldBook->getPages()) - ->setAuthor($packet->author) - ->setTitle($packet->title) + ->setAuthor($author) + ->setTitle($title) ->setGeneration(WrittenBook::GENERATION_ORIGINAL); break; default: @@ -801,7 +829,23 @@ class InGamePacketHandler extends PacketHandler{ BookEditPacket::TYPE_SIGN_BOOK => PlayerEditBookEvent::ACTION_SIGN_BOOK, default => throw new AssumptionFailedError("We already filtered unknown types in the switch above") }; + + /* + * Plugins may have created books with more than 50 pages; we allow plugins to do this, but not players. + * Don't allow the page count to grow past 50, but allow deleting, swapping or altering text of existing pages. + */ + $oldPageCount = count($oldBook->getPages()); + $newPageCount = count($newBook->getPages()); + if(($newPageCount > $oldPageCount && $newPageCount > 50)){ + $this->session->getLogger()->debug("Cancelled book edit due to adding too many pages (new page count would be $newPageCount)"); + $cancel = true; + } + $event = new PlayerEditBookEvent($this->player, $oldBook, $newBook, $action, $modifiedPages); + if($cancel){ + $event->cancel(); + } + $event->call(); if($event->isCancelled()){ return true; diff --git a/src/world/World.php b/src/world/World.php index 29c8cf121..777a2e1f4 100644 --- a/src/world/World.php +++ b/src/world/World.php @@ -2258,6 +2258,11 @@ class World implements ChunkManager{ $previousSpawn = $this->getSpawnLocation(); $this->provider->getWorldData()->setSpawn($pos); (new SpawnChangeEvent($this, $previousSpawn))->call(); + + $location = Position::fromObject($pos, $this); + foreach($this->players as $player){ + $player->getNetworkSession()->syncWorldSpawnPoint($location); + } } /**