Skip to content

Commit c3c7b0a

Browse files
committed
Add length limit checks for CompoundTag and TreeRoot names
I am amazed we weren't checking this already. This ensures the error shows up in the place that caused it, and not when it gets encoded.
1 parent cfd53a8 commit c3c7b0a

File tree

4 files changed

+39
-0
lines changed

4 files changed

+39
-0
lines changed

src/TreeRoot.php

+6
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@
2525

2626
use pocketmine\nbt\tag\CompoundTag;
2727
use pocketmine\nbt\tag\Tag;
28+
use pocketmine\utils\Limits;
29+
use function sprintf;
30+
use function strlen;
2831

2932
/**
3033
* This class wraps around the root Tag for NBT files to avoid losing the name information.
@@ -37,6 +40,9 @@ class TreeRoot{
3740
private $name;
3841

3942
public function __construct(Tag $root, string $name = ""){
43+
if(strlen($name) > Limits::INT16_MAX){
44+
throw new \InvalidArgumentException(sprintf("Tag name must be at most %d bytes, but got %d bytes", Limits::INT16_MAX, strlen($name)));
45+
}
4046
$this->root = $root;
4147
$this->name = $name;
4248
}

src/tag/CompoundTag.php

+6
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,14 @@
2929
use pocketmine\nbt\NoSuchTagException;
3030
use pocketmine\nbt\ReaderTracker;
3131
use pocketmine\nbt\UnexpectedTagTypeException;
32+
use pocketmine\utils\Limits;
3233
use function count;
3334
use function func_num_args;
3435
use function get_class;
3536
use function is_int;
37+
use function sprintf;
3638
use function str_repeat;
39+
use function strlen;
3740
use function strval;
3841

3942
/**
@@ -115,6 +118,9 @@ public function getCompoundTag(string $name) : ?CompoundTag{
115118
* @return $this
116119
*/
117120
public function setTag(string $name, Tag $tag) : self{
121+
if(strlen($name) > Limits::INT16_MAX){
122+
throw new \InvalidArgumentException(sprintf("Tag name must be at most %d bytes, but got %d bytes", Limits::INT16_MAX, strlen($name)));
123+
}
118124
$this->value[$name] = $tag;
119125
return $this;
120126
}

tests/phpunit/TreeRootTest.php

+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
<?php
2+
3+
namespace pocketmine\nbt;
4+
5+
use PHPUnit\Framework\TestCase;
6+
use pocketmine\nbt\tag\IntTag;
7+
use pocketmine\utils\Limits;
8+
use function str_repeat;
9+
10+
class TreeRootTest extends TestCase{
11+
12+
public function testNameLength() : void{
13+
new TreeRoot(new IntTag(1), str_repeat(".", Limits::INT16_MAX)); //ok
14+
15+
$this->expectException(\InvalidArgumentException::class);
16+
new TreeRoot(new IntTag(1), str_repeat(".", Limits::INT16_MAX + 1)); //error
17+
}
18+
}

tests/phpunit/tag/CompoundTagTest.php

+9
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
namespace pocketmine\nbt\tag;
2525

2626
use PHPUnit\Framework\TestCase;
27+
use pocketmine\utils\Limits;
2728
use function array_fill;
2829
use function str_repeat;
2930

@@ -184,5 +185,13 @@ public function testModificationDuringIteration() : void{
184185
self::assertCount(0, $tag);
185186
}
186187

188+
public function testNameLength() : void{
189+
$tag = CompoundTag::create();
190+
$tag->setTag(str_repeat(".", Limits::INT16_MAX), new IntTag(1)); //ok
191+
192+
$this->expectException(\InvalidArgumentException::class);
193+
$tag->setTag(str_repeat(".", Limits::INT16_MAX + 1), new IntTag(1)); //error
194+
}
195+
187196
//TODO: add more tests
188197
}

0 commit comments

Comments
 (0)