-
-
Notifications
You must be signed in to change notification settings - Fork 493
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Flag any non-validated/sanitized super global input vars.
Closes #72 Switch list of sanitizing/autoescaping function to a static var to be used by other classes
- Loading branch information
Showing
4 changed files
with
225 additions
and
4 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,143 @@ | ||
<?php | ||
/** | ||
* Flag any non-validated/sanitized input ( _GET / _POST / _REQUEST / _SERVER ) | ||
* | ||
* PHP version 5 | ||
* | ||
* @category PHP | ||
* @package PHP_CodeSniffer | ||
* @author Shady Sharaf <[email protected]> | ||
* @link https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/issues/69 | ||
*/ | ||
class WordPress_Sniffs_VIP_ValidatedSanitizedInputSniff implements PHP_CodeSniffer_Sniff | ||
{ | ||
|
||
/** | ||
* Returns an array of tokens this test wants to listen for. | ||
* | ||
* @return array | ||
*/ | ||
public function register() | ||
{ | ||
return array( | ||
T_VARIABLE, | ||
); | ||
|
||
}//end register() | ||
|
||
|
||
/** | ||
* Processes this test, when one of its tokens is encountered. | ||
* | ||
* @param PHP_CodeSniffer_File $phpcsFile The file being scanned. | ||
* @param int $stackPtr The position of the current token | ||
* in the stack passed in $tokens. | ||
* | ||
* @return void | ||
*/ | ||
public function process( PHP_CodeSniffer_File $phpcsFile, $stackPtr ) | ||
{ | ||
$tokens = $phpcsFile->getTokens(); | ||
|
||
// Check for $wpdb variable | ||
if ( ! in_array( $tokens[$stackPtr]['content'], array( '$_GET', '$_POST', '$_REQUEST', '$_SERVER' ) ) ) | ||
return; | ||
|
||
$instance = $tokens[$stackPtr]; | ||
$varName = $instance['content']; | ||
|
||
$nested = $instance['nested_parenthesis']; | ||
|
||
// Ignore if wrapped inside ISSET | ||
end($nested); // Get closest parenthesis | ||
if ( T_ISSET === $tokens[ key( $nested ) - 1 ]['code'] ) | ||
return; | ||
|
||
$varKey = $this->getArrayIndexKey( $phpcsFile, $tokens, $stackPtr ); | ||
|
||
if ( empty( $varKey ) ) { | ||
$phpcsFile->addWarning( 'Detected access of super global var %s without targeting a member variable.', $stackPtr, null, array( $varName ) ); | ||
return; | ||
} | ||
|
||
// Check for validation first | ||
$is_validated = false; | ||
|
||
// Wrapped in a condition? check existence of isset with the variable as an argument | ||
if ( ! empty( $tokens[$stackPtr]['conditions'] ) ) { | ||
$conditionPtr = key( $tokens[$stackPtr]['conditions'] ); | ||
$condition = $tokens[$conditionPtr]; | ||
|
||
$issetPtr = $phpcsFile->findNext( array( T_ISSET, T_EMPTY ), $condition['parenthesis_opener'], $condition['parenthesis_closer'] ); | ||
if ( ! empty( $issetPtr ) ) { | ||
$isset = $tokens[$issetPtr]; | ||
$issetOpener = $issetPtr + 1; | ||
$issetCloser = $tokens[$issetOpener]['parenthesis_closer']; | ||
|
||
// Check that it is the same variable name | ||
if ( $validated = $phpcsFile->findNext( array( T_VARIABLE ), $issetOpener, $issetCloser, null, $varName ) ) { | ||
// Double check the $varKey inside the variable, ex: 'hello' in $_POST['hello'] | ||
|
||
$varKeyValidated = $this->getArrayIndexKey( $phpcsFile, $tokens, $validated ); | ||
|
||
if ( $varKeyValidated == $varKey ) { | ||
// everything matches, variable IS validated afterall .. | ||
$is_validated = true; | ||
} | ||
} | ||
} | ||
} | ||
|
||
if ( ! $is_validated ) { | ||
$phpcsFile->addError( 'Detected usage of a non-validated input variable: %s', $stackPtr, null, array( $tokens[$stackPtr]['content'] ) ); | ||
// return; // Should we just return and not look for sanitizing functions ? | ||
} | ||
|
||
// Now look for sanitizing functions | ||
$is_sanitized = false; | ||
|
||
$functionPtr = key( $nested ) - 1; | ||
$function = $tokens[$functionPtr]; | ||
if ( T_STRING === $function['code'] ) { | ||
$functionName = $function['content']; | ||
if ( | ||
in_array( $functionName, WordPress_Sniffs_XSS_EscapeOutputSniff::$autoEscapedFunctions ) | ||
|| | ||
in_array( $functionName, WordPress_Sniffs_XSS_EscapeOutputSniff::$sanitizingFunctions ) | ||
) { | ||
$is_sanitized = true; | ||
} | ||
} | ||
|
||
if ( ! $is_sanitized ) { | ||
$phpcsFile->addError( 'Detected usage of a non-sanitized input variable: %s', $stackPtr, null, array( $tokens[$stackPtr]['content'] ) ); | ||
} | ||
|
||
|
||
return; | ||
}//end process() | ||
|
||
/** | ||
* Get array index key of the variable requested | ||
* @param [type] $phpcsFile [description] | ||
* @param [type] $tokens [description] | ||
* @param [type] $stackPtr [description] | ||
* @return [type] [description] | ||
*/ | ||
public function getArrayIndexKey( $phpcsFile, $tokens, $stackPtr ) { | ||
// Find next bracket | ||
$bracketOpener = $phpcsFile->findNext( array( T_OPEN_SQUARE_BRACKET ), $stackPtr, $stackPtr + 3 ); | ||
|
||
// If no brackets, exit with a warning, this is a non-typical usage of super globals | ||
if ( empty ( $bracketOpener ) ) { | ||
return false; | ||
} | ||
|
||
$bracketCloser = $tokens[$bracketOpener]['bracket_closer']; | ||
|
||
$varKey = trim( $phpcsFile->getTokensAsString( $bracketOpener + 1, $bracketCloser - $bracketOpener - 1 ) ); // aka 'hello' in $_POST['hello'] | ||
|
||
return $varKey; | ||
} | ||
|
||
}//end class |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
<?php | ||
|
||
do_something( $_POST ); // Warning, unusual usage | ||
|
||
do_something_with( $_POST['hello'] ); // Error for no validation, Error for no sanitizing | ||
|
||
esc_html( $_POST['foo'] ); // Error for no validation | ||
|
||
if ( isset( $_POST['foo'] ) ) { | ||
bar( $_POST['foo'] );} // Error for no sanitizing | ||
} | ||
|
||
// @TODO: Cover non-parenthesis'd conditions | ||
// if ( isset( $_POST['foo'] ) ) | ||
// bar( $_POST['foo'] ); | ||
|
||
|
||
if ( isset( $_POST['foo'] ) ) { | ||
bar( esc_html( $_POST['foo'] ) ); // Good, validated and sanitized | ||
bar( $_POST['foo'] ); // Error, validated but not sanitized | ||
bar( esc_html( $_POST['foo'] ) ); // Good, validated and sanitized | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
<?php | ||
/** | ||
* Unit test class for the ValidatedSanitizedInputSniff | ||
* | ||
* PHP version 5 | ||
* | ||
* @category PHP | ||
* @package PHP_CodeSniffer | ||
* @author Shady Sharaf <[email protected]> | ||
* @link http://pear.php.net/package/PHP_CodeSniffer | ||
*/ | ||
|
||
class WordPress_Tests_VIP_ValidatedSanitizedInputUnitTest extends AbstractSniffUnitTest | ||
{ | ||
|
||
|
||
/** | ||
* Returns the lines where errors should occur. | ||
* | ||
* The key of the array should represent the line number and the value | ||
* should represent the number of errors that should occur on that line. | ||
* | ||
* @return array(int => int) | ||
*/ | ||
public function getErrorList() | ||
{ | ||
return array( | ||
5 => 2, | ||
7 => 1, | ||
10 => 1, | ||
20 => 1, | ||
); | ||
|
||
}//end getErrorList() | ||
|
||
|
||
/** | ||
* Returns the lines where warnings should occur. | ||
* | ||
* The key of the array should represent the line number and the value | ||
* should represent the number of warnings that should occur on that line. | ||
* | ||
* @return array(int => int) | ||
*/ | ||
public function getWarningList() | ||
{ | ||
return array( | ||
3 => 1, | ||
); | ||
|
||
}//end getWarningList() | ||
|
||
|
||
}//end class | ||
|
||
?> |