From f53758d2bf2984f7ca627e7111c4c22ad98dc47d Mon Sep 17 00:00:00 2001 From: Lena Orobei Date: Tue, 19 Mar 2019 13:53:32 -0500 Subject: [PATCH 1/2] [Enhancement] DiscouragedFunction rule improvement --- .../DiscouragedFunctionSniff.php | 44 ++++--------------- .../Sniffs/Security/InsecureFunctionSniff.php | 37 ++++++++++++++++ .../DiscouragedFunctionUnitTest.inc | 0 .../DiscouragedFunctionUnitTest.php | 8 +--- .../Security/InsecureFunctionUnitTest.inc | 18 ++++++++ .../Security/InsecureFunctionUnitTest.php | 39 ++++++++++++++++ Magento2/ruleset.xml | 12 ++++- 7 files changed, 114 insertions(+), 44 deletions(-) rename Magento2/Sniffs/{PHP => Functions}/DiscouragedFunctionSniff.php (83%) create mode 100644 Magento2/Sniffs/Security/InsecureFunctionSniff.php rename Magento2/Tests/{PHP => Functions}/DiscouragedFunctionUnitTest.inc (100%) rename Magento2/Tests/{PHP => Functions}/DiscouragedFunctionUnitTest.php (97%) create mode 100644 Magento2/Tests/Security/InsecureFunctionUnitTest.inc create mode 100644 Magento2/Tests/Security/InsecureFunctionUnitTest.php diff --git a/Magento2/Sniffs/PHP/DiscouragedFunctionSniff.php b/Magento2/Sniffs/Functions/DiscouragedFunctionSniff.php similarity index 83% rename from Magento2/Sniffs/PHP/DiscouragedFunctionSniff.php rename to Magento2/Sniffs/Functions/DiscouragedFunctionSniff.php index abb9d2d7..eb7ef313 100644 --- a/Magento2/Sniffs/PHP/DiscouragedFunctionSniff.php +++ b/Magento2/Sniffs/Functions/DiscouragedFunctionSniff.php @@ -3,9 +3,8 @@ * Copyright © Magento. All rights reserved. * See COPYING.txt for license details. */ -namespace Magento2\Sniffs\PHP; +namespace Magento2\Sniffs\Functions; -use PHP_CodeSniffer\Files\File; use PHP_CodeSniffer\Standards\Generic\Sniffs\PHP\ForbiddenFunctionsSniff; /** @@ -20,13 +19,19 @@ class DiscouragedFunctionSniff extends ForbiddenFunctionsSniff */ protected $patternMatch = true; + /** + * If true, an error will be thrown; otherwise a warning. + * + * @var boolean + */ + public $error = false; + /** * List of patterns for forbidden functions. * * @var array */ public $forbiddenFunctions = [ - '^assert$' => null, '^bind_textdomain_codeset$' => null, '^bindtextdomain$' => null, '^bz.*$' => null, @@ -52,7 +57,6 @@ class DiscouragedFunctionSniff extends ForbiddenFunctionsSniff '^dirname$' => null, '^dngettext$' => null, '^domxml_.*$' => null, - '^exec$' => null, '^fbsql_.*$' => null, '^fdf_add_doc_javascript$' => null, '^fdf_open$' => null, @@ -93,7 +97,6 @@ class DiscouragedFunctionSniff extends ForbiddenFunctionsSniff '^parse_str$' => null, '^parse_url$' => null, '^parsekit_compile_string$' => null, - '^passthru$' => null, '^pathinfo$' => null, '^pcntl_.*$' => null, '^posix_.*$' => null, @@ -122,14 +125,12 @@ class DiscouragedFunctionSniff extends ForbiddenFunctionsSniff '^setcookie$' => null, '^setlocale$' => null, '^setrawcookie$' => null, - '^shell_exec$' => null, '^sleep$' => null, '^socket_.*$' => null, '^stream_.*$' => null, '^sybase_.*$' => null, '^symlink$' => null, '^syslog$' => null, - '^system$' => null, '^touch$' => null, '^trigger_error$' => null, '^unlink$' => null, @@ -220,34 +221,5 @@ class DiscouragedFunctionSniff extends ForbiddenFunctionsSniff '^is_null$' => 'strict comparison "=== null"', '^intval$' => '(int) construction', '^strval$' => '(string) construction', - '^md5$' => 'improved hash functions (SHA-256, SHA-512 etc.)', - '^serialize$' => 'json_encode', - '^unserialize$' => 'json_decode', ]; - - /** - * Generates warning for this sniff. - * - * @param File $phpcsFile The file being scanned. - * @param int $stackPtr The position of the forbidden function in the token array. - * @param string $function The name of the forbidden function. - * @param string $pattern The pattern used for the match. - * - * @return void - */ - protected function addError($phpcsFile, $stackPtr, $function, $pattern = null) - { - $data = [$function]; - $warningMessage = 'The use of function %s() is discouraged'; - $warningCode = 'Found'; - if ($pattern === null) { - $pattern = $function; - } - if ($this->forbiddenFunctions[$pattern] !== null) { - $warningCode .= 'WithAlternative'; - $data[] = $this->forbiddenFunctions[$pattern]; - $warningMessage .= '; use %s instead.'; - } - $phpcsFile->addWarning($warningMessage, $stackPtr, $warningCode, $data); - } } diff --git a/Magento2/Sniffs/Security/InsecureFunctionSniff.php b/Magento2/Sniffs/Security/InsecureFunctionSniff.php new file mode 100644 index 00000000..71c9689a --- /dev/null +++ b/Magento2/Sniffs/Security/InsecureFunctionSniff.php @@ -0,0 +1,37 @@ + null, + 'exec' => null, + 'passthru' => null, + 'shell_exec' => null, + 'system' => null, + 'md5' => 'improved hash functions (SHA-256, SHA-512 etc.)', + 'serialize' => 'json_encode', + 'unserialize' => 'json_decode', + ]; +} diff --git a/Magento2/Tests/PHP/DiscouragedFunctionUnitTest.inc b/Magento2/Tests/Functions/DiscouragedFunctionUnitTest.inc similarity index 100% rename from Magento2/Tests/PHP/DiscouragedFunctionUnitTest.inc rename to Magento2/Tests/Functions/DiscouragedFunctionUnitTest.inc diff --git a/Magento2/Tests/PHP/DiscouragedFunctionUnitTest.php b/Magento2/Tests/Functions/DiscouragedFunctionUnitTest.php similarity index 97% rename from Magento2/Tests/PHP/DiscouragedFunctionUnitTest.php rename to Magento2/Tests/Functions/DiscouragedFunctionUnitTest.php index ce2759c2..98b29606 100644 --- a/Magento2/Tests/PHP/DiscouragedFunctionUnitTest.php +++ b/Magento2/Tests/Functions/DiscouragedFunctionUnitTest.php @@ -3,7 +3,7 @@ * Copyright © Magento. All rights reserved. * See COPYING.txt for license details. */ -namespace Magento2\Tests\PHP; +namespace Magento2\Tests\Functions; use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest; @@ -26,7 +26,6 @@ public function getErrorList() public function getWarningList() { return [ - 3 => 1, 6 => 1, 7 => 1, 9 => 1, @@ -63,7 +62,6 @@ public function getWarningList() 65 => 1, 67 => 1, 69 => 1, - 71 => 1, 73 => 1, 74 => 1, 76 => 1, @@ -122,7 +120,6 @@ public function getWarningList() 166 => 1, 169 => 1, 171 => 1, - 173 => 1, 175 => 1, 177 => 1, 179 => 1, @@ -152,7 +149,6 @@ public function getWarningList() 229 => 1, 231 => 1, 233 => 1, - 235 => 1, 237 => 1, 239 => 1, 241 => 1, @@ -162,7 +158,6 @@ public function getWarningList() 247 => 1, 249 => 1, 251 => 1, - 253 => 1, 255 => 1, 258 => 1, 261 => 1, @@ -257,7 +252,6 @@ public function getWarningList() 458 => 1, 460 => 1, 462 => 1, - 464 => 1, ]; } } diff --git a/Magento2/Tests/Security/InsecureFunctionUnitTest.inc b/Magento2/Tests/Security/InsecureFunctionUnitTest.inc new file mode 100644 index 00000000..4ce38897 --- /dev/null +++ b/Magento2/Tests/Security/InsecureFunctionUnitTest.inc @@ -0,0 +1,18 @@ + 1, + 5 => 1, + 7 => 1, + 9 => 1, + 11 => 1, + 13 => 1, + 15 => 1, + 17 => 1, + ]; + } +} diff --git a/Magento2/ruleset.xml b/Magento2/ruleset.xml index dafa4c74..56cc16f4 100644 --- a/Magento2/ruleset.xml +++ b/Magento2/ruleset.xml @@ -50,6 +50,7 @@ 10 error + */Test/* 10 @@ -86,7 +87,7 @@ 9 warning - + 9 warning @@ -116,6 +117,7 @@ 8 warning + */Test/* 8 @@ -125,9 +127,16 @@ 8 warning + + 8 + warning + */lib/* + */Test/* + 8 warning + */Test/* 8 @@ -226,6 +235,7 @@ 7 warning + */Test/* 7 From e8d0916abd83ac45d709fdeebab4c60c8191bb4d Mon Sep 17 00:00:00 2001 From: Lena Orobei Date: Tue, 19 Mar 2019 14:28:13 -0500 Subject: [PATCH 2/2] [Enhancement] DiscouragedFunction rule improvement --- Magento2/Sniffs/Functions/DiscouragedFunctionSniff.php | 3 --- Magento2/Sniffs/Security/InsecureFunctionSniff.php | 10 +++++++--- .../Tests/Functions/DiscouragedFunctionUnitTest.php | 3 --- Magento2/Tests/Security/InsecureFunctionUnitTest.inc | 7 +++++++ Magento2/Tests/Security/InsecureFunctionUnitTest.php | 6 +++++- Magento2/ruleset.xml | 4 ++++ 6 files changed, 23 insertions(+), 10 deletions(-) diff --git a/Magento2/Sniffs/Functions/DiscouragedFunctionSniff.php b/Magento2/Sniffs/Functions/DiscouragedFunctionSniff.php index eb7ef313..56bfa9d3 100644 --- a/Magento2/Sniffs/Functions/DiscouragedFunctionSniff.php +++ b/Magento2/Sniffs/Functions/DiscouragedFunctionSniff.php @@ -44,7 +44,6 @@ class DiscouragedFunctionSniff extends ForbiddenFunctionsSniff '^chroot$' => null, '^com_load_typelib$' => null, '^copy$' => null, - '^create_function$' => null, '^curl_.*$' => null, '^cyrus_connect$' => null, '^dba_.*$' => null, @@ -104,10 +103,8 @@ class DiscouragedFunctionSniff extends ForbiddenFunctionsSniff '^pfsockopen$' => null, '^pg_.*$' => null, '^php_check_syntax$' => null, - '^popen$' => null, '^print_r$' => null, '^printf$' => null, - '^proc_open$' => null, '^putenv$' => null, '^readfile$' => null, '^readgzfile$' => null, diff --git a/Magento2/Sniffs/Security/InsecureFunctionSniff.php b/Magento2/Sniffs/Security/InsecureFunctionSniff.php index 71c9689a..ddc80f7e 100644 --- a/Magento2/Sniffs/Security/InsecureFunctionSniff.php +++ b/Magento2/Sniffs/Security/InsecureFunctionSniff.php @@ -3,7 +3,7 @@ * Copyright © Magento. All rights reserved. * See COPYING.txt for license details. */ -namespace Magento2\Sniffs\Functions; +namespace Magento2\Sniffs\Security; use PHP_CodeSniffer\Standards\Generic\Sniffs\PHP\ForbiddenFunctionsSniff; @@ -26,12 +26,16 @@ class InsecureFunctionSniff extends ForbiddenFunctionsSniff */ public $forbiddenFunctions = [ 'assert' => null, + 'create_function' => null, 'exec' => null, + 'md5' => 'improved hash functions (SHA-256, SHA-512 etc.)', 'passthru' => null, + 'pcntl_exec' => null, + 'popen' => null, + 'proc_open' => null, + 'serialize' => 'json_encode', 'shell_exec' => null, 'system' => null, - 'md5' => 'improved hash functions (SHA-256, SHA-512 etc.)', - 'serialize' => 'json_encode', 'unserialize' => 'json_decode', ]; } diff --git a/Magento2/Tests/Functions/DiscouragedFunctionUnitTest.php b/Magento2/Tests/Functions/DiscouragedFunctionUnitTest.php index 98b29606..dbb1f9b8 100644 --- a/Magento2/Tests/Functions/DiscouragedFunctionUnitTest.php +++ b/Magento2/Tests/Functions/DiscouragedFunctionUnitTest.php @@ -39,7 +39,6 @@ public function getWarningList() 28 => 1, 30 => 1, 32 => 1, - 34 => 1, 36 => 1, 37 => 1, 38 => 1, @@ -128,10 +127,8 @@ public function getWarningList() 184 => 1, 185 => 1, 187 => 1, - 189 => 1, 191 => 1, 193 => 1, - 195 => 1, 197 => 1, 199 => 1, 201 => 1, diff --git a/Magento2/Tests/Security/InsecureFunctionUnitTest.inc b/Magento2/Tests/Security/InsecureFunctionUnitTest.inc index 4ce38897..6f62c45d 100644 --- a/Magento2/Tests/Security/InsecureFunctionUnitTest.inc +++ b/Magento2/Tests/Security/InsecureFunctionUnitTest.inc @@ -16,3 +16,10 @@ serialize([]); unserialize(''); +popen('echo 1;'); + +proc_open('echo 1;'); + +create_function('args', 'code'); + +pcntl_exec('path/goes/here'); diff --git a/Magento2/Tests/Security/InsecureFunctionUnitTest.php b/Magento2/Tests/Security/InsecureFunctionUnitTest.php index c5dcb1e5..33054944 100644 --- a/Magento2/Tests/Security/InsecureFunctionUnitTest.php +++ b/Magento2/Tests/Security/InsecureFunctionUnitTest.php @@ -3,7 +3,7 @@ * Copyright © Magento. All rights reserved. * See COPYING.txt for license details. */ -namespace Magento2\Tests\Functions; +namespace Magento2\Tests\Security; use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest; @@ -34,6 +34,10 @@ public function getWarningList() 13 => 1, 15 => 1, 17 => 1, + 19 => 1, + 21 => 1, + 23 => 1, + 25 => 1, ]; } } diff --git a/Magento2/ruleset.xml b/Magento2/ruleset.xml index 56cc16f4..be5d59c1 100644 --- a/Magento2/ruleset.xml +++ b/Magento2/ruleset.xml @@ -59,6 +59,7 @@ 10 error + */lib/* 10 @@ -86,6 +87,7 @@ 9 warning + */lib/* 9 @@ -94,6 +96,7 @@ 9 warning + */lib/* *.phtml @@ -137,6 +140,7 @@ 8 warning */Test/* + */Setup/* 8