From c4c02698414934def38e0e1a4a002a2c139d9242 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 23 Jul 2024 22:37:14 +0200 Subject: [PATCH] Bleeding edge - check preg_quote delimiter sanity --- conf/bleedingEdge.neon | 1 + conf/config.level0.neon | 5 + conf/config.neon | 1 + conf/parametersSchema.neon | 1 + .../Regexp/RegularExpressionQuotingRule.php | 274 ++++++++++++++++++ .../RegularExpressionQuotingRuleTest.php | 94 ++++++ .../Rules/Regexp/data/preg-quote-php8.php | 13 + .../PHPStan/Rules/Regexp/data/preg-quote.php | 96 ++++++ 8 files changed, 485 insertions(+) create mode 100644 src/Rules/Regexp/RegularExpressionQuotingRule.php create mode 100644 tests/PHPStan/Rules/Regexp/RegularExpressionQuotingRuleTest.php create mode 100644 tests/PHPStan/Rules/Regexp/data/preg-quote-php8.php create mode 100644 tests/PHPStan/Rules/Regexp/data/preg-quote.php diff --git a/conf/bleedingEdge.neon b/conf/bleedingEdge.neon index 3bbbeb6bc0..61482ac786 100644 --- a/conf/bleedingEdge.neon +++ b/conf/bleedingEdge.neon @@ -58,5 +58,6 @@ parameters: uselessReturnValue: true printfArrayParameters: true preciseMissingReturn: true + validatePregQuote: true stubFiles: - ../stubs/bleedingEdge/Rule.stub diff --git a/conf/config.level0.neon b/conf/config.level0.neon index 18bc1b0cd5..8052e5f5de 100644 --- a/conf/config.level0.neon +++ b/conf/config.level0.neon @@ -28,6 +28,8 @@ conditionalTags: phpstan.rules.rule: %featureToggles.uselessReturnValue% PHPStan\Rules\Functions\PrintfArrayParametersRule: phpstan.rules.rule: %featureToggles.printfArrayParameters% + PHPStan\Rules\Regexp\RegularExpressionQuotingRule: + phpstan.rules.rule: %featureToggles.validatePregQuote% rules: - PHPStan\Rules\Api\ApiInstantiationRule @@ -304,3 +306,6 @@ services: - class: PHPStan\Rules\Functions\PrintfArrayParametersRule + + - + class: PHPStan\Rules\Regexp\RegularExpressionQuotingRule diff --git a/conf/config.neon b/conf/config.neon index ac8f18be39..60fb7e44f1 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -93,6 +93,7 @@ parameters: uselessReturnValue: false printfArrayParameters: false preciseMissingReturn: false + validatePregQuote: false fileExtensions: - php checkAdvancedIsset: false diff --git a/conf/parametersSchema.neon b/conf/parametersSchema.neon index f33fc0ba5d..29efd1a8b3 100644 --- a/conf/parametersSchema.neon +++ b/conf/parametersSchema.neon @@ -88,6 +88,7 @@ parametersSchema: uselessReturnValue: bool() printfArrayParameters: bool() preciseMissingReturn: bool() + validatePregQuote: bool() ]) fileExtensions: listOf(string()) checkAdvancedIsset: bool() diff --git a/src/Rules/Regexp/RegularExpressionQuotingRule.php b/src/Rules/Regexp/RegularExpressionQuotingRule.php new file mode 100644 index 0000000000..873209c00f --- /dev/null +++ b/src/Rules/Regexp/RegularExpressionQuotingRule.php @@ -0,0 +1,274 @@ + + */ +class RegularExpressionQuotingRule implements Rule +{ + + public function __construct(private ReflectionProvider $reflectionProvider) + { + } + + public function getNodeType(): string + { + return FuncCall::class; + } + + public function processNode(Node $node, Scope $scope): array + { + if (!$node->name instanceof Node\Name) { + return []; + } + + if (!$this->reflectionProvider->hasFunction($node->name, $scope)) { + return []; + } + + $functionReflection = $this->reflectionProvider->getFunction($node->name, $scope); + if ( + !in_array($functionReflection->getName(), [ + 'preg_match', + 'preg_match_all', + 'preg_filter', + 'preg_grep', + 'preg_replace', + 'preg_replace_callback', + 'preg_split', + ], true) + ) { + return []; + } + + $normalizedArgs = $this->getNormalizedArgs($node, $scope, $functionReflection); + if ($normalizedArgs === null) { + return []; + } + if (!isset($normalizedArgs[0])) { + return []; + } + if (!$normalizedArgs[0]->value instanceof Concat) { + return []; + } + + $patternDelimiters = $this->getDelimitersFromConcat($normalizedArgs[0]->value, $scope); + return $this->validateQuoteDelimiters($normalizedArgs[0]->value, $scope, $patternDelimiters); + } + + /** + * @param string[] $patternDelimiters + * + * @return list + */ + private function validateQuoteDelimiters(Concat $concat, Scope $scope, array $patternDelimiters): array + { + if ($patternDelimiters === []) { + return []; + } + + $errors = []; + if ( + $concat->left instanceof FuncCall + && $concat->left->name instanceof Name + && $concat->left->name->toLowerString() === 'preg_quote' + ) { + $pregError = $this->validatePregQuote($concat->left, $scope, $patternDelimiters); + if ($pregError !== null) { + $errors[] = $pregError; + } + } elseif ($concat->left instanceof Concat) { + $errors = array_merge($errors, $this->validateQuoteDelimiters($concat->left, $scope, $patternDelimiters)); + } + + if ( + $concat->right instanceof FuncCall + && $concat->right->name instanceof Name + && $concat->right->name->toLowerString() === 'preg_quote' + ) { + $pregError = $this->validatePregQuote($concat->right, $scope, $patternDelimiters); + if ($pregError !== null) { + $errors[] = $pregError; + } + } elseif ($concat->right instanceof Concat) { + $errors = array_merge($errors, $this->validateQuoteDelimiters($concat->right, $scope, $patternDelimiters)); + } + + return $errors; + } + + /** + * @param string[] $patternDelimiters + */ + private function validatePregQuote(FuncCall $pregQuote, Scope $scope, array $patternDelimiters): ?IdentifierRuleError + { + if (!$pregQuote->name instanceof Node\Name) { + return null; + } + + if (!$this->reflectionProvider->hasFunction($pregQuote->name, $scope)) { + return null; + } + $functionReflection = $this->reflectionProvider->getFunction($pregQuote->name, $scope); + + $args = $this->getNormalizedArgs($pregQuote, $scope, $functionReflection); + if ($args === null) { + return null; + } + + $patternDelimiters = $this->removeDefaultEscapedDelimiters($patternDelimiters); + if ($patternDelimiters === []) { + return null; + } + + if (count($args) === 1) { + if (count($patternDelimiters) === 1) { + return RuleErrorBuilder::message(sprintf('Call to preg_quote() is missing delimiter %s to be effective.', $patternDelimiters[0])) + ->line($pregQuote->getStartLine()) + ->identifier('argument.invalidPregQuote') + ->build(); + } + + return RuleErrorBuilder::message('Call to preg_quote() is missing delimiter parameter to be effective.') + ->line($pregQuote->getStartLine()) + ->identifier('argument.invalidPregQuote') + ->build(); + } + + if (count($args) >= 2) { + + foreach ($scope->getType($args[1]->value)->getConstantStrings() as $quoteDelimiterType) { + $quoteDelimiter = $quoteDelimiterType->getValue(); + + $quoteDelimiters = $this->removeDefaultEscapedDelimiters([$quoteDelimiter]); + if ($quoteDelimiters === []) { + continue; + } + + if (count($quoteDelimiters) !== 1) { + throw new ShouldNotHappenException(); + } + $quoteDelimiter = $quoteDelimiters[0]; + + if (!in_array($quoteDelimiter, $patternDelimiters, true)) { + if (count($patternDelimiters) === 1) { + return RuleErrorBuilder::message(sprintf('Call to preg_quote() uses invalid delimiter %s while pattern uses %s.', $quoteDelimiter, $patternDelimiters[0])) + ->line($pregQuote->getStartLine()) + ->identifier('argument.invalidPregQuote') + ->build(); + } + + return RuleErrorBuilder::message(sprintf('Call to preg_quote() uses invalid delimiter %s.', $quoteDelimiter)) + ->line($pregQuote->getStartLine()) + ->identifier('argument.invalidPregQuote') + ->build(); + } + } + } + + return null; + } + + /** + * Get delimiters from non-constant patterns, if possible. + * + * @return string[] + */ + private function getDelimitersFromConcat(Concat $concat, Scope $scope): array + { + if ($concat->left instanceof Concat) { + return $this->getDelimitersFromConcat($concat->left, $scope); + } + + $left = $scope->getType($concat->left); + + $delimiters = []; + foreach ($left->getConstantStrings() as $leftString) { + $delimiter = $this->getDelimiterFromString($leftString); + if ($delimiter === null) { + continue; + } + + $delimiters[] = $delimiter; + } + return $delimiters; + } + + private function getDelimiterFromString(ConstantStringType $string): ?string + { + if ($string->getValue() === '') { + return null; + } + + return substr($string->getValue(), 0, 1); + } + + /** + * @param string[] $delimiters + * + * @return list + */ + private function removeDefaultEscapedDelimiters(array $delimiters): array + { + return array_values(array_filter($delimiters, fn (string $delimiter): bool => !$this->isDefaultEscaped($delimiter))); + } + + private function isDefaultEscaped(string $delimiter): bool + { + if (strlen($delimiter) !== 1) { + return false; + } + + return in_array( + $delimiter, + // these delimiters are escaped, no matter what preg_quote() 2nd arg looks like + ['.', '\\', '+', '*', '?', '[', '^', ']', '$', '(', ')', '{', '}', '=', '!', '<', '>', '|', ':', '-', '#'], + true, + ); + } + + /** + * @return Node\Arg[]|null + */ + private function getNormalizedArgs(FuncCall $functionCall, Scope $scope, FunctionReflection $functionReflection): ?array + { + $parametersAcceptor = ParametersAcceptorSelector::selectFromArgs( + $scope, + $functionCall->getArgs(), + $functionReflection->getVariants(), + $functionReflection->getNamedArgumentsVariants(), + ); + + $normalizedFuncCall = ArgumentsNormalizer::reorderFuncArguments($parametersAcceptor, $functionCall); + if ($normalizedFuncCall === null) { + return null; + } + + return $normalizedFuncCall->getArgs(); + } + +} diff --git a/tests/PHPStan/Rules/Regexp/RegularExpressionQuotingRuleTest.php b/tests/PHPStan/Rules/Regexp/RegularExpressionQuotingRuleTest.php new file mode 100644 index 0000000000..fc1db91b7f --- /dev/null +++ b/tests/PHPStan/Rules/Regexp/RegularExpressionQuotingRuleTest.php @@ -0,0 +1,94 @@ + + */ +class RegularExpressionQuotingRuleTest extends RuleTestCase +{ + + protected function getRule(): Rule + { + return new RegularExpressionQuotingRule($this->createReflectionProvider()); + } + + public function testRule(): void + { + $this->analyse( + [__DIR__ . '/data/preg-quote.php'], + [ + [ + 'Call to preg_quote() is missing delimiter & to be effective.', + 6, + ], + [ + 'Call to preg_quote() uses invalid delimiter / while pattern uses &.', + 7, + ], + [ + 'Call to preg_quote() uses invalid delimiter / while pattern uses &.', + 11, + ], + [ + 'Call to preg_quote() is missing delimiter & to be effective.', + 12, + ], + [ + 'Call to preg_quote() uses invalid delimiter / while pattern uses &.', + 18, + ], + [ + 'Call to preg_quote() uses invalid delimiter / while pattern uses &.', + 20, + ], + [ + 'Call to preg_quote() uses invalid delimiter / while pattern uses &.', + 21, + ], + [ + 'Call to preg_quote() uses invalid delimiter / while pattern uses &.', + 22, + ], + [ + 'Call to preg_quote() uses invalid delimiter / while pattern uses &.', + 23, + ], + [ + 'Call to preg_quote() uses invalid delimiter / while pattern uses &.', + 24, + ], + [ + 'Call to preg_quote() is missing delimiter parameter to be effective.', + 77, + ], + ], + ); + } + + public function testRulePhp8(): void + { + if (PHP_VERSION_ID < 80000) { + $this->markTestSkipped('Test requires PHP 8.0.'); + } + + $this->analyse( + [__DIR__ . '/data/preg-quote-php8.php'], + [ + [ + 'Call to preg_quote() uses invalid delimiter / while pattern uses &.', + 6, + ], + [ + 'Call to preg_quote() uses invalid delimiter / while pattern uses &.', + 7, + ], + ], + ); + } + +} diff --git a/tests/PHPStan/Rules/Regexp/data/preg-quote-php8.php b/tests/PHPStan/Rules/Regexp/data/preg-quote-php8.php new file mode 100644 index 0000000000..83e19cf03b --- /dev/null +++ b/tests/PHPStan/Rules/Regexp/data/preg-quote-php8.php @@ -0,0 +1,13 @@ += 8.0 + +namespace PregQuotingPhp8; + +function doFoo(string $s, callable $cb): void { // errors + preg_split(subject: $s, pattern: '&' . preg_quote('&oops', '/') . 'pattern&'); + preg_split(subject: $s, pattern: '&' . preg_quote(delimiter: '/', str: '&oops') . 'pattern&'); +} + +function ok(string $s): void { // ok + preg_split(subject: $s, pattern: '&' . preg_quote('&oops', '&') . 'pattern&'); + preg_split(subject: $s, pattern: '&' . preg_quote(delimiter: '&', str: '&oops') . 'pattern&'); +} diff --git a/tests/PHPStan/Rules/Regexp/data/preg-quote.php b/tests/PHPStan/Rules/Regexp/data/preg-quote.php new file mode 100644 index 0000000000..5333d72f58 --- /dev/null +++ b/tests/PHPStan/Rules/Regexp/data/preg-quote.php @@ -0,0 +1,96 @@ +