diff --git a/WordPressVIPMinimum/Sniffs/Security/UnderscorejsSniff.php b/WordPressVIPMinimum/Sniffs/Security/UnderscorejsSniff.php index fb1c0af0..6ea36135 100644 --- a/WordPressVIPMinimum/Sniffs/Security/UnderscorejsSniff.php +++ b/WordPressVIPMinimum/Sniffs/Security/UnderscorejsSniff.php @@ -8,6 +8,7 @@ namespace WordPressVIPMinimum\Sniffs\Security; +use PHP_CodeSniffer\Util\Tokens; use WordPressVIPMinimum\Sniffs\Sniff; /** @@ -17,6 +18,29 @@ */ class UnderscorejsSniff extends Sniff { + /** + * Regex to match unescaped output notations containing variable interpolation + * and retrieve a code snippet. + * + * @var string + */ + const UNESCAPED_INTERPOLATE_REGEX = '`<%=\s*(?:.+?%>|$)`'; + + /** + * Regex to match execute notations containing a print command + * and retrieve a code snippet. + * + * @var string + */ + const UNESCAPED_PRINT_REGEX = '`<%\s*(?:print\s*\(.+?\)\s*;|__p\s*\+=.+?)\s*%>`'; + + /** + * Regex to match the "interpolate" keyword when used to overrule the ERB-style delimiters. + * + * @var string + */ + const INTERPOLATE_KEYWORD_REGEX = '`(?:templateSettings\.interpolate|\.interpolate\s*=\s*/|interpolate\s*:\s*/)`'; + /** * A list of tokenizers this sniff supports. * @@ -30,12 +54,11 @@ class UnderscorejsSniff extends Sniff { * @return array */ public function register() { - return [ - T_CONSTANT_ENCAPSED_STRING, - T_PROPERTY, - T_INLINE_HTML, - T_HEREDOC, - ]; + $targets = Tokens::$textStringTokens; + $targets[] = T_PROPERTY; + $targets[] = T_STRING; + + return $targets; } /** @@ -46,15 +69,91 @@ public function register() { * @return void */ public function process_token( $stackPtr ) { + /* + * Ignore Gruntfile.js files as they are configuration, not code. + */ + $file_name = $this->strip_quotes( $this->phpcsFile->getFileName() ); + $file_name = strtolower( basename( $file_name ) ); + + if ( $file_name === 'gruntfile.js' ) { + return; + } + + /* + * Check for delimiter change in JS files. + */ + if ( $this->tokens[ $stackPtr ]['code'] === T_STRING + || $this->tokens[ $stackPtr ]['code'] === T_PROPERTY + ) { + if ( $this->phpcsFile->tokenizerType !== 'JS' ) { + // These tokens are only relevant for JS files. + return; + } + + if ( $this->tokens[ $stackPtr ]['content'] !== 'interpolate' ) { + return; + } + + // Check the context to prevent false positives. + if ( $this->tokens[ $stackPtr ]['code'] === T_STRING ) { + $prev = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $stackPtr - 1 ), null, true ); + if ( $prev === false || $this->tokens[ $prev ]['code'] !== T_OBJECT_OPERATOR ) { + return; + } + + $prevPrev = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $stackPtr - 1 ), null, true ); + $next = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true ); + if ( ( $prevPrev === false + || $this->tokens[ $prevPrev ]['code'] !== T_STRING + || $this->tokens[ $prevPrev ]['content'] !== 'templateSettings' ) + && ( $next === false + || $this->tokens[ $next ]['code'] !== T_EQUAL ) + ) { + return; + } + } + + // Underscore.js delimiter change. + $message = 'Found Underscore.js delimiter change notation.'; + $this->phpcsFile->addWarning( $message, $stackPtr, 'InterpolateFound' ); + + return; + } + + $content = $this->strip_quotes( $this->tokens[ $stackPtr ]['content'] ); + + $match_count = preg_match_all( self::UNESCAPED_INTERPOLATE_REGEX, $content, $matches ); + if ( $match_count > 0 ) { + foreach ( $matches[0] as $match ) { + if ( strpos( $match, '_.escape(' ) !== false ) { + continue; + } + + // Underscore.js unescaped output. + $message = 'Found Underscore.js unescaped output notation: "%s".'; + $data = [ $match ]; + $this->phpcsFile->addWarning( $message, $stackPtr, 'OutputNotation', $data ); + } + } + + $match_count = preg_match_all( self::UNESCAPED_PRINT_REGEX, $content, $matches ); + if ( $match_count > 0 ) { + foreach ( $matches[0] as $match ) { + if ( strpos( $match, '_.escape(' ) !== false ) { + continue; + } - if ( strpos( $this->tokens[ $stackPtr ]['content'], '<%=' ) !== false ) { - // Underscore.js unescaped output. - $message = 'Found Underscore.js unescaped output notation: "<%=".'; - $this->phpcsFile->addWarning( $message, $stackPtr, 'OutputNotation' ); + // Underscore.js unescaped output. + $message = 'Found Underscore.js unescaped print execution: "%s".'; + $data = [ $match ]; + $this->phpcsFile->addWarning( $message, $stackPtr, 'PrintExecution', $data ); + } } - if ( strpos( $this->tokens[ $stackPtr ]['content'], 'interpolate' ) !== false ) { - // Underscore.js unescaped output. + if ( $this->phpcsFile->tokenizerType !== 'JS' + && preg_match( self::INTERPOLATE_KEYWORD_REGEX, $content ) > 0 + ) { + // Underscore.js delimiter change. $message = 'Found Underscore.js delimiter change notation.'; $this->phpcsFile->addWarning( $message, $stackPtr, 'InterpolateFound' ); } diff --git a/WordPressVIPMinimum/Tests/Security/Gruntfile.js b/WordPressVIPMinimum/Tests/Security/Gruntfile.js new file mode 100644 index 00000000..b423d7af --- /dev/null +++ b/WordPressVIPMinimum/Tests/Security/Gruntfile.js @@ -0,0 +1,100 @@ + +module.exports = function(grunt) { + + require('load-grunt-tasks')(grunt); + + // Project configuration. + grunt.initConfig({ + pkg: grunt.file.readJSON('package.json'), + + checktextdomain: { + options:{ + text_domain: '<%= pkg.name %>', + correct_domain: true, + keywords: [ + '__:1,2d', + '_e:1,2d', + '_x:1,2c,3d', + 'esc_html__:1,2d', + 'esc_html_e:1,2d', + 'esc_html_x:1,2c,3d', + 'esc_attr__:1,2d', + 'esc_attr_e:1,2d', + 'esc_attr_x:1,2c,3d', + '_ex:1,2c,3d', + '_n:1,2,4d', + '_nx:1,2,4c,5d', + '_n_noop:1,2,3d', + '_nx_noop:1,2,3c,4d' + ] + }, + files: { + src: [ + '**/*.php', + ], + expand: true + } + }, + + makepot: { + target: { + options: { + domainPath: '/languages/', // Where to save the POT file. + mainFile: 'style.css', // Main project file. + potFilename: '<%= pkg.name %>.pot', // Name of the POT file. + type: 'wp-theme', // Type of project (wp-plugin or wp-theme). + processPot: function( pot, options ) { + pot.headers['plural-forms'] = 'nplurals=2; plural=n != 1;'; + pot.headers['x-poedit-basepath'] = '.\n'; + pot.headers['x-poedit-language'] = 'English\n'; + pot.headers['x-poedit-country'] = 'UNITED STATES\n'; + pot.headers['x-poedit-sourcecharset'] = 'utf-8\n'; + pot.headers['X-Poedit-KeywordsList'] = '__;_e;__ngettext:1,2;_n:1,2;__ngettext_noop:1,2;_n_noop:1,2;_c,_nc:4c,1,2;_x:1,2c;_ex:1,2c;_nx:4c,1,2;_nx_noop:4c,1,2;\n'; + pot.headers['x-textdomain-support'] = 'yes\n'; + return pot; + } + } + } + }, + + // Clean up build directory + clean: { + main: ['build/<%= pkg.name %>'] + }, + + // Copy the theme into the build directory + copy: { + main: { + src: [ + '**', + '!build/**', + '!.git/**', + '!Gruntfile.js', + '!package.json', + '!.gitignore', + '!.gitmodules', + ], + dest: 'build/<%= pkg.name %>/' + } + }, + + //Compress build directory into .zip and -.zip + compress: { + main: { + options: { + mode: 'zip', + archive: './build/<%= pkg.name %>.zip' + }, + expand: true, + cwd: 'build/<%= pkg.name %>/', + src: ['**/*'], + dest: '<%= pkg.name %>/' + } + } + + }); + + // Default task(s). + grunt.registerTask( 'build', [ 'clean', 'copy', 'compress' ] ); + grunt.registerTask( 'i18n', [ 'checktextdomain', 'makepot' ] ); +}; diff --git a/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.inc b/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.inc index 2e8b7cd6..addf103d 100644 --- a/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.inc @@ -12,6 +12,109 @@ EOT; + +" data-origsrc="<%- data.originalSrc %>">'. // NOK x 1. + '<%=data.alt%>'. // NOK x 1. + ''; +} + +function single_quoted_string_with_concatenation( $data ) { + echo '<%- data.alt %>'; // NOK x 1. +} + +function double_quoted_string( $name, $value, $is_template ) { + echo $is_template ? "<%={$name}%>" : esc_attr( $value ); // NOK. +} + +$nowdoc = <<<'EOT' + +EOT; + +$heredoc = << + <%= name %> + +EOD; + +// Make sure the JS specific check does not trigger on PHP code. +$obj->interpolate = true; + +// Test matching the "interpolate" keyword with higher precision (mirrors same check in JS). +function test_interpolate_match_precision() { + ?> + + <%= _.escape(name) %>', { name: 'John Smith' }); // OK. + + var html = _.template( + "
The \"<% __p+=_.escape(o.text) %>\" is the same
" + // OK. + "as the \"<%= _.escape(o.text) %>\" and the same
" + // OK. + "as the \"<%- o.text %>\"
", // OK. + { + text: "some text and \n it's a line break" + }, + { + variable: "o" + } + ); +EOD; + + echo $script; +} + +function display_foo { +?> + + + +<%- name %>', { name: 'John Smith' }); // OK. + +var html = _.template('
  • <%= name %>
  • ', { name: 'John Smith' }); // NOK. +var html = _.template('
  • <%=type.item%>
  • ', { name: 'John Smith' }); // NOK. + +_.templateSettings.interpolate = /\{\{(.+?)\}\}/g; /* NOK */ +_.templateSettings = { + interpolate: /\{\{(.+?)\}\}/g /* NOK */ +}; + +options.interpolate=_.templateSettings.interpolate; /* NOK */ +var interpolate = options.interpolate || reNoMatch, /* Ignore */ + source = "__p += '"; + +var template = _.template('
  • {{ name }}
  • '); /* NOK, due to the interpolate, but not flagged. */ + +// Prevent false positives on "interpolate". +var preventMisidentification = 'text interpolate text'; // OK. +var interpolate = THREE.CurveUtils.interpolate; // OK. + +var p = function(f, d) { + return s.interpolate(m(f), _(d), 0.5, e.color_space) // OK. +} + +y.interpolate.bezier = b; // OK. + +// Recognize escaping. +var html = _.template('
  • <%= _.escape(name) %>
  • ', { name: 'John Smith' }); // OK. + +var html = _.template( + "
    The \"<% __p+=_.escape(o.text) %>\" is the same
    " + // OK. + "as the \"<%= _.escape(o.text) %>\" and the same
    " + // OK. + "as the \"<%- o.text %>\"
    ", // OK. + { + text: "some text and \n it's a line break" + }, + { + variable: "o" + } +); + +var compiled = _.template("<% print('Hello ' + _.escape(epithet)); %>"); /* OK */ +var compiled = _.template("<% print('Hello ' + epithet); %>"); /* NOK */ +var compiled = _.template("<% __p+=o.text %>"); /* NOK */ diff --git a/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.php b/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.php index 1043c3e8..0c0e08c5 100644 --- a/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.php +++ b/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.php @@ -18,6 +18,21 @@ */ class UnderscorejsUnitTest extends AbstractSniffUnitTest { + /** + * Get a list of all test files to check. + * + * @param string $testFileBase The base path that the unit tests files will have. + * + * @return string[] + */ + protected function getTestFiles( $testFileBase ) { + return [ + $testFileBase . 'inc', + $testFileBase . 'js', + __DIR__ . DIRECTORY_SEPARATOR . 'Gruntfile.js', + ]; + } + /** * Returns the lines where errors should occur. * @@ -30,13 +45,44 @@ public function getErrorList() { /** * Returns the lines where warnings should occur. * + * @param string $testFile The name of the file being tested. + * * @return array => */ - public function getWarningList() { - return [ - 6 => 1, - 14 => 1, - ]; + public function getWarningList( $testFile = '' ) { + switch ( $testFile ) { + case 'UnderscorejsUnitTest.inc': + return [ + 6 => 1, + 14 => 1, + 22 => 1, + 23 => 1, + 28 => 1, + 32 => 1, + 38 => 3, + 45 => 1, + 46 => 1, + 47 => 1, + 58 => 1, + 60 => 1, + 114 => 1, + 115 => 1, + ]; + + case 'UnderscorejsUnitTest.js': + return [ + 4 => 1, + 5 => 1, + 7 => 1, + 9 => 1, + 12 => 1, + 44 => 1, + 45 => 1, + ]; + + default: + return []; + } } }