Flag any non-validated/sanitized super global input vars

Sniffs/VIP/ValidatedSanitizedInputSniff.php

@@ -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 <shady@x-team.com>
+ * @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

Sniffs/XSS/EscapeOutputSniff.php

@@ -20,7 +20,7 @@
 class WordPress_Sniffs_XSS_EscapeOutputSniff implements PHP_CodeSniffer_Sniff
 {
 
-    public $autoEscapedFunctions = array(
+    public static $autoEscapedFunctions = array(
                                     'get_header',
                                     'get_footer',
                                     'get_sidebar',
@@ -196,7 +196,7 @@ class WordPress_Sniffs_XSS_EscapeOutputSniff implements PHP_CodeSniffer_Sniff
                                     'do_shortcode_tag',
                                    );
 
-    public $sanitizingFunctions = array(
+    public static $sanitizingFunctions = array(
                                    'wp_kses_post',
                                    'wp_kses_data',
                                    'wp_kses_allowed_html',
@@ -319,11 +319,11 @@ public function process(PHP_CodeSniffer_File $phpcsFile, $stackPtr)
         }
 
         $functionName = $tokens[$stackPtr]['content'];
-        if (in_array($functionName, $this->autoEscapedFunctions) === true) {
+        if (in_array($functionName, self::$autoEscapedFunctions) === true) {
             return;
         }
 
-        if (in_array($functionName, $this->sanitizingFunctions) === false) {
+        if (in_array($functionName, self::$sanitizingFunctions) === false) {
             $error = sprintf("Expected a sanitizing function (see Codex for 'Data Validation'), but instead saw '%s'", $tokens[$stackPtr]['content']);
             $phpcsFile->addError($error, $stackPtr);
             return;

Tests/VIP/ValidatedSanitizedInputUnitTest.inc

@@ -0,0 +1,31 @@
+<?php
+
+do_something( $_POST ); // Warning, unusual usage
+
+do_something_with( $_POST['hello'] ); // Error for no validation, Error for no sanitizing
+
+echo 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
+}
+
+// Should all be OK
+$empty = (
+	empty( $_GET['foo'] )
+	||
+	empty( $_REQUEST['foo'] )
+	||
+	empty( $_POST['foo'] )
+);

Tests/VIP/ValidatedSanitizedInputUnitTest.php

@@ -0,0 +1,56 @@
+<?php
+/**
+ * Unit test class for the ValidatedSanitizedInputSniff
+ *
+ * PHP version 5
+ *
+ * @category  PHP
+ * @package   PHP_CodeSniffer
+ * @author    Shady Sharaf <shady@x-team.com>
+ * @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
+
+?>