From bc709efb770dadc0825692c7557fbc4f10950930 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Sat, 19 Jan 2019 15:14:51 +0000 Subject: [PATCH 1/4] Prevent stupidity with /enchant --- src/pocketmine/command/defaults/EnchantCommand.php | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/pocketmine/command/defaults/EnchantCommand.php b/src/pocketmine/command/defaults/EnchantCommand.php index 238e91aa2..c40886297 100644 --- a/src/pocketmine/command/defaults/EnchantCommand.php +++ b/src/pocketmine/command/defaults/EnchantCommand.php @@ -77,7 +77,15 @@ class EnchantCommand extends VanillaCommand{ return true; } - $item->addEnchantment(new EnchantmentInstance($enchantment, (int) ($args[2] ?? 1))); + $level = 1; + if(isset($args[2])){ + $level = $this->getBoundedInt($sender, $args[2], 1, $enchantment->getMaxLevel()); + if($level === null){ + return false; + } + } + + $item->addEnchantment(new EnchantmentInstance($enchantment, $level)); $player->getInventory()->setItemInHand($item); From 82dddde159c249eccbaebf5f5ab0543c3075553d Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Sat, 19 Jan 2019 15:15:04 +0000 Subject: [PATCH 2/4] Remove dead code in /time command handler --- src/pocketmine/command/defaults/TimeCommand.php | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/pocketmine/command/defaults/TimeCommand.php b/src/pocketmine/command/defaults/TimeCommand.php index 510709855..61a9967e1 100644 --- a/src/pocketmine/command/defaults/TimeCommand.php +++ b/src/pocketmine/command/defaults/TimeCommand.php @@ -55,9 +55,7 @@ class TimeCommand extends VanillaCommand{ return true; } foreach($sender->getServer()->getLevels() as $level){ - $level->checkTime(); $level->startTime(); - $level->checkTime(); } Command::broadcastCommandMessage($sender, "Restarted the time"); return true; @@ -68,9 +66,7 @@ class TimeCommand extends VanillaCommand{ return true; } foreach($sender->getServer()->getLevels() as $level){ - $level->checkTime(); $level->stopTime(); - $level->checkTime(); } Command::broadcastCommandMessage($sender, "Stopped the time"); return true; @@ -110,9 +106,7 @@ class TimeCommand extends VanillaCommand{ } foreach($sender->getServer()->getLevels() as $level){ - $level->checkTime(); $level->setTime($value); - $level->checkTime(); } Command::broadcastCommandMessage($sender, new TranslationContainer("commands.time.set", [$value])); }elseif($args[0] === "add"){ @@ -124,9 +118,7 @@ class TimeCommand extends VanillaCommand{ $value = $this->getInteger($sender, $args[1], 0); foreach($sender->getServer()->getLevels() as $level){ - $level->checkTime(); $level->setTime($level->getTime() + $value); - $level->checkTime(); } Command::broadcastCommandMessage($sender, new TranslationContainer("commands.time.added", [$value])); }else{ From 41fd7545e307c171426e8cd00a7d2b589d63381e Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Sat, 19 Jan 2019 15:49:19 +0000 Subject: [PATCH 3/4] RegionLoader: Account for unexpected EOF when reading chunks, closes #2676 --- .../level/format/io/region/RegionLoader.php | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/src/pocketmine/level/format/io/region/RegionLoader.php b/src/pocketmine/level/format/io/region/RegionLoader.php index 42c6ddf3c..dfcd74dc9 100644 --- a/src/pocketmine/level/format/io/region/RegionLoader.php +++ b/src/pocketmine/level/format/io/region/RegionLoader.php @@ -46,6 +46,7 @@ use function str_pad; use function stream_set_read_buffer; use function stream_set_write_buffer; use function strlen; +use function substr; use function time; use function touch; use function unpack; @@ -134,8 +135,11 @@ class RegionLoader{ } fseek($this->filePointer, $this->locationTable[$index][0] << 12); - $length = Binary::readInt(fread($this->filePointer, 4)); - $compression = ord(fgetc($this->filePointer)); + $prefix = fread($this->filePointer, 4); + if($prefix === false or strlen($prefix) !== 4){ + throw new CorruptedChunkException("Corrupted chunk header detected (unexpected end of file reading length prefix)"); + } + $length = Binary::readInt($prefix); if($length <= 0 or $length > self::MAX_SECTOR_LENGTH){ //Not yet generated / corrupted if($length >= self::MAX_SECTOR_LENGTH){ @@ -150,17 +154,19 @@ class RegionLoader{ MainLogger::getLogger()->error("Corrupted bigger chunk detected (bigger than number of sectors given in header)"); $this->locationTable[$index][1] = $length >> 12; $this->writeLocationIndex($index); - }elseif($compression !== self::COMPRESSION_ZLIB and $compression !== self::COMPRESSION_GZIP){ + } + + $chunkData = fread($this->filePointer, $length); + if($chunkData === false or strlen($chunkData) !== $length){ + throw new CorruptedChunkException("Corrupted chunk detected (unexpected end of file reading chunk data)"); + } + + $compression = ord($chunkData[0]); + if($compression !== self::COMPRESSION_ZLIB and $compression !== self::COMPRESSION_GZIP){ throw new CorruptedChunkException("Invalid compression type (got $compression, expected " . self::COMPRESSION_ZLIB . " or " . self::COMPRESSION_GZIP . ")"); } - $chunkData = fread($this->filePointer, $length - 1); - if($chunkData === false){ - throw new CorruptedChunkException("Corrupted chunk detected (failed to read chunk data from disk)"); - - } - - return $chunkData; + return substr($chunkData, 1); } public function chunkExists(int $x, int $z) : bool{ From 4fd3bee3607daf090fc235547f23fe64c96db2d1 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Sat, 19 Jan 2019 16:05:10 +0000 Subject: [PATCH 4/4] Entity: Address fireticks crashdumps This will now throw an exception at the source instead of crashing when the entity is saved, which should put the blame on the correct plugin responsible for this. This also includes magic method hacks to preserve backwards compatibility, since the fireTicks field is now protected. --- src/pocketmine/block/Water.php | 2 +- src/pocketmine/entity/Entity.php | 53 +++++++++++++++++-- .../entity/projectile/Projectile.php | 2 +- 3 files changed, 52 insertions(+), 5 deletions(-) diff --git a/src/pocketmine/block/Water.php b/src/pocketmine/block/Water.php index 00eeb3a9d..8f490c4aa 100644 --- a/src/pocketmine/block/Water.php +++ b/src/pocketmine/block/Water.php @@ -67,7 +67,7 @@ class Water extends Liquid{ public function onEntityCollide(Entity $entity) : void{ $entity->resetFallDistance(); - if($entity->fireTicks > 0){ + if($entity->isOnFire()){ $entity->extinguish(); } diff --git a/src/pocketmine/entity/Entity.php b/src/pocketmine/entity/Entity.php index 0f3702c6c..3cade37c1 100644 --- a/src/pocketmine/entity/Entity.php +++ b/src/pocketmine/entity/Entity.php @@ -477,7 +477,7 @@ abstract class Entity extends Location implements Metadatable, EntityIds{ /** @var int */ public $lastUpdate; /** @var int */ - public $fireTicks = 0; + protected $fireTicks = 0; /** @var CompoundTag */ public $namedtag; /** @var bool */ @@ -1093,8 +1093,8 @@ abstract class Entity extends Location implements Metadatable, EntityIds{ public function setOnFire(int $seconds) : void{ $ticks = $seconds * 20; - if($ticks > $this->fireTicks){ - $this->fireTicks = $ticks; + if($ticks > $this->getFireTicks()){ + $this->setFireTicks($ticks); } $this->setGenericFlag(self::DATA_FLAG_ONFIRE, true); @@ -1109,8 +1109,12 @@ abstract class Entity extends Location implements Metadatable, EntityIds{ /** * @param int $fireTicks + * @throws \InvalidArgumentException */ public function setFireTicks(int $fireTicks) : void{ + if($fireTicks < 0 or $fireTicks > 0x7fff){ + throw new \InvalidArgumentException("Fire ticks must be in range 0 ... " . 0x7fff . ", got $fireTicks"); + } $this->fireTicks = $fireTicks; } @@ -2187,4 +2191,47 @@ abstract class Entity extends Location implements Metadatable, EntityIds{ public function __toString(){ return (new \ReflectionClass($this))->getShortName() . "(" . $this->getId() . ")"; } + + /** + * TODO: remove this BC hack in 4.0 + * + * @param string $name + * + * @return mixed + * @throws \ErrorException + */ + public function __get($name){ + if($name === "fireTicks"){ + return $this->fireTicks; + } + throw new \ErrorException("Undefined property: " . get_class($this) . "::\$" . $name); + } + + /** + * TODO: remove this BC hack in 4.0 + * + * @param string $name + * @param mixed $value + * + * @throws \ErrorException + * @throws \InvalidArgumentException + */ + public function __set($name, $value){ + if($name === "fireTicks"){ + $this->setFireTicks($value); + }else{ + throw new \ErrorException("Undefined property: " . get_class($this) . "::\$" . $name); + } + } + + /** + * TODO: remove this BC hack in 4.0 + * + * @param string $name + * + * @return bool + */ + public function __isset($name){ + return $name === "fireTicks"; + } } diff --git a/src/pocketmine/entity/projectile/Projectile.php b/src/pocketmine/entity/projectile/Projectile.php index fd561a968..c1bb3e881 100644 --- a/src/pocketmine/entity/projectile/Projectile.php +++ b/src/pocketmine/entity/projectile/Projectile.php @@ -319,7 +319,7 @@ abstract class Projectile extends Entity{ $entityHit->attack($ev); - if($this->fireTicks > 0){ + if($this->isOnFire()){ $ev = new EntityCombustByEntityEvent($this, $entityHit, 5); $ev->call(); if(!$ev->isCancelled()){