Security/EscapeOutput: fix false negatives when handling anonymous classes (#2559)

* Security/EscapeOutput: fix false negatives when handling anonymous classes

This commit fixes false negatives when the sniff handles readonly anonymous classes and anonymous classes with attributes that are part of a throw statement.

When stepping over tokens after `T_THROW` to find the `T_OPEN_PARENTHESIS` of the exception creation function call/class instantiation, the sniff was not considering that it might need to step over `T_READONLY` tokens or attribute declarations when dealing with anonymous classes.

Fixes #2552

---------

Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
This commit is contained in:
Rodrigo Primo 2025-08-02 05:46:58 -03:00 committed by GitHub
parent fa2b44e1b7
commit afcb17eddd
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 67 additions and 11 deletions

View file

@ -68,6 +68,7 @@
<exclude name="PHPCompatibility.Constants.NewConstants.t_coalesceFound"/>
<exclude name="PHPCompatibility.Constants.NewConstants.t_coalesce_equalFound"/>
<exclude name="PHPCompatibility.Constants.NewConstants.t_yield_fromFound"/>
<exclude name="PHPCompatibility.Constants.NewConstants.t_readonlyFound"/>
</rule>
<!-- Enforce PSR1 compatible namespaces. -->

View file

@ -216,22 +216,35 @@ class EscapeOutputSniff extends AbstractFunctionRestrictionsSniff {
case \T_THROW:
// Find the open parentheses, while stepping over the exception creation tokens.
$ignore = Tokens::$emptyTokens;
$ignore += Collections::namespacedNameTokens();
$ignore += Collections::functionCallTokens();
$ignore += Collections::objectOperators();
$ignore = Tokens::$emptyTokens;
$ignore += Collections::namespacedNameTokens();
$ignore += Collections::functionCallTokens();
$ignore += Collections::objectOperators();
$ignore[ \T_READONLY ] = \T_READONLY;
$next_relevant = $this->phpcsFile->findNext( $ignore, ( $stackPtr + 1 ), null, true );
if ( false === $next_relevant ) {
return;
}
if ( \T_NEW === $this->tokens[ $next_relevant ]['code'] ) {
$next_relevant = $stackPtr;
do {
$next_relevant = $this->phpcsFile->findNext( $ignore, ( $next_relevant + 1 ), null, true );
if ( false === $next_relevant ) {
return;
}
}
if ( \T_NEW === $this->tokens[ $next_relevant ]['code'] ) {
continue;
}
// Skip over attribute declarations when searching for the open parenthesis.
if ( \T_ATTRIBUTE === $this->tokens[ $next_relevant ]['code'] ) {
if ( isset( $this->tokens[ $next_relevant ]['attribute_closer'] ) === false ) {
return;
}
$next_relevant = $this->tokens[ $next_relevant ]['attribute_closer'];
continue;
}
break;
} while ( $next_relevant < ( $this->phpcsFile->numTokens - 1 ) );
if ( \T_OPEN_PARENTHESIS !== $this->tokens[ $next_relevant ]['code']
|| isset( $this->tokens[ $next_relevant ]['parenthesis_closer'] ) === false

View file

@ -662,3 +662,17 @@ die( status: esc_html( $foo ) ); // Ok.
exit( status: $foo ); // Bad.
die( status: $foo ); // Bad.
/*
* Issue https://github.com/WordPress/WordPress-Coding-Standards/issues/2552
* Ensure that readonly anonymous classes and anonymous classes with attributes are handled
* correctly when part of a throw statement.
*/
throw new #[MyAttribute] readonly class( esc_html( $message ) ) extends Exception {}; // Good.
throw new readonly class( $unescaped ) {}; // Bad.
throw new #[MyAttribute] class( $unescaped ) extends Exception {}; // Bad.
throw new
#[Attribute1]
/* some comment */
#[Attribute2('text', 10)]
readonly class( $unescaped ) {}; // Bad.

View file

@ -0,0 +1,8 @@
<?php
/*
* Intentional parse error (nothing after T_ATTRIBUTE).
* This should be the only test in this file.
*/
throw new #[

View file

@ -0,0 +1,8 @@
<?php
/*
* Intentional parse error (only whitespaces after T_ATTRIBUTE_END).
* This should be the only test in this file.
*/
throw new #[MyAttribute]

View file

@ -0,0 +1,9 @@
<?php
/*
* Intentional parse error (nothing after T_ATTRIBUTE_END).
* There should be no whitespaces at the end of this file.
* This should be the only test in this file.
*/
throw new #[MyAttribute]

View file

@ -161,6 +161,9 @@ final class EscapeOutputUnitTest extends AbstractSniffUnitTest {
657 => 1,
663 => 1,
664 => 1,
672 => 1,
673 => 1,
678 => 1,
);
case 'EscapeOutputUnitTest.6.inc':