Skip to content

Commit 36322b6

Browse files
committed
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
1 parent 5511d78 commit 36322b6

File tree

5 files changed

+52
-4
lines changed

5 files changed

+52
-4
lines changed

WordPress/Sniffs/Security/EscapeOutputSniff.php

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -216,10 +216,11 @@ public function process_token( $stackPtr ) {
216216

217217
case \T_THROW:
218218
// Find the open parentheses, while stepping over the exception creation tokens.
219-
$ignore = Tokens::$emptyTokens;
220-
$ignore += Collections::namespacedNameTokens();
221-
$ignore += Collections::functionCallTokens();
222-
$ignore += Collections::objectOperators();
219+
$ignore = Tokens::$emptyTokens;
220+
$ignore += Collections::namespacedNameTokens();
221+
$ignore += Collections::functionCallTokens();
222+
$ignore += Collections::objectOperators();
223+
$ignore[ T_READONLY ] = T_READONLY;
223224

224225
$next_relevant = $this->phpcsFile->findNext( $ignore, ( $stackPtr + 1 ), null, true );
225226
if ( false === $next_relevant ) {
@@ -233,6 +234,24 @@ public function process_token( $stackPtr ) {
233234
}
234235
}
235236

237+
// Skip over attribute declarations when searching for the open parenthesis.
238+
if ( \T_ATTRIBUTE === $this->tokens[ $next_relevant ]['code'] ) {
239+
if ( isset( $this->tokens[ $next_relevant ]['attribute_closer'] ) === false ) {
240+
return;
241+
}
242+
243+
$next_relevant = $this->phpcsFile->findNext(
244+
$ignore,
245+
( $this->tokens[ $next_relevant ]['attribute_closer'] + 1 ),
246+
null,
247+
true
248+
);
249+
250+
if ( false === $next_relevant ) {
251+
return;
252+
}
253+
}
254+
236255
if ( \T_OPEN_PARENTHESIS !== $this->tokens[ $next_relevant ]['code']
237256
|| isset( $this->tokens[ $next_relevant ]['parenthesis_closer'] ) === false
238257
) {

WordPress/Tests/Security/EscapeOutputUnitTest.1.inc

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -655,3 +655,13 @@ 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+
/*
660+
* Issue https://github.com/WordPress/WordPress-Coding-Standards/issues/2552
661+
* Ensure that readonly anonymous classes and anonymous classes with attributes are handled
662+
* correctly when part of a throw statement.
663+
*/
664+
throw new #[MyAttribute] readonly class( esc_html( $message ) ) extends Exception {}; // Good.
665+
throw new readonly class( $unescaped ) {}; // Bad.
666+
throw new #[MyAttribute] class( $unescaped ) extends Exception {}; // Bad.
667+
throw new #[MyAttribute] readonly class( $unescaped ) extends Exception {}; // Bad.
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<?php
2+
3+
/*
4+
* Intentional parse error (nothing after T_ATTRIBUTE).
5+
* This should be the only test in this file.
6+
*/
7+
8+
throw new #[
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<?php
2+
3+
/*
4+
* Intentional parse error (nothing after T_ATTRIBUTE_END).
5+
* This should be the only test in this file.
6+
*/
7+
8+
throw new #[MyAttribute]

WordPress/Tests/Security/EscapeOutputUnitTest.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,9 @@ public function getErrorList( $testFile = '' ) {
159159
654 => 1,
160160
655 => 1,
161161
657 => 1,
162+
665 => 1,
163+
668 => 1,
164+
669 => 1,
162165
);
163166

164167
case 'EscapeOutputUnitTest.6.inc':

0 commit comments

Comments
 (0)