Introduce dedicated NBT data exceptions, fix up some corrupted chunk handling

composer.lock

@@ -372,12 +372,12 @@
             "source": {
                 "type": "git",
                 "url": "https://github.com/pmmp/NBT.git",
-                "reference": "8b211471159b4aca329c665bdbd9149046ca6550"
+                "reference": "6736e85ec7600309d7ef1dc5b965ff7e4b1c5357"
             },
             "dist": {
                 "type": "zip",
-                "url": "https://api.github.com/repos/pmmp/NBT/zipball/8b211471159b4aca329c665bdbd9149046ca6550",
+                "url": "https://api.github.com/repos/pmmp/NBT/zipball/6736e85ec7600309d7ef1dc5b965ff7e4b1c5357",
-                "reference": "8b211471159b4aca329c665bdbd9149046ca6550",
+                "reference": "6736e85ec7600309d7ef1dc5b965ff7e4b1c5357",
                 "shasum": ""
             },
             "require": {
@@ -405,7 +405,7 @@
                 "source": "https://github.com/pmmp/NBT/tree/master",
                 "issues": "https://github.com/pmmp/NBT/issues"
             },
-            "time": "2019-01-09T00:12:13+00:00"
+            "time": "2019-01-18T15:28:20+00:00"
         },
         {
             "name": "pocketmine/raklib",

src/pocketmine/Player.php

@@ -88,10 +88,12 @@ use pocketmine\level\Level;
 use pocketmine\level\Position;
 use pocketmine\math\Vector3;
 use pocketmine\metadata\MetadataValue;
+use pocketmine\nbt\NbtDataException;
 use pocketmine\nbt\tag\ByteTag;
 use pocketmine\nbt\tag\CompoundTag;
 use pocketmine\nbt\tag\DoubleTag;
 use pocketmine\nbt\tag\ListTag;
+use pocketmine\network\BadPacketException;
 use pocketmine\network\mcpe\CompressBatchPromise;
 use pocketmine\network\mcpe\NetworkCipher;
 use pocketmine\network\mcpe\NetworkNbtSerializer;
@@ -2451,6 +2453,12 @@ class Player extends Human implements CommandSender, ChunkLoader, IPlayer{
 		return $handled;
 	}
 
+	/**
+	 * @param BlockEntityDataPacket $packet
+	 *
+	 * @return bool
+	 * @throws BadPacketException
+	 */
 	public function handleBlockEntityData(BlockEntityDataPacket $packet) : bool{
 		$this->doCloseInventory();
 
@@ -2462,7 +2470,11 @@ class Player extends Human implements CommandSender, ChunkLoader, IPlayer{
 		$t = $this->level->getTile($pos);
 		if($t instanceof Spawnable){
 			$nbt = new NetworkNbtSerializer();
+			try{
 				$compound = $nbt->read($packet->namedtag);
+			}catch(NbtDataException $e){
+				throw new BadPacketException($e->getMessage(), 0, $e);
+			}
 			if(!$t->updateCompoundTag($compound, $this)){
 				$t->spawnTo($this);
 			}

src/pocketmine/Server.php

@@ -59,6 +59,7 @@ use pocketmine\metadata\LevelMetadataStore;
 use pocketmine\metadata\PlayerMetadataStore;
 use pocketmine\nbt\BigEndianNbtSerializer;
 use pocketmine\nbt\NBT;
+use pocketmine\nbt\NbtDataException;
 use pocketmine\nbt\tag\ByteTag;
 use pocketmine\nbt\tag\CompoundTag;
 use pocketmine\nbt\tag\DoubleTag;
@@ -676,16 +677,14 @@ class Server{
 		if($this->shouldSavePlayerData()){
 			if(file_exists($path . "$name.dat")){
 				try{
-					$nbt = new BigEndianNbtSerializer();
+					return (new BigEndianNbtSerializer())->readCompressed(file_get_contents($path . "$name.dat"));
-					return $nbt->readCompressed(file_get_contents($path . "$name.dat"));
+				}catch(NbtDataException $e){ //zlib decode error / corrupt data
-				}catch(\Throwable $e){ //zlib decode error / corrupt data
 					rename($path . "$name.dat", $path . "$name.dat.bak");
-					$this->logger->notice($this->getLanguage()->translateString("pocketmine.data.playerCorrupted", [$name]));
+					$this->logger->error($this->getLanguage()->translateString("pocketmine.data.playerCorrupted", [$name]));
+				}
 			}
-			}else{
 			$this->logger->notice($this->getLanguage()->translateString("pocketmine.data.playerNotFound", [$name]));
 		}
-		}
 		$spawn = $this->levelManager->getDefaultLevel()->getSafeSpawn();
 		$currentTimeMillis = (int) (microtime(true) * 1000);
 

src/pocketmine/command/defaults/GiveCommand.php

@@ -29,7 +29,7 @@ use pocketmine\command\utils\InvalidCommandSyntaxException;
 use pocketmine\item\ItemFactory;
 use pocketmine\lang\TranslationContainer;
 use pocketmine\nbt\JsonNbtParser;
-use pocketmine\nbt\tag\CompoundTag;
+use pocketmine\nbt\NbtDataException;
 use pocketmine\utils\TextFormat;
 use function array_slice;
 use function count;
@@ -75,16 +75,11 @@ class GiveCommand extends VanillaCommand{
 		}
 
 		if(isset($args[3])){
-			$tags = $exception = null;
 			$data = implode(" ", array_slice($args, 3));
 			try{
 				$tags = JsonNbtParser::parseJson($data);
-			}catch(\Exception $ex){
+			}catch(NbtDataException $e){
-				$exception = $ex;
+				$sender->sendMessage(new TranslationContainer("commands.give.tagError", [$e->getMessage()]));
-			}
-
-			if(!($tags instanceof CompoundTag) or $exception !== null){
-				$sender->sendMessage(new TranslationContainer("commands.give.tagError", [$exception !== null ? $exception->getMessage() : "Invalid tag conversion"]));
 				return true;
 			}
 

src/pocketmine/item/Item.php

@@ -35,6 +35,7 @@ use pocketmine\item\enchantment\EnchantmentInstance;
 use pocketmine\math\Vector3;
 use pocketmine\nbt\LittleEndianNbtSerializer;
 use pocketmine\nbt\NBT;
+use pocketmine\nbt\NbtDataException;
 use pocketmine\nbt\tag\ByteTag;
 use pocketmine\nbt\tag\CompoundTag;
 use pocketmine\nbt\tag\ListTag;
@@ -64,11 +65,13 @@ class Item implements ItemIds, \JsonSerializable{
 	/** @var LittleEndianNbtSerializer */
 	private static $cachedParser = null;
 
+	/**
+	 * @param string $tag
+	 *
+	 * @return CompoundTag
+	 * @throws NbtDataException
+	 */
 	private static function parseCompoundTag(string $tag) : CompoundTag{
-		if($tag === ""){
-			throw new \InvalidArgumentException("No NBT data found in supplied string");
-		}
-
 		if(self::$cachedParser === null){
 			self::$cachedParser = new LittleEndianNbtSerializer();
 		}

src/pocketmine/level/format/io/leveldb/LevelDB.php

@@ -27,13 +27,16 @@ use pocketmine\level\format\Chunk;
 use pocketmine\level\format\io\BaseLevelProvider;
 use pocketmine\level\format\io\ChunkUtils;
 use pocketmine\level\format\io\data\BedrockLevelData;
+use pocketmine\level\format\io\exception\CorruptedChunkException;
 use pocketmine\level\format\io\exception\UnsupportedChunkFormatException;
 use pocketmine\level\format\io\exception\UnsupportedLevelFormatException;
 use pocketmine\level\format\io\LevelData;
 use pocketmine\level\format\SubChunk;
 use pocketmine\nbt\LittleEndianNbtSerializer;
+use pocketmine\nbt\NbtDataException;
 use pocketmine\nbt\tag\CompoundTag;
 use pocketmine\utils\Binary;
+use pocketmine\utils\BinaryDataException;
 use pocketmine\utils\BinaryStream;
 use function array_values;
 use function chr;
@@ -130,6 +133,7 @@ class LevelDB extends BaseLevelProvider{
 	 * @param int $chunkZ
 	 *
 	 * @return Chunk|null
+	 * @throws CorruptedChunkException
 	 * @throws UnsupportedChunkFormatException
 	 */
 	protected function readChunk(int $chunkX, int $chunkZ) : ?Chunk{
@@ -164,11 +168,15 @@ class LevelDB extends BaseLevelProvider{
 						continue;
 					}
 
-					$binaryStream->setBuffer($data, 0);
+					$binaryStream->setBuffer($data);
+					if($binaryStream->feof()){
+						throw new CorruptedChunkException("Unexpected empty data for subchunk $y");
+					}
 					$subChunkVersion = $binaryStream->getByte();
 
 					switch($subChunkVersion){
 						case 0:
+							try{
 								$blocks = $binaryStream->get(4096);
 								$blockData = $binaryStream->get(2048);
 								if($chunkVersion < 4){
@@ -180,6 +188,9 @@ class LevelDB extends BaseLevelProvider{
 									$blockLight = "";
 									$lightPopulated = false;
 								}
+							}catch(BinaryDataException $e){
+								throw new CorruptedChunkException($e->getMessage(), 0, $e);
+							}
 
 							$subChunks[$y] = new SubChunk($blocks, $blockData, $blockSkyLight, $blockLight);
 							break;
@@ -190,18 +201,30 @@ class LevelDB extends BaseLevelProvider{
 				}
 
 				if(($maps2d = $this->db->get($index . self::TAG_DATA_2D)) !== false){
-					$binaryStream->setBuffer($maps2d, 0);
+					$binaryStream->setBuffer($maps2d);
 
+					try{
 						$heightMap = array_values(unpack("v*", $binaryStream->get(512)));
 						$biomeIds = $binaryStream->get(256);
+					}catch(BinaryDataException $e){
+						throw new CorruptedChunkException($e->getMessage(), 0, $e);
+					}
 				}
 				break;
 			case 2: // < MCPE 1.0
-				$binaryStream->setBuffer($this->db->get($index . self::TAG_LEGACY_TERRAIN));
+				$legacyTerrain = $this->db->get($index . self::TAG_LEGACY_TERRAIN);
+				if($legacyTerrain === false){
+					throw new CorruptedChunkException("Missing expected LEGACY_TERRAIN tag for format version $chunkVersion");
+				}
+				$binaryStream->setBuffer($legacyTerrain);
+				try{
 					$fullIds = $binaryStream->get(32768);
 					$fullData = $binaryStream->get(16384);
 					$fullSkyLight = $binaryStream->get(16384);
 					$fullBlockLight = $binaryStream->get(16384);
+				}catch(BinaryDataException $e){
+					throw new CorruptedChunkException($e->getMessage(), 0, $e);
+				}
 
 				for($yy = 0; $yy < 8; ++$yy){
 					$subOffset = ($yy << 4);
@@ -231,8 +254,12 @@ class LevelDB extends BaseLevelProvider{
 					$subChunks[$yy] = new SubChunk($ids, $data, $skyLight, $blockLight);
 				}
 
+				try{
 					$heightMap = array_values(unpack("C*", $binaryStream->get(256)));
 					$biomeIds = ChunkUtils::convertBiomeColors(array_values(unpack("N*", $binaryStream->get(1024))));
+				}catch(BinaryDataException $e){
+					throw new CorruptedChunkException($e->getMessage(), 0, $e);
+				}
 				break;
 			default:
 				//TODO: set chunks read-only so the version on disk doesn't get overwritten
@@ -244,13 +271,21 @@ class LevelDB extends BaseLevelProvider{
 		/** @var CompoundTag[] $entities */
 		$entities = [];
 		if(($entityData = $this->db->get($index . self::TAG_ENTITY)) !== false and $entityData !== ""){
+			try{
 				$entities = $nbt->readMultiple($entityData);
+			}catch(NbtDataException $e){
+				throw new CorruptedChunkException($e->getMessage(), 0, $e);
+			}
 		}
 
 		/** @var CompoundTag[] $tiles */
 		$tiles = [];
 		if(($tileData = $this->db->get($index . self::TAG_BLOCK_ENTITY)) !== false and $tileData !== ""){
+			try{
 				$tiles = $nbt->readMultiple($tileData);
+			}catch(NbtDataException $e){
+				throw new CorruptedChunkException($e->getMessage(), 0, $e);
+			}
 		}
 
 		//TODO: extra data should be converted into blockstorage layers (first they need to be implemented!)

src/pocketmine/level/format/io/region/LegacyAnvilChunkTrait.php

@@ -29,6 +29,7 @@ use pocketmine\level\format\io\exception\CorruptedChunkException;
 use pocketmine\level\format\SubChunk;
 use pocketmine\nbt\BigEndianNbtSerializer;
 use pocketmine\nbt\NBT;
+use pocketmine\nbt\NbtDataException;
 use pocketmine\nbt\tag\CompoundTag;
 use pocketmine\nbt\tag\IntArrayTag;
 use pocketmine\nbt\tag\ListTag;
@@ -95,9 +96,19 @@ trait LegacyAnvilChunkTrait{
 
 	abstract protected function serializeSubChunk(SubChunk $subChunk) : CompoundTag;
 
+	/**
+	 * @param string $data
+	 *
+	 * @return Chunk
+	 * @throws CorruptedChunkException
+	 */
 	protected function deserializeChunk(string $data) : Chunk{
 		$nbt = new BigEndianNbtSerializer();
+		try{
 			$chunk = $nbt->readCompressed($data);
+		}catch(NbtDataException $e){
+			throw new CorruptedChunkException($e->getMessage(), 0, $e);
+		}
 		if(!$chunk->hasTag("Level")){
 			throw new CorruptedChunkException("'Level' key is missing from chunk NBT");
 		}

src/pocketmine/level/format/io/region/McRegion.php

@@ -29,6 +29,7 @@ use pocketmine\level\format\io\exception\CorruptedChunkException;
 use pocketmine\level\format\SubChunk;
 use pocketmine\nbt\BigEndianNbtSerializer;
 use pocketmine\nbt\NBT;
+use pocketmine\nbt\NbtDataException;
 use pocketmine\nbt\tag\ByteArrayTag;
 use pocketmine\nbt\tag\CompoundTag;
 use pocketmine\nbt\tag\IntArrayTag;
@@ -107,7 +108,11 @@ class McRegion extends RegionLevelProvider{
 	 */
 	protected function deserializeChunk(string $data) : Chunk{
 		$nbt = new BigEndianNbtSerializer();
+		try{
 			$chunk = $nbt->readCompressed($data);
+		}catch(NbtDataException $e){
+			throw new CorruptedChunkException($e->getMessage(), 0, $e);
+		}
 		if(!$chunk->hasTag("Level")){
 			throw new CorruptedChunkException("'Level' key is missing from chunk NBT");
 		}

src/pocketmine/level/format/io/region/RegionLevelProvider.php

@@ -176,6 +176,13 @@ abstract class RegionLevelProvider extends BaseLevelProvider{
 	 */
 	abstract protected function deserializeChunk(string $data) : Chunk;
 
+	/**
+	 * @param int $chunkX
+	 * @param int $chunkZ
+	 *
+	 * @return Chunk|null
+	 * @throws CorruptedChunkException
+	 */
 	protected function readChunk(int $chunkX, int $chunkZ) : ?Chunk{
 		$regionX = $regionZ = null;
 		self::getRegionIndex($chunkX, $chunkZ, $regionX, $regionZ);

src/pocketmine/network/mcpe/NetworkBinaryStream.php

@@ -31,6 +31,7 @@ use pocketmine\item\Item;
 use pocketmine\item\ItemFactory;
 use pocketmine\math\Vector3;
 use pocketmine\nbt\LittleEndianNbtSerializer;
+use pocketmine\nbt\NbtDataException;
 use pocketmine\network\BadPacketException;
 use pocketmine\network\mcpe\protocol\types\CommandOriginData;
 use pocketmine\network\mcpe\protocol\types\EntityLink;
@@ -105,7 +106,7 @@ class NetworkBinaryStream extends BinaryStream{
 			}
 			try{
 				$compound = self::$itemNbtSerializer->read($this->get($nbtLen));
-			}catch(\UnexpectedValueException $e){
+			}catch(NbtDataException $e){
 				throw new BadPacketException($e->getMessage(), 0, $e);
 			}
 		}