Skip to content

Commit 2db499f

Browse files
committed
Security/EscapeOutput: improve handling of params in formatting functions
Similar to multiple other fixes made before, the parameters passed to a formatting function should be examined individually and not as one big code block. Fixed now. Includes tests.
1 parent 1689ea5 commit 2db499f

File tree

3 files changed

+20
-5
lines changed

3 files changed

+20
-5
lines changed

WordPress/Sniffs/Security/EscapeOutputSniff.php

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -693,13 +693,20 @@ protected function check_code_is_escaped( $start, $end ) {
693693
}
694694
}
695695

696-
// Skip pointer to after the function.
697-
// If this is a formatting function we just skip over the opening
698-
// parenthesis. Otherwise we skip all the way to the closing.
696+
// If this is a formatting function, we examine the parameters individually.
699697
if ( $is_formatting_function ) {
700-
$i = ( $function_opener + 1 );
698+
$formatting_params = PassedParameters::getParameters( $this->phpcsFile, $i );
699+
if ( ! empty( $formatting_params ) ) {
700+
foreach ( $formatting_params as $format_param ) {
701+
$this->check_code_is_escaped( $format_param['start'], ( $format_param['end'] + 1 ) );
702+
}
703+
}
704+
701705
$watch = true;
702-
} elseif ( isset( $this->tokens[ $function_opener ]['parenthesis_closer'] ) ) {
706+
}
707+
708+
// Skip pointer to after the function.
709+
if ( isset( $this->tokens[ $function_opener ]['parenthesis_closer'] ) ) {
703710
$i = $this->tokens[ $function_opener ]['parenthesis_closer'];
704711
} else {
705712
// Live coding or parse error.

WordPress/Tests/Security/EscapeOutputUnitTest.1.inc

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -641,3 +641,10 @@ Some text without interpolation.
641641
Some text {$with->interpolation}.
642642
Some text without interpolation.
643643
EOD;
644+
645+
// Parameters in formatting functions should be examined individually.
646+
echo sprintf(
647+
'<li><a href="#%1$s" class="%2$s"%3$s%4$s>%5$s</a></li>',
648+
( '' !== $this->link_title ) ? ' title="' . esc_attr( $this->link_title ) . '"' : '',
649+
( '' !== $this->link_aria_label ) ? ' aria-label="' . $this->link_aria_label . '"' : '',
650+
); // Bad x 1.

WordPress/Tests/Security/EscapeOutputUnitTest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ public function getErrorList( $testFile = '' ) {
157157
635 => 1,
158158
636 => 1,
159159
641 => 1,
160+
649 => 1,
160161
);
161162

162163
case 'EscapeOutputUnitTest.6.inc':

0 commit comments

Comments
 (0)