Skip to content

Commit d0d84d4

Browse files
committed
New rule: explode() limit parameter must be set
1 parent 3a2d0d7 commit d0d84d4

19 files changed

+136
-25
lines changed

build/dump-version-info.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636
*/
3737
$options = [
3838
"base_version" => VersionInfo::BASE_VERSION,
39-
"major_version" => fn() => explode(".", VersionInfo::BASE_VERSION)[0],
39+
"major_version" => fn() => explode(".", VersionInfo::BASE_VERSION, limit: 2)[0],
4040
"mcpe_version" => ProtocolInfo::MINECRAFT_VERSION_NETWORK,
4141
"is_dev" => VersionInfo::IS_DEVELOPMENT_BUILD,
4242
"changelog_file_name" => function() : string{

phpstan.neon.dist

+1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ rules:
1313
- pocketmine\phpstan\rules\DeprecatedLegacyEnumAccessRule
1414
- pocketmine\phpstan\rules\DisallowEnumComparisonRule
1515
- pocketmine\phpstan\rules\DisallowForeachByReferenceRule
16+
- pocketmine\phpstan\rules\ExplodeLimitRule
1617
- pocketmine\phpstan\rules\UnsafeForeachArrayOfStringRule
1718
# - pocketmine\phpstan\rules\ThreadedSupportedTypesRule
1819

src/PocketMine.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,7 @@ function server(){
264264
$composerGitHash = InstalledVersions::getReference('pocketmine/pocketmine-mp');
265265
if($composerGitHash !== null){
266266
//we can't verify dependency versions if we were installed without using git
267-
$currentGitHash = explode("-", VersionInfo::GIT_HASH())[0];
267+
$currentGitHash = explode("-", VersionInfo::GIT_HASH(), 2)[0];
268268
if($currentGitHash !== $composerGitHash){
269269
critical_error("Composer dependencies and/or autoloader are out of sync.");
270270
critical_error("- Current revision is $currentGitHash");

src/block/tile/Sign.php

+2-1
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,10 @@ class Sign extends Spawnable{
6262

6363
/**
6464
* @return string[]
65+
* @deprecated
6566
*/
6667
public static function fixTextBlob(string $blob) : array{
67-
return array_slice(array_pad(explode("\n", $blob), 4, ""), 0, 4);
68+
return array_slice(array_pad(explode("\n", $blob, limit: 5), 4, ""), 0, 4);
6869
}
6970

7071
protected SignText $text;

src/block/utils/SignText.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ public function __construct(?array $lines = null, ?Color $baseColor = null, bool
7979
* @throws \InvalidArgumentException if the text is not valid UTF-8
8080
*/
8181
public static function fromBlob(string $blob, ?Color $baseColor = null, bool $glowing = false) : SignText{
82-
return new self(array_slice(array_pad(explode("\n", $blob), self::LINE_COUNT, ""), 0, self::LINE_COUNT), $baseColor, $glowing);
82+
return new self(array_slice(array_pad(explode("\n", $blob, limit: self::LINE_COUNT + 1), self::LINE_COUNT, ""), 0, self::LINE_COUNT), $baseColor, $glowing);
8383
}
8484

8585
/**

src/command/Command.php

+2-1
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
use function explode;
3838
use function implode;
3939
use function str_replace;
40+
use const PHP_INT_MAX;
4041

4142
abstract class Command{
4243

@@ -113,7 +114,7 @@ public function setPermissions(array $permissions) : void{
113114
}
114115

115116
public function setPermission(?string $permission) : void{
116-
$this->setPermissions($permission === null ? [] : explode(";", $permission));
117+
$this->setPermissions($permission === null ? [] : explode(";", $permission, limit: PHP_INT_MAX));
117118
}
118119

119120
public function testPermission(CommandSender $target, ?string $permission = null) : bool{

src/command/defaults/HelpCommand.php

+2-1
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
use function min;
4040
use function sort;
4141
use function strtolower;
42+
use const PHP_INT_MAX;
4243
use const SORT_FLAG_CASE;
4344
use const SORT_NATURAL;
4445

@@ -108,7 +109,7 @@ public function execute(CommandSender $sender, string $commandLabel, array $args
108109

109110
$usage = $cmd->getUsage();
110111
$usageString = $usage instanceof Translatable ? $lang->translate($usage) : $usage;
111-
$sender->sendMessage(KnownTranslationFactory::pocketmine_command_help_specificCommand_usage(TextFormat::RESET . implode("\n" . TextFormat::RESET, explode("\n", $usageString)))
112+
$sender->sendMessage(KnownTranslationFactory::pocketmine_command_help_specificCommand_usage(TextFormat::RESET . implode("\n" . TextFormat::RESET, explode("\n", $usageString, limit: PHP_INT_MAX)))
112113
->prefix(TextFormat::GOLD));
113114

114115
$aliases = $cmd->getAliases();

src/command/defaults/ParticleCommand.php

+5-1
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,11 @@ private function getParticle(string $name, ?string $data = null) : ?Particle{
219219
break;
220220
case "blockdust":
221221
if($data !== null){
222-
$d = explode("_", $data);
222+
//to preserve the old unlimited explode behaviour, allow this to split into at most 5 parts
223+
//this allows the 4th argument to be processed normally if given without forcing it to also consume
224+
//any unexpected parts
225+
//we probably ought to error in this case, but this will do for now
226+
$d = explode("_", $data, limit: 5);
223227
if(count($d) >= 3){
224228
return new DustParticle(new Color(
225229
((int) $d[0]) & 0xff,

src/console/ConsoleCommandSender.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ public function sendMessage(Translatable|string $message) : void{
6262
$message = $this->getLanguage()->translate($message);
6363
}
6464

65-
foreach(explode("\n", trim($message)) as $line){
65+
foreach(explode("\n", trim($message), limit: PHP_INT_MAX) as $line){
6666
Terminal::writeLine(TextFormat::GREEN . "Command output | " . TextFormat::addBase(TextFormat::WHITE, $line));
6767
}
6868
}

src/item/LegacyStringToItemParser.php

+2-1
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,8 @@ public function getMappings() : array{
111111
*/
112112
public function parse(string $input) : Item{
113113
$key = $this->reprocess($input);
114-
$b = explode(":", $key);
114+
//TODO: this should be limited to 2 parts, but 3 preserves old behaviour when given a string like 351:4:1
115+
$b = explode(":", $key, limit: 3);
115116

116117
if(!isset($b[1])){
117118
$meta = 0;

src/lang/Language.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ public static function getLanguageList(string $path = "") : array{
7171

7272
foreach($files as $file){
7373
try{
74-
$code = explode(".", $file)[0];
74+
$code = explode(".", $file, limit: 2)[0];
7575
$strings = self::loadLang($path, $code);
7676
if(isset($strings[KnownTranslationKeys::LANGUAGE_NAME])){
7777
$result[$code] = $strings[KnownTranslationKeys::LANGUAGE_NAME];

src/network/mcpe/JwtUtils.php

+4-2
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,11 @@ final class JwtUtils{
7272
* @throws JwtException
7373
*/
7474
public static function split(string $jwt) : array{
75-
$v = explode(".", $jwt);
75+
//limit of 4 allows us to detect too many parts without having to split the string up into a potentially large
76+
//number of parts
77+
$v = explode(".", $jwt, limit: 4);
7678
if(count($v) !== 3){
77-
throw new JwtException("Expected exactly 3 JWT parts, got " . count($v));
79+
throw new JwtException("Expected exactly 3 JWT parts delimited by a period");
7880
}
7981
return [$v[0], $v[1], $v[2]]; //workaround phpstan bug
8082
}

src/permission/BanEntry.php

+3-1
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,9 @@ public static function fromString(string $str) : ?BanEntry{
148148
return null;
149149
}
150150

151-
$parts = explode("|", trim($str));
151+
//we expect at most 5 parts, but accept 6 in case of an extra unexpected delimiter
152+
//we don't want to include unexpected data into the ban reason
153+
$parts = explode("|", trim($str), limit: 6);
152154
$entry = new BanEntry(trim(array_shift($parts)));
153155
if(count($parts) > 0){
154156
$entry->setCreated(self::parseDate(array_shift($parts)));

src/utils/Config.php

+5-4
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
use const JSON_BIGINT_AS_STRING;
5555
use const JSON_PRETTY_PRINT;
5656
use const JSON_THROW_ON_ERROR;
57+
use const PHP_INT_MAX;
5758
use const YAML_UTF8_ENCODING;
5859

5960
/**
@@ -339,7 +340,7 @@ public function __unset($k){
339340
}
340341

341342
public function setNested(string $key, mixed $value) : void{
342-
$vars = explode(".", $key);
343+
$vars = explode(".", $key, limit: PHP_INT_MAX);
343344
$base = array_shift($vars);
344345

345346
if(!isset($this->config[$base])){
@@ -366,7 +367,7 @@ public function getNested(string $key, mixed $default = null) : mixed{
366367
return $this->nestedCache[$key];
367368
}
368369

369-
$vars = explode(".", $key);
370+
$vars = explode(".", $key, limit: PHP_INT_MAX);
370371
$base = array_shift($vars);
371372
if(isset($this->config[$base])){
372373
$base = $this->config[$base];
@@ -390,7 +391,7 @@ public function removeNested(string $key) : void{
390391
$this->nestedCache = [];
391392
$this->changed = true;
392393

393-
$vars = explode(".", $key);
394+
$vars = explode(".", $key, limit: PHP_INT_MAX);
394395

395396
$currentNode = &$this->config;
396397
while(count($vars) > 0){
@@ -495,7 +496,7 @@ private function fillDefaults(array $default, array &$data) : int{
495496
*/
496497
public static function parseList(string $content) : array{
497498
$result = [];
498-
foreach(explode("\n", trim(str_replace("\r\n", "\n", $content))) as $v){
499+
foreach(explode("\n", trim(str_replace("\r\n", "\n", $content)), limit: PHP_INT_MAX) as $v){
499500
$v = trim($v);
500501
if($v === ""){
501502
continue;

src/utils/Internet.php

+4-2
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060
use const CURLOPT_SSL_VERIFYHOST;
6161
use const CURLOPT_SSL_VERIFYPEER;
6262
use const CURLOPT_TIMEOUT_MS;
63+
use const PHP_INT_MAX;
6364
use const SOCK_DGRAM;
6465
use const SOL_UDP;
6566

@@ -227,9 +228,10 @@ public static function simpleCurl(string $page, float $timeout = 10, array $extr
227228
$rawHeaders = substr($raw, 0, $headerSize);
228229
$body = substr($raw, $headerSize);
229230
$headers = [];
230-
foreach(explode("\r\n\r\n", $rawHeaders) as $rawHeaderGroup){
231+
//TODO: explore if we can set these limits lower
232+
foreach(explode("\r\n\r\n", $rawHeaders, limit: PHP_INT_MAX) as $rawHeaderGroup){
231233
$headerGroup = [];
232-
foreach(explode("\r\n", $rawHeaderGroup) as $line){
234+
foreach(explode("\r\n", $rawHeaderGroup, limit: PHP_INT_MAX) as $line){
233235
$nameValue = explode(":", $line, 2);
234236
if(isset($nameValue[1])){
235237
$headerGroup[trim(strtolower($nameValue[0]))] = trim($nameValue[1]);

src/utils/Utils.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -369,7 +369,7 @@ public static function getReferenceCount(object $value, bool $includeCurrent = t
369369
debug_zval_dump($value);
370370
$contents = ob_get_contents();
371371
if($contents === false) throw new AssumptionFailedError("ob_get_contents() should never return false here");
372-
$ret = explode("\n", $contents);
372+
$ret = explode("\n", $contents, limit: 2);
373373
ob_end_clean();
374374

375375
if(preg_match('/^.* refcount\\(([0-9]+)\\)\\{$/', trim($ret[0]), $m) > 0){

src/world/generator/FlatGeneratorOptions.php

+7-4
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,12 @@
2626
use pocketmine\data\bedrock\BiomeIds;
2727
use pocketmine\item\LegacyStringToItemParser;
2828
use pocketmine\item\LegacyStringToItemParserException;
29+
use pocketmine\world\World;
2930
use function array_map;
3031
use function explode;
3132
use function preg_match;
3233
use function preg_match_all;
34+
use const PHP_INT_MAX;
3335

3436
/**
3537
* @internal
@@ -70,7 +72,7 @@ public function getExtraOptions() : array{ return $this->extraOptions; }
7072
*/
7173
public static function parseLayers(string $layers) : array{
7274
$result = [];
73-
$split = array_map('\trim', explode(',', $layers));
75+
$split = array_map('\trim', explode(',', $layers, limit: World::Y_MAX - World::Y_MIN));
7476
$y = 0;
7577
$itemParser = LegacyStringToItemParser::getInstance();
7678
foreach($split as $line){
@@ -96,7 +98,7 @@ public static function parseLayers(string $layers) : array{
9698
* @throws InvalidGeneratorOptionsException
9799
*/
98100
public static function parsePreset(string $presetString) : self{
99-
$preset = explode(";", $presetString);
101+
$preset = explode(";", $presetString, limit: 4);
100102
$blocks = $preset[1] ?? "";
101103
$biomeId = (int) ($preset[2] ?? BiomeIds::PLAINS);
102104
$optionsString = $preset[3] ?? "";
@@ -109,9 +111,10 @@ public static function parsePreset(string $presetString) : self{
109111
$params = true;
110112
if($matches[3][$i] !== ""){
111113
$params = [];
112-
$p = explode(" ", $matches[3][$i]);
114+
$p = explode(" ", $matches[3][$i], limit: PHP_INT_MAX);
113115
foreach($p as $k){
114-
$k = explode("=", $k);
116+
//TODO: this should be limited to 2 parts, but 3 preserves old behaviour when given e.g. treecount=20=1
117+
$k = explode("=", $k, limit: 3);
115118
if(isset($k[1])){
116119
$params[$k[0]] = $k[1];
117120
}
+92
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
<?php
2+
3+
/*
4+
*
5+
* ____ _ _ __ __ _ __ __ ____
6+
* | _ \ ___ ___| | _____| |_| \/ (_)_ __ ___ | \/ | _ \
7+
* | |_) / _ \ / __| |/ / _ \ __| |\/| | | '_ \ / _ \_____| |\/| | |_) |
8+
* | __/ (_) | (__| < __/ |_| | | | | | | | __/_____| | | | __/
9+
* |_| \___/ \___|_|\_\___|\__|_| |_|_|_| |_|\___| |_| |_|_|
10+
*
11+
* This program is free software: you can redistribute it and/or modify
12+
* it under the terms of the GNU Lesser General Public License as published by
13+
* the Free Software Foundation, either version 3 of the License, or
14+
* (at your option) any later version.
15+
*
16+
* @author PocketMine Team
17+
* @link http://www.pocketmine.net/
18+
*
19+
*
20+
*/
21+
22+
declare(strict_types=1);
23+
24+
namespace pocketmine\phpstan\rules;
25+
26+
use PhpParser\Node;
27+
use PhpParser\Node\Expr\FuncCall;
28+
use PhpParser\Node\Name;
29+
use PHPStan\Analyser\ArgumentsNormalizer;
30+
use PHPStan\Analyser\Scope;
31+
use PHPStan\Reflection\ParametersAcceptorSelector;
32+
use PHPStan\Reflection\ReflectionProvider;
33+
use PHPStan\Rules\Rule;
34+
use PHPStan\Rules\RuleErrorBuilder;
35+
use function count;
36+
37+
/**
38+
* @phpstan-implements Rule<FuncCall>
39+
*/
40+
final class ExplodeLimitRule implements Rule{
41+
private ReflectionProvider $reflectionProvider;
42+
43+
public function __construct(
44+
ReflectionProvider $reflectionProvider
45+
){
46+
$this->reflectionProvider = $reflectionProvider;
47+
}
48+
49+
public function getNodeType() : string{
50+
return FuncCall::class;
51+
}
52+
53+
public function processNode(Node $node, Scope $scope) : array{
54+
if(!$node->name instanceof Name){
55+
return [];
56+
}
57+
58+
if(!$this->reflectionProvider->hasFunction($node->name, $scope)){
59+
return [];
60+
}
61+
62+
$functionReflection = $this->reflectionProvider->getFunction($node->name, $scope);
63+
64+
if($functionReflection->getName() !== 'explode'){
65+
return [];
66+
}
67+
68+
$parametersAcceptor = ParametersAcceptorSelector::selectFromArgs(
69+
$scope,
70+
$node->getArgs(),
71+
$functionReflection->getVariants(),
72+
$functionReflection->getNamedArgumentsVariants(),
73+
);
74+
75+
$normalizedFuncCall = ArgumentsNormalizer::reorderFuncArguments($parametersAcceptor, $node);
76+
77+
if($normalizedFuncCall === null){
78+
return [];
79+
}
80+
81+
$count = count($normalizedFuncCall->getArgs());
82+
if($count !== 3){
83+
return [
84+
RuleErrorBuilder::message('The $limit parameter of explode() must be set to prevent malicious client data wasting resources.')
85+
->identifier("pocketmine.explode.limit")
86+
->build()
87+
];
88+
}
89+
90+
return [];
91+
}
92+
}

tools/generate-bedrock-data-from-packets.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -624,7 +624,7 @@ function main(array $argv) : int{
624624
}
625625

626626
foreach($packets as $lineNum => $line){
627-
$parts = explode(':', $line);
627+
$parts = explode(':', $line, limit: 3);
628628
if(count($parts) !== 2){
629629
fwrite(STDERR, 'Wrong packet format at line ' . ($lineNum + 1) . ', expected read:base64 or write:base64');
630630
return 1;

0 commit comments

Comments
 (0)