From 6f5d4d6b80248568c17b84b0b941769814568832 Mon Sep 17 00:00:00 2001
From: "Dylan K. Taylor" <odigiman@gmail.com>
Date: Tue, 5 Mar 2019 10:52:36 +0000
Subject: [PATCH] RegionLoader: fixed handling of invalid chunk coordinates

---
 .../level/format/io/region/RegionLoader.php   | 36 ++++++++++++--
 .../format/io/region/RegionLoaderTest.php     | 47 +++++++++++++++++++
 2 files changed, 79 insertions(+), 4 deletions(-)

diff --git a/src/pocketmine/level/format/io/region/RegionLoader.php b/src/pocketmine/level/format/io/region/RegionLoader.php
index dfcd74dc9..decb9c1c5 100644
--- a/src/pocketmine/level/format/io/region/RegionLoader.php
+++ b/src/pocketmine/level/format/io/region/RegionLoader.php
@@ -124,9 +124,6 @@ class RegionLoader{
 	 */
 	public function readChunk(int $x, int $z) : ?string{
 		$index = self::getChunkOffset($x, $z);
-		if($index < 0 or $index >= 4096){
-			throw new \InvalidArgumentException("Invalid chunk position in region, expected x/z in range 0-31, got x=$x, z=$z");
-		}
 
 		$this->lastUsed = time();
 
@@ -169,10 +166,25 @@ class RegionLoader{
 		return substr($chunkData, 1);
 	}
 
+	/**
+	 * @param int $x
+	 * @param int $z
+	 *
+	 * @return bool
+	 * @throws \InvalidArgumentException
+	 */
 	public function chunkExists(int $x, int $z) : bool{
 		return $this->isChunkGenerated(self::getChunkOffset($x, $z));
 	}
 
+	/**
+	 * @param int    $x
+	 * @param int    $z
+	 * @param string $chunkData
+	 *
+	 * @throws ChunkException
+	 * @throws \InvalidArgumentException
+	 */
 	public function writeChunk(int $x, int $z, string $chunkData){
 		$this->lastUsed = time();
 
@@ -202,14 +214,30 @@ class RegionLoader{
 		}
 	}
 
+	/**
+	 * @param int $x
+	 * @param int $z
+	 *
+	 * @throws \InvalidArgumentException
+	 */
 	public function removeChunk(int $x, int $z){
 		$index = self::getChunkOffset($x, $z);
 		$this->locationTable[$index][0] = 0;
 		$this->locationTable[$index][1] = 0;
 	}
 
+	/**
+	 * @param int $x
+	 * @param int $z
+	 *
+	 * @return int
+	 * @throws \InvalidArgumentException
+	 */
 	protected static function getChunkOffset(int $x, int $z) : int{
-		return $x + ($z << 5);
+		if($x < 0 or $x > 31 or $z < 0 or $z > 31){
+			throw new \InvalidArgumentException("Invalid chunk position in region, expected x/z in range 0-31, got x=$x, z=$z");
+		}
+		return $x | ($z << 5);
 	}
 
 	/**
diff --git a/tests/phpunit/level/format/io/region/RegionLoaderTest.php b/tests/phpunit/level/format/io/region/RegionLoaderTest.php
index 5a80d73c0..5eed6482c 100644
--- a/tests/phpunit/level/format/io/region/RegionLoaderTest.php
+++ b/tests/phpunit/level/format/io/region/RegionLoaderTest.php
@@ -68,4 +68,51 @@ class RegionLoaderTest extends TestCase{
 		$r->open();
 		self::assertSame($data, $r->readChunk(0, 0));
 	}
+
+	public function outOfBoundsCoordsProvider() : \Generator{
+		yield [-1, -1];
+		yield [32, 32];
+		yield [-1, 32];
+		yield [32, -1];
+	}
+
+	/**
+	 * @dataProvider outOfBoundsCoordsProvider
+	 * @param int $x
+	 * @param int $z
+	 *
+	 * @throws ChunkException
+	 * @throws \InvalidArgumentException
+	 */
+	public function testWriteChunkOutOfBounds(int $x, int $z) : void{
+		$this->expectException(\InvalidArgumentException::class);
+		$this->region->writeChunk($x, $z, str_repeat("\x00", 1000));
+	}
+
+	public function testReadWriteChunkInBounds() : void{
+		$dat = random_bytes(1000);
+		for($x = 0; $x < 32; ++$x){
+			for($z = 0; $z < 32; ++$z){
+				$this->region->writeChunk($x, $z, $dat);
+			}
+		}
+		for($x = 0; $x < 32; ++$x){
+			for($z = 0; $z < 32; ++$z){
+				self::assertSame($dat, $this->region->readChunk($x, $z));
+			}
+		}
+	}
+
+	/**
+	 * @dataProvider outOfBoundsCoordsProvider
+	 * @param int $x
+	 * @param int $z
+	 *
+	 * @throws \InvalidArgumentException
+	 * @throws \pocketmine\level\format\io\exception\CorruptedChunkException
+	 */
+	public function testReadChunkOutOfBounds(int $x, int $z) : void{
+		$this->expectException(\InvalidArgumentException::class);
+		$this->region->readChunk($x, $z);
+	}
 }