Skip to content

Commit c4c0269

Browse files
staabmstaabm
authored
Bleeding edge - check preg_quote delimiter sanity
1 parent c9a6d2e commit c4c0269

File tree

8 files changed

+485
-0
lines changed

8 files changed

+485
-0
lines changed

‎conf/bleedingEdge.neon

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,5 +58,6 @@ parameters:
5858
uselessReturnValue: true
5959
printfArrayParameters: true
6060
preciseMissingReturn: true
61+
validatePregQuote: true
6162
stubFiles:
6263
- ../stubs/bleedingEdge/Rule.stub

‎conf/config.level0.neon

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ conditionalTags:
2828
phpstan.rules.rule: %featureToggles.uselessReturnValue%
2929
PHPStan\Rules\Functions\PrintfArrayParametersRule:
3030
phpstan.rules.rule: %featureToggles.printfArrayParameters%
31+
PHPStan\Rules\Regexp\RegularExpressionQuotingRule:
32+
phpstan.rules.rule: %featureToggles.validatePregQuote%
3133

3234
rules:
3335
- PHPStan\Rules\Api\ApiInstantiationRule
@@ -304,3 +306,6 @@ services:
304306

305307
-
306308
class: PHPStan\Rules\Functions\PrintfArrayParametersRule
309+
310+
-
311+
class: PHPStan\Rules\Regexp\RegularExpressionQuotingRule

‎conf/config.neon

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ parameters:
9393
uselessReturnValue: false
9494
printfArrayParameters: false
9595
preciseMissingReturn: false
96+
validatePregQuote: false
9697
fileExtensions:
9798
- php
9899
checkAdvancedIsset: false

‎conf/parametersSchema.neon

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ parametersSchema:
8888
uselessReturnValue: bool()
8989
printfArrayParameters: bool()
9090
preciseMissingReturn: bool()
91+
validatePregQuote: bool()
9192
])
9293
fileExtensions: listOf(string())
9394
checkAdvancedIsset: bool()

‎src/Rules/Regexp/RegularExpressionQuotingRule.php

Lines changed: 274 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,274 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\Regexp;
4+
5+
use PhpParser\Node;
6+
use PhpParser\Node\Expr\BinaryOp\Concat;
7+
use PhpParser\Node\Expr\FuncCall;
8+
use PhpParser\Node\Name;
9+
use PHPStan\Analyser\ArgumentsNormalizer;
10+
use PHPStan\Analyser\Scope;
11+
use PHPStan\Reflection\FunctionReflection;
12+
use PHPStan\Reflection\ParametersAcceptorSelector;
13+
use PHPStan\Reflection\ReflectionProvider;
14+
use PHPStan\Rules\IdentifierRuleError;
15+
use PHPStan\Rules\Rule;
16+
use PHPStan\Rules\RuleErrorBuilder;
17+
use PHPStan\ShouldNotHappenException;
18+
use PHPStan\Type\Constant\ConstantStringType;
19+
use function array_filter;
20+
use function array_merge;
21+
use function array_values;
22+
use function count;
23+
use function in_array;
24+
use function sprintf;
25+
use function strlen;
26+
use function substr;
27+
28+
/**
29+
* @implements Rule<Node\Expr\FuncCall>
30+
*/
31+
class RegularExpressionQuotingRule implements Rule
32+
{
33+
34+
public function __construct(private ReflectionProvider $reflectionProvider)
35+
{
36+
}
37+
38+
public function getNodeType(): string
39+
{
40+
return FuncCall::class;
41+
}
42+
43+
public function processNode(Node $node, Scope $scope): array
44+
{
45+
if (!$node->name instanceof Node\Name) {
46+
return [];
47+
}
48+
49+
if (!$this->reflectionProvider->hasFunction($node->name, $scope)) {
50+
return [];
51+
}
52+
53+
$functionReflection = $this->reflectionProvider->getFunction($node->name, $scope);
54+
if (
55+
!in_array($functionReflection->getName(), [
56+
'preg_match',
57+
'preg_match_all',
58+
'preg_filter',
59+
'preg_grep',
60+
'preg_replace',
61+
'preg_replace_callback',
62+
'preg_split',
63+
], true)
64+
) {
65+
return [];
66+
}
67+
68+
$normalizedArgs = $this->getNormalizedArgs($node, $scope, $functionReflection);
69+
if ($normalizedArgs === null) {
70+
return [];
71+
}
72+
if (!isset($normalizedArgs[0])) {
73+
return [];
74+
}
75+
if (!$normalizedArgs[0]->value instanceof Concat) {
76+
return [];
77+
}
78+
79+
$patternDelimiters = $this->getDelimitersFromConcat($normalizedArgs[0]->value, $scope);
80+
return $this->validateQuoteDelimiters($normalizedArgs[0]->value, $scope, $patternDelimiters);
81+
}
82+
83+
/**
84+
* @param string[] $patternDelimiters
85+
*
86+
* @return list<IdentifierRuleError>
87+
*/
88+
private function validateQuoteDelimiters(Concat $concat, Scope $scope, array $patternDelimiters): array
89+
{
90+
if ($patternDelimiters === []) {
91+
return [];
92+
}
93+
94+
$errors = [];
95+
if (
96+
$concat->left instanceof FuncCall
97+
&& $concat->left->name instanceof Name
98+
&& $concat->left->name->toLowerString() === 'preg_quote'
99+
) {
100+
$pregError = $this->validatePregQuote($concat->left, $scope, $patternDelimiters);
101+
if ($pregError !== null) {
102+
$errors[] = $pregError;
103+
}
104+
} elseif ($concat->left instanceof Concat) {
105+
$errors = array_merge($errors, $this->validateQuoteDelimiters($concat->left, $scope, $patternDelimiters));
106+
}
107+
108+
if (
109+
$concat->right instanceof FuncCall
110+
&& $concat->right->name instanceof Name
111+
&& $concat->right->name->toLowerString() === 'preg_quote'
112+
) {
113+
$pregError = $this->validatePregQuote($concat->right, $scope, $patternDelimiters);
114+
if ($pregError !== null) {
115+
$errors[] = $pregError;
116+
}
117+
} elseif ($concat->right instanceof Concat) {
118+
$errors = array_merge($errors, $this->validateQuoteDelimiters($concat->right, $scope, $patternDelimiters));
119+
}
120+
121+
return $errors;
122+
}
123+
124+
/**
125+
* @param string[] $patternDelimiters
126+
*/
127+
private function validatePregQuote(FuncCall $pregQuote, Scope $scope, array $patternDelimiters): ?IdentifierRuleError
128+
{
129+
if (!$pregQuote->name instanceof Node\Name) {
130+
return null;
131+
}
132+
133+
if (!$this->reflectionProvider->hasFunction($pregQuote->name, $scope)) {
134+
return null;
135+
}
136+
$functionReflection = $this->reflectionProvider->getFunction($pregQuote->name, $scope);
137+
138+
$args = $this->getNormalizedArgs($pregQuote, $scope, $functionReflection);
139+
if ($args === null) {
140+
return null;
141+
}
142+
143+
$patternDelimiters = $this->removeDefaultEscapedDelimiters($patternDelimiters);
144+
if ($patternDelimiters === []) {
145+
return null;
146+
}
147+
148+
if (count($args) === 1) {
149+
if (count($patternDelimiters) === 1) {
150+
return RuleErrorBuilder::message(sprintf('Call to preg_quote() is missing delimiter %s to be effective.', $patternDelimiters[0]))
151+
->line($pregQuote->getStartLine())
152+
->identifier('argument.invalidPregQuote')
153+
->build();
154+
}
155+
156+
return RuleErrorBuilder::message('Call to preg_quote() is missing delimiter parameter to be effective.')
157+
->line($pregQuote->getStartLine())
158+
->identifier('argument.invalidPregQuote')
159+
->build();
160+
}
161+
162+
if (count($args) >= 2) {
163+
164+
foreach ($scope->getType($args[1]->value)->getConstantStrings() as $quoteDelimiterType) {
165+
$quoteDelimiter = $quoteDelimiterType->getValue();
166+
167+
$quoteDelimiters = $this->removeDefaultEscapedDelimiters([$quoteDelimiter]);
168+
if ($quoteDelimiters === []) {
169+
continue;
170+
}
171+
172+
if (count($quoteDelimiters) !== 1) {
173+
throw new ShouldNotHappenException();
174+
}
175+
$quoteDelimiter = $quoteDelimiters[0];
176+
177+
if (!in_array($quoteDelimiter, $patternDelimiters, true)) {
178+
if (count($patternDelimiters) === 1) {
179+
return RuleErrorBuilder::message(sprintf('Call to preg_quote() uses invalid delimiter %s while pattern uses %s.', $quoteDelimiter, $patternDelimiters[0]))
180+
->line($pregQuote->getStartLine())
181+
->identifier('argument.invalidPregQuote')
182+
->build();
183+
}
184+
185+
return RuleErrorBuilder::message(sprintf('Call to preg_quote() uses invalid delimiter %s.', $quoteDelimiter))
186+
->line($pregQuote->getStartLine())
187+
->identifier('argument.invalidPregQuote')
188+
->build();
189+
}
190+
}
191+
}
192+
193+
return null;
194+
}
195+
196+
/**
197+
* Get delimiters from non-constant patterns, if possible.
198+
*
199+
* @return string[]
200+
*/
201+
private function getDelimitersFromConcat(Concat $concat, Scope $scope): array
202+
{
203+
if ($concat->left instanceof Concat) {
204+
return $this->getDelimitersFromConcat($concat->left, $scope);
205+
}
206+
207+
$left = $scope->getType($concat->left);
208+
209+
$delimiters = [];
210+
foreach ($left->getConstantStrings() as $leftString) {
211+
$delimiter = $this->getDelimiterFromString($leftString);
212+
if ($delimiter === null) {
213+
continue;
214+
}
215+
216+
$delimiters[] = $delimiter;
217+
}
218+
return $delimiters;
219+
}
220+
221+
private function getDelimiterFromString(ConstantStringType $string): ?string
222+
{
223+
if ($string->getValue() === '') {
224+
return null;
225+
}
226+
227+
return substr($string->getValue(), 0, 1);
228+
}
229+
230+
/**
231+
* @param string[] $delimiters
232+
*
233+
* @return list<string>
234+
*/
235+
private function removeDefaultEscapedDelimiters(array $delimiters): array
236+
{
237+
return array_values(array_filter($delimiters, fn (string $delimiter): bool => !$this->isDefaultEscaped($delimiter)));
238+
}
239+
240+
private function isDefaultEscaped(string $delimiter): bool
241+
{
242+
if (strlen($delimiter) !== 1) {
243+
return false;
244+
}
245+
246+
return in_array(
247+
$delimiter,
248+
// these delimiters are escaped, no matter what preg_quote() 2nd arg looks like
249+
['.', '\\', '+', '*', '?', '[', '^', ']', '$', '(', ')', '{', '}', '=', '!', '<', '>', '|', ':', '-', '#'],
250+
true,
251+
);
252+
}
253+
254+
/**
255+
* @return Node\Arg[]|null
256+
*/
257+
private function getNormalizedArgs(FuncCall $functionCall, Scope $scope, FunctionReflection $functionReflection): ?array
258+
{
259+
$parametersAcceptor = ParametersAcceptorSelector::selectFromArgs(
260+
$scope,
261+
$functionCall->getArgs(),
262+
$functionReflection->getVariants(),
263+
$functionReflection->getNamedArgumentsVariants(),
264+
);
265+
266+
$normalizedFuncCall = ArgumentsNormalizer::reorderFuncArguments($parametersAcceptor, $functionCall);
267+
if ($normalizedFuncCall === null) {
268+
return null;
269+
}
270+
271+
return $normalizedFuncCall->getArgs();
272+
}
273+
274+
}

0 commit comments

Comments
 (0)