From caf141bb3ce59bf9d3e6bd0afc7cb1a62243acf5 Mon Sep 17 00:00:00 2001 From: "Alexander M. Turek" Date: Tue, 9 Jun 2026 10:34:36 +0200 Subject: [PATCH] Error on unserialize() without allowed_classes option --- README.md | 2 + rules.neon | 7 ++ .../UnserializeAllowedClassesRule.php | 81 +++++++++++++++++++ .../UnserializeAllowedClassesRuleTest.php | 39 +++++++++ .../data/unserialize-good-usages.php | 32 ++++++++ .../Functions/data/unserialize-usages.php | 51 ++++++++++++ 6 files changed, 212 insertions(+) create mode 100644 src/Rules/Functions/UnserializeAllowedClassesRule.php create mode 100644 tests/Rules/Functions/UnserializeAllowedClassesRuleTest.php create mode 100644 tests/Rules/Functions/data/unserialize-good-usages.php create mode 100644 tests/Rules/Functions/data/unserialize-usages.php diff --git a/README.md b/README.md index 52ec368..e00d027 100644 --- a/README.md +++ b/README.md @@ -31,6 +31,7 @@ | `strictArrayFilter` | Require `array_filter()` to have a callback parameter to avoid loose comparison semantics. | | `illegalConstructorMethodCall` | Disallow calling `__construct()` on an existing object or as a static call outside of parent constructor. | | `closureUsesThis` | Closure should use `$this` directly instead of using `$this` variable indirectly. | +| `unserializeAllowedClasses` | Calls to `unserialize()` must include the `allowed_classes` option. | Additional rules are coming in subsequent releases! @@ -74,6 +75,7 @@ parameters: disallowedShortTernary: false overwriteVariablesWithLoop: false closureUsesThis: false + unserializeAllowedClasses: false matchingInheritedMethodNames: false numericOperandsInArithmeticOperators: false strictFunctionCalls: false diff --git a/rules.neon b/rules.neon index 92ca7e0..4a1b012 100644 --- a/rules.neon +++ b/rules.neon @@ -26,6 +26,7 @@ parameters: disallowedShortTernary: %strictRules.allRules% overwriteVariablesWithLoop: %strictRules.allRules% closureUsesThis: %strictRules.allRules% + unserializeAllowedClasses: %strictRules.allRules% matchingInheritedMethodNames: %strictRules.allRules% numericOperandsInArithmeticOperators: %strictRules.allRules% strictFunctionCalls: %strictRules.allRules% @@ -49,6 +50,7 @@ parametersSchema: disallowedShortTernary: anyOf(bool(), arrayOf(bool())) overwriteVariablesWithLoop: anyOf(bool(), arrayOf(bool())) closureUsesThis: anyOf(bool(), arrayOf(bool())) + unserializeAllowedClasses: anyOf(bool(), arrayOf(bool())) matchingInheritedMethodNames: anyOf(bool(), arrayOf(bool())) numericOperandsInArithmeticOperators: anyOf(bool(), arrayOf(bool())) strictFunctionCalls: anyOf(bool(), arrayOf(bool())) @@ -98,6 +100,8 @@ conditionalTags: phpstan.rules.rule: %strictRules.strictArrayFilter% PHPStan\Rules\Functions\ClosureUsesThisRule: phpstan.rules.rule: %strictRules.closureUsesThis% + PHPStan\Rules\Functions\UnserializeAllowedClassesRule: + phpstan.rules.rule: %strictRules.unserializeAllowedClasses% PHPStan\Rules\Methods\WrongCaseOfInheritedMethodRule: phpstan.rules.rule: %strictRules.matchingInheritedMethodNames% PHPStan\Rules\Operators\OperandInArithmeticPostDecrementRule: @@ -229,6 +233,9 @@ services: - class: PHPStan\Rules\Functions\ClosureUsesThisRule + - + class: PHPStan\Rules\Functions\UnserializeAllowedClassesRule + - class: PHPStan\Rules\Methods\WrongCaseOfInheritedMethodRule diff --git a/src/Rules/Functions/UnserializeAllowedClassesRule.php b/src/Rules/Functions/UnserializeAllowedClassesRule.php new file mode 100644 index 0000000..d4eb089 --- /dev/null +++ b/src/Rules/Functions/UnserializeAllowedClassesRule.php @@ -0,0 +1,81 @@ + */ +class UnserializeAllowedClassesRule implements Rule +{ + + public function getNodeType(): string + { + return FuncCall::class; + } + + public function processNode(Node $node, Scope $scope): array + { + if (!$node->name instanceof Node\Name) { + return []; + } + + if (!in_array(strtolower((string) $node->name), ['unserialize'], true)) { + return []; + } + + $args = $node->getArgs(); + + if (count($args) < 2) { + return [ + RuleErrorBuilder::message( + 'unserialize() called without the $options argument. Always pass ' + . "['allowed_classes' => false] or an explicit list of safe classes. " + . "If you truly need to allow all classes, pass ['allowed_classes' => true] " + . 'to make that decision explicit.', + ) + ->identifier('unserialize.allowedClasses') + ->build(), + ]; + } + + $optionsArg = $args[1]->value; + + if (!$optionsArg instanceof Node\Expr\Array_) { + return []; + } + + foreach ($optionsArg->items as $item) { + if ($item === null) { + continue; + } + + $key = $item->key; + if (!$key instanceof Node\Scalar\String_) { + continue; + } + + if ($key->value === 'allowed_classes') { + return []; + } + } + + return [ + RuleErrorBuilder::message( + "unserialize() called without the 'allowed_classes' key in \$options. " + . "Always pass ['allowed_classes' => false] or an explicit list of safe classes. " + . "If you truly need to allow all classes, pass ['allowed_classes' => true] " + . 'to make that decision explicit.', + ) + ->identifier('unserialize.allowedClasses') + ->build(), + ]; + } + +} diff --git a/tests/Rules/Functions/UnserializeAllowedClassesRuleTest.php b/tests/Rules/Functions/UnserializeAllowedClassesRuleTest.php new file mode 100644 index 0000000..e5f632e --- /dev/null +++ b/tests/Rules/Functions/UnserializeAllowedClassesRuleTest.php @@ -0,0 +1,39 @@ + */ +class UnserializeAllowedClassesRuleTest extends RuleTestCase +{ + + protected function getRule(): UnserializeAllowedClassesRule + { + return new UnserializeAllowedClassesRule(); + } + + public function testViolations(): void + { + $this->analyse([__DIR__ . '/data/unserialize-usages.php'], [ + [ + "unserialize() called without the \$options argument. Always pass ['allowed_classes' => false] or an explicit list of safe classes. If you truly need to allow all classes, pass ['allowed_classes' => true] to make that decision explicit.", + 10, + ], + [ + "unserialize() called without the 'allowed_classes' key in \$options. Always pass ['allowed_classes' => false] or an explicit list of safe classes. If you truly need to allow all classes, pass ['allowed_classes' => true] to make that decision explicit.", + 15, + ], + [ + "unserialize() called without the 'allowed_classes' key in \$options. Always pass ['allowed_classes' => false] or an explicit list of safe classes. If you truly need to allow all classes, pass ['allowed_classes' => true] to make that decision explicit.", + 20, + ], + ]); + } + + public function testNoErrors(): void + { + $this->analyse([__DIR__ . '/data/unserialize-good-usages.php'], []); + } + +} diff --git a/tests/Rules/Functions/data/unserialize-good-usages.php b/tests/Rules/Functions/data/unserialize-good-usages.php new file mode 100644 index 0000000..f31661a --- /dev/null +++ b/tests/Rules/Functions/data/unserialize-good-usages.php @@ -0,0 +1,32 @@ + */ + public function allowedClassesFalse(string $data): array + { + /** @var array $result */ + $result = unserialize($data, ['allowed_classes' => false]); + + return $result; + } + + public function allowedClassesExplicitTrue(string $data): mixed + { + return unserialize($data, ['allowed_classes' => true]); + } + + public function allowedClassesList(string $data): mixed + { + return unserialize($data, ['allowed_classes' => [\stdClass::class]]); + } + + public function dynamicOptions(string $data, mixed $options): mixed + { + return unserialize($data, $options); + } + +} diff --git a/tests/Rules/Functions/data/unserialize-usages.php b/tests/Rules/Functions/data/unserialize-usages.php new file mode 100644 index 0000000..64b3a45 --- /dev/null +++ b/tests/Rules/Functions/data/unserialize-usages.php @@ -0,0 +1,51 @@ + true]); + } + + /** @return array */ + public function allowedClassesFalse(string $data): array + { + /** @var array $result */ + $result = unserialize($data, ['allowed_classes' => false]); + + return $result; + } + + /** @return array */ + public function allowedClassesExplicitTrue(string $data): array + { + /** @var array $result */ + $result = unserialize($data, ['allowed_classes' => true]); + + return $result; + } + + public function allowedClassesList(string $data): mixed + { + return unserialize($data, ['allowed_classes' => [\stdClass::class]]); + } + + public function dynamicOptions(string $data, mixed $options): mixed + { + return unserialize($data, $options); + } + +}