Skip to content

Commit 86b43ec

Browse files
committed
PHP 8.4 | Security/EscapeOutput: handle exit/die using named parameters
As of PHP 8.4, the internal of `T_EXIT` are changing in PHP itself and `exit`/`die` will internally be treated as a function call. The net effect of this is that named parameters can now be used with `exit()`/`die()`. For the `WordPress.Security.EscapeOutput` sniff this would lead to false positives as the parameter name would be seen as "not escaped". As of PHPCSUtils 1.1.0, the methods within the `PassedParameters` class will allow for passing a `T_EXIT` token. This, in turn, allows for handling `exit`/`die` with named parameters correctly in the context of the `EscapeOutput` sniff. This commit implements using the `PassedParameters` class for parsing calls to `exit`/`die`, which fixes the issue. Includes tests. Ref: https://wiki.php.net/rfc/exit-as-function
1 parent a74a579 commit 86b43ec

File tree

3 files changed

+19
-10
lines changed

3 files changed

+19
-10
lines changed

WordPress/Sniffs/Security/EscapeOutputSniff.php

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -200,19 +200,19 @@ public function process_token( $stackPtr ) {
200200
return parent::process_token( $stackPtr );
201201

202202
case \T_EXIT:
203-
$next_non_empty = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true );
204-
if ( false === $next_non_empty
205-
|| \T_OPEN_PARENTHESIS !== $this->tokens[ $next_non_empty ]['code']
206-
|| isset( $this->tokens[ $next_non_empty ]['parenthesis_closer'] ) === false
207-
) {
208-
// Live coding/parse error or an exit/die which doesn't pass a status code. Ignore.
203+
$params = PassedParameters::getParameters( $this->phpcsFile, $stackPtr );
204+
if ( empty( $params ) ) {
205+
// Live coding/parse error or an exit/die which doesn't pass a status. Ignore.
209206
return;
210207
}
211208

212-
// $end is not examined, so make sure the parentheses are balanced.
213-
$start = $next_non_empty;
214-
$end = ( $this->tokens[ $next_non_empty ]['parenthesis_closer'] + 1 );
215-
break;
209+
// There should only be one parameter ($status), but just to be on the safe side.
210+
foreach ( $params as $param ) {
211+
$this->check_code_is_escaped( $param['start'], ( $param['end'] + 1 ) );
212+
}
213+
214+
// Skip to the end of the last found parameter.
215+
return ( $param['end'] + 1 );
216216

217217
case \T_THROW:
218218
// Find the open parentheses, while stepping over the exception creation tokens.

WordPress/Tests/Security/EscapeOutputUnitTest.1.inc

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -655,3 +655,10 @@ echo '<input type="search" value="' . get_search_query( false ) . '">'; // Bad.
655655
echo '<input type="search" value="' . get_search_query( 0 ) . '">'; // Bad.
656656
echo '<input type="search" value="' . get_search_query( escape: false ) . '">'; // OK, well not really, typo in param name, but that's not our concern.
657657
echo '<input type="search" value="' . get_search_query( escaped: false ) . '">'; // Bad.
658+
659+
// PHP 8.4: exit/die using named parameters.
660+
exit( status: esc_html( $foo ) ); // Ok.
661+
die( status: esc_html( $foo ) ); // Ok.
662+
663+
exit( status: $foo ); // Bad.
664+
die( status: $foo ); // Bad.

WordPress/Tests/Security/EscapeOutputUnitTest.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,8 @@ public function getErrorList( $testFile = '' ) {
159159
654 => 1,
160160
655 => 1,
161161
657 => 1,
162+
663 => 1,
163+
664 => 1,
162164
);
163165

164166
case 'EscapeOutputUnitTest.6.inc':

0 commit comments

Comments
 (0)