diff --git a/includes/Checker/Checks/Plugin_Repo/Setting_Sanitization_Check.php b/includes/Checker/Checks/Plugin_Repo/Setting_Sanitization_Check.php new file mode 100644 index 000000000..68f331627 --- /dev/null +++ b/includes/Checker/Checks/Plugin_Repo/Setting_Sanitization_Check.php @@ -0,0 +1,113 @@ + 'php', + 'standard' => 'PluginCheck', + 'sniffs' => 'PluginCheck.CodeAnalysis.SettingSanitization', + ); + } + + /** + * Gets the description for the check. + * + * Every check must have a short description explaining what the check does. + * + * @since 1.4.0 + * + * @return string Description. + */ + public function get_description(): string { + return __( 'Ensures sanitization in register_setting().', 'plugin-check' ); + } + + /** + * Gets the documentation URL for the check. + * + * Every check must have a URL with further information about the check. + * + * @since 1.4.0 + * + * @return string The documentation URL. + */ + public function get_documentation_url(): string { + return __( 'https://developer.wordpress.org/reference/functions/register_setting/', 'plugin-check' ); + } + + /** + * Amends the given result with a message for the specified file, including error information. + * + * @since 1.4.0 + * + * @param Check_Result $result The check result to amend, including the plugin context to check. + * @param bool $error Whether it is an error or notice. + * @param string $message Error message. + * @param string $code Error code. + * @param string $file Absolute path to the file where the issue was found. + * @param int $line The line on which the message occurred. Default is 0 (unknown line). + * @param int $column The column on which the message occurred. Default is 0 (unknown column). + * @param string $docs URL for further information about the message. + * @param int $severity Severity level. Default is 5. + */ + protected function add_result_message_for_file( Check_Result $result, $error, $message, $code, $file, $line = 0, $column = 0, string $docs = '', $severity = 5 ) { + // Update severity. + switch ( $code ) { + case 'PluginCheck.CodeAnalysis.SettingSanitization.register_settingMissing': + case 'PluginCheck.CodeAnalysis.SettingSanitization.register_settingInvalid': + $severity = 7; + break; + + default: + break; + } + + // Add docs link. + $docs = __( 'https://developer.wordpress.org/reference/functions/register_setting/', 'plugin-check' ); + + parent::add_result_message_for_file( $result, $error, $message, $code, $file, $line, $column, $docs, $severity ); + } +} diff --git a/includes/Checker/Default_Check_Repository.php b/includes/Checker/Default_Check_Repository.php index 247140677..71319d746 100644 --- a/includes/Checker/Default_Check_Repository.php +++ b/includes/Checker/Default_Check_Repository.php @@ -93,6 +93,7 @@ private function register_default_checks() { 'non_blocking_scripts' => new Checks\Performance\Non_Blocking_Scripts_Check(), 'offloading_files' => new Checks\Plugin_Repo\Offloading_Files_Check(), 'image_functions' => new Checks\Performance\Image_Functions_Check(), + 'setting_sanitization' => new Checks\Plugin_Repo\Setting_Sanitization_Check(), ) ); diff --git a/phpcs-sniffs/PluginCheck/Sniffs/CodeAnalysis/SettingSanitizationSniff.php b/phpcs-sniffs/PluginCheck/Sniffs/CodeAnalysis/SettingSanitizationSniff.php new file mode 100644 index 000000000..d5286203b --- /dev/null +++ b/phpcs-sniffs/PluginCheck/Sniffs/CodeAnalysis/SettingSanitizationSniff.php @@ -0,0 +1,102 @@ + Key is function name, value irrelevant. + */ + + protected $target_functions = array( + 'register_setting' => true, + ); + + /** + * Process the parameters of a matched function. + * + * @since 1.4.0 + * + * @param int $stackPtr The position of the current token in the stack. + * @param string $group_name The name of the group which was matched. + * @param string $matched_content The token content (function name) which was matched in lowercase. + * @param array $parameters Array with information about the parameters. + * + * @return void + */ + public function process_parameters( $stackPtr, $group_name, $matched_content, $parameters ) { + $third_param = PassedParameters::getParameterFromStack( $parameters, 3, 'args' ); + $error_code = MessageHelper::stringToErrorCode( $matched_content, true ); + + if ( false === $third_param ) { + $this->phpcsFile->addError( + 'Sanitization missing for %s().', + $stackPtr, + $error_code . 'Missing', + array( $matched_content ) + ); + + return; + } + + $content = TextStrings::stripQuotes( $third_param['clean'] ); + + if ( is_numeric( $content ) || in_array( strtolower( $content ), array( 'true', 'false', 'null' ), true ) ) { + $this->phpcsFile->addError( + 'Invalid sanitization provided for %s().', + $stackPtr, + $error_code . 'Invalid', + array( $matched_content ) + ); + + return; + } + + $next_token = $this->phpcsFile->findNext( Tokens::$emptyTokens, $third_param['start'], ( $third_param['end'] + 1 ), true ); + + if ( false !== $next_token ) { + $next_type = $this->tokens[ $next_token ]['type']; + + if ( in_array( $next_type, array( 'T_ARRAY', 'T_OPEN_SHORT_ARRAY', 'T_VARIABLE', 'T_CALLABLE' ), true ) ) { + $this->phpcsFile->addWarning( + 'Dynamic argument passed in third parameter of %s(). Please ensure proper sanitization.', + $stackPtr, + $error_code . 'Dynamic', + array( $matched_content ) + ); + } + } + } +} diff --git a/phpcs-sniffs/PluginCheck/Tests/CodeAnalysis/SettingSanitizationUnitTest.inc b/phpcs-sniffs/PluginCheck/Tests/CodeAnalysis/SettingSanitizationUnitTest.inc new file mode 100644 index 000000000..8c4fa63f0 --- /dev/null +++ b/phpcs-sniffs/PluginCheck/Tests/CodeAnalysis/SettingSanitizationUnitTest.inc @@ -0,0 +1,27 @@ + 'sanitize_text_field']); // Warning. +$args = array( 'sanitize_callback' => 'absint' ); +register_setting( 'my_options_group', 'my_option_name', $args ); // Warning. + +class TestClass { + public function register_setting() { + } +} + +$obj = new TestClass(); +$obj->register_setting(); // Good. + +register_setting('my_options_group', 'my_option_name',array(&$this,'validate')); // Warning. + +register_setting( + 'my_options_group', + $setting_key, + array( + 'sanitize_callback' => $setting['sanitize_callback'] + ) +); // Warning. diff --git a/phpcs-sniffs/PluginCheck/Tests/CodeAnalysis/SettingSanitizationUnitTest.php b/phpcs-sniffs/PluginCheck/Tests/CodeAnalysis/SettingSanitizationUnitTest.php new file mode 100644 index 000000000..d63a3d5bc --- /dev/null +++ b/phpcs-sniffs/PluginCheck/Tests/CodeAnalysis/SettingSanitizationUnitTest.php @@ -0,0 +1,64 @@ + => + */ + public function getErrorList() { + return array( + 2 => 1, + 3 => 1, + 4 => 1, + ); + } + + /** + * Returns the lines where warnings should occur. + * + * @return array => + */ + public function getWarningList() { + return array( + 7 => 1, + 9 => 1, + 19 => 1, + 21 => 1, + ); + } + + /** + * Returns the fully qualified class name (FQCN) of the sniff. + * + * @return string The fully qualified class name of the sniff. + */ + protected function get_sniff_fqcn() { + return SettingSanitizationSniff::class; + } + + /** + * Sets the parameters for the sniff. + * + * @throws \RuntimeException If unable to set the ruleset parameters required for the test. + * + * @param Sniff $sniff The sniff being tested. + */ + public function set_sniff_parameters( Sniff $sniff ) { + } +} diff --git a/phpcs-sniffs/PluginCheck/ruleset.xml b/phpcs-sniffs/PluginCheck/ruleset.xml index 4243c3285..901d7e6ae 100644 --- a/phpcs-sniffs/PluginCheck/ruleset.xml +++ b/phpcs-sniffs/PluginCheck/ruleset.xml @@ -8,5 +8,6 @@ + diff --git a/tests/phpunit/testdata/plugins/test-plugin-setting-sanitization-check-with-errors/load.php b/tests/phpunit/testdata/plugins/test-plugin-setting-sanitization-check-with-errors/load.php new file mode 100644 index 000000000..6debcb4b6 --- /dev/null +++ b/tests/phpunit/testdata/plugins/test-plugin-setting-sanitization-check-with-errors/load.php @@ -0,0 +1,23 @@ + 'absint' ); +register_setting( 'my_options_group', 'option_4', $args ); // Warning. diff --git a/tests/phpunit/tests/Checker/Checks/Setting_Sanitization_Check_Test.php b/tests/phpunit/tests/Checker/Checks/Setting_Sanitization_Check_Test.php new file mode 100644 index 000000000..a3622c1c4 --- /dev/null +++ b/tests/phpunit/tests/Checker/Checks/Setting_Sanitization_Check_Test.php @@ -0,0 +1,33 @@ +run( $check_result ); + + $errors = $check_result->get_errors(); + $warnings = $check_result->get_warnings(); + + $this->assertNotEmpty( $errors ); + $this->assertNotEmpty( $warnings ); + $this->assertArrayHasKey( 'load.php', $errors ); + $this->assertArrayHasKey( 'load.php', $warnings ); + + $this->assertSame( 'PluginCheck.CodeAnalysis.SettingSanitization.register_settingMissing', $errors['load.php'][19][1][0]['code'] ); + $this->assertSame( 'PluginCheck.CodeAnalysis.SettingSanitization.register_settingInvalid', $errors['load.php'][20][1][0]['code'] ); + $this->assertSame( 'PluginCheck.CodeAnalysis.SettingSanitization.register_settingDynamic', $warnings['load.php'][23][1][0]['code'] ); + } +}