Skip to content

Commit 0638f0b

Browse files
committed
Security/EscapeOutput: fix false positive for get_search_query() with FQN/mixed-case true
The sniff was incorrectly flagging `get_search_query()` calls as unsafe when the `$escaped` parameter was passed as fully qualified true or as true using a non-standard case. The comparison `'true' !== $escaped_param['clean']` failed because the parameter value wasn't normalized. This commit fixes this by stripping any leading backslash and converting to lowercase before comparison.
1 parent 6b7601d commit 0638f0b

File tree

3 files changed

+14
-1
lines changed

3 files changed

+14
-1
lines changed

WordPress/Sniffs/Security/EscapeOutputSniff.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -741,7 +741,7 @@ protected function check_code_is_escaped( $start, $end, $code = 'OutputNotEscape
741741
// Special case get_search_query() which is unsafe if $escaped = false.
742742
if ( 'get_search_query' === strtolower( $functionName ) ) {
743743
$escaped_param = PassedParameters::getParameter( $this->phpcsFile, $ptr, 1, 'escaped' );
744-
if ( false !== $escaped_param && 'true' !== $escaped_param['clean'] ) {
744+
if ( false !== $escaped_param && 'true' !== strtolower( ltrim( $escaped_param['clean'], '\\' ) ) ) {
745745
$this->phpcsFile->addError(
746746
'Output from get_search_query() is unsafe due to $escaped parameter being set to "false".',
747747
$ptr,

WordPress/Tests/Security/EscapeOutputUnitTest.1.inc

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -801,3 +801,13 @@ echo implode( '<br>', MyNamespace\array_map( 'esc_html', $items ) ); // Bad x 2.
801801
echo implode( '<br>', \MyNamespace\map_deep( $items, 'esc_html' ) ); // Bad.
802802
echo implode( '<br>', namespace\array_map( 'esc_html', $items ) ); // Bad x 2. The sniff should stop flagging this once it can resolve relative namespaces.
803803
echo implode( '<br>', namespace\Sub\map_deep( $items, 'esc_html' ) ); // Bad.
804+
805+
/*
806+
* Safeguard correct handling of get_search_query() with FQN and non-standard case booleans for the $escaped parameter.
807+
*/
808+
echo \get_search_query( TRUE ); // Ok.
809+
echo \get_search_query( \true ); // Ok.
810+
echo \get_search_query( \True ); // Ok.
811+
echo \get_search_query( FaLsE ); // Bad.
812+
echo \get_search_query( \false ); // Bad.
813+
echo \get_search_query( \FALSE ); // Bad.

WordPress/Tests/Security/EscapeOutputUnitTest.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,9 @@ public function getErrorList( $testFile = '' ) {
205205
801 => 1,
206206
802 => 2,
207207
803 => 1,
208+
811 => 1,
209+
812 => 1,
210+
813 => 1,
208211
);
209212

210213
case 'EscapeOutputUnitTest.6.inc':

0 commit comments

Comments
 (0)