Skip to content

Commit 6d3cf93

Browse files
lenaorobeilenaorobei
authored
Merge pull request #63 from magento/DiscouragedFunction-improvement
[Enhancement] DiscouragedFunction rule improvement
2 parents 1b65bd6 + e8d0916 commit 6d3cf93

7 files changed

+133
-50
lines changed

‎Magento2/Sniffs/PHP/DiscouragedFunctionSniff.php renamed to ‎Magento2/Sniffs/Functions/DiscouragedFunctionSniff.php

Lines changed: 8 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,8 @@
33
* Copyright © Magento. All rights reserved.
44
* See COPYING.txt for license details.
55
*/
6-
namespace Magento2\Sniffs\PHP;
6+
namespace Magento2\Sniffs\Functions;
77

8-
use PHP_CodeSniffer\Files\File;
98
use PHP_CodeSniffer\Standards\Generic\Sniffs\PHP\ForbiddenFunctionsSniff;
109

1110
/**
@@ -20,13 +19,19 @@ class DiscouragedFunctionSniff extends ForbiddenFunctionsSniff
2019
*/
2120
protected $patternMatch = true;
2221

22+
/**
23+
* If true, an error will be thrown; otherwise a warning.
24+
*
25+
* @var boolean
26+
*/
27+
public $error = false;
28+
2329
/**
2430
* List of patterns for forbidden functions.
2531
*
2632
* @var array
2733
*/
2834
public $forbiddenFunctions = [
29-
'^assert$' => null,
3035
'^bind_textdomain_codeset$' => null,
3136
'^bindtextdomain$' => null,
3237
'^bz.*$' => null,
@@ -39,7 +44,6 @@ class DiscouragedFunctionSniff extends ForbiddenFunctionsSniff
3944
'^chroot$' => null,
4045
'^com_load_typelib$' => null,
4146
'^copy$' => null,
42-
'^create_function$' => null,
4347
'^curl_.*$' => null,
4448
'^cyrus_connect$' => null,
4549
'^dba_.*$' => null,
@@ -52,7 +56,6 @@ class DiscouragedFunctionSniff extends ForbiddenFunctionsSniff
5256
'^dirname$' => null,
5357
'^dngettext$' => null,
5458
'^domxml_.*$' => null,
55-
'^exec$' => null,
5659
'^fbsql_.*$' => null,
5760
'^fdf_add_doc_javascript$' => null,
5861
'^fdf_open$' => null,
@@ -93,18 +96,15 @@ class DiscouragedFunctionSniff extends ForbiddenFunctionsSniff
9396
'^parse_str$' => null,
9497
'^parse_url$' => null,
9598
'^parsekit_compile_string$' => null,
96-
'^passthru$' => null,
9799
'^pathinfo$' => null,
98100
'^pcntl_.*$' => null,
99101
'^posix_.*$' => null,
100102
'^pfpro_.*$' => null,
101103
'^pfsockopen$' => null,
102104
'^pg_.*$' => null,
103105
'^php_check_syntax$' => null,
104-
'^popen$' => null,
105106
'^print_r$' => null,
106107
'^printf$' => null,
107-
'^proc_open$' => null,
108108
'^putenv$' => null,
109109
'^readfile$' => null,
110110
'^readgzfile$' => null,
@@ -122,14 +122,12 @@ class DiscouragedFunctionSniff extends ForbiddenFunctionsSniff
122122
'^setcookie$' => null,
123123
'^setlocale$' => null,
124124
'^setrawcookie$' => null,
125-
'^shell_exec$' => null,
126125
'^sleep$' => null,
127126
'^socket_.*$' => null,
128127
'^stream_.*$' => null,
129128
'^sybase_.*$' => null,
130129
'^symlink$' => null,
131130
'^syslog$' => null,
132-
'^system$' => null,
133131
'^touch$' => null,
134132
'^trigger_error$' => null,
135133
'^unlink$' => null,
@@ -220,34 +218,5 @@ class DiscouragedFunctionSniff extends ForbiddenFunctionsSniff
220218
'^is_null$' => 'strict comparison "=== null"',
221219
'^intval$' => '(int) construction',
222220
'^strval$' => '(string) construction',
223-
'^md5$' => 'improved hash functions (SHA-256, SHA-512 etc.)',
224-
'^serialize$' => 'json_encode',
225-
'^unserialize$' => 'json_decode',
226221
];
227-
228-
/**
229-
* Generates warning for this sniff.
230-
*
231-
* @param File $phpcsFile The file being scanned.
232-
* @param int $stackPtr The position of the forbidden function in the token array.
233-
* @param string $function The name of the forbidden function.
234-
* @param string $pattern The pattern used for the match.
235-
*
236-
* @return void
237-
*/
238-
protected function addError($phpcsFile, $stackPtr, $function, $pattern = null)
239-
{
240-
$data = [$function];
241-
$warningMessage = 'The use of function %s() is discouraged';
242-
$warningCode = 'Found';
243-
if ($pattern === null) {
244-
$pattern = $function;
245-
}
246-
if ($this->forbiddenFunctions[$pattern] !== null) {
247-
$warningCode .= 'WithAlternative';
248-
$data[] = $this->forbiddenFunctions[$pattern];
249-
$warningMessage .= '; use %s instead.';
250-
}
251-
$phpcsFile->addWarning($warningMessage, $stackPtr, $warningCode, $data);
252-
}
253222
}

‎Magento2/Sniffs/Security/InsecureFunctionSniff.php

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
<?php
2+
/**
3+
* Copyright © Magento. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
namespace Magento2\Sniffs\Security;
7+
8+
use PHP_CodeSniffer\Standards\Generic\Sniffs\PHP\ForbiddenFunctionsSniff;
9+
10+
/**
11+
* Detects the use of insecure functions.
12+
*/
13+
class InsecureFunctionSniff extends ForbiddenFunctionsSniff
14+
{
15+
/**
16+
* If true, an error will be thrown; otherwise a warning.
17+
*
18+
* @var boolean
19+
*/
20+
public $error = false;
21+
22+
/**
23+
* List of patterns for forbidden functions.
24+
*
25+
* @var array
26+
*/
27+
public $forbiddenFunctions = [
28+
'assert' => null,
29+
'create_function' => null,
30+
'exec' => null,
31+
'md5' => 'improved hash functions (SHA-256, SHA-512 etc.)',
32+
'passthru' => null,
33+
'pcntl_exec' => null,
34+
'popen' => null,
35+
'proc_open' => null,
36+
'serialize' => 'json_encode',
37+
'shell_exec' => null,
38+
'system' => null,
39+
'unserialize' => 'json_decode',
40+
];
41+
}

‎Magento2/Tests/PHP/DiscouragedFunctionUnitTest.inc renamed to ‎Magento2/Tests/Functions/DiscouragedFunctionUnitTest.inc

File renamed without changes.

‎Magento2/Tests/PHP/DiscouragedFunctionUnitTest.php renamed to ‎Magento2/Tests/Functions/DiscouragedFunctionUnitTest.php

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* Copyright © Magento. All rights reserved.
44
* See COPYING.txt for license details.
55
*/
6-
namespace Magento2\Tests\PHP;
6+
namespace Magento2\Tests\Functions;
77

88
use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest;
99

@@ -26,7 +26,6 @@ public function getErrorList()
2626
public function getWarningList()
2727
{
2828
return [
29-
3 => 1,
3029
6 => 1,
3130
7 => 1,
3231
9 => 1,
@@ -40,7 +39,6 @@ public function getWarningList()
4039
28 => 1,
4140
30 => 1,
4241
32 => 1,
43-
34 => 1,
4442
36 => 1,
4543
37 => 1,
4644
38 => 1,
@@ -63,7 +61,6 @@ public function getWarningList()
6361
65 => 1,
6462
67 => 1,
6563
69 => 1,
66-
71 => 1,
6764
73 => 1,
6865
74 => 1,
6966
76 => 1,
@@ -122,7 +119,6 @@ public function getWarningList()
122119
166 => 1,
123120
169 => 1,
124121
171 => 1,
125-
173 => 1,
126122
175 => 1,
127123
177 => 1,
128124
179 => 1,
@@ -131,10 +127,8 @@ public function getWarningList()
131127
184 => 1,
132128
185 => 1,
133129
187 => 1,
134-
189 => 1,
135130
191 => 1,
136131
193 => 1,
137-
195 => 1,
138132
197 => 1,
139133
199 => 1,
140134
201 => 1,
@@ -152,7 +146,6 @@ public function getWarningList()
152146
229 => 1,
153147
231 => 1,
154148
233 => 1,
155-
235 => 1,
156149
237 => 1,
157150
239 => 1,
158151
241 => 1,
@@ -162,7 +155,6 @@ public function getWarningList()
162155
247 => 1,
163156
249 => 1,
164157
251 => 1,
165-
253 => 1,
166158
255 => 1,
167159
258 => 1,
168160
261 => 1,
@@ -257,7 +249,6 @@ public function getWarningList()
257249
458 => 1,
258250
460 => 1,
259251
462 => 1,
260-
464 => 1,
261252
];
262253
}
263254
}

‎Magento2/Tests/Security/InsecureFunctionUnitTest.inc

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
<?php
2+
3+
assert($a === true);
4+
5+
exec('echo 1;');
6+
7+
passthru('echo 1;');
8+
9+
shell_exec('echo 1;');
10+
11+
system('echo 1;');
12+
13+
md5($text);
14+
15+
serialize([]);
16+
17+
unserialize('');
18+
19+
popen('echo 1;');
20+
21+
proc_open('echo 1;');
22+
23+
create_function('args', 'code');
24+
25+
pcntl_exec('path/goes/here');

‎Magento2/Tests/Security/InsecureFunctionUnitTest.php

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
<?php
2+
/**
3+
* Copyright © Magento. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
namespace Magento2\Tests\Security;
7+
8+
use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest;
9+
10+
/**
11+
* Class InsecureFunctionUnitTest
12+
*/
13+
class InsecureFunctionUnitTest extends AbstractSniffUnitTest
14+
{
15+
/**
16+
* @inheritdoc
17+
*/
18+
public function getErrorList()
19+
{
20+
return [];
21+
}
22+
23+
/**
24+
* @inheritdoc
25+
*/
26+
public function getWarningList()
27+
{
28+
return [
29+
3 => 1,
30+
5 => 1,
31+
7 => 1,
32+
9 => 1,
33+
11 => 1,
34+
13 => 1,
35+
15 => 1,
36+
17 => 1,
37+
19 => 1,
38+
21 => 1,
39+
23 => 1,
40+
25 => 1,
41+
];
42+
}
43+
}

‎Magento2/ruleset.xml

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
<rule ref="Magento2.Security.IncludeFile">
5151
<severity>10</severity>
5252
<type>error</type>
53+
<exclude-pattern>*/Test/*</exclude-pattern>
5354
</rule>
5455
<rule ref="Magento2.Security.LanguageConstruct">
5556
<severity>10</severity>
@@ -58,6 +59,7 @@
5859
<rule ref="Magento2.Security.Superglobal.SuperglobalUsageError">
5960
<severity>10</severity>
6061
<type>error</type>
62+
<exclude-pattern>*/lib/*</exclude-pattern>
6163
</rule>
6264
<rule ref="Magento2.Strings.ExecutableRegEx">
6365
<severity>10</severity>
@@ -85,14 +87,16 @@
8587
<rule ref="Magento2.PHP.DateTime">
8688
<severity>9</severity>
8789
<type>warning</type>
90+
<exclude-pattern>*/lib/*</exclude-pattern>
8891
</rule>
89-
<rule ref="Magento2.PHP.DiscouragedFunction">
92+
<rule ref="Magento2.Security.InsecureFunction">
9093
<severity>9</severity>
9194
<type>warning</type>
9295
</rule>
9396
<rule ref="Magento2.Security.Superglobal.SuperglobalUsageWarning">
9497
<severity>9</severity>
9598
<type>warning</type>
99+
<exclude-pattern>*/lib/*</exclude-pattern>
96100
</rule>
97101
<rule ref="Magento2.Security.XssTemplate">
98102
<include-pattern>*.phtml</include-pattern>
@@ -116,6 +120,7 @@
116120
<rule ref="Magento2.Classes.ObjectInstantiation">
117121
<severity>8</severity>
118122
<type>warning</type>
123+
<exclude-pattern>*/Test/*</exclude-pattern>
119124
</rule>
120125
<rule ref="Magento2.Exceptions.DirectThrow">
121126
<severity>8</severity>
@@ -125,9 +130,17 @@
125130
<severity>8</severity>
126131
<type>warning</type>
127132
</rule>
133+
<rule ref="Magento2.Functions.DiscouragedFunction">
134+
<severity>8</severity>
135+
<type>warning</type>
136+
<exclude-pattern>*/lib/*</exclude-pattern>
137+
<exclude-pattern>*/Test/*</exclude-pattern>
138+
</rule>
128139
<rule ref="Magento2.Functions.StaticFunction">
129140
<severity>8</severity>
130141
<type>warning</type>
142+
<exclude-pattern>*/Test/*</exclude-pattern>
143+
<exclude-pattern>*/Setup/*</exclude-pattern>
131144
</rule>
132145
<rule ref="Magento2.Files.LineLength">
133146
<severity>8</severity>
@@ -226,6 +239,7 @@
226239
<rule ref="Squiz.Functions.GlobalFunction">
227240
<severity>7</severity>
228241
<type>warning</type>
242+
<exclude-pattern>*/Test/*</exclude-pattern>
229243
</rule>
230244
<rule ref="Squiz.Operators.IncrementDecrementUsage">
231245
<severity>7</severity>

0 commit comments

Comments
 (0)