Security/PHPFilterFunctions: add support for PHP 8.0+ named parameters

1. Changed the `$target_functions` property to contain information about the target parameter name and position.
2. Adjusted the logic in the sniff to allow for named parameters using the new PHPCSUtils 1.0.0-alpha4 `PassedParameters::getParameterFromStack()` method.
3. The parameter names used are in line with the name as per the PHP 8.0 release.
    PHP itself renamed a lot of parameters in PHP 8.0. As named parameters did not exist before PHP 8.0, the parameter name as per PHP 8.0 (or above) is the only relevant name.
    Also see: php/doc-en#2044
4. Updated the error messages to use the parameter name instead of its position.

As a lot of the logic is now independent of which function is called, this commit also reduces code duplication in the sniff by some logic changes.

Includes additional unit tests.

Note: in the context of named parameters, it would be advisable to rename the `MissingSecondParameter` and `MissingThirdParameter` error codes to a dynamic error code using the parameter name instead, but as that would be a BC-break, this will need to wait for the next major (if deemed worth making the change).

Author: jrfnl

WordPressVIPMinimum/Sniffs/Security/PHPFilterFunctionsSniff.php

@@ -9,6 +9,7 @@
 
 namespace WordPressVIPMinimum\Sniffs\Security;
 
+use PHPCSUtils\Utils\PassedParameters;
 use WordPressCS\WordPress\AbstractFunctionParameterSniff;
 
 /**
@@ -26,15 +27,27 @@ class PHPFilterFunctionsSniff extends AbstractFunctionParameterSniff {
 	protected $group_name = 'php_filter_functions';
 
 	/**
-	 * Functions this sniff is looking for.
+	 * Functions this sniff is looking for and information on which parameter to check for in those function calls.
 	 *
-	 * @var array<string, bool> Key is the function name, value irrelevant.
+	 * @var array<string, array{param_position: int, param_name: string}> Key is the function name.
 	 */
 	protected $target_functions = [
-		'filter_var'         => true,
-		'filter_input'       => true,
-		'filter_var_array'   => true,
-		'filter_input_array' => true,
+		'filter_var'         => [
+			'param_position' => 2,
+			'param_name'     => 'filter',
+		],
+		'filter_input'       => [
+			'param_position' => 3,
+			'param_name'     => 'filter',
+		],
+		'filter_var_array'   => [
+			'param_position' => 2,
+			'param_name'     => 'options',
+		],
+		'filter_input_array' => [
+			'param_position' => 2,
+			'param_name'     => 'options',
+		],
 	];
 
 	/**
@@ -60,30 +73,28 @@ class PHPFilterFunctionsSniff extends AbstractFunctionParameterSniff {
 	 *                  normal file processing.
 	 */
 	public function process_parameters( $stackPtr, $group_name, $matched_content, $parameters ) {
-		if ( $matched_content === 'filter_input' ) {
-			if ( count( $parameters ) === 2 ) {
-				$message = 'Missing third parameter for "%s".';
-				$data    = [ $matched_content ];
-				$this->phpcsFile->addWarning( $message, $stackPtr, 'MissingThirdParameter', $data );
-			}
+		$param_position = $this->target_functions[ $matched_content ]['param_position'];
+		$param_name     = $this->target_functions[ $matched_content ]['param_name'];
 
-			if ( isset( $parameters[3], $this->restricted_filters[ $parameters[3]['clean'] ] ) ) {
-				$message = 'Please use an appropriate filter to sanitize, as "%s" does no filtering, see: http://php.net/manual/en/filter.filters.sanitize.php.';
-				$data    = [ $parameters[3]['clean'] ];
-				$this->phpcsFile->addWarning( $message, $stackPtr, 'RestrictedFilter', $data );
-			}
-		} else {
-			if ( count( $parameters ) === 1 ) {
-				$message = 'Missing second parameter for "%s".';
-				$data    = [ $matched_content ];
-				$this->phpcsFile->addWarning( $message, $stackPtr, 'MissingSecondParameter', $data );
-			}
+		$target_param = PassedParameters::getParameterFromStack( $parameters, $param_position, $param_name );
+		if ( $target_param === false ) {
+			$message = 'Missing $%s parameter for "%s()".';
+			$data    = [ $param_name, $matched_content ];
 
-			if ( isset( $parameters[2], $this->restricted_filters[ $parameters[2]['clean'] ] ) ) {
-				$message = 'Please use an appropriate filter to sanitize, as "%s" does no filtering, see http://php.net/manual/en/filter.filters.sanitize.php.';
-				$data    = [ $parameters[2]['clean'] ];
-				$this->phpcsFile->addWarning( $message, $stackPtr, 'RestrictedFilter', $data );
+			// Error codes should probably be made more descriptive, but that would be a BC-break.
+			$error_code = 'MissingSecondParameter';
+			if ( $matched_content === 'filter_input' ) {
+				$error_code = 'MissingThirdParameter';
 			}
+
+			$this->phpcsFile->addWarning( $message, $stackPtr, $error_code, $data );
+			return;
+		}
+
+		if ( isset( $this->restricted_filters[ $target_param['clean'] ] ) ) {
+			$message = 'Please use an appropriate filter to sanitize, as "%s" does no filtering, see: http://php.net/manual/en/filter.filters.sanitize.php.';
+			$data    = [ $target_param['clean'] ];
+			$this->phpcsFile->addWarning( $message, $stackPtr, 'RestrictedFilter', $data );
 		}
 	}
 }

WordPressVIPMinimum/Tests/Security/PHPFilterFunctionsUnitTest.inc

@@ -41,18 +41,35 @@ $incorrect_but_ok = filter_input();
 /*
  * These should all be flagged with a warning.
  */
-filter_input( INPUT_GET, 'foo' ); // Missing third parameter.
+filter_input( INPUT_GET, 'foo' ); // Missing $filter parameter.
 \filter_input( INPUT_GET, 'foo', FILTER_DEFAULT ); // This filter ID does nothing.
 filter_input( INPUT_GET, "foo", FILTER_UNSAFE_RAW /* comment */ ,); // This filter ID does nothing.
 
-filter_var( $url ); // Missing second parameter.
+filter_var( $url ); // Missing $filter parameter.
 Filter_Var( $url, FILTER_DEFAULT ); // This filter ID does nothing.
 filter_var( 'https://google.com', FILTER_UNSAFE_RAW ); // This filter ID does nothing.
 
-filter_var_array( $array, ); // Missing second parameter.
+filter_var_array( $array, ); // Missing $options parameter.
 filter_var_array( $array, FILTER_DEFAULT ); // This filter ID does nothing.
 filter_var_array( $array, FILTER_UNSAFE_RAW ); // This filter ID does nothing.
 
-filter_input_array( $array ); // Missing second parameter.
+filter_input_array( $array ); // Missing $options parameter.
 \FILTER_INPUT_ARRAY( $array, FILTER_DEFAULT ); // This filter ID does nothing.
 filter_input_array( $array, FILTER_UNSAFE_RAW, ); // This filter ID does nothing.
+
+// Safeguard handling of function calls using PHP 8.0+ named parameters.
+filter_input(var_name: $var_name, filter: FILTER_SANITIZE_STRING, type: FILTER_DEFAULT); // OK, invalid input value for $type, but that's not our concern.
+filter_input(var_name: $var_name, filter: $filter, type: $type); // Ignore, undetermined.
+filter_input(
+    var_name: $var_name,
+    filter: FILTER_DEFAULT, // This filter ID does nothing.
+    type: $type,
+);
+
+filter_var(filter: FILTER_SANITIZE_URL); // OK, well not really, missing required parameter, but that's not our concern.
+filter_var($value, options: FILTER_NULL_ON_FAILURE); // Missing $filter parameter.
+filter_var(value: $value, filters: FILTER_SANITIZE_URL); // Typo in parameter name, report as missing $filter parameter.
+
+filter_var_array(options: FILTER_UNSAFE_RAW, array: $array); // This filter ID does nothing.
+
+filter_input_array($type, add_empty: false, options: FILTER_DEFAULT,); // This filter ID does nothing.

WordPressVIPMinimum/Tests/Security/PHPFilterFunctionsUnitTest.php

@@ -44,6 +44,11 @@ public function getWarningList() {
 			56 => 1,
 			57 => 1,
 			58 => 1,
+			63 => 1,
+			70 => 1,
+			71 => 1,
+			73 => 1,
+			75 => 1,
 		];
 	}
 }