diff --git a/WordPressVIPMinimum/Sniffs/Variables/ServerVariablesSniff.php b/WordPressVIPMinimum/Sniffs/Variables/ServerVariablesSniff.php index ffd039e2..f3769380 100644 --- a/WordPressVIPMinimum/Sniffs/Variables/ServerVariablesSniff.php +++ b/WordPressVIPMinimum/Sniffs/Variables/ServerVariablesSniff.php @@ -9,6 +9,8 @@ namespace WordPressVIPMinimum\Sniffs\Variables; +use PHP_CodeSniffer\Util\Tokens; +use PHPCSUtils\Utils\TextStrings; use WordPressVIPMinimum\Sniffs\Sniff; /** @@ -17,7 +19,7 @@ class ServerVariablesSniff extends Sniff { /** - * List of restricted constant names. + * List of restricted indices. * * @var array> */ @@ -53,21 +55,94 @@ public function register() { */ public function process_token( $stackPtr ) { - if ( $this->tokens[ $stackPtr ]['content'] !== '$_SERVER' ) { - // Not the variable we are looking for. + if ( $this->tokens[ $stackPtr ]['content'] !== '$_SERVER' + && $this->tokens[ $stackPtr ]['content'] !== '$GLOBALS' + ) { + // Not a variable we are looking for. return; } - $variableNamePtr = $this->phpcsFile->findNext( [ T_CONSTANT_ENCAPSED_STRING ], $stackPtr + 1, null, false, null, true ); - $variableName = str_replace( [ "'", '"' ], '', $this->tokens[ $variableNamePtr ]['content'] ); + $searchStart = $stackPtr; + if ( $this->tokens[ $stackPtr ]['content'] === '$GLOBALS' ) { + $globalsIndexPtr = $this->get_array_access_key( $stackPtr ); + if ( $globalsIndexPtr === false ) { + // Couldn't find an array index token usable for the purposes of this sniff. Bow out. + return; + } - if ( isset( $this->restrictedVariables['authVariables'][ $variableName ] ) ) { + $globalsIndexName = TextStrings::stripQuotes( $this->tokens[ $globalsIndexPtr ]['content'] ); + if ( $globalsIndexName !== '_SERVER' ) { + // Not access to `$GLOBALS['_SERVER']`. + return; + } + + // Set the start point for the next array access key search to the close bracket of this array index. + // No need for defensive coding as we already know there will be a valid close bracket next. + $searchStart = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $globalsIndexPtr + 1 ), null, true ); + } + + $prevNonEmpty = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $stackPtr - 1 ), null, true ); + if ( $this->tokens[ $prevNonEmpty ]['code'] === T_DOUBLE_COLON ) { + // Access to OO property mirroring the name of the superglobal. Not our concern. + return; + } + + $indexPtr = $this->get_array_access_key( $searchStart ); + if ( $indexPtr === false ) { + // Couldn't find an array index token usable for the purposes of this sniff. Bow out as undetermined. + return; + } + + $indexName = TextStrings::stripQuotes( $this->tokens[ $indexPtr ]['content'] ); + + if ( isset( $this->restrictedVariables['authVariables'][ $indexName ] ) ) { $message = 'Basic authentication should not be handled via PHP code.'; $this->phpcsFile->addError( $message, $stackPtr, 'BasicAuthentication' ); - } elseif ( isset( $this->restrictedVariables['userControlledVariables'][ $variableName ] ) ) { + } elseif ( isset( $this->restrictedVariables['userControlledVariables'][ $indexName ] ) ) { $message = 'Header "%s" is user-controlled and should be properly validated before use.'; - $data = [ $variableName ]; + $data = [ $indexName ]; $this->phpcsFile->addError( $message, $stackPtr, 'UserControlledHeaders', $data ); } } + + /** + * Get the array access key. + * + * Find the array access key and check if it is: + * - comprised of a single functional token. + * - that token is a T_CONSTANT_ENCAPSED_STRING. + * + * @param int $stackPtr The position of either a variable or the close bracket of a previous array access. + * + * @return int|false Stack pointer to the index token; or FALSE for + * live coding, non-indexed array assignment, or non plain text array keys. + */ + private function get_array_access_key( $stackPtr ) { + $openBracket = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true ); + if ( $openBracket === false + || $this->tokens[ $openBracket ]['code'] !== T_OPEN_SQUARE_BRACKET + || isset( $this->tokens[ $openBracket ]['bracket_closer'] ) === false + ) { + // If it isn't an open bracket, this isn't array access. And without closer, it is a parse error/live coding. + return false; + } + + $closeBracket = $this->tokens[ $openBracket ]['bracket_closer']; + + $indexPtr = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $openBracket + 1 ), $closeBracket, true ); + if ( $indexPtr === false + || $this->tokens[ $indexPtr ]['code'] !== T_CONSTANT_ENCAPSED_STRING + ) { + // No array access (like for array assignment without key) or key is not plain text. + return false; + } + + $hasOtherTokens = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $indexPtr + 1 ), $closeBracket, true ); + if ( $hasOtherTokens !== false ) { + // The array index is comprised of multiple tokens. Bow out as undetermined. + return false; + } + + return $indexPtr; + } } diff --git a/WordPressVIPMinimum/Tests/Variables/ServerVariablesUnitTest.inc b/WordPressVIPMinimum/Tests/Variables/ServerVariablesUnitTest.inc index a22bae0c..28de0071 100644 --- a/WordPressVIPMinimum/Tests/Variables/ServerVariablesUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Variables/ServerVariablesUnitTest.inc @@ -1,9 +1,75 @@ 1, - 5 => 1, - 6 => 1, - 7 => 1, + 31 => 1, + 32 => 1, + 33 => 1, + 36 => 1, + 37 => 1, + 71 => 1, + 72 => 1, + 73 => 1, + 74 => 1, + 75 => 1, ]; }