Skip to content

Commit

Permalink
Fix "needs sanitising" functions spilling over
Browse files Browse the repository at this point in the history
When a function call to a function in the list of those that are
specifically marked as needing sanitization is nested within a call to
another function, any later parameters of the super-function will be
marked ass needing sanitization under some circumstances.

This is fixed by marking the end of the statement as the closing
parenthesis of the “needs sanitizing” function, rather than the next
semi-colon.
  • Loading branch information
JDGrimes committed Feb 16, 2015
1 parent 39881e8 commit b416691
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 1 deletion.
7 changes: 6 additions & 1 deletion WordPress/Sniffs/XSS/EscapeOutputSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,8 @@ public function process( PHP_CodeSniffer_File $phpcsFile, $stackPtr )
$needs_sanitizing_function = true;

$stackPtr++; // Ignore the starting bracket

$end_of_statement = $tokens[ $stackPtr ]['parenthesis_closer'];
}

if ( $tokens[ $stackPtr ]['code'] === T_EXIT ) {
Expand Down Expand Up @@ -323,7 +325,10 @@ public function process( PHP_CodeSniffer_File $phpcsFile, $stackPtr )
}
}

$end_of_statement = $phpcsFile->findNext( T_SEMICOLON, $stackPtr );
// This is already determined if $needs_sanitizing_function.
if ( ! isset( $end_of_statement ) ) {
$end_of_statement = $phpcsFile->findNext( T_SEMICOLON, $stackPtr );
}

// Check for the ternary operator.
$ternary = $phpcsFile->findNext( T_INLINE_THEN, $stackPtr, $end_of_statement );
Expand Down
9 changes: 9 additions & 0 deletions WordPress/Tests/XSS/EscapeOutputUnitTest.inc
Original file line number Diff line number Diff line change
Expand Up @@ -74,3 +74,12 @@ printf( 'Hello %s! Hi %s!', esc_html( $foo ), $bar ); // Bad

vprintf( 'Hello %s', array( $foo ) ); // Bad
vprintf( 'Hello %s', array( esc_html( $foo ) ) ); // OK

// The below checks that functions which are marked as needed further sanitization
// don't spill over into later arguments when nested in a function call. There was
// a bug which would cause line 84 to be marked as needing sanitization because _x()
// is marked as needing sanitization.
do_something(
_x( 'Some string', 'context', 'domain' )
, array( $foo ) // OK
);

0 comments on commit b416691

Please sign in to comment.