Skip to content

Commit

Permalink
Always consider casting sanitization
Browse files Browse the repository at this point in the history
There are two different sanitization checks in this sniff. Only one of
them was considering that casting is a form of input sanitization. I’ve
moved the casting check out of the conditional it was in, and now the
`$is_casted` value is being checked for all sanitization checking paths.

Includes unit tests.

Fixes #179
  • Loading branch information
JDGrimes committed Dec 24, 2014
1 parent e68ce32 commit 53ea105
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 5 deletions.
10 changes: 6 additions & 4 deletions WordPress/Sniffs/VIP/ValidatedSanitizedInputSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,17 +59,17 @@ public function process( PHP_CodeSniffer_File $phpcsFile, $stackPtr )
return;
}

$is_casted = false;
// Search for casting
$prev = $phpcsFile->findPrevious( array( T_WHITESPACE ), $stackPtr - 1, null, true, null, true );
$is_casted = in_array( $tokens[ $prev ]['code'], array( T_INT_CAST, T_DOUBLE_CAST, T_BOOL_CAST ) );

if ( isset( $instance['nested_parenthesis'] ) ) {
$nested = $instance['nested_parenthesis'];
// Ignore if wrapped inside ISSET
end( $nested ); // Get closest parenthesis
if ( in_array( $tokens[ key( $nested ) - 1 ]['code'], array( T_ISSET, T_EMPTY, T_UNSET ) ) )
return;
} else {
// Search for casting
$prev = $phpcsFile->findPrevious( array( T_WHITESPACE ), $stackPtr -1, null, true, null, true );
$is_casted = in_array( $tokens[ $prev ]['code'], array( T_INT_CAST, T_DOUBLE_CAST, T_BOOL_CAST ) );
if ( ! $is_casted ) {
$phpcsFile->addError( 'Detected usage of a non-sanitized input variable: %s', $stackPtr, null, array( $tokens[$stackPtr]['content'] ) );
return;
Expand Down Expand Up @@ -163,6 +163,8 @@ public function process( PHP_CodeSniffer_File $phpcsFile, $stackPtr )
}
} elseif ( T_UNSET === $function['code'] ) {
$is_sanitized = true;
} elseif ( $is_casted ) {
$is_sanitized = true;
}
}

Expand Down
3 changes: 2 additions & 1 deletion WordPress/Tests/VIP/ValidatedSanitizedInputUnitTest.inc
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ foreach( $_POST as $key => $value ) {

unset( $_GET['test'] ); // ok

echo (int) $_GET['test'];
echo (int) $_GET['test']; // ok
some_func( $some_arg, (int) $_GET['test'] ); // ok

function zebra() {
if ( isset( $_GET['foo'], $_POST['bar'] ) ) {
Expand Down

0 comments on commit 53ea105

Please sign in to comment.