Skip to content

Commit

Permalink
Adding missing escaping check for <?=
Browse files Browse the repository at this point in the history
This commit registers `T_OPEN_TAG_WITH_ECHO` in EscapeOutputSniff and adds new test for those cases
  • Loading branch information
david-binda authored and jrfnl committed Jun 29, 2017
1 parent bbcc832 commit 04d997c
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 2 deletions.
4 changes: 4 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ matrix:
env: PHPCS_BRANCH=2.9
- php: nightly
env: PHPCS_BRANCH=2.9
# Test PHP 5.3 with short_open_tags set to On (is Off by default)
- php: 5.3
env: PHPCS_BRANCH=master SHORT_OPEN_TAGS=true
allow_failures:
# Allow failures for unstable builds.
- php: nightly
Expand All @@ -52,6 +55,7 @@ before_install:
# test suite is currently not compatible with PHPUnit 6.x.
# Fixed at a very specific PHPUnit version which is also compatible with HHVM.
- if [[ ${TRAVIS_PHP_VERSION:0:2} != "5." ]]; then wget -P $PHPUNIT_DIR https://phar.phpunit.de/phpunit-5.7.17.phar && chmod +x $PHPUNIT_DIR/phpunit-5.7.17.phar; fi
- if [[ "$SHORT_OPEN_TAGS" == "true" ]]; then echo "short_open_tag = On" >> ~/.phpenv/versions/$(phpenv version-name)/etc/php.ini; fi

script:
# Lint the PHP files against parse errors.
Expand Down
43 changes: 42 additions & 1 deletion WordPress/Sniffs/XSS/EscapeOutputSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -144,19 +144,42 @@ class WordPress_Sniffs_XSS_EscapeOutputSniff extends WordPress_Sniff {
'T_END_NOWDOC' => true,
);

/**
* Status of short_open_tag feature
*
* @since 0.11.0
*
* @var bool
*/
private $short_open_tag_enabled = true;

/**
* Returns an array of tokens this test wants to listen for.
*
* @return array
*/
public function register() {
return array(
// Check whether short_open_tag is disabled on PHP version < 5.4 (it's enabled by default in later versions).
if ( PHP_VERSION_ID < 50400 && false === (bool) ini_get( 'short_open_tag' ) ) {
$this->short_open_tag_enabled = false;
}

$tokens = array(
T_ECHO,
T_PRINT,
T_EXIT,
T_STRING,
T_OPEN_TAG_WITH_ECHO,
);

/*
* In case short_open_tag is turned off, we can attempt to regex T_INLINE_HTML
* which is how short open tags are being handled in that case.
*/
if ( false === $this->short_open_tag_enabled ) {
$tokens[] = T_INLINE_HTML;
}
return $tokens;
}

/**
Expand Down Expand Up @@ -191,6 +214,24 @@ public function process_token( $stackPtr ) {
if ( in_array( $function, array( 'trigger_error', 'user_error' ), true ) ) {
$end_of_statement = $this->phpcsFile->findEndOfStatement( $open_paren + 1 );
}
} else if ( false === $this->short_open_tag_enabled && T_INLINE_HTML === $this->tokens[ $stackPtr ]['code'] ) {
// Skip if no PHP short_open_tag is in the string.
if ( false === strpos( $this->tokens[ $stackPtr ]['content'], '<?=' ) ) {
return;
}

// Report on what very likely is a PHP short open tag outputting variable.
if ( preg_match( '/\<\?\=[\s]*(\$[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*(?:(?:->\S+|\[[^\]]+\]))*)[\s]*;?[\s]*\?\>/', $this->tokens[ $stackPtr ]['content'], $matches ) ) {
$this->phpcsFile->addError( 'Expected next thing to be an escaping function, not %s.', $stackPtr, 'OutputNotEscaped', $matches[1] );
return;
}

// Throw warning in case the T_INLINE_HTML looks like a open_short_tag.
if ( false !== strpos( $this->tokens[ $stackPtr ]['content'], '<?=' ) ) {
$this->phpcsFile->addWarning( 'Possible use of PHP short open tag ( "<?=" ) detected. Needs manual inspection.', $stackPtr, 'PossibleShortOpenTag' );
return;
}
return;
}

// Checking for the ignore comment, ex: //xss ok.
Expand Down
7 changes: 7 additions & 0 deletions WordPress/Tests/XSS/EscapeOutputUnitTest.inc
Original file line number Diff line number Diff line change
Expand Up @@ -218,3 +218,10 @@ echo 1.234; // Ok.
echo ( 1.234 + 10 + 2.5 ); // Ok.
echo 10 % 2; // Ok.
echo 8 * 1.2; // Ok.

?>
<?= $var ?><!-- Bad. -->
<?= esc_html( $var ); ?><!-- Ok. -->
<?= $var['foo']; ?><!-- Bad. -->
<?= $var->foo ?><!-- Bad. -->
<?php
11 changes: 10 additions & 1 deletion WordPress/Tests/XSS/EscapeOutputUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ public function getErrorList() {
206 => 1,
207 => 1,
212 => ( PHP_VERSION_ID < 50300 ) ? 1 : 0, // PHPCS on PHP 5.2 does not recognize T_NOWDOC.
210 => 1,
212 => 1,
213 => 1,
);

} // end getErrorList()
Expand All @@ -74,8 +77,14 @@ public function getErrorList() {
* @return array <int line number> => <int number of warnings>
*/
public function getWarningList() {
return array();
$list = array();

// Adding Warning which is triggerred in case open_short_tag is set to Off.
if ( PHP_VERSION_ID < 50400 && false === (bool) ini_get( 'short_open_tag' ) ) {
$list[211] = 1;
}

return $list;
}

} // End class.

0 comments on commit 04d997c

Please sign in to comment.