Skip to content

Commit f165714

Browse files
committed
Security/EscapeOutput: special case get_search_query( false )
As per the reported issue, `get_search_query()` is unsafe when the `$escaped` parameter is set, but not set to `true`. This commit special cases the function and checks the parameter. If the parameter is passed, but not set to `true`, a custom error message will be thrown. Includes tests. Fixes 1354
1 parent 2db499f commit f165714

File tree

3 files changed

+22
-0
lines changed

3 files changed

+22
-0
lines changed

WordPress/Sniffs/Security/EscapeOutputSniff.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -719,6 +719,18 @@ protected function check_code_is_escaped( $start, $end ) {
719719
|| $this->is_escaping_function( $functionName )
720720
|| $this->is_auto_escaped_function( $functionName )
721721
) {
722+
// Special case get_search_query() which is unsafe if $escaped = false.
723+
if ( 'get_search_query' === strtolower( $functionName ) ) {
724+
$escaped_param = PassedParameters::getParameter( $this->phpcsFile, $ptr, 1, 'escaped' );
725+
if ( false !== $escaped_param && 'true' !== $escaped_param['clean'] ) {
726+
$this->phpcsFile->addError(
727+
'Output from get_search_query() is unsafe due to $escaped parameter being set to "false".',
728+
$ptr,
729+
'UnsafeSearchQuery'
730+
);
731+
}
732+
}
733+
722734
continue;
723735
}
724736

WordPress/Tests/Security/EscapeOutputUnitTest.1.inc

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -648,3 +648,10 @@ echo sprintf(
648648
( '' !== $this->link_title ) ? ' title="' . esc_attr( $this->link_title ) . '"' : '',
649649
( '' !== $this->link_aria_label ) ? ' aria-label="' . $this->link_aria_label . '"' : '',
650650
); // Bad x 1.
651+
652+
echo '<input type="search" value="' . get_search_query() . '">'; // OK.
653+
echo '<input type="search" value="' . get_search_query( /*comment*/ true ) . '">'; // OK.
654+
echo '<input type="search" value="' . get_search_query( false ) . '">'; // Bad.
655+
echo '<input type="search" value="' . get_search_query( 0 ) . '">'; // Bad.
656+
echo '<input type="search" value="' . get_search_query( escape: false ) . '">'; // OK, well not really, typo in param name, but that's not our concern.
657+
echo '<input type="search" value="' . get_search_query( escaped: false ) . '">'; // Bad.

WordPress/Tests/Security/EscapeOutputUnitTest.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,9 @@ public function getErrorList( $testFile = '' ) {
158158
636 => 1,
159159
641 => 1,
160160
649 => 1,
161+
654 => 1,
162+
655 => 1,
163+
657 => 1,
161164
);
162165

163166
case 'EscapeOutputUnitTest.6.inc':

0 commit comments

Comments
 (0)